2019-03-29 15:51:44

by Yannick FERTRE

[permalink] [raw]
Subject: [PATCH] drm/stm: ltdc: fix data enable polarity

Wrong DISPLAY_FLAGS used to set the data enable polarity.

Signed-off-by: Yannick Fertré <[email protected]>
---
drivers/gpu/drm/stm/ltdc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index b1741a9..6ba326a 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -555,7 +555,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
val |= GCR_VSPOL;

- if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
+ if (vm.flags & DISPLAY_FLAGS_DE_LOW)
val |= GCR_DEPOL;

if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
--
2.7.4



2019-03-29 21:34:00

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH] drm/stm: ltdc: fix data enable polarity


On 3/29/2019 9:20 PM, Yannick Fertré wrote:
> Wrong DISPLAY_FLAGS used to set the data enable polarity.
Used or checked.
Can you also explain how it is wrong to check against this FLAG in commit?
>
> Signed-off-by: Yannick Fertré <[email protected]>
> ---
> drivers/gpu/drm/stm/ltdc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index b1741a9..6ba326a 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -555,7 +555,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
> if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
> val |= GCR_VSPOL;
>
> - if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> + if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> val |= GCR_DEPOL;
>
> if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)

2019-03-29 21:35:35

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH] drm/stm: ltdc: fix data enable polarity


On 3/29/2019 9:20 PM, Yannick Fertré wrote:
> Wrong DISPLAY_FLAGS used to set the data enable polarity.
Used or checked?
Can you also explain how it is wrong to check against this FLAG in commit?
>
> Signed-off-by: Yannick Fertré <[email protected]>
> ---
> drivers/gpu/drm/stm/ltdc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index b1741a9..6ba326a 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -555,7 +555,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
> if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
> val |= GCR_VSPOL;
>
> - if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> + if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> val |= GCR_DEPOL;
>
> if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)

2019-04-01 09:19:54

by Philippe Cornu

[permalink] [raw]
Subject: Re: [PATCH] drm/stm: ltdc: fix data enable polarity



On 3/29/19 10:32 PM, Mukesh Ojha wrote:
>
> On 3/29/2019 9:20 PM, Yannick Fertré wrote:
>> Wrong DISPLAY_FLAGS used to set the data enable polarity.
> Used or checked?
> Can you also explain how it is wrong to check against this FLAG in commit?

Dear Yannick,
Many thanks for your patch,

Dear Mukesh,
Many thanks for your comment,

Looking deeper in the stm32 LTDC Ref man chapter, we can read:

Bit 30 VSPOL: vertical synchronization polarity
This bit is set and cleared by software.
0: vertical synchronization is active low.
1: vertical synchronization is active high.

Bit 29 DEPOL: not data enable polarity
This bit is set and cleared by software.
0: not data enable polarity is active low.
1: not data enable polarity is active high.

So I suggest the following commit message:
"According to the STM32 LTDC documentation, GCR_DEPOL bit need to be set
with DISPLAY_FLAGS_DE_LOW but not with DISPLAY_FLAGS_DE_HIGH."

With that or something similar

Acked-by: Philippe Cornu <[email protected]>

Philippe :-)


>>
>> Signed-off-by: Yannick Fertré <[email protected]>
>> ---
>> drivers/gpu/drm/stm/ltdc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
>> index b1741a9..6ba326a 100644
>> --- a/drivers/gpu/drm/stm/ltdc.c
>> +++ b/drivers/gpu/drm/stm/ltdc.c
>> @@ -555,7 +555,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
>> if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
>> val |= GCR_VSPOL;
>>
>> - if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
>> + if (vm.flags & DISPLAY_FLAGS_DE_LOW)
>> val |= GCR_DEPOL;
>>
>> if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)

2019-04-24 19:36:14

by Benjamin Gaignard

[permalink] [raw]
Subject: Re: [PATCH] drm/stm: ltdc: fix data enable polarity

Le lun. 1 avr. 2019 à 11:18, Philippe CORNU <[email protected]> a écrit :
>
>
>
> On 3/29/19 10:32 PM, Mukesh Ojha wrote:
> >
> > On 3/29/2019 9:20 PM, Yannick Fertré wrote:
> >> Wrong DISPLAY_FLAGS used to set the data enable polarity.
> > Used or checked?
> > Can you also explain how it is wrong to check against this FLAG in commit?
>
> Dear Yannick,
> Many thanks for your patch,
>
> Dear Mukesh,
> Many thanks for your comment,
>
> Looking deeper in the stm32 LTDC Ref man chapter, we can read:
>
> Bit 30 VSPOL: vertical synchronization polarity
> This bit is set and cleared by software.
> 0: vertical synchronization is active low.
> 1: vertical synchronization is active high.
>
> Bit 29 DEPOL: not data enable polarity
> This bit is set and cleared by software.
> 0: not data enable polarity is active low.
> 1: not data enable polarity is active high.
>
> So I suggest the following commit message:
> "According to the STM32 LTDC documentation, GCR_DEPOL bit need to be set
> with DISPLAY_FLAGS_DE_LOW but not with DISPLAY_FLAGS_DE_HIGH."
>
> With that or something similar
>
> Acked-by: Philippe Cornu <[email protected]>
>

Applied on drm-misc-next,
Thanks,
Benjamin

> Philippe :-)
>
>
> >>
> >> Signed-off-by: Yannick Fertré <[email protected]>
> >> ---
> >> drivers/gpu/drm/stm/ltdc.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> >> index b1741a9..6ba326a 100644
> >> --- a/drivers/gpu/drm/stm/ltdc.c
> >> +++ b/drivers/gpu/drm/stm/ltdc.c
> >> @@ -555,7 +555,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
> >> if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
> >> val |= GCR_VSPOL;
> >>
> >> - if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> >> + if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> >> val |= GCR_DEPOL;
> >>
> >> if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel