Hi Jacopo,
On Thu, Mar 07, 2019 at 11:35:11AM +0100, Jacopo Mondi wrote:
> On Wed, Mar 06, 2019 at 09:15:21PM +0200, Laurent Pinchart wrote:
> > 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.
I think this needs more investigation. It feels to me that you're
working around an issue by chance, and it will come back to bite us
later :-(
> 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?
It also doesn't make too much sense to keep them both enabled when they
don't need to be :-) You'll end up consuming more power.
> >> 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
