diff mbox

[v2,03/37] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*

Message ID 1382022155-21954-4-git-send-email-denis@eukrea.com (mailing list archive)
State New, archived
Headers show

Commit Message

Denis Carikli Oct. 17, 2013, 3:02 p.m. UTC 
Without that fix, drivers using the drm_display_mode_from_videomode
  function will not be able to get certain information because
  some DISPLAY_FLAGS_* have no corresponding DRM_MODE_FLAG_*.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: driverdev-devel@linuxdriverproject.org
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 drivers/gpu/drm/drm_modes.c |    9 +++++++++
 include/uapi/drm/drm_mode.h |    4 ++++
 2 files changed, 13 insertions(+)

Comments

Ville Syrjala Oct. 18, 2013, 7:46 a.m. UTC | #1 
On Thu, Oct 17, 2013 at 05:02:01PM +0200, Denis Carikli wrote:
> Without that fix, drivers using the drm_display_mode_from_videomode
>   function will not be able to get certain information because
>   some DISPLAY_FLAGS_* have no corresponding DRM_MODE_FLAG_*.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: driverdev-devel@linuxdriverproject.org
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
>  drivers/gpu/drm/drm_modes.c |    9 +++++++++
>  include/uapi/drm/drm_mode.h |    4 ++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index b073315..353aaae 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -537,6 +537,15 @@ int 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_DE_LOW)
> +		dmode->flags |= DRM_MODE_FLAG_NDATEN;
> +	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> +		dmode->flags |= DRM_MODE_FLAG_PDATEN;
> +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> +		dmode->flags |= DRM_MODE_FLAG_PPIXDATEDGE;
> +	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> +		dmode->flags |= DRM_MODE_FLAG_NPIXDATEDGE;
> +
>  	drm_mode_set_name(dmode);
>  
>  	return 0;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index bafe612..13843c7 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -66,6 +66,10 @@
>  #define DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH	(1<<19)
>  #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM		(1<<20)
>  #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF	(1<<21)
> +#define DRM_MODE_FLAG_PDATEN			(1<<22)
> +#define DRM_MODE_FLAG_NDATEN			(1<<23)
> +#define DRM_MODE_FLAG_PPIXDATEDGE		(1<<24)
> +#define DRM_MODE_FLAG_NPIXDATEDGE		(1<<25)

Do we really need to make this stuff visible to userspace?
And there's no documentation to explain any of it.

>  
>  /* DPMS flags */
>  /* bit compatible with the xorg definitions. */
> -- 
> 1.7.9.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Denis Carikli Oct. 23, 2013, 2:48 p.m. UTC | #2 
Hi,

On 10/18/2013 09:46 AM, Ville Syrjälä wrote:
>> +#define DRM_MODE_FLAG_PDATEN			(1<<22)
>> +#define DRM_MODE_FLAG_NDATEN			(1<<23)
>> +#define DRM_MODE_FLAG_PPIXDATEDGE		(1<<24)
>> +#define DRM_MODE_FLAG_NPIXDATEDGE		(1<<25)
>
> Do we really need to make this stuff visible to userspace?
> And there's no documentation to explain any of it.

DRM_MODE_FLAG_PDATEN and DRM_MODE_FLAG_NDATEN were meant to represent 
the data enable polarity.

DRM_MODE_FLAG_PPIXDATEDGE and DRM_MODE_FLAG_NPIXDATEDGE were meant to 
represent the clock polarity.

What would you recommend to represent theses polarities?

Denis.
Ville Syrjala Oct. 23, 2013, 3:38 p.m. UTC | #3 
On Wed, Oct 23, 2013 at 04:48:51PM +0200, Denis Carikli wrote:
> Hi,
> 
> On 10/18/2013 09:46 AM, Ville Syrjälä wrote:
> >> +#define DRM_MODE_FLAG_PDATEN			(1<<22)
> >> +#define DRM_MODE_FLAG_NDATEN			(1<<23)
> >> +#define DRM_MODE_FLAG_PPIXDATEDGE		(1<<24)
> >> +#define DRM_MODE_FLAG_NPIXDATEDGE		(1<<25)
> >
> > Do we really need to make this stuff visible to userspace?
> > And there's no documentation to explain any of it.
> 
> DRM_MODE_FLAG_PDATEN and DRM_MODE_FLAG_NDATEN were meant to represent 
> the data enable polarity.
> 
> DRM_MODE_FLAG_PPIXDATEDGE and DRM_MODE_FLAG_NPIXDATEDGE were meant to 
> represent the clock polarity.
> 
> What would you recommend to represent theses polarities?

I don't really care that much how you represent them. Just wondering
if userspace has any business dictating those, and it not, then they
shouldn't be DRM_MODE flags.
Matt Sealey Oct. 23, 2013, 3:47 p.m. UTC | #4 
On Thu, Oct 17, 2013 at 10:02 AM, Denis Carikli <denis@eukrea.com> wrote:
> Without that fix, drivers using the drm_display_mode_from_videomode
>   function will not be able to get certain information because
>   some DISPLAY_FLAGS_* have no corresponding DRM_MODE_FLAG_*.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: driverdev-devel@linuxdriverproject.org
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
>  drivers/gpu/drm/drm_modes.c |    9 +++++++++
>  include/uapi/drm/drm_mode.h |    4 ++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index b073315..353aaae 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -537,6 +537,15 @@ int 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_DE_LOW)
> +               dmode->flags |= DRM_MODE_FLAG_NDATEN;
> +       if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
> +               dmode->flags |= DRM_MODE_FLAG_PDATEN;
> +       if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
> +               dmode->flags |= DRM_MODE_FLAG_PPIXDATEDGE;
> +       if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
> +               dmode->flags |= DRM_MODE_FLAG_NPIXDATEDGE;

Is there any reason these aren't named after the original
DISPLAY_FLAGS? DRM_MODE_FLAG_PIXDATA_POSEDGE is easier to get your
head around if you know it is mapped from the DISPLAY_FLAGS_ version.
PDATEN and PPIXDATEDGE don't exactly map to things like EDID field
names either..

> +#define DRM_MODE_FLAG_PPIXDATEDGE              (1<<24)
> +#define DRM_MODE_FLAG_NPIXDATEDGE              (1<<25)
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index b073315..353aaae 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -537,6 +537,15 @@  int 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_DE_LOW)
+		dmode->flags |= DRM_MODE_FLAG_NDATEN;
+	if (vm->flags & DISPLAY_FLAGS_DE_HIGH)
+		dmode->flags |= DRM_MODE_FLAG_PDATEN;
+	if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE)
+		dmode->flags |= DRM_MODE_FLAG_PPIXDATEDGE;
+	if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE)
+		dmode->flags |= DRM_MODE_FLAG_NPIXDATEDGE;
+
 	drm_mode_set_name(dmode);
 
 	return 0;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index bafe612..13843c7 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -66,6 +66,10 @@ 
 #define DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH	(1<<19)
 #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM		(1<<20)
 #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF	(1<<21)
+#define DRM_MODE_FLAG_PDATEN			(1<<22)
+#define DRM_MODE_FLAG_NDATEN			(1<<23)
+#define DRM_MODE_FLAG_PPIXDATEDGE		(1<<24)
+#define DRM_MODE_FLAG_NPIXDATEDGE		(1<<25)
 
 /* DPMS flags */
 /* bit compatible with the xorg definitions. */