Re: [PATCH] media: adv748x: Don't disable CSI-2 on link_setup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

On Wed, Mar 06, 2019 at 09:15:21PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wed, Mar 06, 2019 at 12:26:59PM +0100, Jacopo Mondi wrote:
> > When both the media links between AFE and HDMI and the two TX CSI-2 outputs
> > gets disabled, the routing register ADV748X_IO_10 gets zeroed causing both
> > TXA and TXB output to get disabled.
> >
> > This causes some HDMI transmitters to stop working after both AFE and
> > HDMI links are disabled.
>
> Could you elaborate on why this would be the case ? By HDMI transmitter,
> I assume you mean the device connected to the HDMI input of the ADV748x.
> Why makes it fail (and how ?) when the TXA and TXB are both disabled ?
>

I know, it's weird, the HDMI transmitter is connected to the HDMI
input of adv748x and should not be bothered by CSI-2 outputs
enablement/disablement.

BUT, when I developed the initial adv748x AFE->TXA patches I was
testing HDMI capture using a laptop, and things were smooth.

I recently started using a chrome cast device I found in some drawer
to test HDMI, as with it I don't need to go through xrandr as I had to
do when using a laptop for testing, but it seems the two behaves differently.

Failures are of different types: from detecting a non-realisting
resolution from the HDMI subdevice, and then messing up the pipeline
configuration, to capture operations apparently completing properly
but resulting in mangled images.

Do not deactivate the CSI-2 ouputs seems to fix the issue for the
Chromecast, and still work when capturing from laptop. There might be
something I am missing about HDMI maybe, but the patch not just fixes
the issue for me, but it might make sense on its own as disabling the
TXes might trigger some internal power saving state, or simply mess up
the HDMI link.

As disabling both TXes usually happens at media link reset time, just
before enabling one of them (or both), going through a full disable
makes little sense, even more if it triggers any sort of malfunctioning.

Does this make sense to you?

Thanks
  j

> > Fix this by preventing writing 0 to
> > ADV748X_IO_10 register, which gets only updated when links are enabled
> > again.
> >
> > Fixes: 9423ca350df7 ("media: adv748x: Implement TX link_setup callback")
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> > ---
> > The issue presents itself only on some HDMI transmitters, and went unnoticed
> > during the development of:
> > "[PATCH v3 0/6] media: adv748x: Implement dynamic routing support"
> >
> > Patch intended to be applied on top of latest media-master, where the
> > "[PATCH v3 0/6] media: adv748x: Implement dynamic routing support"
> > series is applied.
> >
> > The patch reports a "Fixes" tag, but should actually be merged with the above
> > mentioned series.
> >
> > ---
> >  drivers/media/i2c/adv748x/adv748x-core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/media/i2c/adv748x/adv748x-core.c b/drivers/media/i2c/adv748x/adv748x-core.c
> > index f57cd77a32fa..0e5a75eb6d75 100644
> > --- a/drivers/media/i2c/adv748x/adv748x-core.c
> > +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> > @@ -354,6 +354,9 @@ static int adv748x_link_setup(struct media_entity *entity,
> >
> >  	tx->src = enable ? rsd : NULL;
> >
> > +	if (!enable)
> > +		return 0;
> > +
> >  	if (state->afe.tx) {
> >  		/* AFE Requires TXA enabled, even when output to TXB */
> >  		io10 |= ADV748X_IO_10_CSI4_EN;
>
> --
> Regards,
>
> Laurent Pinchart

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux