| Message ID | 1396874691-27954-7-git-send-email-denis@eukrea.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Denis, On 04/07/2014 02:44 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 v11->v12: > - Rebased: This patch now applies against drm_modes.h > - Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names > > 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_modes.h | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h > index 2dbbf99..b3789e2 100644 > --- a/include/drm/drm_modes.h > +++ b/include/drm/drm_modes.h > @@ -93,6 +93,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) What is the purpose of DRM_MODE_FLAG_POL_PIXDATA_PRESERVE? If 'preserve' means 'ignore' we can set to zero negedge and posedge bits instead of adding new bit. If it is something different please describe it. > +#define DRM_MODE_FLAG_POL_DE_LOW BIT(4) > +#define DRM_MODE_FLAG_POL_DE_HIGH BIT(5) > +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) > + The same comments here. > struct drm_display_mode { > /* Header */ > struct list_head head; > @@ -144,6 +151,7 @@ struct drm_display_mode { > int vrefresh; /* in Hz */ > int hsync; /* in kHz */ > enum hdmi_picture_aspect picture_aspect_ratio; > + unsigned int pol_flags; Adding field and macros description to the DocBook would be nice. Regards Andrzej > }; > > /* mode specified on the command line */ >
On 04/08/2014 08:36 AM, Andrzej Hajda wrote: > > Hi Denis, Hi, >> +#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) > > What is the purpose of DRM_MODE_FLAG_POL_PIXDATA_PRESERVE? > If 'preserve' means 'ignore' we can set to zero negedge and posedge bits > instead of adding new bit. If it is something different please describe it. Yes, it meant 'ignore'. The goal was to be able to have a way to keep the old behavior while still being able to set the flags. So, with the imx-drm driver, if none of the DRM_MODE_FLAG_POL_PIXDATA were set(that is POSEDGE, NEGEDGE, PRESERVE), then in ipuv3-crtc.c, it went using the old flags settings that were previously hardcoded. The same applied for DRM_MODE_FLAG_POL_DE. The patch using theses flags is the 08/12 of this same serie. >> struct drm_display_mode { [..] >> + unsigned int pol_flags; > > Adding field and macros description to the DocBook would be nice. So I will have to describe it in the "Connector Helper Operations" section of drm.tmpl, right before the mode_valid synopsis ? Denis.
On 04/08/2014 10:08 AM, Denis Carikli wrote: > On 04/08/2014 08:36 AM, Andrzej Hajda wrote: >> >> Hi Denis, > Hi, > >>> +#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) >> >> What is the purpose of DRM_MODE_FLAG_POL_PIXDATA_PRESERVE? >> If 'preserve' means 'ignore' we can set to zero negedge and posedge bits >> instead of adding new bit. If it is something different please describe it. > Yes, it meant 'ignore'. > > The goal was to be able to have a way to keep the old behavior while > still being able to set the flags. > > So, with the imx-drm driver, if none of the DRM_MODE_FLAG_POL_PIXDATA > were set(that is POSEDGE, NEGEDGE, PRESERVE), then in ipuv3-crtc.c, it > went using the old flags settings that were previously hardcoded. > > The same applied for DRM_MODE_FLAG_POL_DE. > The patch using theses flags is the 08/12 of this same serie. So as I understand you want to: - do not change hw polarity settings by using preserve bit, - keep the old behavior of the driver by setting all bits to zero. I think this is the issue of the specific driver, and it should not influence core structs. I am not familiar with imx but I guess it should not be a problem to solve the issue in the driver. > >>> struct drm_display_mode { > [..] >>> + unsigned int pol_flags; >> >> Adding field and macros description to the DocBook would be nice. > So I will have to describe it in the "Connector Helper Operations" > section of drm.tmpl, right before the mode_valid synopsis ? Yes, I think so. Andrzej > > Denis. >
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 2dbbf99..b3789e2 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -93,6 +93,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_LOW BIT(4) +#define DRM_MODE_FLAG_POL_DE_HIGH BIT(5) +#define DRM_MODE_FLAG_POL_DE_PRESERVE BIT(6) + struct drm_display_mode { /* Header */ struct list_head head; @@ -144,6 +151,7 @@ struct drm_display_mode { int vrefresh; /* in Hz */ int hsync; /* in kHz */ enum hdmi_picture_aspect picture_aspect_ratio; + unsigned int pol_flags; }; /* mode specified on the command line */
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 v11->v12: - Rebased: This patch now applies against drm_modes.h - Rebased: It now uses the new DRM_MODE_FLAG_POL_DE flags defines names 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_modes.h | 8 ++++++++ 1 file changed, 8 insertions(+)