Linux-Renesas-SoC Archive on lore.kernel.org
 help / color / Atom feed
From: "Niklas Söderlund" <niklas.soderlund@ragnatech.se>
To: Jacopo Mondi <jacopo+renesas@jmondi.org>
Cc: laurent.pinchart@ideasonboard.com,
	kieran.bingham@ideasonboard.com, linux-media@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org,
	Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Subject: Re: [PATCH v3 3/6] media: adv748x: csi2: Link AFE with TXA and TXB
Date: Mon, 14 Jan 2019 15:55:33 +0100
Message-ID: <20190114145533.GK30160@bigcity.dyn.berto.se> (raw)
In-Reply-To: <20190110140213.5198-4-jacopo+renesas@jmondi.org>

Hi Jacopo,

Thanks for your patch.

On 2019-01-10 15:02:10 +0100, Jacopo Mondi wrote:
> The ADV748x chip supports routing AFE output to either TXA or TXB.
> In order to support run-time configuration of video stream path, create an
> additional (not enabled) "AFE:8->TXA:0" link, and remove the IMMUTABLE flag
> from existing ones.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/i2c/adv748x/adv748x-csi2.c | 44 +++++++++++++-----------
>  1 file changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c
> index b6b5d8c7ea7c..8c3714495e11 100644
> --- a/drivers/media/i2c/adv748x/adv748x-csi2.c
> +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c
> @@ -27,6 +27,7 @@ static int adv748x_csi2_set_virtual_channel(struct adv748x_csi2 *tx,
>   * @v4l2_dev: Video registration device
>   * @src: Source subdevice to establish link
>   * @src_pad: Pad number of source to link to this @tx
> + * @enable: Link enabled flag
>   *
>   * Ensure that the subdevice is registered against the v4l2_device, and link the
>   * source pad to the sink pad of the CSI2 bus entity.
> @@ -34,17 +35,11 @@ static int adv748x_csi2_set_virtual_channel(struct adv748x_csi2 *tx,
>  static int adv748x_csi2_register_link(struct adv748x_csi2 *tx,
>  				      struct v4l2_device *v4l2_dev,
>  				      struct v4l2_subdev *src,
> -				      unsigned int src_pad)
> +				      unsigned int src_pad,
> +				      bool enable)
>  {
> -	int enabled = MEDIA_LNK_FL_ENABLED;
>  	int ret;
>  
> -	/*
> -	 * Dynamic linking of the AFE is not supported.
> -	 * Register the links as immutable.
> -	 */
> -	enabled |= MEDIA_LNK_FL_IMMUTABLE;
> -
>  	if (!src->v4l2_dev) {
>  		ret = v4l2_device_register_subdev(v4l2_dev, src);
>  		if (ret)
> @@ -53,7 +48,7 @@ static int adv748x_csi2_register_link(struct adv748x_csi2 *tx,
>  
>  	return media_create_pad_link(&src->entity, src_pad,
>  				     &tx->sd.entity, ADV748X_CSI2_SINK,
> -				     enabled);
> +				     enable ? MEDIA_LNK_FL_ENABLED : 0);
>  }
>  
>  /* -----------------------------------------------------------------------------
> @@ -68,25 +63,32 @@ static int adv748x_csi2_registered(struct v4l2_subdev *sd)
>  {
>  	struct adv748x_csi2 *tx = adv748x_sd_to_csi2(sd);
>  	struct adv748x_state *state = tx->state;
> +	int ret;
>  
>  	adv_dbg(state, "Registered %s (%s)", is_txa(tx) ? "TXA":"TXB",
>  			sd->name);
>  
>  	/*
> -	 * The adv748x hardware allows the AFE to route through the TXA, however
> -	 * this is not currently supported in this driver.
> +	 * Link TXA to AFE and HDMI, and TXB to AFE only as TXB cannot output
> +	 * HDMI.
>  	 *
> -	 * Link HDMI->TXA, and AFE->TXB directly.
> +	 * The HDMI->TXA link is enabled by default, as is the AFE->TXB one.
>  	 */
> -	if (is_txa(tx) && is_hdmi_enabled(state))
> -		return adv748x_csi2_register_link(tx, sd->v4l2_dev,
> -						  &state->hdmi.sd,
> -						  ADV748X_HDMI_SOURCE);
> -	if (is_txb(tx) && is_afe_enabled(state))
> -		return adv748x_csi2_register_link(tx, sd->v4l2_dev,
> -						  &state->afe.sd,
> -						  ADV748X_AFE_SOURCE);
> -	return 0;
> +	if (is_afe_enabled(state)) {
> +		ret = adv748x_csi2_register_link(tx, sd->v4l2_dev,
> +						 &state->afe.sd,
> +						 ADV748X_AFE_SOURCE,
> +						 is_txb(tx));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Register link to HDMI for TXA only. */
> +	if (is_txb(tx) || !is_hdmi_enabled(state))

Small nit, I would s/is_txb(tx)/!is_txa(tx)/ here as to me it becomes 
easier to read. With or without this change,

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> +		return 0;
> +
> +	return adv748x_csi2_register_link(tx, sd->v4l2_dev, &state->hdmi.sd,
> +					  ADV748X_HDMI_SOURCE, true);
>  }
>  
>  static const struct v4l2_subdev_internal_ops adv748x_csi2_internal_ops = {
> -- 
> 2.20.1
> 

-- 
Regards,
Niklas Söderlund

  reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-10 14:02 [PATCH v3 0/6] media: adv748x: Implement dynamic routing support Jacopo Mondi
2019-01-10 14:02 ` [PATCH v3 1/6] media: adv748x: Add is_txb() Jacopo Mondi
2019-01-14 14:47   ` Niklas Söderlund
2019-01-10 14:02 ` [PATCH v3 2/6] media: adv748x: Rename reset procedures Jacopo Mondi
2019-01-14 14:49   ` Niklas Söderlund
2019-01-10 14:02 ` [PATCH v3 3/6] media: adv748x: csi2: Link AFE with TXA and TXB Jacopo Mondi
2019-01-14 14:55   ` Niklas Söderlund [this message]
2019-01-16  9:10     ` Jacopo Mondi
2019-01-16 12:28       ` Niklas Söderlund
2019-01-16 13:44     ` Niklas Söderlund
2019-01-28 14:47       ` Jacopo Mondi
2019-02-07 15:57         ` Niklas Söderlund
2019-01-10 14:02 ` [PATCH v3 4/6] media: adv748x: Store the source subdevice in TX Jacopo Mondi
2019-01-16 13:33   ` Niklas Söderlund
2019-01-10 14:02 ` [PATCH v3 5/6] media: adv748x: Store the TX sink in HDMI/AFE Jacopo Mondi
2019-01-16 13:45   ` Niklas Söderlund
2019-01-10 14:02 ` [PATCH v3 6/6] media: adv748x: Implement TX link_setup callback Jacopo Mondi
2019-01-10 14:36   ` Kieran Bingham
2019-01-16 13:46   ` Niklas Söderlund
2019-01-14 14:22 ` [PATCH v3 0/6] media: adv748x: Implement dynamic routing support Kieran Bingham

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190114145533.GK30160@bigcity.dyn.berto.se \
    --to=niklas.soderlund@ragnatech.se \
    --cc=jacopo+renesas@jmondi.org \
    --cc=kieran.bingham+renesas@ideasonboard.com \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Renesas-SoC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-renesas-soc/0 linux-renesas-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-renesas-soc linux-renesas-soc/ https://lore.kernel.org/linux-renesas-soc \
		linux-renesas-soc@vger.kernel.org linux-renesas-soc@archiver.kernel.org
	public-inbox-index linux-renesas-soc


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-renesas-soc


AGPL code for this site: git clone https://public-inbox.org/ public-inbox