From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755731AbcFAQcm (ORCPT ); Wed, 1 Jun 2016 12:32:42 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:47507 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751570AbcFAQck (ORCPT ); Wed, 1 Jun 2016 12:32:40 -0400 Date: Wed, 1 Jun 2016 17:32:28 +0100 From: Russell King - ARM Linux To: Joao Pinto Cc: linux-kernel@vger.kernel.org, ijc+devicetree@hellion.org.uk, liviu.dudau@arm.com, ryan.harkin@linaro.org Subject: Re: [PATCH 3/3] tda998x: add HPD delay to avoid disabling sound when EDID checksum fails. Message-ID: <20160601163228.GF19428@n2100.arm.linux.org.uk> References: <2f242f545c99fc604fb70eeffcfd5d7703ace18d.1464619185.git.jpinto@synopsys.com> <20160530191044.GR19428@n2100.arm.linux.org.uk> <801ce163-b8ac-3375-56f2-9a8d7cd70ded@synopsys.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <801ce163-b8ac-3375-56f2-9a8d7cd70ded@synopsys.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.