From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Jacopo Mondi <jacopo+renesas@jmondi.org>
Cc: niklas.soderlund+renesas@ragnatech.se,
kieran.bingham@ideasonboard.com, 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: Wed, 09 Jan 2019 02:11:47 +0200 [thread overview]
Message-ID: <2046845.fJaF8tx5Pb@avalon> (raw)
In-Reply-To: <20190106155413.30666-4-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thank you for the patch.
On Sunday, 6 January 2019 17:54:10 EET 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.
>
> 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.
> */
> - 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. */
I would have written "Create link", but then realized that the function is
named adv748x_csi2_register_link(). Looking at its definition, I now see this
means "register and link". That seems a bit of a hack, especially seeing how
double registration is skipped in the function by checking src->v4l2_dev.
Kieran, could this be fixed ?
> + 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,
Laurent Pinchart
next prev parent reply other threads:[~2019-01-09 0:10 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
2019-01-09 0:11 ` Laurent Pinchart [this message]
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=2046845.fJaF8tx5Pb@avalon \
--to=laurent.pinchart@ideasonboard.com \
--cc=jacopo+renesas@jmondi.org \
--cc=kieran.bingham@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).