Re: [PATCH v1] drm/tegra: dpaux: Fix always-failing probing of the driver

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4841 bytes --]

On Mon, Sep 24, 2018 at 04:56:28PM +0300, Dmitry Osipenko wrote:
> On 9/24/18 4:36 PM, Thierry Reding wrote:
> > On Mon, Sep 24, 2018 at 03:40:35PM +0300, Dmitry Osipenko wrote:
> > > On 9/24/18 3:32 PM, Jon Hunter wrote:
> > > > 
> > > > On 24/09/18 12:59, Thierry Reding wrote:
> > > > > On Fri, Sep 21, 2018 at 02:42:41PM +0300, Dmitry Osipenko wrote:
> > > > > > Some of definitions in the code changed the meaning, unfortunately one
> > > > > > place missed the change.
> > > > > > 
> > > > > > Fixes: 0751bb5c44fe ("drm/tegra: dpaux: Add pinctrl support")
> > > > > > Cc: <stable@vger.kernel.org> # v4.8+
> > > > > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> > > > > > ---
> > > > > > 
> > > > > > I don't have HW to test DPAUX driver, apparently it has been broken for
> > > > > > 2+ years now. There is also a known issue on with the DPAUX driver that
> > > > > > prevents it from probing, that was discussed on the #tegra IRC. Thierry,
> > > > > > please take a closer look at this driver and test it thoroughly, it has
> > > > > > some obvious problems.
> > > > > > 
> > > > > >    drivers/gpu/drm/tegra/dpaux.c | 2 +-
> > > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > It's odd that you claim that the driver is always failing probe and at
> > > > > the same time you say that you don't have hardware to test the driver.
> > > > > =)
> > > > > 
> > > > > I know for a fact that this driver does not usually fail because it is
> > > > > required on all recent chips (Tegra210 and later) to drive HDMI, which
> > > > > we support on all boards, so it is indeed thoroughly tested.
> > > > > 
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> > > > > > index d84e81ff36ad..ba5681fab73b 100644
> > > > > > --- a/drivers/gpu/drm/tegra/dpaux.c
> > > > > > +++ b/drivers/gpu/drm/tegra/dpaux.c
> > > > > > @@ -521,7 +521,7 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
> > > > > >    	 * is no possibility to perform the I2C mode configuration in the
> > > > > >    	 * HDMI path.
> > > > > >    	 */
> > > > > > -	err = tegra_dpaux_pad_config(dpaux, DPAUX_HYBRID_PADCTL_MODE_I2C);
> > > > > > +	err = tegra_dpaux_pad_config(dpaux, DPAUX_PADCTL_FUNC_I2C);
> > > > > >    	if (err < 0)
> > > > > >    		return err;
> > > > > 
> > > > > If you look at the definitions of both DPAUX_HYBRID_PADCTL_MODE_I2C and
> > > > > DPAUX_PADCTL_FUNC_I2C, you'll see that both are actually the same, which
> > > > > is a good explanation for why the driver performs flawlessly.
> > > > > 
> > > > > That said, your change is obviously correct. I've applied it, but since
> > > > > it doesn't actually fix anything, and doesn't change anything from a
> > > > > binary point of view, I've removed the Fixes: and Cc: stable tags.
> > > > 
> > > > Did you change the subject for the patch because as you mentioned it
> > > > does not seem related to the change?
> > > > 
> > > > Otherwise for the fix you can have my ...
> > > > 
> > > > Acked-by: Jon Hunter <jonathanh@nvidia.com>
> > > > 
> > > > The dpaux driver has been working fine for me on Tegra210 Smaug when
> > > > using the pins for I2C and I have not seen any probing problems.
> > > 
> > > Guy with nickname "vlado" said on the IRC that the panel stopped to work on
> > > T124 chromebook since 4.16 kernel, reverting commit [0] helps.
> > > 
> > > [0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/drm_dp_helper.c?id=17ab7806de0c10d27cdbda8ef57ca680bdd24315
> > 
> > I'm pretty sure that's something that we fixed, but I'll have to check.
> > 
> > Is this patch reported to help with that issue, or where's the
> > connection?
> 
> Reverting of the patch was reported to help with the problem. Check the IRC
> history, you can also just ask the original reporter for more details on
> #tegra. IIUC, the DPAUX driver probing is getting deferred because regulator
> isn't ready during of the first invocation and then for some reason driver
> is never getting re-probed.

My IRC logs are a bit spotty because I had some intermittent failure on
the server that my client was running on. But I think you're referring
to the drm_dp_helper.c patch. I was just getting confused because you
referred to it from this thread, and I don't see the connection between
the two.

Okay, so my other reply has all the background on why this is failing.
It would also explain why the driver "always" fails. panel-simple fails
to find the I2C adapter for the ddc-i2c-bus property, so it reports
-EPROBE_DEFER, which is then propagated to Tegra DRM.

So let me resume the discussion on the lists about the offending revert
and see if we can get that fixed.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel