From: Kieran Bingham <kieran.bingham@ideasonboard.com>
To: Jacopo Mondi <jacopo+renesas@jmondi.org>,
laurent.pinchart@ideasonboard.com,
niklas.soderlund+renesas@ragnatech.se
Cc: linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH v2 3/6] media: adv748x: csi2: Link AFE with TXA and TXB
Date: Mon, 7 Jan 2019 10:35:52 +0000 [thread overview]
Message-ID: <1800f3fa-fb0d-f6b3-31bb-2de3e520c68a@ideasonboard.com> (raw)
In-Reply-To: <20190106155413.30666-4-jacopo+renesas@jmondi.org>
Hi Jacopo,
On 06/01/2019 15:54, 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.
Great - this looks good to me.
Small nit below - but it's not a big deal.
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..9d391d6f752e 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 the AFE->TXB is.
I might have worded this "as is the AFE->TXB." but it's understandable.
I can update this when applying if you wish.
> */
> - 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))
> + 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 = {
>
--
Regards
--
Kieran
next prev parent reply other threads:[~2019-01-07 10:35 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-06 15:54 [PATCH v2 0/6] media: adv748x: Implement dynamic routing support Jacopo Mondi
2019-01-06 15:54 ` [PATCH v2 1/6] media: adv748x: Add is_txb() Jacopo Mondi
2019-01-07 9:49 ` Kieran Bingham
2019-01-07 10:05 ` Jacopo Mondi
2019-01-07 10:38 ` Kieran Bingham
2019-01-09 0:04 ` Laurent Pinchart
2019-01-06 15:54 ` [PATCH v2 2/6] media: adv748x: Rename reset procedures Jacopo Mondi
2019-01-07 9:59 ` Kieran Bingham
2019-01-06 15:54 ` [PATCH v2 3/6] media: adv748x: csi2: Link AFE with TXA and TXB Jacopo Mondi
2019-01-07 10:35 ` Kieran Bingham [this message]
2019-01-09 0:11 ` Laurent Pinchart
2019-01-06 15:54 ` [PATCH v2 4/6] media: adv748x: Store the source subdevice in TX Jacopo Mondi
2019-01-07 10:41 ` Kieran Bingham
2019-01-06 15:54 ` [PATCH v2 5/6] media: adv748x: Store the TX sink in HDMI/AFE Jacopo Mondi
2019-01-07 10:45 ` Kieran Bingham
2019-01-06 15:54 ` [PATCH v2 6/6] media: adv748x: Implement TX link_setup callback Jacopo Mondi
2019-01-07 12:36 ` Kieran Bingham
2019-01-09 0:15 ` Laurent Pinchart
2019-01-09 14:15 ` Kieran Bingham
2019-01-10 8:58 ` Jacopo Mondi
2019-01-10 10:05 ` Kieran Bingham
2019-01-10 8:51 ` Jacopo Mondi
2019-01-10 9:27 ` Laurent Pinchart
2019-01-09 14:17 ` Kieran Bingham
2019-01-10 10:01 ` Jacopo Mondi
2019-01-10 13:40 ` Jacopo Mondi
Reply instructions:
You may reply publicly 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=1800f3fa-fb0d-f6b3-31bb-2de3e520c68a@ideasonboard.com \
--to=kieran.bingham@ideasonboard.com \
--cc=jacopo+renesas@jmondi.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=niklas.soderlund+renesas@ragnatech.se \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).