| Message ID | 1553874620-31468-1-git-send-email-yannick.fertre@st.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm/stm: ltdc: fix data enable polarity | expand |
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é <yannick.fertre@st.com> > --- > 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)
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é <yannick.fertre@st.com> > --- > 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)
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 <philippe.cornu@st.com> Philippe :-) >> >> Signed-off-by: Yannick Fertré <yannick.fertre@st.com> >> --- >> 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)
Le lun. 1 avr. 2019 à 11:18, Philippe CORNU <philippe.cornu@st.com> 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 <philippe.cornu@st.com> > Applied on drm-misc-next, Thanks, Benjamin > Philippe :-) > > > >> > >> Signed-off-by: Yannick Fertré <yannick.fertre@st.com> > >> --- > >> 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 > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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)
Wrong DISPLAY_FLAGS used to set the data enable polarity. Signed-off-by: Yannick Fertré <yannick.fertre@st.com> --- drivers/gpu/drm/stm/ltdc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)