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

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

[Dmitry Osipenko]

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(-)

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;
 
-- 
2.19.0

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

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

[Thierry Reding]

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

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.

Thanks,
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

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

[Jon Hunter]

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.

Cheers
Jon

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

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

[Dmitry Osipenko]

On 9/24/18 2:59 PM, 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.

That's the great news! Somehow it even didn't strike me to check that the actual values 
are the same. In a longer term it should be better to use enum's to avoid such confusions 
in the future, compiler will complain about the wrong types.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Dmitry Osipenko]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Thierry Reding]

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

On Mon, Sep 24, 2018 at 01:32:41PM +0100, 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?

Yes, I did.

> Otherwise for the fix you can have my ...
> 
> Acked-by: Jon Hunter <jonathanh@nvidia.com>

Added, thanks.

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

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

[Thierry Reding]

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

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?

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

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

[Dmitry Osipenko]

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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Thierry Reding]

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

On Mon, Sep 24, 2018 at 03:36:29PM +0200, 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.

Looks like we didn't fix that after all. Going through the patchwork log
linked to from that commit message I thought there wasn't actually an
issue with the code, but it turns out that I was wrong. The problem is
that for HDMI we don't use DPAUX in I2C mode, so we don't need to look
up the corresponding I2C controller and we just need the DPAUX driver to
program the pins into I2C mode, then use a real I2C controller for
communication.

What's special about Nyan is that it uses eDP and for eDP we use the
DPAUX for access to the DPCD *and* for DDC. eDP panels will look up the
phandle to the I2C controller used for EDID (DDC) via the ddc-i2c-bus
property. In order for that to work, we need to register the DPAUX I2C
controller with its OF node set to that of the DPAUX device.

Technically I don't think we need to query the EDID because we always
have at least one hard-coded mode in panel-simple. However, if the panel
has a slightly different mode from that, it'd still be good if the
driver used that instead of the built-in mode.

So I think we need to either bring back that patch or find some
alternative way to make the panel aware of the DDC bus that it needs to
talk to.

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

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

[Thierry Reding]

[-- 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

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

[Dmitry Osipenko]

On 9/24/18 5:04 PM, Thierry Reding wrote:
> On Mon, Sep 24, 2018 at 03:36:29PM +0200, 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.
> 
> Looks like we didn't fix that after all. Going through the patchwork log
> linked to from that commit message I thought there wasn't actually an
> issue with the code, but it turns out that I was wrong. The problem is
> that for HDMI we don't use DPAUX in I2C mode, so we don't need to look
> up the corresponding I2C controller and we just need the DPAUX driver to
> program the pins into I2C mode, then use a real I2C controller for
> communication.
> 
> What's special about Nyan is that it uses eDP and for eDP we use the
> DPAUX for access to the DPCD *and* for DDC. eDP panels will look up the
> phandle to the I2C controller used for EDID (DDC) via the ddc-i2c-bus
> property. In order for that to work, we need to register the DPAUX I2C
> controller with its OF node set to that of the DPAUX device.
> 
> Technically I don't think we need to query the EDID because we always
> have at least one hard-coded mode in panel-simple. However, if the panel
> has a slightly different mode from that, it'd still be good if the
> driver used that instead of the built-in mode.

That also could be bad. The panels EDID mode worked fine in the past,
but it didn't work when I tried it last time. The timing was incorrect
or something like that, maybe something is wrong with the mode parser
(or HW).
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel