| Message ID | 1385998768-23512-1-git-send-email-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 02, 2013 at 04:39:26PM +0100, Marek Vasut wrote: > Add DRM flags for the LCD display clock polarity so the pixelclk-active DT > property can be properly handled by drivers using the DRM API. I still say that not even this should be part of the DRM mode API to userspace. The hint that you're changing the user API is that you're modifying a header file below a 'uapi' directory. The settings of double scan, sync polarity etc are all part of the display mode specification (check CEA-861 documents). Things like pixel clock polarity are not part of the mode specification, they're a property of the display itself and are independent of the mode. Therefore, they should not be part of struct drm_mode_modeinfo.
Hello Russell, > On Mon, Dec 02, 2013 at 04:39:26PM +0100, Marek Vasut wrote: > > Add DRM flags for the LCD display clock polarity so the pixelclk-active > > DT property can be properly handled by drivers using the DRM API. > > I still say that not even this should be part of the DRM mode API to > userspace. The hint that you're changing the user API is that you're > modifying a header file below a 'uapi' directory. OK, you are right. > The settings of double scan, sync polarity etc are all part of the > display mode specification (check CEA-861 documents). Things like > pixel clock polarity are not part of the mode specification, they're > a property of the display itself and are independent of the mode. > > Therefore, they should not be part of struct drm_mode_modeinfo. Can you please provide me with some more hint here ? As for as I understand, using the 'pixelclk-active' DT prop in the 'display-timings' node for this purpose is correct, yes ? What I don't quite grok down is how do I get this information from DT into the imx-drm driver. Thanks! Best regards, Marek Vasut
On Mon, Dec 02, 2013 at 03:32:30PM -0500, Rob Clark wrote: > On Mon, Dec 2, 2013 at 3:01 PM, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Mon, Dec 02, 2013 at 04:39:26PM +0100, Marek Vasut wrote: > >> Add DRM flags for the LCD display clock polarity so the pixelclk-active DT > >> property can be properly handled by drivers using the DRM API. > > > > I still say that not even this should be part of the DRM mode API to > > userspace. The hint that you're changing the user API is that you're > > modifying a header file below a 'uapi' directory. > > > > The settings of double scan, sync polarity etc are all part of the > > display mode specification (check CEA-861 documents). Things like > > pixel clock polarity are not part of the mode specification, they're > > a property of the display itself and are independent of the mode. > > > > Therefore, they should not be part of struct drm_mode_modeinfo. > > Note that we could make them part of drm_display_mode (but > drm_crtc_convert_to_umode() should filter them out when copying from > drm_mode_modeinfo.. I agree this information should not be coming from > userspace). Seems like the sort of thing that the bridge or encoder > could set on the adjusted_mode. Probably better to not mix user visible and internal flags in drm_display_mode.flags. Just makes it more difficult to add real flags. Maybe just stick them into private_flags for the drivers that care.
On Mon, Dec 02, 2013 at 04:39:26PM +0100, Marek Vasut wrote: > Add DRM flags for the LCD display clock polarity so the pixelclk-active DT > property can be properly handled by drivers using the DRM API. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Dave Airlie <airlied@gmail.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Shawn Guo <shawn.guo@linaro.org> > --- > drivers/gpu/drm/drm_modes.c | 5 +++++ > include/uapi/drm/drm_mode.h | 3 +++ > 2 files changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 85071a1..d1f3bfc 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -537,6 +537,11 @@ 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_PIXDATA_POSEDGE) > + dmode->flags |= DRM_MODE_FLAG_PIXELCLK_PPOL; > + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) > + dmode->flags |= DRM_MODE_FLAG_PIXELCLK_NPOL; > + > drm_mode_set_name(dmode); > > return 0; > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index f104c26..a6169ca 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -73,6 +73,9 @@ > #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14) > #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14) > > +/* CRTC LCD clock polarity flags. */ > +#define DRM_MODE_FLAG_PIXELCLK_PPOL (1<<19) > +#define DRM_MODE_FLAG_PIXELCLK_NPOL (1<<20) Marek, It looks that Denis (copied) is working on the same problem, so you may want to be aware of his effort [1][2]. Shawn [1] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/42832 [2] http://thread.gmane.org/gmane.linux.drivers.driver-project.devel/42850/focus=279646 > > /* DPMS flags */ > /* bit compatible with the xorg definitions. */ > -- > 1.8.4.2 >
On Tue, Dec 03, 2013 at 07:44:52PM +0800, Shawn Guo wrote: > On Mon, Dec 02, 2013 at 04:39:26PM +0100, Marek Vasut wrote: > > Add DRM flags for the LCD display clock polarity so the pixelclk-active DT > > property can be properly handled by drivers using the DRM API. > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > Cc: Dave Airlie <airlied@gmail.com> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > Cc: Shawn Guo <shawn.guo@linaro.org> > > --- > > drivers/gpu/drm/drm_modes.c | 5 +++++ > > include/uapi/drm/drm_mode.h | 3 +++ > > 2 files changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > index 85071a1..d1f3bfc 100644 > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -537,6 +537,11 @@ 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_PIXDATA_POSEDGE) > > + dmode->flags |= DRM_MODE_FLAG_PIXELCLK_PPOL; > > + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) > > + dmode->flags |= DRM_MODE_FLAG_PIXELCLK_NPOL; > > + > > drm_mode_set_name(dmode); > > > > return 0; > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > > index f104c26..a6169ca 100644 > > --- a/include/uapi/drm/drm_mode.h > > +++ b/include/uapi/drm/drm_mode.h > > @@ -73,6 +73,9 @@ > > #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14) > > #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14) > > > > +/* CRTC LCD clock polarity flags. */ > > +#define DRM_MODE_FLAG_PIXELCLK_PPOL (1<<19) > > +#define DRM_MODE_FLAG_PIXELCLK_NPOL (1<<20) > > Marek, > > It looks that Denis (copied) is working on the same problem, so you may > want to be aware of his effort [1][2]. Indeed, and I made very similar comments in that thread.
On Tuesday, December 03, 2013 at 12:44:52 PM, Shawn Guo wrote: > On Mon, Dec 02, 2013 at 04:39:26PM +0100, Marek Vasut wrote: > > Add DRM flags for the LCD display clock polarity so the pixelclk-active > > DT property can be properly handled by drivers using the DRM API. > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > Cc: Dave Airlie <airlied@gmail.com> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > Cc: Shawn Guo <shawn.guo@linaro.org> > > --- > > > > drivers/gpu/drm/drm_modes.c | 5 +++++ > > include/uapi/drm/drm_mode.h | 3 +++ > > 2 files changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > index 85071a1..d1f3bfc 100644 > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -537,6 +537,11 @@ 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_PIXDATA_POSEDGE) > > + dmode->flags |= DRM_MODE_FLAG_PIXELCLK_PPOL; > > + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) > > + dmode->flags |= DRM_MODE_FLAG_PIXELCLK_NPOL; > > + > > > > drm_mode_set_name(dmode); > > > > return 0; > > > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > > index f104c26..a6169ca 100644 > > --- a/include/uapi/drm/drm_mode.h > > +++ b/include/uapi/drm/drm_mode.h > > @@ -73,6 +73,9 @@ > > > > #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14) > > #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14) > > > > +/* CRTC LCD clock polarity flags. */ > > +#define DRM_MODE_FLAG_PIXELCLK_PPOL (1<<19) > > +#define DRM_MODE_FLAG_PIXELCLK_NPOL (1<<20) > > Marek, > > It looks that Denis (copied) is working on the same problem, so you may > want to be aware of his effort [1][2]. He also just sent a new patchset, so I would focus on his work instead. It makes more sense as he is not breaking the userland stuff. Thank you Shawn, Russell! Best regards, Marek Vasut
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 85071a1..d1f3bfc 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -537,6 +537,11 @@ 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_PIXDATA_POSEDGE) + dmode->flags |= DRM_MODE_FLAG_PIXELCLK_PPOL; + else if (vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) + dmode->flags |= DRM_MODE_FLAG_PIXELCLK_NPOL; + drm_mode_set_name(dmode); return 0; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index f104c26..a6169ca 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -73,6 +73,9 @@ #define DRM_MODE_FLAG_3D_TOP_AND_BOTTOM (7<<14) #define DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF (8<<14) +/* CRTC LCD clock polarity flags. */ +#define DRM_MODE_FLAG_PIXELCLK_PPOL (1<<19) +#define DRM_MODE_FLAG_PIXELCLK_NPOL (1<<20) /* DPMS flags */ /* bit compatible with the xorg definitions. */
Add DRM flags for the LCD display clock polarity so the pixelclk-active DT property can be properly handled by drivers using the DRM API. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Dave Airlie <airlied@gmail.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Philipp Zabel <p.zabel@pengutronix.de> Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Shawn Guo <shawn.guo@linaro.org> --- drivers/gpu/drm/drm_modes.c | 5 +++++ include/uapi/drm/drm_mode.h | 3 +++ 2 files changed, 8 insertions(+)