| Message ID | 1394731053-6118-7-git-send-email-denis@eukrea.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Denis, Thanks for the patch. On 03/13/2014 06:17 PM, Denis Carikli wrote: > We need a way to pass signal polarity informations > between DRM panels, and the display drivers. > > To do that, a pol_flags field was added to drm_display_mode. > > Signed-off-by: Denis Carikli <denis@eukrea.com> > --- > ChangeLog v10->v11: > - Since the imx-drm won't be able to retrive its regulators > from the device tree when using display-timings nodes, > and that I was told that the drm simple-panel driver > already supported that, I then, instead, added what was > lacking to make the eukrea displays work with the > drm-simple-panel driver. > > That required a way to get back the display polarity > informations from the imx-drm driver without affecting > userspace. > --- > include/drm/drm_crtc.h | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index f764654..61a4fe1 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -131,6 +131,13 @@ enum drm_mode_status { > > #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1) > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2) > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3) > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4) > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5) > +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) Could you add some description to these flags. What are *_PRESERVE flags for? Are those flags 1:1 compatible with respective 'videomode:flags'? I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH), am I right? Regards Andrzej > + > struct drm_display_mode { > /* Header */ > struct list_head head; > @@ -183,6 +190,7 @@ struct drm_display_mode { > int vrefresh; /* in Hz */ > int hsync; /* in kHz */ > enum hdmi_picture_aspect picture_aspect_ratio; > + unsigned int pol_flags; > }; > > static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) >
Hello, On Monday 17 March 2014 14:41:09 Andrzej Hajda wrote: > On 03/13/2014 06:17 PM, Denis Carikli wrote: > > We need a way to pass signal polarity informations > > between DRM panels, and the display drivers. > > > > To do that, a pol_flags field was added to drm_display_mode. > > > > Signed-off-by: Denis Carikli <denis@eukrea.com> > > --- > > ChangeLog v10->v11: > > - Since the imx-drm won't be able to retrive its regulators > > > > from the device tree when using display-timings nodes, > > and that I was told that the drm simple-panel driver > > already supported that, I then, instead, added what was > > lacking to make the eukrea displays work with the > > drm-simple-panel driver. > > > > That required a way to get back the display polarity > > informations from the imx-drm driver without affecting > > userspace. > > > > --- > > > > include/drm/drm_crtc.h | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > index f764654..61a4fe1 100644 > > --- a/include/drm/drm_crtc.h > > +++ b/include/drm/drm_crtc.h > > @@ -131,6 +131,13 @@ enum drm_mode_status { > > > > #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1) > > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2) > > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3) > > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4) > > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5) > > +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) > > Could you add some description to these flags. > What are *_PRESERVE flags for? > Are those flags 1:1 compatible with respective 'videomode:flags'? > I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH), am I > right? Possibly nitpicking, I wouldn't call the clock edge on which data signals are generated/sampled "data polarity". This is clock polarity information. Have you seen cases where pixel data and DE are geenrated or need to be sampled on different edges ? > > + > > struct drm_display_mode { > > /* Header */ > > struct list_head head; > > @@ -183,6 +190,7 @@ struct drm_display_mode { > > int vrefresh; /* in Hz */ > > int hsync; /* in kHz */ > > enum hdmi_picture_aspect picture_aspect_ratio; > > + unsigned int pol_flags; > > }; > > > > static inline bool drm_mode_is_stereo(const struct drm_display_mode > > *mode)
Hi, Laurent Pinchart wrote: > Hello, > > On Monday 17 March 2014 14:41:09 Andrzej Hajda wrote: > > On 03/13/2014 06:17 PM, Denis Carikli wrote: > > > We need a way to pass signal polarity informations > > > between DRM panels, and the display drivers. > > > > > > To do that, a pol_flags field was added to drm_display_mode. > > > > > > Signed-off-by: Denis Carikli <denis@eukrea.com> > > > --- > > > ChangeLog v10->v11: > > > - Since the imx-drm won't be able to retrive its regulators > > > > > > from the device tree when using display-timings nodes, > > > and that I was told that the drm simple-panel driver > > > already supported that, I then, instead, added what was > > > lacking to make the eukrea displays work with the > > > drm-simple-panel driver. > > > > > > That required a way to get back the display polarity > > > informations from the imx-drm driver without affecting > > > userspace. > > > > > > --- > > > > > > include/drm/drm_crtc.h | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > > index f764654..61a4fe1 100644 > > > --- a/include/drm/drm_crtc.h > > > +++ b/include/drm/drm_crtc.h > > > @@ -131,6 +131,13 @@ enum drm_mode_status { > > > > > > #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1) > > > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2) > > > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3) > > > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4) > > > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5) > > > +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) > > > > Could you add some description to these flags. > > What are *_PRESERVE flags for? > > Are those flags 1:1 compatible with respective 'videomode:flags'? > > I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH), am I > > right? > > Possibly nitpicking, I wouldn't call the clock edge on which data signals are > generated/sampled "data polarity". This is clock polarity information. > > Have you seen cases where pixel data and DE are geenrated or need to be > sampled on different edges ? > DE is not a clock signal, but an 'Enable' signal whose value (high or low) defines the window in which the pixel data is valid. The flag defines whether data is valid during the HIGH or LOW period of DE. Lothar Waßmann
Hi Lothar, On Monday 17 March 2014 16:14:36 Lothar Waßmann wrote: > Laurent Pinchart wrote: > > On Monday 17 March 2014 14:41:09 Andrzej Hajda wrote: > > > On 03/13/2014 06:17 PM, Denis Carikli wrote: > > > > We need a way to pass signal polarity informations > > > > between DRM panels, and the display drivers. > > > > > > > > To do that, a pol_flags field was added to drm_display_mode. > > > > > > > > Signed-off-by: Denis Carikli <denis@eukrea.com> > > > > --- > > > > ChangeLog v10->v11: > > > > - Since the imx-drm won't be able to retrive its regulators > > > > > > > > from the device tree when using display-timings nodes, > > > > and that I was told that the drm simple-panel driver > > > > already supported that, I then, instead, added what was > > > > lacking to make the eukrea displays work with the > > > > drm-simple-panel driver. > > > > > > > > That required a way to get back the display polarity > > > > informations from the imx-drm driver without affecting > > > > userspace. > > > > > > > > --- > > > > > > > > include/drm/drm_crtc.h | 8 ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > > > index f764654..61a4fe1 100644 > > > > --- a/include/drm/drm_crtc.h > > > > +++ b/include/drm/drm_crtc.h > > > > @@ -131,6 +131,13 @@ enum drm_mode_status { > > > > > > > > #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF > > > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1) > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2) > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3) > > > > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4) > > > > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5) > > > > +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) > > > > > > Could you add some description to these flags. > > > What are *_PRESERVE flags for? > > > Are those flags 1:1 compatible with respective 'videomode:flags'? > > > I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH), am I > > > right? > > > > Possibly nitpicking, I wouldn't call the clock edge on which data signals > > are generated/sampled "data polarity". This is clock polarity > > information. > > > > Have you seen cases where pixel data and DE are geenrated or need to be > > sampled on different edges ? > > DE is not a clock signal, but an 'Enable' signal whose value (high or > low) defines the window in which the pixel data is valid. > The flag defines whether data is valid during the HIGH or LOW period of > DE. The DRM_MODE_FLAG_POL_DE_(LOW|HIGH) do, by my impression of the proposed new DRM_MODE_FLAG_POL_DE_*EDGE flags is that they define sampling clock edges, not active levels.
Hi, Laurent Pinchart wrote: > Hi Lothar, > > On Monday 17 March 2014 16:14:36 Lothar Waßmann wrote: > > Laurent Pinchart wrote: > > > On Monday 17 March 2014 14:41:09 Andrzej Hajda wrote: > > > > On 03/13/2014 06:17 PM, Denis Carikli wrote: > > > > > We need a way to pass signal polarity informations > > > > > between DRM panels, and the display drivers. > > > > > > > > > > To do that, a pol_flags field was added to drm_display_mode. > > > > > > > > > > Signed-off-by: Denis Carikli <denis@eukrea.com> > > > > > --- > > > > > ChangeLog v10->v11: > > > > > - Since the imx-drm won't be able to retrive its regulators > > > > > > > > > > from the device tree when using display-timings nodes, > > > > > and that I was told that the drm simple-panel driver > > > > > already supported that, I then, instead, added what was > > > > > lacking to make the eukrea displays work with the > > > > > drm-simple-panel driver. > > > > > > > > > > That required a way to get back the display polarity > > > > > informations from the imx-drm driver without affecting > > > > > userspace. > > > > > > > > > > --- > > > > > > > > > > include/drm/drm_crtc.h | 8 ++++++++ > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > > > > index f764654..61a4fe1 100644 > > > > > --- a/include/drm/drm_crtc.h > > > > > +++ b/include/drm/drm_crtc.h > > > > > @@ -131,6 +131,13 @@ enum drm_mode_status { > > > > > > > > > > #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF > > > > > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1) > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2) > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3) > > > > > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4) > > > > > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5) > > > > > +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) > > > > > > > > Could you add some description to these flags. > > > > What are *_PRESERVE flags for? > > > > Are those flags 1:1 compatible with respective 'videomode:flags'? > > > > I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH), am I > > > > right? > > > > > > Possibly nitpicking, I wouldn't call the clock edge on which data signals > > > are generated/sampled "data polarity". This is clock polarity > > > information. > > > > > > Have you seen cases where pixel data and DE are geenrated or need to be > > > sampled on different edges ? > > > > DE is not a clock signal, but an 'Enable' signal whose value (high or > > low) defines the window in which the pixel data is valid. > > The flag defines whether data is valid during the HIGH or LOW period of > > DE. > > The DRM_MODE_FLAG_POL_DE_(LOW|HIGH) do, by my impression of the proposed new > DRM_MODE_FLAG_POL_DE_*EDGE flags is that they define sampling clock edges, not > active levels. > The current naming of the flags gives the impression that they describe the sampling edges of a clock signal. But the DE signal in fact is not a clock signal but a level sensitive gating signal. Lothar Waßmann
On Tue, Mar 18, 2014 at 08:50:30AM +0100, Lothar Waßmann wrote: > Hi, > > Laurent Pinchart wrote: > > Hi Lothar, > > > > On Monday 17 March 2014 16:14:36 Lothar Waßmann wrote: > > > DE is not a clock signal, but an 'Enable' signal whose value (high or > > > low) defines the window in which the pixel data is valid. > > > The flag defines whether data is valid during the HIGH or LOW period of > > > DE. > > > > The DRM_MODE_FLAG_POL_DE_(LOW|HIGH) do, by my impression of the proposed new > > DRM_MODE_FLAG_POL_DE_*EDGE flags is that they define sampling clock edges, not > > active levels. > > > The current naming of the flags gives the impression that they describe > the sampling edges of a clock signal. But the DE signal in fact is not > a clock signal but a level sensitive gating signal. +1
Hi Lothar, On Tuesday 18 March 2014 08:50:30 Lothar Waßmann wrote: > Laurent Pinchart wrote: > > On Monday 17 March 2014 16:14:36 Lothar Waßmann wrote: > > > Laurent Pinchart wrote: > > > > On Monday 17 March 2014 14:41:09 Andrzej Hajda wrote: > > > > > On 03/13/2014 06:17 PM, Denis Carikli wrote: > > > > > > We need a way to pass signal polarity informations > > > > > > between DRM panels, and the display drivers. > > > > > > > > > > > > To do that, a pol_flags field was added to drm_display_mode. > > > > > > > > > > > > Signed-off-by: Denis Carikli <denis@eukrea.com> > > > > > > --- > > > > > > ChangeLog v10->v11: > > > > > > - Since the imx-drm won't be able to retrive its regulators > > > > > > > > > > > > from the device tree when using display-timings nodes, > > > > > > and that I was told that the drm simple-panel driver > > > > > > already supported that, I then, instead, added what was > > > > > > lacking to make the eukrea displays work with the > > > > > > drm-simple-panel driver. > > > > > > > > > > > > That required a way to get back the display polarity > > > > > > informations from the imx-drm driver without affecting > > > > > > userspace. > > > > > > > > > > > > --- > > > > > > > > > > > > include/drm/drm_crtc.h | 8 ++++++++ > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > > > > > index f764654..61a4fe1 100644 > > > > > > --- a/include/drm/drm_crtc.h > > > > > > +++ b/include/drm/drm_crtc.h > > > > > > @@ -131,6 +131,13 @@ enum drm_mode_status { > > > > > > > > > > > > #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF > > > > > > > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1) > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2) > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3) > > > > > > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4) > > > > > > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5) > > > > > > +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) > > > > > > > > > > Could you add some description to these flags. > > > > > What are *_PRESERVE flags for? > > > > > Are those flags 1:1 compatible with respective 'videomode:flags'? > > > > > I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH), > > > > > am I right? > > > > > > > > Possibly nitpicking, I wouldn't call the clock edge on which data > > > > signals are generated/sampled "data polarity". This is clock polarity > > > > information. > > > > > > > > Have you seen cases where pixel data and DE are geenrated or need to > > > > be sampled on different edges ? > > > > > > DE is not a clock signal, but an 'Enable' signal whose value (high or > > > low) defines the window in which the pixel data is valid. > > > The flag defines whether data is valid during the HIGH or LOW period of > > > DE. > > > > The DRM_MODE_FLAG_POL_DE_(LOW|HIGH) do, by my impression of the proposed > > new DRM_MODE_FLAG_POL_DE_*EDGE flags is that they define sampling clock > > edges, not active levels. > > The current naming of the flags gives the impression that they describe > the sampling edges of a clock signal. But the DE signal in fact is not > a clock signal but a level sensitive gating signal. That's not my point. I *know* that DE is a data gating signal with a polarity already defined by the DRM_MODE_FLAG_POL_DE_(LOW|HIGH) flags. Like all other signals it gets generated on a clock edge and is sampled on a clock edge. The DRM_MODE_FLAG_POL_DE_*EDGE flags proposed above describe seem to describe just that, *not* the signal polarity. I thus want to know whether there are systems where the data signals and the DE signal need to be sampled on different edges of the pixel clock.
On Tue, Mar 18, 2014 at 01:41:54PM +0100, Laurent Pinchart wrote: > Hi Lothar, > > That's not my point. I *know* that DE is a data gating signal with a polarity > already defined by the DRM_MODE_FLAG_POL_DE_(LOW|HIGH) flags. Like all other > signals it gets generated on a clock edge and is sampled on a clock edge. The > DRM_MODE_FLAG_POL_DE_*EDGE flags proposed above describe seem to describe just > that, *not* the signal polarity. I thus want to know whether there are systems > where the data signals and the DE signal need to be sampled on different edges > of the pixel clock. That's not relevant to this patch series though. This patch series is about adding configuration which will allow iMX6 SoCs to be properly configured for their displays. iMX has the ability to: - define the polarity of the clock signal - define the polarity of the other signals It does not have the ability to define which clock edge individual signals like vsync (frame clock), hsync (line clock), disable enable change on independently. So, it doesn't make sense _here_ for the display enable to be defined by an edge - it's not something that can be changed here. What can only be changed is it's active level. Of course, there may be some which can do this, and that's a separate problem that would need to be addressed when there's hardware that does support it. The objection which Lothar is raising is that _this_ definition does not match the _hardware_ capabilities which it is intended to be used with, which is a very valid objection.
Hi Russell, On Tuesday 18 March 2014 12:56:23 Russell King - ARM Linux wrote: > On Tue, Mar 18, 2014 at 01:41:54PM +0100, Laurent Pinchart wrote: > > Hi Lothar, > > > > That's not my point. I *know* that DE is a data gating signal with a > > polarity already defined by the DRM_MODE_FLAG_POL_DE_(LOW|HIGH) flags. > > Like all other signals it gets generated on a clock edge and is sampled > > on a clock edge. The DRM_MODE_FLAG_POL_DE_*EDGE flags proposed above > > describe seem to describe just that, *not* the signal polarity. I thus > > want to know whether there are systems where the data signals and the DE > > signal need to be sampled on different edges of the pixel clock. > > That's not relevant to this patch series though. This patch series is > about adding configuration which will allow iMX6 SoCs to be properly > configured for their displays. > > iMX has the ability to: > > - define the polarity of the clock signal > - define the polarity of the other signals > > It does not have the ability to define which clock edge individual signals > like vsync (frame clock), hsync (line clock), disable enable change on > independently. > > So, it doesn't make sense _here_ for the display enable to be defined by > an edge - it's not something that can be changed here. What can only be > changed is it's active level. > > Of course, there may be some which can do this, and that's a separate > problem that would need to be addressed when there's hardware that does > support it. > > The objection which Lothar is raising is that _this_ definition does not > match the _hardware_ capabilities which it is intended to be used with, > which is a very valid objection. Thank you for the clarification. That absolutely makes sense.
Hi, Laurent Pinchart wrote: > Hi Lothar, > > On Tuesday 18 March 2014 08:50:30 Lothar Waßmann wrote: > > Laurent Pinchart wrote: > > > On Monday 17 March 2014 16:14:36 Lothar Waßmann wrote: > > > > Laurent Pinchart wrote: > > > > > On Monday 17 March 2014 14:41:09 Andrzej Hajda wrote: > > > > > > On 03/13/2014 06:17 PM, Denis Carikli wrote: > > > > > > > We need a way to pass signal polarity informations > > > > > > > between DRM panels, and the display drivers. > > > > > > > > > > > > > > To do that, a pol_flags field was added to drm_display_mode. > > > > > > > > > > > > > > Signed-off-by: Denis Carikli <denis@eukrea.com> > > > > > > > --- > > > > > > > ChangeLog v10->v11: > > > > > > > - Since the imx-drm won't be able to retrive its regulators > > > > > > > > > > > > > > from the device tree when using display-timings nodes, > > > > > > > and that I was told that the drm simple-panel driver > > > > > > > already supported that, I then, instead, added what was > > > > > > > lacking to make the eukrea displays work with the > > > > > > > drm-simple-panel driver. > > > > > > > > > > > > > > That required a way to get back the display polarity > > > > > > > informations from the imx-drm driver without affecting > > > > > > > userspace. > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > include/drm/drm_crtc.h | 8 ++++++++ > > > > > > > 1 file changed, 8 insertions(+) > > > > > > > > > > > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > > > > > > index f764654..61a4fe1 100644 > > > > > > > --- a/include/drm/drm_crtc.h > > > > > > > +++ b/include/drm/drm_crtc.h > > > > > > > @@ -131,6 +131,13 @@ enum drm_mode_status { > > > > > > > > > > > > > > #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF > > > > > > > > > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1) > > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2) > > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3) > > > > > > > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4) > > > > > > > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5) > > > > > > > +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) > > > > > > > > > > > > Could you add some description to these flags. > > > > > > What are *_PRESERVE flags for? > > > > > > Are those flags 1:1 compatible with respective 'videomode:flags'? > > > > > > I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH), > > > > > > am I right? > > > > > > > > > > Possibly nitpicking, I wouldn't call the clock edge on which data > > > > > signals are generated/sampled "data polarity". This is clock polarity > > > > > information. > > > > > > > > > > Have you seen cases where pixel data and DE are geenrated or need to > > > > > be sampled on different edges ? > > > > > > > > DE is not a clock signal, but an 'Enable' signal whose value (high or > > > > low) defines the window in which the pixel data is valid. > > > > The flag defines whether data is valid during the HIGH or LOW period of > > > > DE. > > > > > > The DRM_MODE_FLAG_POL_DE_(LOW|HIGH) do, by my impression of the proposed > > > new DRM_MODE_FLAG_POL_DE_*EDGE flags is that they define sampling clock > > > edges, not active levels. > > > > The current naming of the flags gives the impression that they describe > > the sampling edges of a clock signal. But the DE signal in fact is not > > a clock signal but a level sensitive gating signal. > > That's not my point. I *know* that DE is a data gating signal with a polarity > already defined by the DRM_MODE_FLAG_POL_DE_(LOW|HIGH) flags. Like all other > signals it gets generated on a clock edge and is sampled on a clock edge. The > DRM_MODE_FLAG_POL_DE_*EDGE flags proposed above describe seem to describe just > The important word here is 'seem'. Lothar Waßann
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index f764654..61a4fe1 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -131,6 +131,13 @@ enum drm_mode_status { #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGE BIT(1) +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGE BIT(2) +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE BIT(3) +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4) +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5) +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) + struct drm_display_mode { /* Header */ struct list_head head; @@ -183,6 +190,7 @@ struct drm_display_mode { int vrefresh; /* in Hz */ int hsync; /* in kHz */ enum hdmi_picture_aspect picture_aspect_ratio; + unsigned int pol_flags; }; static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode)
We need a way to pass signal polarity informations between DRM panels, and the display drivers. To do that, a pol_flags field was added to drm_display_mode. Signed-off-by: Denis Carikli <denis@eukrea.com> --- ChangeLog v10->v11: - Since the imx-drm won't be able to retrive its regulators from the device tree when using display-timings nodes, and that I was told that the drm simple-panel driver already supported that, I then, instead, added what was lacking to make the eukrea displays work with the drm-simple-panel driver. That required a way to get back the display polarity informations from the imx-drm driver without affecting userspace. --- include/drm/drm_crtc.h | 8 ++++++++ 1 file changed, 8 insertions(+)