Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1664321 > unrolled thread
| Started by | Kieran Bingham <kbingham@kernel.org> |
|---|---|
| First post | 2017-06-13 02:40 +0200 |
| Last post | 2017-06-13 14:30 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v4 0/2] ADV748x HDMI/Analog video receiver Kieran Bingham <kbingham@kernel.org> - 2017-06-13 02:40 +0200
Re: [PATCH v4 1/2] media: i2c: adv748x: add adv748x driver Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-13 11:30 +0200
Re: [PATCH v4 1/2] media: i2c: adv748x: add adv748x driver Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> - 2017-06-13 11:40 +0200
Re: [PATCH v4 1/2] media: i2c: adv748x: add adv748x driver Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-13 11:40 +0200
Re: [PATCH v4 1/2] media: i2c: adv748x: add adv748x driver Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> - 2017-06-13 14:30 +0200
| From | Kieran Bingham <kbingham@kernel.org> |
|---|---|
| Date | 2017-06-13 02:40 +0200 |
| Subject | [PATCH v4 0/2] ADV748x HDMI/Analog video receiver |
| Message-ID | <tRI2d-Sj-5@gated-at.bofh.it> |
From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
This is a driver for the Analog Devices ADV748x device, and follows on from a
previous posting by Niklas Söderlund [0] of an earlier incarnation of this
driver.
Aside from a few bug fixes, and considerable refactoring this driver:
- is refactored to multiple object files
- defines multiple sub devices for the output paths.
- has independent controls for both HDMI and Analog video paths
- Specifies 'endpoint' matching instead of 'device' in async framework
These patches are based up on Niklas' pending RVin work [1] and Sakari's fwnode
series [2]
This version is the culmination of large refactoring and development, and I
believe is ready (or near) for mainline integration.
ADV748x
=======
The ADV7481 and ADV7482 support two video pipelines which can run independently
of each other, with each pipeline terminating in a CSI-2 output: TXA (4-Lane)
and TXB (1-Lane)
The ADV7480 (Not yet included here), ADV7481, and ADV7482 are all derivatives,
with the following features
Analog HDMI MHL 4-Lane 1-Lane
In In CSI CSI
ADV7480 X X X
ADV7481 X X X X X
ADV7482 X X X X
Implementation
==============
This RFC creates 4 entities. AFE (CVBS/Analog In), HDMI, TXA and TXB. At probe
time, the DT is parsed to identify the endpoints for each of these nodes, and
those are used for async matching of the CSI2 (TXA/TXB) entities in the master
driver. The HDMI and AFE entities are then registered after a successful
registration of both the CSI2 entities.
(Known) Future Todo's
=====================
Further potential development areas include:
- ADV7480 Support (No AFE)
- MHL support (Not present on ADV7482)
- EDID support
- CEC Support
- Configurable I2C addressing
- Interrupt handling for format changes and hotplug detect.
However, this driver is functional without the above, and these developments
can be written when required.
References
==========
[0] http://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg05196.html
[1] https://git.ragnatech.se/linux rcar-vin-elinux-v7
[2] https://www.mail-archive.com/linux-media@vger.kernel.org/msg111332.html
v1/RFC:
- Initial posting
v2:
- Reworked DT parsing and entities
v3:
- Refreshed with lots of fixups from Sakari's review comments
v4:
- Many changes all round, following Laurent's review and extensive development
- Now uses regmap
- AFE port numbering has been changed to match the entity pads
-
Kieran Bingham (2):
media: i2c: adv748x: add adv748x driver
arm64: dts: renesas: salvator-x: Add ADV7482 support
Documentation/devicetree/bindings/media/i2c/adv748x.txt | 96 +-
MAINTAINERS | 6 +-
arch/arm64/boot/dts/renesas/salvator-x.dtsi | 123 +-
drivers/media/i2c/Kconfig | 11 +-
drivers/media/i2c/Makefile | 1 +-
drivers/media/i2c/adv748x/Makefile | 7 +-
drivers/media/i2c/adv748x/adv748x-afe.c | 571 ++++++-
drivers/media/i2c/adv748x/adv748x-core.c | 907 +++++++++-
drivers/media/i2c/adv748x/adv748x-csi2.c | 323 +++-
drivers/media/i2c/adv748x/adv748x-hdmi.c | 652 ++++++-
drivers/media/i2c/adv748x/adv748x.h | 415 ++++-
11 files changed, 3112 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/adv748x.txt
create mode 100644 drivers/media/i2c/adv748x/Makefile
create mode 100644 drivers/media/i2c/adv748x/adv748x-afe.c
create mode 100644 drivers/media/i2c/adv748x/adv748x-core.c
create mode 100644 drivers/media/i2c/adv748x/adv748x-csi2.c
create mode 100644 drivers/media/i2c/adv748x/adv748x-hdmi.c
create mode 100644 drivers/media/i2c/adv748x/adv748x.h
base-commit: 287d20fda775908006c5d64a15cd65244578ed01
--
git-series 0.9.1
[toc] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2017-06-13 11:30 +0200 |
| Subject | Re: [PATCH v4 1/2] media: i2c: adv748x: add adv748x driver |
| Message-ID | <tRQj8-6bD-17@gated-at.bofh.it> |
| In reply to | #1664321 |
Hi Kieran,
On Tue, Jun 13, 2017 at 2:35 AM, Kieran Bingham <kbingham@kernel.org> wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
> Provide support for the ADV7481 and ADV7482.
>
> The driver is modelled with 4 subdevices to allow simultaneous streaming
> from the AFE (Analog front end) and HDMI inputs though two CSI TX
> entities.
>
> The HDMI entity is linked to the TXA CSI bus, whilst the AFE is linked
> to the TXB CSI bus.
>
> The driver is based on a prototype by Koji Matsuoka in the Renesas BSP,
> and an earlier rework by Niklas Söderlund.
>
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> --- /dev/null
> +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
> +static int adv748x_hdmi_set_pixelrate(struct adv748x_hdmi *hdmi)
> +{
> + struct v4l2_subdev *tx;
> + struct v4l2_dv_timings timings;
> + struct v4l2_bt_timings *bt = &timings.bt;
> + unsigned int fps;
> +
> + tx = adv748x_get_remote_sd(&hdmi->pads[ADV748X_HDMI_SOURCE]);
> + if (!tx)
> + return -ENOLINK;
> +
> + adv748x_hdmi_query_dv_timings(&hdmi->sd, &timings);
> +
> + fps = DIV_ROUND_CLOSEST(bt->pixelclock,
> + V4L2_DV_BT_FRAME_WIDTH(bt) *
> + V4L2_DV_BT_FRAME_HEIGHT(bt));
On arm32:
drivers/built-in.o: In function `adv748x_hdmi_set_pixelrate':
:(.text+0x1b8b1c): undefined reference to `__aeabi_uldivmod'
v4l2_bt_timings.pixelclock is u64, so you should use DIV_ROUND_CLOSEST_ULL()
instead.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[toc] | [prev] | [next] | [standalone]
| From | Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> |
|---|---|
| Date | 2017-06-13 11:40 +0200 |
| Subject | Re: [PATCH v4 1/2] media: i2c: adv748x: add adv748x driver |
| Message-ID | <tRQsN-6eE-7@gated-at.bofh.it> |
| In reply to | #1664631 |
On 13/06/17 10:24, Geert Uytterhoeven wrote:
> Hi Kieran,
>
> On Tue, Jun 13, 2017 at 2:35 AM, Kieran Bingham <kbingham@kernel.org> wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> Provide support for the ADV7481 and ADV7482.
>>
>> The driver is modelled with 4 subdevices to allow simultaneous streaming
>> from the AFE (Analog front end) and HDMI inputs though two CSI TX
>> entities.
>>
>> The HDMI entity is linked to the TXA CSI bus, whilst the AFE is linked
>> to the TXB CSI bus.
>>
>> The driver is based on a prototype by Koji Matsuoka in the Renesas BSP,
>> and an earlier rework by Niklas Söderlund.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
>> --- /dev/null
>> +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
>
>> +static int adv748x_hdmi_set_pixelrate(struct adv748x_hdmi *hdmi)
>> +{
>> + struct v4l2_subdev *tx;
>> + struct v4l2_dv_timings timings;
>> + struct v4l2_bt_timings *bt = &timings.bt;
>> + unsigned int fps;
>> +
>> + tx = adv748x_get_remote_sd(&hdmi->pads[ADV748X_HDMI_SOURCE]);
>> + if (!tx)
>> + return -ENOLINK;
>> +
>> + adv748x_hdmi_query_dv_timings(&hdmi->sd, &timings);
>> +
>> + fps = DIV_ROUND_CLOSEST(bt->pixelclock,
>> + V4L2_DV_BT_FRAME_WIDTH(bt) *
>> + V4L2_DV_BT_FRAME_HEIGHT(bt));
>
> On arm32:
>
> drivers/built-in.o: In function `adv748x_hdmi_set_pixelrate':
> :(.text+0x1b8b1c): undefined reference to `__aeabi_uldivmod'
>
> v4l2_bt_timings.pixelclock is u64, so you should use DIV_ROUND_CLOSEST_ULL()
> instead.
Aha, thanks.
/me ponders why I didn't get spammed from the bot-builders about this?
Fix applied locally ready for v5.
Would you like the remote updated for renesas-drivers or will you patch locally?
--
Kieran
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2017-06-13 11:40 +0200 |
| Subject | Re: [PATCH v4 1/2] media: i2c: adv748x: add adv748x driver |
| Message-ID | <tRQsN-6eE-9@gated-at.bofh.it> |
| In reply to | #1664646 |
Hi Kieran,
On Tue, Jun 13, 2017 at 11:32 AM, Kieran Bingham
<kieran.bingham+renesas@ideasonboard.com> wrote:
> On 13/06/17 10:24, Geert Uytterhoeven wrote:
>> On Tue, Jun 13, 2017 at 2:35 AM, Kieran Bingham <kbingham@kernel.org> wrote:
>>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>
>>> Provide support for the ADV7481 and ADV7482.
>>>
>>> The driver is modelled with 4 subdevices to allow simultaneous streaming
>>> from the AFE (Analog front end) and HDMI inputs though two CSI TX
>>> entities.
>>>
>>> The HDMI entity is linked to the TXA CSI bus, whilst the AFE is linked
>>> to the TXB CSI bus.
>>>
>>> The driver is based on a prototype by Koji Matsuoka in the Renesas BSP,
>>> and an earlier rework by Niklas Söderlund.
>>>
>>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>>> --- /dev/null
>>> +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
>>
>>> +static int adv748x_hdmi_set_pixelrate(struct adv748x_hdmi *hdmi)
>>> +{
>>> + struct v4l2_subdev *tx;
>>> + struct v4l2_dv_timings timings;
>>> + struct v4l2_bt_timings *bt = &timings.bt;
>>> + unsigned int fps;
>>> +
>>> + tx = adv748x_get_remote_sd(&hdmi->pads[ADV748X_HDMI_SOURCE]);
>>> + if (!tx)
>>> + return -ENOLINK;
>>> +
>>> + adv748x_hdmi_query_dv_timings(&hdmi->sd, &timings);
>>> +
>>> + fps = DIV_ROUND_CLOSEST(bt->pixelclock,
>>> + V4L2_DV_BT_FRAME_WIDTH(bt) *
>>> + V4L2_DV_BT_FRAME_HEIGHT(bt));
>>
>> On arm32:
>>
>> drivers/built-in.o: In function `adv748x_hdmi_set_pixelrate':
>> :(.text+0x1b8b1c): undefined reference to `__aeabi_uldivmod'
>>
>> v4l2_bt_timings.pixelclock is u64, so you should use DIV_ROUND_CLOSEST_ULL()
>> instead.
>
> Aha, thanks.
>
> /me ponders why I didn't get spammed from the bot-builders about this?
-EBUSY?
> Fix applied locally ready for v5.
>
> Would you like the remote updated for renesas-drivers or will you patch locally?
I'll patch it locally just to avoid receiving more spam from the builders soon
(we don't use adv748x on arm32 boards).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[toc] | [prev] | [next] | [standalone]
| From | Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> |
|---|---|
| Date | 2017-06-13 14:30 +0200 |
| Subject | Re: [PATCH v4 1/2] media: i2c: adv748x: add adv748x driver |
| Message-ID | <tRT7k-7UN-31@gated-at.bofh.it> |
| In reply to | #1664321 |
Hi Niklas
On 13/06/17 08:33, Niklas Söderlund wrote:
> Hi Kieran,
>
> Thanks for your patch, and great work!
Thanks for taking a look.
> On 2017-06-13 01:35:07 +0100, Kieran Bingham wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> Provide support for the ADV7481 and ADV7482.
>>
>> The driver is modelled with 4 subdevices to allow simultaneous streaming
>> from the AFE (Analog front end) and HDMI inputs though two CSI TX
>> entities.
>>
>> The HDMI entity is linked to the TXA CSI bus, whilst the AFE is linked
>> to the TXB CSI bus.
>>
>> The driver is based on a prototype by Koji Matsuoka in the Renesas BSP,
>> and an earlier rework by Niklas Söderlund.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
<snip>
>> +static int adv748x_afe_get_format(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *sdformat)
>> +{
>> + struct adv748x_afe *afe = adv748x_sd_to_afe(sd);
>> + struct v4l2_mbus_framefmt *mbusformat;
>> +
>> + /* The format of the analog sink pads is nonsensical */
>> + if (sdformat->pad != ADV748X_AFE_SOURCE)
>> + return -EINVAL;
>> +
>> + if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY) {
>> + mbusformat = v4l2_subdev_get_try_format(sd, cfg, sdformat->pad);
>> + sdformat->format = *mbusformat;
>> + } else {
>> + adv748x_afe_fill_format(afe, &sdformat->format);
>> + adv748x_afe_set_pixelrate(afe);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int adv748x_afe_set_format(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *sdformat)
>> +{
>> + struct v4l2_mbus_framefmt *mbusformat;
>> +
>> + /* The format of the analog sink pads is nonsensical */
>> + if (sdformat->pad != ADV748X_AFE_SOURCE)
>> + return -EINVAL;
>> +
>> + if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> + return adv748x_afe_get_format(sd, cfg, sdformat);
>> +
>> + mbusformat = v4l2_subdev_get_try_format(sd, cfg, sdformat->pad);
>> + *mbusformat = sdformat->format;
>
> Hum, for the V4L2_SUBDEV_FORMAT_TRY case will this not accept any format
> provided to the function? Should you not limit this to the device
> capabilities before assigning it to mbusformat ?
Hrmmm maybe it got too late last night :)
I was trying to remove the effect of the active setting on the TRY format, and
I've gone too far :)
>
>> +
>> + return 0;
>> +}
<snip>
>> +
>> +static int adv748x_setup_links(struct adv748x_state *state)
>> +{
>> + int ret;
>> + int enabled = MEDIA_LNK_FL_ENABLED;
>> +
>> +/*
>> + * HACK/Workaround:
>> + *
>> + * Currently non-immutable link resets go through the RVin
>> + * driver, and cause the links to fail, due to not being part of RVIN.
>> + * As a temporary workaround until the RVIN driver knows better than to parse
>> + * links that do not belong to it, use static immutable links for our internal
>> + * media paths.
>> + */
>
> The problem is a bigger then just the VIN ignoring the links not
> belonging to it self I think. The ADV7482 driver must have a link
> notification handler to deal with the links that belong to it.
>
> Else if all links of the media device is reset there is no way to setup
> the links between the different ADV7482 subdevices, or am I missing
> something?
Ahhh -- this function shouldn't even be in here ! It's not meant to be used -
Links are now created in adv748x_csi2_register_link() so now I'm concerned why I
didn't get an unused function compiler warning :)
However - your point still stands.
>
>> +#define ADV748x_DEV_STATIC_LINKS
>> +#ifdef ADV748x_DEV_STATIC_LINKS
>> + enabled |= MEDIA_LNK_FL_IMMUTABLE;
>> +#endif
>> +
>> + /* TXA - Default link is with HDMI */
>> + ret = media_create_pad_link(&state->hdmi.sd.entity, 1,
>> + &state->txa.sd.entity, 0, enabled);
>> + if (ret) {
>> + adv_err(state, "Failed to create HDMI-TXA pad link");
>> + return ret;
>> + }
>> +
>> +#ifndef ADV748x_DEV_STATIC_LINKS
>> + ret = media_create_pad_link(&state->afe.sd.entity, ADV748X_AFE_SOURCE,
>> + &state->txa.sd.entity, 0, 0);
>> + if (ret) {
>> + adv_err(state, "Failed to create AFE-TXA pad link");
>> + return ret;
>> + }
>> +#endif
>> +
>> + /* TXB - Can only output from the AFE */
>> + ret = media_create_pad_link(&state->afe.sd.entity, ADV748X_AFE_SOURCE,
>> + &state->txb.sd.entity, 0, enabled);
>> + if (ret) {
>> + adv_err(state, "Failed to create AFE-TXB pad link");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int adv748x_register_subdevs(struct adv748x_state *state,
>> + struct v4l2_device *v4l2_dev)
And that's why I didn't get a function unused warning from the compiler.
adv748x_setup_links() is called by adv748x_register_subdevs() which is not
static, but is never used.
I've just removed these two functions. - the new linking mechanism is handled
during the registration of the CSI2 entitiy.
However your consideration about links resetting is still valid - as I have only
been testing with immutable links which will have prevented me from seeing the
issues.
>> +{
>> + int ret;
>> +
>> + ret = v4l2_device_register_subdev(v4l2_dev, &state->hdmi.sd);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = v4l2_device_register_subdev(v4l2_dev, &state->afe.sd);
>> + if (ret < 0)
>> + goto err_unregister_hdmi;
>> +
>> + ret = adv748x_setup_links(state);
>> + if (ret < 0)
>> + goto err_unregister_afe;
>> +
>> + return 0;
>> +
>> +err_unregister_afe:
>> + v4l2_device_unregister_subdev(&state->afe.sd);
>> +err_unregister_hdmi:
>> + v4l2_device_unregister_subdev(&state->hdmi.sd);
>> +
>> + return ret;
>> +}
>> +
<snip>
>> +static int adv748x_csi2_get_format(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *sdformat)
>> +{
>> + struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>> + struct adv748x_state *state = tx->state;
>> + struct v4l2_mbus_framefmt *mbusformat;
>> +
>> + mbusformat = adv748x_csi2_get_pad_format(sd, cfg, sdformat->pad,
>> + sdformat->which);
>> + if (!mbusformat)
>> + return -EINVAL;
>> +
>> + mutex_lock(&state->mutex);
>
> Why do you need to take the lock here? I'm not saying it's wrong just
> curious :-)
>
I think the get/set formats are both userspace API's that 'could' be accessed in
parallel which read/modify the same context ...
>> +
>> + sdformat->format = *mbusformat;
>> +
>> + mutex_unlock(&state->mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static int adv748x_csi2_set_format(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *sdformat)
>> +{
>> + struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>> + struct adv748x_state *state = tx->state;
>> + struct v4l2_mbus_framefmt *mbusformat;
>> + int ret = 0;
>> +
>> + mbusformat = adv748x_csi2_get_pad_format(sd, cfg, sdformat->pad,
>> + sdformat->which);
>> + if (!mbusformat)
>> + return -EINVAL;
>> +
>> + mutex_lock(&state->mutex);
>
> Also why you need to lock here?
As above...
if this is extraneous I'll remove it.
>> +
>> +static int adv748x_hdmi_read_pixelclock(struct adv748x_state *state)
>> +{
>> + int a, b;
>> +
>> + a = hdmi_read(state, ADV748X_HDMI_TMDS_1);
>> + b = hdmi_read(state, ADV748X_HDMI_TMDS_2);
>> + if (a < 0 || b < 0)
>> + return -ENODATA;
>> +
>> + /*
>> + * The High 9 bits store TMDS frequency measurement in MHz
>
> s/High/high/
>
fixed.
>> + * The low 7 bits of TMDS_2 store the 7-bit TMDS fractional frequency
>> + * measurement in 1/128 MHz
>> + */
>> + return ((a << 1) | (b >> 7)) * 1000000 + (b & 0x7f) * 1000000 / 128;
>> +}
>> +
>> +/*
<snip>
>> +
>> +static int adv748x_hdmi_g_dv_timings(struct v4l2_subdev *sd,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
>> + struct adv748x_state *state = adv748x_hdmi_to_state(hdmi);
>> +
>> + mutex_lock(&state->mutex);
>
> Why you need to take the lock here?
User accessible get/setters...
But it doesn't look like other drivers do the same here.
Maybe I went overkill adding extra locking earlier.
>
>> +
>> + *timings = hdmi->timings;
>> +
>> + mutex_unlock(&state->mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static int adv748x_hdmi_query_dv_timings(struct v4l2_subdev *sd,
>> + struct v4l2_dv_timings *timings)
>> +{
>> + struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
>> + struct adv748x_state *state = adv748x_hdmi_to_state(hdmi);
>> + struct v4l2_bt_timings *bt = &timings->bt;
>> + int pixelclock;
>> + int polarity;
>> +
>> + if (!timings)
>> + return -EINVAL;
>> +
>> + memset(timings, 0, sizeof(struct v4l2_dv_timings));
>> +
>> + if (!adv748x_hdmi_has_signal(state))
>> + return -ENOLINK;
>> +
>> + pixelclock = adv748x_hdmi_read_pixelclock(state);
>> + if (pixelclock < 0)
>> + return -ENODATA;
>> +
>> + timings->type = V4L2_DV_BT_656_1120;
>> +
>> + bt->pixelclock = pixelclock;
>> + bt->interlaced = hdmi_read(state, ADV748X_HDMI_F1H1) &
>> + ADV748X_HDMI_F1H1_INTERLACED ?
>> + V4L2_DV_INTERLACED : V4L2_DV_PROGRESSIVE;
>> + bt->width = hdmi_read16(state, ADV748X_HDMI_LW1,
>> + ADV748X_HDMI_LW1_WIDTH_MASK);
>> + bt->height = hdmi_read16(state, ADV748X_HDMI_F0H1,
>> + ADV748X_HDMI_F0H1_HEIGHT_MASK);
>> + bt->hfrontporch = hdmi_read16(state, ADV748X_HDMI_HFRONT_PORCH,
>> + ADV748X_HDMI_HFRONT_PORCH_MASK);
>> + bt->hsync = hdmi_read16(state, ADV748X_HDMI_HSYNC_WIDTH,
>> + ADV748X_HDMI_HSYNC_WIDTH_MASK);
>> + bt->hbackporch = hdmi_read16(state, ADV748X_HDMI_HBACK_PORCH,
>> + ADV748X_HDMI_HBACK_PORCH_MASK);
>> + bt->vfrontporch = hdmi_read16(state, ADV748X_HDMI_VFRONT_PORCH,
>> + ADV748X_HDMI_VFRONT_PORCH_MASK) / 2;
>> + bt->vsync = hdmi_read16(state, ADV748X_HDMI_VSYNC_WIDTH,
>> + ADV748X_HDMI_VSYNC_WIDTH_MASK) / 2;
>> + bt->vbackporch = hdmi_read16(state, ADV748X_HDMI_VBACK_PORCH,
>> + ADV748X_HDMI_VBACK_PORCH_MASK) / 2;
>> +
>
> Extra newline.
Ah yes,
I just found a nice sed to catch and remove these automatically.
sed 'N;/^\n$/D;P;D;'
Picked up 2 more lines from -core.c :)
That should be part of checkpatch.pl somewhere ... Then I'd catch them as I commit.
>
>> +
>> + polarity = hdmi_read(state, 0x05);
>> + bt->polarities = (polarity & BIT(4) ? V4L2_DV_VSYNC_POS_POL : 0) |
>> + (polarity & BIT(5) ? V4L2_DV_HSYNC_POS_POL : 0);
>> +
>> + if (bt->interlaced == V4L2_DV_INTERLACED) {
>> + bt->height += hdmi_read16(state, 0x0b, 0x1fff);
>> + bt->il_vfrontporch = hdmi_read16(state, 0x2c, 0x3fff) / 2;
>> + bt->il_vsync = hdmi_read16(state, 0x30, 0x3fff) / 2;
>> + bt->il_vbackporch = hdmi_read16(state, 0x34, 0x3fff) / 2;
>> + }
>> +
>> + adv748x_fill_optional_dv_timings(timings);
>> +
>> + /*
>> + * No interrupt handling is implemented yet.
>> + * There should be an IRQ when a cable is plugged and the new timings
>> + * should be figured out and stored to state.
>> + */
>> + hdmi->timings = *timings;
>> +
>> + return 0;
>> +}
>> +
>> +static int adv748x_hdmi_g_input_status(struct v4l2_subdev *sd, u32 *status)
>> +{
>> + struct adv748x_hdmi *hdmi = adv748x_sd_to_hdmi(sd);
>> + struct adv748x_state *state = adv748x_hdmi_to_state(hdmi);
>> +
>> + mutex_lock(&state->mutex);
>
> Lock ? :-)
>
Now this one is talking to the i2c bus ... so I want to keep that - although now
I've converted to regmap - it might well be locking the bus for me ... I'll have
to check.
>> +
>> + *status = adv748x_hdmi_has_signal(state) ? 0 : V4L2_IN_ST_NO_SIGNAL;
>> +
>> + mutex_unlock(&state->mutex);
>> +
>> + return 0;
>> +}
>> +
<snip>
>> +
>> +static int adv748x_hdmi_set_format(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *sdformat)
>> +{
>> + struct v4l2_mbus_framefmt *mbusformat;
>> +
>> + if (sdformat->pad != ADV748X_HDMI_SOURCE)
>> + return -EINVAL;
>> +
>> + if (sdformat->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>> + return adv748x_hdmi_get_format(sd, cfg, sdformat);
>> +
>> + mbusformat = v4l2_subdev_get_try_format(sd, cfg, sdformat->pad);
>> + *mbusformat = sdformat->format;
>
> Same comment as for adv748x_afe_set_format().
Ack.
>
>> +
>> + return 0;
>> +}
>> +
Thanks :)
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web