[Patch v2][ 03/37] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*
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. -- Ville Syrj?l? Intel OTC
[Patch v2][ 03/37] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*
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.
[Patch v2][ 03/37] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*
On Thu, Oct 17, 2013 at 10:02 AM, 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 > Cc: driverdev-devel at linuxdriverproject.org > Cc: David Airlie > Cc: dri-devel at lists.freedesktop.org > Cc: linux-arm-kernel at lists.infradead.org > Cc: Fabio Estevam > Cc: Sascha Hauer > Cc: linux-arm-kernel at lists.infradead.org > Cc: Eric B?nard > Signed-off-by: Denis Carikli > --- > 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) -- Matt Sealey
[Patch v2][ 03/37] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*
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 > Cc: driverdev-devel at linuxdriverproject.org > Cc: David Airlie > Cc: dri-devel at lists.freedesktop.org > Cc: linux-arm-kernel at lists.infradead.org > Cc: Fabio Estevam > Cc: Sascha Hauer > Cc: linux-arm-kernel at lists.infradead.org > Cc: Eric B?nard > Signed-off-by: Denis Carikli > --- > 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 at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrj?l? Intel OTC
[Patch v2][ 03/37] drm: Add the lacking DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*
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 Cc: driverdev-devel at linuxdriverproject.org Cc: David Airlie Cc: dri-devel at lists.freedesktop.org Cc: linux-arm-kernel at lists.infradead.org Cc: Fabio Estevam Cc: Sascha Hauer Cc: linux-arm-kernel at lists.infradead.org Cc: Eric B?nard Signed-off-by: Denis Carikli --- 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) /* DPMS flags */ /* bit compatible with the xorg definitions. */ -- 1.7.9.5