Hi Jacopo,
Thank you for the patch.
On Tuesday, 11 December 2018 17:16:12 EET Jacopo Mondi wrote:
> Both the AFE and HDMI s_stream routines (adv748x_afe_s_stream() and
> adv748x_hdmi_s_stream()) have to enable the CSI-2 TX they are streaming
> video data to.
>
> With the introduction of dynamic routing between HDMI and AFE entities to
> TXA, the video stream sink needs to be set at run time, and not statically
> selected as the s_stream functions are currently doing.
>
> To fix this, store a reference to the active CSI-2 TX sink for both HDMI and
> AFE sources, and operate on it when starting/stopping the stream.
>
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
> ---
> drivers/media/i2c/adv748x/adv748x-afe.c | 2 +-
> drivers/media/i2c/adv748x/adv748x-csi2.c | 19 ++++++++++++++-----
> drivers/media/i2c/adv748x/adv748x-hdmi.c | 2 +-
> drivers/media/i2c/adv748x/adv748x.h | 4 ++++
> 4 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c
> b/drivers/media/i2c/adv748x/adv748x-afe.c index 71714634efb0..dbbb1e4d6363
> 100644
> --- a/drivers/media/i2c/adv748x/adv748x-afe.c
> +++ b/drivers/media/i2c/adv748x/adv748x-afe.c
> @@ -282,7 +282,7 @@ static int adv748x_afe_s_stream(struct v4l2_subdev *sd,
> int enable) goto unlock;
> }
>
> - ret = adv748x_tx_power(&state->txb, enable);
> + ret = adv748x_tx_power(afe->tx, enable);
> if (ret)
> goto unlock;
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c
> b/drivers/media/i2c/adv748x/adv748x-csi2.c index 307966f4c736..0d6344a51795
> 100644
> --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> @@ -85,6 +85,9 @@ static int adv748x_csi2_registered(struct v4l2_subdev *sd)
> MEDIA_LNK_FL_ENABLED);
> if (ret)
> return ret;
> +
> + /* The default HDMI output is TXA. */
> + state->hdmi.tx = tx;
> }
>
> if (is_afe_enabled(state)) {
> @@ -95,11 +98,17 @@ static int adv748x_csi2_registered(struct v4l2_subdev
> *sd) if (ret)
> return ret;
> }
> - } else if (is_afe_enabled(state))
> - return adv748x_csi2_register_link(tx, sd->v4l2_dev,
> - &state->afe.sd,
> - ADV748X_AFE_SOURCE,
> - MEDIA_LNK_FL_ENABLED);
> + } else if (is_afe_enabled(state)) {
> + ret = adv748x_csi2_register_link(tx, sd->v4l2_dev,
> + &state->afe.sd,
> + ADV748X_AFE_SOURCE,
> + MEDIA_LNK_FL_ENABLED);
> + if (ret)
> + return ret;
> +
> + /* The default AFE output is TXB. */
> + state->afe.tx = tx;
> + }
>
> return 0;
> }
> diff --git a/drivers/media/i2c/adv748x/adv748x-hdmi.c
> b/drivers/media/i2c/adv748x/adv748x-hdmi.c index 35d027941482..c557f8fdf11a
> 100644
> --- a/drivers/media/i2c/adv748x/adv748x-hdmi.c
> +++ b/drivers/media/i2c/adv748x/adv748x-hdmi.c
> @@ -358,7 +358,7 @@ static int adv748x_hdmi_s_stream(struct v4l2_subdev *sd,
> int enable)
>
> mutex_lock(&state->mutex);
>
> - ret = adv748x_tx_power(&state->txa, enable);
> + ret = adv748x_tx_power(hdmi->tx, enable);
> if (ret)
> goto done;
>
> diff --git a/drivers/media/i2c/adv748x/adv748x.h
> b/drivers/media/i2c/adv748x/adv748x.h index 387002d6da65..0ee3b8d5c795
> 100644
> --- a/drivers/media/i2c/adv748x/adv748x.h
> +++ b/drivers/media/i2c/adv748x/adv748x.h
> @@ -118,6 +118,8 @@ struct adv748x_hdmi {
> struct v4l2_dv_timings timings;
> struct v4l2_fract aspect_ratio;
>
> + struct adv748x_csi2 *tx;
> +
> struct {
> u8 edid[512];
> u32 present;
> @@ -148,6 +150,8 @@ struct adv748x_afe {
> struct v4l2_subdev sd;
> struct v4l2_mbus_framefmt format;
>
> + struct adv748x_csi2 *tx;
> +
> bool streaming;
> v4l2_std_id curr_norm;
> unsigned int input;
This may call for defining a common structure to store the common fields of
adv748x_hdmi and adv748x_afe. Out of scope for this patch, but please keep it
in mind.
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
--
Regards,
Laurent Pinchart
