2018-09-24 11:48:57

by Yannick FERTRE

[permalink] [raw]
Subject: [PATCH v1 0/2] Manage pixel clock & data enable polarities

Version 1:
- Initial commit

This serie contains all patchsets needed for control the pixel clock & data
enable polarities by the display controller driver.


Yannick Fertré (2):
drm: Add missing flags for pixel clock & data enable
drm/stm: ltdc: Solve issue on pixel clock & data enable polarity

drivers/gpu/drm/drm_modes.c | 19 ++++++++++++++++++-
drivers/gpu/drm/stm/ltdc.c | 23 +++++++++++++++++++----
include/uapi/drm/drm_mode.h | 6 ++++++
3 files changed, 43 insertions(+), 5 deletions(-)

--
2.7.4



2018-09-24 11:38:20

by Yannick FERTRE

[permalink] [raw]
Subject: [PATCH v1 1/2] drm: Add missing flags for pixel clock & data enable

Add missing flags for pixel clock & data enable polarities.
These flags are similar to other synchronization signals (hsync, vsync...).

Signed-off-by: Yannick Fertré <[email protected]>
---
drivers/gpu/drm/drm_modes.c | 19 ++++++++++++++++++-
include/uapi/drm/drm_mode.h | 6 ++++++
2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 02db9ac..596f8b3 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -130,7 +130,7 @@ EXPORT_SYMBOL(drm_mode_probed_add);
* according to the hdisplay, vdisplay, vrefresh.
* It is based from the VESA(TM) Coordinated Video Timing Generator by
* Graham Loveridge April 9, 2003 available at
- * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
+ * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
*
* And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c.
* What I have done is to translate it by using integer calculation.
@@ -611,6 +611,15 @@ void drm_display_mode_from_videomode(const struct videomode *vm,
dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
dmode->flags |= DRM_MODE_FLAG_DBLCLK;
+ if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+ dmode->flags |= DRM_MODE_FLAG_PPIXCLK;
+ else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+ dmode->flags |= DRM_MODE_FLAG_NPIXCLK;
+ if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+ dmode->flags |= DRM_MODE_FLAG_PDATAEN;
+ else if (vm->flags & DISPLAY_FLAGS_DE_LOW)
+ dmode->flags |= DRM_MODE_FLAG_NDE;
+
drm_mode_set_name(dmode);
}
EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
@@ -652,6 +661,14 @@ void drm_display_mode_to_videomode(const struct drm_display_mode *dmode,
vm->flags |= DISPLAY_FLAGS_DOUBLESCAN;
if (dmode->flags & DRM_MODE_FLAG_DBLCLK)
vm->flags |= DISPLAY_FLAGS_DOUBLECLK;
+ if (dmode->flags & DRM_MODE_FLAG_PPIXDATA)
+ vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
+ else if (dmode->flags & DRM_MODE_FLAG_NPIXDATA)
+ vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
+ if (dmode->flags & DRM_MODE_FLAG_PDE)
+ vm->flags |= DISPLAY_FLAGS_DE_HIGH;
+ else if (dmode->flags & DRM_MODE_FLAG_NDE)
+ vm->flags |= DISPLAY_FLAGS_DE_LOW;
}
EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index d3e0fe3..b335a17 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -89,6 +89,12 @@ extern "C" {
#define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14)
#define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14)

+/* flags for polarity clock & data enable polarities */
+#define DRM_MODE_FLAG_PPIXDATA (1 << 19)
+#define DRM_MODE_FLAG_NPIXDATA (1 << 20)
+#define DRM_MODE_FLAG_PDE (1 << 21)
+#define DRM_MODE_FLAG_NDE (1 << 22)
+
/* Picture aspect ratio options */
#define DRM_MODE_PICTURE_ASPECT_NONE 0
#define DRM_MODE_PICTURE_ASPECT_4_3 1
--
2.7.4


2018-09-24 11:48:47

by Yannick FERTRE

[permalink] [raw]
Subject: [PATCH v1 2/2] drm/stm: ltdc: Solve issue on pixel clock & data enable polarity

Wrong flags used for set the pixel clock & data enable polarities.
Add trace for polarities of hsync, vsync, data enabled & pixel clock.

Signed-off-by: Yannick Fertré <[email protected]>
---
drivers/gpu/drm/stm/ltdc.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 808d9fb..f671abc 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -517,7 +517,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
struct videomode vm;
u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h;
u32 total_width, total_height;
- u32 val;
+ u32 val = 0;

drm_display_mode_to_videomode(mode, &vm);

@@ -538,7 +538,22 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
total_height = accum_act_h + vm.vfront_porch;

/* Configures the HS, VS, DE and PC polarities. Default Active Low */
- val = 0;
+ if (vm.flags & DISPLAY_FLAGS_HSYNC_LOW)
+ DRM_DEBUG_DRIVER("Horizontal Synchronization polarity is active low");
+ if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH)
+ DRM_DEBUG_DRIVER("Horizontal Synchronization polarity is active high");
+ if (vm.flags & DISPLAY_FLAGS_VSYNC_LOW)
+ DRM_DEBUG_DRIVER("Vertical Synchronization polarity is active low");
+ if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
+ DRM_DEBUG_DRIVER("Vertical Synchronization polarity is active high");
+ if (vm.flags & DISPLAY_FLAGS_DE_LOW)
+ DRM_DEBUG_DRIVER("Data Enable polarity is active low");
+ if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
+ DRM_DEBUG_DRIVER("Data Enable polarity is active high");
+ if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+ DRM_DEBUG_DRIVER("Pixel clock polarity is active low");
+ if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+ DRM_DEBUG_DRIVER("Pixel clock polarity is active high");

if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH)
val |= GCR_HSPOL;
@@ -546,10 +561,10 @@ 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)
+ if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
val |= GCR_PCPOL;

reg_update_bits(ldev->regs, LTDC_GCR,
--
2.7.4


2018-10-15 11:16:20

by Benjamin Gaignard

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] drm: Add missing flags for pixel clock & data enable

Le lun. 24 sept. 2018 à 13:59, Yannick Fertré <[email protected]> a écrit :
>
> Add missing flags for pixel clock & data enable polarities.
> These flags are similar to other synchronization signals (hsync, vsync...).
>
> Signed-off-by: Yannick Fertré <[email protected]>

Reviewed-by: Benjamin Gaignard <[email protected]>

> ---
> drivers/gpu/drm/drm_modes.c | 19 ++++++++++++++++++-
> include/uapi/drm/drm_mode.h | 6 ++++++
> 2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 02db9ac..596f8b3 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -130,7 +130,7 @@ EXPORT_SYMBOL(drm_mode_probed_add);
> * according to the hdisplay, vdisplay, vrefresh.
> * It is based from the VESA(TM) Coordinated Video Timing Generator by
> * Graham Loveridge April 9, 2003 available at
> - * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> + * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> *
> * And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c.
> * What I have done is to translate it by using integer calculation.
> @@ -611,6 +611,15 @@ void drm_display_mode_from_videomode(const struct videomode *vm,
> dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
> dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> + if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> + dmode->flags |= DRM_MODE_FLAG_PPIXCLK;
> + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> + dmode->flags |= DRM_MODE_FLAG_NPIXCLK;
> + if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> + dmode->flags |= DRM_MODE_FLAG_PDATAEN;
> + else if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> + dmode->flags |= DRM_MODE_FLAG_NDE;
> +
> drm_mode_set_name(dmode);
> }
> EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
> @@ -652,6 +661,14 @@ void drm_display_mode_to_videomode(const struct drm_display_mode *dmode,
> vm->flags |= DISPLAY_FLAGS_DOUBLESCAN;
> if (dmode->flags & DRM_MODE_FLAG_DBLCLK)
> vm->flags |= DISPLAY_FLAGS_DOUBLECLK;
> + if (dmode->flags & DRM_MODE_FLAG_PPIXDATA)
> + vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
> + else if (dmode->flags & DRM_MODE_FLAG_NPIXDATA)
> + vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
> + if (dmode->flags & DRM_MODE_FLAG_PDE)
> + vm->flags |= DISPLAY_FLAGS_DE_HIGH;
> + else if (dmode->flags & DRM_MODE_FLAG_NDE)
> + vm->flags |= DISPLAY_FLAGS_DE_LOW;
> }
> EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index d3e0fe3..b335a17 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -89,6 +89,12 @@ extern "C" {
> #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14)
> #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14)
>
> +/* flags for polarity clock & data enable polarities */
> +#define DRM_MODE_FLAG_PPIXDATA (1 << 19)
> +#define DRM_MODE_FLAG_NPIXDATA (1 << 20)
> +#define DRM_MODE_FLAG_PDE (1 << 21)
> +#define DRM_MODE_FLAG_NDE (1 << 22)
> +
> /* Picture aspect ratio options */
> #define DRM_MODE_PICTURE_ASPECT_NONE 0
> #define DRM_MODE_PICTURE_ASPECT_4_3 1
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

2018-10-15 11:16:34

by Benjamin Gaignard

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] drm/stm: ltdc: Solve issue on pixel clock & data enable polarity

Le lun. 24 sept. 2018 à 14:05, Yannick Fertré <[email protected]> a écrit :
>
> Wrong flags used for set the pixel clock & data enable polarities.
> Add trace for polarities of hsync, vsync, data enabled & pixel clock.
>
> Signed-off-by: Yannick Fertré <[email protected]>
Reviewed-by: Benjamin Gaignard <[email protected]>
> ---
> drivers/gpu/drm/stm/ltdc.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 808d9fb..f671abc 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -517,7 +517,7 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
> struct videomode vm;
> u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h;
> u32 total_width, total_height;
> - u32 val;
> + u32 val = 0;
>
> drm_display_mode_to_videomode(mode, &vm);
>
> @@ -538,7 +538,22 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
> total_height = accum_act_h + vm.vfront_porch;
>
> /* Configures the HS, VS, DE and PC polarities. Default Active Low */
> - val = 0;
> + if (vm.flags & DISPLAY_FLAGS_HSYNC_LOW)
> + DRM_DEBUG_DRIVER("Horizontal Synchronization polarity is active low");
> + if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH)
> + DRM_DEBUG_DRIVER("Horizontal Synchronization polarity is active high");
> + if (vm.flags & DISPLAY_FLAGS_VSYNC_LOW)
> + DRM_DEBUG_DRIVER("Vertical Synchronization polarity is active low");
> + if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH)
> + DRM_DEBUG_DRIVER("Vertical Synchronization polarity is active high");
> + if (vm.flags & DISPLAY_FLAGS_DE_LOW)
> + DRM_DEBUG_DRIVER("Data Enable polarity is active low");
> + if (vm.flags & DISPLAY_FLAGS_DE_HIGH)
> + DRM_DEBUG_DRIVER("Data Enable polarity is active high");
> + if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> + DRM_DEBUG_DRIVER("Pixel clock polarity is active low");
> + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> + DRM_DEBUG_DRIVER("Pixel clock polarity is active high");
>
> if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH)
> val |= GCR_HSPOL;
> @@ -546,10 +561,10 @@ 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)
> + if (vm.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> val |= GCR_PCPOL;
>
> reg_update_bits(ldev->regs, LTDC_GCR,
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

2018-10-23 14:51:21

by Benjamin Gaignard

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] drm: Add missing flags for pixel clock & data enable

Le lun. 15 oct. 2018 à 13:15, Benjamin Gaignard
<[email protected]> a écrit :
>
> Le lun. 24 sept. 2018 à 13:59, Yannick Fertré <[email protected]> a écrit :
> >
> > Add missing flags for pixel clock & data enable polarities.
> > These flags are similar to other synchronization signals (hsync, vsync...).
> >
> > Signed-off-by: Yannick Fertré <[email protected]>
>
> Reviewed-by: Benjamin Gaignard <[email protected]>

Dave or Daniel could you give us your PoV on this patch ?
Thanks

>
> > ---
> > drivers/gpu/drm/drm_modes.c | 19 ++++++++++++++++++-
> > include/uapi/drm/drm_mode.h | 6 ++++++
> > 2 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index 02db9ac..596f8b3 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -130,7 +130,7 @@ EXPORT_SYMBOL(drm_mode_probed_add);
> > * according to the hdisplay, vdisplay, vrefresh.
> > * It is based from the VESA(TM) Coordinated Video Timing Generator by
> > * Graham Loveridge April 9, 2003 available at
> > - * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> > + * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> > *
> > * And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c.
> > * What I have done is to translate it by using integer calculation.
> > @@ -611,6 +611,15 @@ void drm_display_mode_from_videomode(const struct videomode *vm,
> > dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> > if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
> > dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> > + if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > + dmode->flags |= DRM_MODE_FLAG_PPIXCLK;
> > + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > + dmode->flags |= DRM_MODE_FLAG_NPIXCLK;
> > + if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> > + dmode->flags |= DRM_MODE_FLAG_PDATAEN;
> > + else if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> > + dmode->flags |= DRM_MODE_FLAG_NDE;
> > +
> > drm_mode_set_name(dmode);
> > }
> > EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
> > @@ -652,6 +661,14 @@ void drm_display_mode_to_videomode(const struct drm_display_mode *dmode,
> > vm->flags |= DISPLAY_FLAGS_DOUBLESCAN;
> > if (dmode->flags & DRM_MODE_FLAG_DBLCLK)
> > vm->flags |= DISPLAY_FLAGS_DOUBLECLK;
> > + if (dmode->flags & DRM_MODE_FLAG_PPIXDATA)
> > + vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
> > + else if (dmode->flags & DRM_MODE_FLAG_NPIXDATA)
> > + vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
> > + if (dmode->flags & DRM_MODE_FLAG_PDE)
> > + vm->flags |= DISPLAY_FLAGS_DE_HIGH;
> > + else if (dmode->flags & DRM_MODE_FLAG_NDE)
> > + vm->flags |= DISPLAY_FLAGS_DE_LOW;
> > }
> > EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
> >
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index d3e0fe3..b335a17 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -89,6 +89,12 @@ extern "C" {
> > #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14)
> > #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14)
> >
> > +/* flags for polarity clock & data enable polarities */
> > +#define DRM_MODE_FLAG_PPIXDATA (1 << 19)
> > +#define DRM_MODE_FLAG_NPIXDATA (1 << 20)
> > +#define DRM_MODE_FLAG_PDE (1 << 21)
> > +#define DRM_MODE_FLAG_NDE (1 << 22)
> > +
> > /* Picture aspect ratio options */
> > #define DRM_MODE_PICTURE_ASPECT_NONE 0
> > #define DRM_MODE_PICTURE_ASPECT_4_3 1
> > --
> > 2.7.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



--
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

2018-10-24 08:24:33

by Lucas Stach

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] drm: Add missing flags for pixel clock & data enable

Am Mittwoch, den 24.10.2018, 10:16 +0200 schrieb Daniel Vetter:
> On Tue, Oct 23, 2018 at 04:50:19PM +0200, Benjamin Gaignard wrote:
> > Le lun. 15 oct. 2018 à 13:15, Benjamin Gaignard
> > <[email protected]> a écrit :
> > >
> > > Le lun. 24 sept. 2018 à 13:59, Yannick Fertré <[email protected]> a écrit :
> > > >
> > > > Add missing flags for pixel clock & data enable polarities.
> > > > These flags are similar to other synchronization signals (hsync, vsync...).
> > > >
> > > > Signed-off-by: Yannick Fertré <[email protected]>
> > >
> > > Reviewed-by: Benjamin Gaignard <[email protected]>
> >
> > Dave or Daniel could you give us your PoV on this patch ?
>
> Does it work? Iirc we had some userspace chocking on new mode flags, and
> needed explicit opt-in. If that looks good (check weston, -modesetting and
> drm_hwc, that should have you covered I hope) then has my ack.

What's the use of exposing those to userspace? There are a number of
drivers that already have to deal with this and they are totally fine
with keeping this information internal to the driver.

For reference see include/video/display_timing.h, specifically the enum
display_flags.

Regards,
Lucas

> -Daniel
>
> > Thanks
> >
> > >
> > > > ---
> > > > drivers/gpu/drm/drm_modes.c | 19 ++++++++++++++++++-
> > > > include/uapi/drm/drm_mode.h | 6 ++++++
> > > > 2 files changed, 24 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > > > index 02db9ac..596f8b3 100644
> > > > --- a/drivers/gpu/drm/drm_modes.c
> > > > +++ b/drivers/gpu/drm/drm_modes.c
> > > > @@ -130,7 +130,7 @@ EXPORT_SYMBOL(drm_mode_probed_add);
> > > > * according to the hdisplay, vdisplay, vrefresh.
> > > > * It is based from the VESA(TM) Coordinated Video Timing Generator by
> > > > * Graham Loveridge April 9, 2003 available at
> > > > - * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> > > > + * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> > > > *
> > > > * And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c.
> > > > * What I have done is to translate it by using integer calculation.
> > > > @@ -611,6 +611,15 @@ void drm_display_mode_from_videomode(const struct videomode *vm,
> > > > dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> > > > if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
> > > > dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> > > > + if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > > > + dmode->flags |= DRM_MODE_FLAG_PPIXCLK;
> > > > + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > > > + dmode->flags |= DRM_MODE_FLAG_NPIXCLK;
> > > > + if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> > > > + dmode->flags |= DRM_MODE_FLAG_PDATAEN;
> > > > + else if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> > > > + dmode->flags |= DRM_MODE_FLAG_NDE;
> > > > +
> > > > drm_mode_set_name(dmode);
> > > > }
> > > > EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
> > > > @@ -652,6 +661,14 @@ void drm_display_mode_to_videomode(const struct drm_display_mode *dmode,
> > > > vm->flags |= DISPLAY_FLAGS_DOUBLESCAN;
> > > > if (dmode->flags & DRM_MODE_FLAG_DBLCLK)
> > > > vm->flags |= DISPLAY_FLAGS_DOUBLECLK;
> > > > + if (dmode->flags & DRM_MODE_FLAG_PPIXDATA)
> > > > + vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
> > > > + else if (dmode->flags & DRM_MODE_FLAG_NPIXDATA)
> > > > + vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
> > > > + if (dmode->flags & DRM_MODE_FLAG_PDE)
> > > > + vm->flags |= DISPLAY_FLAGS_DE_HIGH;
> > > > + else if (dmode->flags & DRM_MODE_FLAG_NDE)
> > > > + vm->flags |= DISPLAY_FLAGS_DE_LOW;
> > > > }
> > > > EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
> > > >
> > > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > > > index d3e0fe3..b335a17 100644
> > > > --- a/include/uapi/drm/drm_mode.h
> > > > +++ b/include/uapi/drm/drm_mode.h
> > > > @@ -89,6 +89,12 @@ extern "C" {
> > > > #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14)
> > > > #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14)
> > > >
> > > > +/* flags for polarity clock & data enable polarities */
> > > > +#define DRM_MODE_FLAG_PPIXDATA (1 << 19)
> > > > +#define DRM_MODE_FLAG_NPIXDATA (1 << 20)
> > > > +#define DRM_MODE_FLAG_PDE (1 << 21)
> > > > +#define DRM_MODE_FLAG_NDE (1 << 22)
> > > > +
> > > > /* Picture aspect ratio options */
> > > > #define DRM_MODE_PICTURE_ASPECT_NONE 0
> > > > #define DRM_MODE_PICTURE_ASPECT_4_3 1
> > > > --
> > > > 2.7.4
> > > >
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > [email protected]
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> >
> >
> > --
> > Benjamin Gaignard
> >
> > Graphic Study Group
> >
> > Linaro.org │ Open source software for ARM SoCs
> >
> > Follow Linaro: Facebook | Twitter | Blog
>
>


2018-10-24 08:25:18

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] drm: Add missing flags for pixel clock & data enable

On Tue, Oct 23, 2018 at 04:50:19PM +0200, Benjamin Gaignard wrote:
> Le lun. 15 oct. 2018 à 13:15, Benjamin Gaignard
> <[email protected]> a écrit :
> >
> > Le lun. 24 sept. 2018 à 13:59, Yannick Fertré <[email protected]> a écrit :
> > >
> > > Add missing flags for pixel clock & data enable polarities.
> > > These flags are similar to other synchronization signals (hsync, vsync...).
> > >
> > > Signed-off-by: Yannick Fertré <[email protected]>
> >
> > Reviewed-by: Benjamin Gaignard <[email protected]>
>
> Dave or Daniel could you give us your PoV on this patch ?

Does it work? Iirc we had some userspace chocking on new mode flags, and
needed explicit opt-in. If that looks good (check weston, -modesetting and
drm_hwc, that should have you covered I hope) then has my ack.
-Daniel

> Thanks
>
> >
> > > ---
> > > drivers/gpu/drm/drm_modes.c | 19 ++++++++++++++++++-
> > > include/uapi/drm/drm_mode.h | 6 ++++++
> > > 2 files changed, 24 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > > index 02db9ac..596f8b3 100644
> > > --- a/drivers/gpu/drm/drm_modes.c
> > > +++ b/drivers/gpu/drm/drm_modes.c
> > > @@ -130,7 +130,7 @@ EXPORT_SYMBOL(drm_mode_probed_add);
> > > * according to the hdisplay, vdisplay, vrefresh.
> > > * It is based from the VESA(TM) Coordinated Video Timing Generator by
> > > * Graham Loveridge April 9, 2003 available at
> > > - * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> > > + * http://www.elo.utfsm.cl/~elo212/docs/CVTd6r1.xls
> > > *
> > > * And it is copied from xf86CVTmode in xserver/hw/xfree86/modes/xf86cvt.c.
> > > * What I have done is to translate it by using integer calculation.
> > > @@ -611,6 +611,15 @@ void drm_display_mode_from_videomode(const struct videomode *vm,
> > > dmode->flags |= DRM_MODE_FLAG_DBLSCAN;
> > > if (vm->flags & DISPLAY_FLAGS_DOUBLECLK)
> > > dmode->flags |= DRM_MODE_FLAG_DBLCLK;
> > > + if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> > > + dmode->flags |= DRM_MODE_FLAG_PPIXCLK;
> > > + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> > > + dmode->flags |= DRM_MODE_FLAG_NPIXCLK;
> > > + if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> > > + dmode->flags |= DRM_MODE_FLAG_PDATAEN;
> > > + else if (vm->flags & DISPLAY_FLAGS_DE_LOW)
> > > + dmode->flags |= DRM_MODE_FLAG_NDE;
> > > +
> > > drm_mode_set_name(dmode);
> > > }
> > > EXPORT_SYMBOL_GPL(drm_display_mode_from_videomode);
> > > @@ -652,6 +661,14 @@ void drm_display_mode_to_videomode(const struct drm_display_mode *dmode,
> > > vm->flags |= DISPLAY_FLAGS_DOUBLESCAN;
> > > if (dmode->flags & DRM_MODE_FLAG_DBLCLK)
> > > vm->flags |= DISPLAY_FLAGS_DOUBLECLK;
> > > + if (dmode->flags & DRM_MODE_FLAG_PPIXDATA)
> > > + vm->flags |= DISPLAY_FLAGS_PIXDATA_POSEDGE;
> > > + else if (dmode->flags & DRM_MODE_FLAG_NPIXDATA)
> > > + vm->flags |= DISPLAY_FLAGS_PIXDATA_NEGEDGE;
> > > + if (dmode->flags & DRM_MODE_FLAG_PDE)
> > > + vm->flags |= DISPLAY_FLAGS_DE_HIGH;
> > > + else if (dmode->flags & DRM_MODE_FLAG_NDE)
> > > + vm->flags |= DISPLAY_FLAGS_DE_LOW;
> > > }
> > > EXPORT_SYMBOL_GPL(drm_display_mode_to_videomode);
> > >
> > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > > index d3e0fe3..b335a17 100644
> > > --- a/include/uapi/drm/drm_mode.h
> > > +++ b/include/uapi/drm/drm_mode.h
> > > @@ -89,6 +89,12 @@ extern "C" {
> > > #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14)
> > > #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14)
> > >
> > > +/* flags for polarity clock & data enable polarities */
> > > +#define DRM_MODE_FLAG_PPIXDATA (1 << 19)
> > > +#define DRM_MODE_FLAG_NPIXDATA (1 << 20)
> > > +#define DRM_MODE_FLAG_PDE (1 << 21)
> > > +#define DRM_MODE_FLAG_NDE (1 << 22)
> > > +
> > > /* Picture aspect ratio options */
> > > #define DRM_MODE_PICTURE_ASPECT_NONE 0
> > > #define DRM_MODE_PICTURE_ASPECT_4_3 1
> > > --
> > > 2.7.4
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > [email protected]
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Benjamin Gaignard
>
> Graphic Study Group
>
> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: Facebook | Twitter | Blog

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch