linux-kernel - Re: [PATCH 3/3] tda998x: add HPD delay to avoid disabling sound when EDID checksum fails.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160601163228.GF19428@n2100.arm.linux.org.uk>
Date:	Wed, 1 Jun 2016 17:32:28 +0100
From:	Russell King - ARM Linux <linux@...linux.org.uk>
To:	Joao Pinto <Joao.Pinto@...opsys.com>
Cc:	linux-kernel@...r.kernel.org, ijc+devicetree@...lion.org.uk,
	liviu.dudau@....com, ryan.harkin@...aro.org
Subject: Re: [PATCH 3/3] tda998x: add HPD delay to avoid disabling sound when
 EDID checksum fails.

On Tue, May 31, 2016 at 05:58:31PM +0100, Joao Pinto wrote:
> Hi Russell,
> 
> On 5/30/2016 8:10 PM, Russell King - ARM Linux wrote:
> > On Mon, May 30, 2016 at 04:15:54PM +0100, Joao Pinto wrote:
> >> When using ffplay to reproduce video+sound it was noticed that sometimes the
> >> sound was disabled. The cause was an initial EDID checksum error that disabled
> 
> (...)
> 
> >> @@ -1313,6 +1324,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
> >>  
> >>  		/* init read EDID waitqueue and HDP work */
> >>  		init_waitqueue_head(&priv->wq_edid);
> >> +		INIT_DELAYED_WORK(&priv->dwork, tda998x_hpd);
> >>  
> >>  		/* clear pending interrupts */
> >>  		reg_read(priv, REG_INT_FLAGS_0);
> > 
> > Clearly, this patch is incomplete.  There's nothing that schedules this
> > work to be run.
> 
> You are right, forgot to include the schedule in the patch!
> 
> > 
> > In any case, this is reintroducing the code which I deleted when I fixed
> > the (rather crappy) previous implemention of delaying the EDID read after
> > a hotplug event.  You should not need this patch.
> > 
> 
> If a checksum validation fails the video reproduction is done muted if
> you use a simple app like ffplay. This does not happen if using mplayer.

So?

We already delay the EDID read to work around reading a corrupted EDID.
If you need an additional delay, then it seems that the existing delay
is not long enough, and maybe we should extend it a little more.

What we should not do is to delay the HPD signalling.  That is definitely
the wrong solution.

In any case, I'm having a hard time understanding what the problem here
is.  You've unplugged the connected HDMI sink, so there's no longer a
display device connected, and the display hardware is shut down.  The
EDID becomes invalid.  You then plug in a HDMI sink, and now you have a
display device connected.  The EDID is read, and the display hardware is
configured according to the EDID details.  Video output is resumed, and
it takes a second or so for the sink to lock on and display the resulting
video.

At what point does the problem occur?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.