Hi,
This is a new & split out version of the last patch of my "drm/fsl-dcu: fixes and enhancements" patchset: https://lkml.org/lkml/2015/11/18/949
Instead of using struct drm_display_mode to convey the pixel clock polarity information, this patchset introduces a new field called bus_flags stored in struct drm_display_info.
-- Stefan
Changes since v1: - Introduce bus_flags to convey the pixel clock polarity from panel-simple.c to the driver.
Stefan Agner (3): drm/fsl-dcu: use mode flags for hsync/vsync polarity drm: introduce bus_flags in drm_display_info drm/fsl-dcu: use bus_flags for pixel clock polarity
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 16 +++++++++++++--- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 4 ++-- drivers/gpu/drm/panel/panel-simple.c | 6 +++++- include/drm/drm_crtc.h | 9 +++++++++ 4 files changed, 29 insertions(+), 6 deletions(-)
The current default configuration is as follows: - Invert VSYNC signal (active LOW) - Invert HSYNC signal (active LOW)
The mode flags allow to specify the required polarity per mode. Furthermore, none of the current driver settings is actually a standard polarity.
This patch applies the current driver default polarities as explicit flags to the display which has been introduced with the driver (NEC WQVGA "nec,nl4827hc19-05b"). The driver now also parses the flags field and applies the configuration accordingly, by using the following values as standard polarities: (e.g. when no flags are specified): - VSYNC signal not inverted (active HIGH) - HSYNC signal not inverted (active HIGH)
Signed-off-by: Stefan Agner stefan@agner.ch --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 11 ++++++++--- drivers/gpu/drm/panel/panel-simple.c | 1 + 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c index 62377e4..b36f815 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c @@ -74,7 +74,7 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; struct drm_display_mode *mode = &crtc->state->mode; - unsigned int hbp, hfp, hsw, vbp, vfp, vsw, div, index; + unsigned int hbp, hfp, hsw, vbp, vfp, vsw, div, index, pol = 0; unsigned long dcuclk;
index = drm_crtc_index(crtc); @@ -89,6 +89,12 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) vfp = mode->vsync_start - mode->vdisplay; vsw = mode->vsync_end - mode->vsync_start;
+ if (mode->flags & DRM_MODE_FLAG_NHSYNC) + pol |= DCU_SYN_POL_INV_HS_LOW; + + if (mode->flags & DRM_MODE_FLAG_NVSYNC) + pol |= DCU_SYN_POL_INV_VS_LOW; + regmap_write(fsl_dev->regmap, DCU_HSYN_PARA, DCU_HSYN_PARA_BP(hbp) | DCU_HSYN_PARA_PW(hsw) | @@ -101,8 +107,7 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) DCU_DISP_SIZE_DELTA_Y(mode->vdisplay) | DCU_DISP_SIZE_DELTA_X(mode->hdisplay)); regmap_write(fsl_dev->regmap, DCU_DIV_RATIO, div); - regmap_write(fsl_dev->regmap, DCU_SYN_POL, - DCU_SYN_POL_INV_VS_LOW | DCU_SYN_POL_INV_HS_LOW); + regmap_write(fsl_dev->regmap, DCU_SYN_POL, pol); regmap_write(fsl_dev->regmap, DCU_BGND, DCU_BGND_R(0) | DCU_BGND_G(0) | DCU_BGND_B(0)); regmap_write(fsl_dev->regmap, DCU_DCU_MODE, diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index f88a631..2164c99 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -1016,6 +1016,7 @@ static const struct drm_display_mode nec_nl4827hc19_05b_mode = { .vsync_end = 272 + 2 + 4, .vtotal = 272 + 2 + 4 + 2, .vrefresh = 74, + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, };
static const struct panel_desc nec_nl4827hc19_05b = {
On Mon, Feb 08, 2016 at 01:57:41PM -0800, Stefan Agner wrote:
The current default configuration is as follows:
- Invert VSYNC signal (active LOW)
- Invert HSYNC signal (active LOW)
The mode flags allow to specify the required polarity per mode. Furthermore, none of the current driver settings is actually a standard polarity.
This patch applies the current driver default polarities as explicit flags to the display which has been introduced with the driver (NEC WQVGA "nec,nl4827hc19-05b"). The driver now also parses the flags field and applies the configuration accordingly, by using the following values as standard polarities: (e.g. when no flags are specified):
- VSYNC signal not inverted (active HIGH)
- HSYNC signal not inverted (active HIGH)
Signed-off-by: Stefan Agner stefan@agner.ch
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 11 ++++++++--- drivers/gpu/drm/panel/panel-simple.c | 1 + 2 files changed, 9 insertions(+), 3 deletions(-)
Acked-by: Thierry Reding treding@nvidia.com
On 2016-02-08 13:57, Stefan Agner wrote:
The current default configuration is as follows:
- Invert VSYNC signal (active LOW)
- Invert HSYNC signal (active LOW)
The mode flags allow to specify the required polarity per mode. Furthermore, none of the current driver settings is actually a standard polarity.
This patch applies the current driver default polarities as explicit flags to the display which has been introduced with the driver (NEC WQVGA "nec,nl4827hc19-05b"). The driver now also parses the flags field and applies the configuration accordingly, by using the following values as standard polarities: (e.g. when no flags are specified):
- VSYNC signal not inverted (active HIGH)
- HSYNC signal not inverted (active HIGH)
Signed-off-by: Stefan Agner stefan@agner.ch
Applied.
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 11 ++++++++--- drivers/gpu/drm/panel/panel-simple.c | 1 + 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c index 62377e4..b36f815 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c @@ -74,7 +74,7 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; struct drm_display_mode *mode = &crtc->state->mode;
- unsigned int hbp, hfp, hsw, vbp, vfp, vsw, div, index;
unsigned int hbp, hfp, hsw, vbp, vfp, vsw, div, index, pol = 0; unsigned long dcuclk;
index = drm_crtc_index(crtc);
@@ -89,6 +89,12 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) vfp = mode->vsync_start - mode->vdisplay; vsw = mode->vsync_end - mode->vsync_start;
- if (mode->flags & DRM_MODE_FLAG_NHSYNC)
pol |= DCU_SYN_POL_INV_HS_LOW;- if (mode->flags & DRM_MODE_FLAG_NVSYNC)
pol |= DCU_SYN_POL_INV_VS_LOW;- regmap_write(fsl_dev->regmap, DCU_HSYN_PARA, DCU_HSYN_PARA_BP(hbp) | DCU_HSYN_PARA_PW(hsw) |
@@ -101,8 +107,7 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) DCU_DISP_SIZE_DELTA_Y(mode->vdisplay) | DCU_DISP_SIZE_DELTA_X(mode->hdisplay)); regmap_write(fsl_dev->regmap, DCU_DIV_RATIO, div);
- regmap_write(fsl_dev->regmap, DCU_SYN_POL,
DCU_SYN_POL_INV_VS_LOW | DCU_SYN_POL_INV_HS_LOW);
- regmap_write(fsl_dev->regmap, DCU_SYN_POL, pol); regmap_write(fsl_dev->regmap, DCU_BGND, DCU_BGND_R(0) | DCU_BGND_G(0) | DCU_BGND_B(0)); regmap_write(fsl_dev->regmap, DCU_DCU_MODE,
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index f88a631..2164c99 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -1016,6 +1016,7 @@ static const struct drm_display_mode nec_nl4827hc19_05b_mode = { .vsync_end = 272 + 2 + 4, .vtotal = 272 + 2 + 4 + 2, .vrefresh = 74,
- .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
};
static const struct panel_desc nec_nl4827hc19_05b = {
Introduce bus_flags to specify display bus properties like signal polarities. This is useful for parallel display buses, e.g. to specify the pixel clock or data enable polarity.
Suggested-by: Thierry Reding thierry.reding@gmail.com Signed-off-by: Stefan Agner stefan@agner.ch --- include/drm/drm_crtc.h | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c65a212..20a489d 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -126,6 +126,14 @@ enum subpixel_order { #define DRM_COLOR_FORMAT_RGB444 (1<<0) #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) + +#define DRM_BUS_FLAG_DE_LOW (1<<0) +#define DRM_BUS_FLAG_DE_HIGH (1<<1) +/* drive data on pos. edge */ +#define DRM_BUS_FLAG_PIXDATA_POSEDGE (1<<2) +/* drive data on neg. edge */ +#define DRM_BUS_FLAG_PIXDATA_NEGEDGE (1<<3) + /* * Describes a given display (e.g. CRT or flat panel) and its limitations. */ @@ -147,6 +155,7 @@ struct drm_display_info {
const u32 *bus_formats; unsigned int num_bus_formats; + u32 bus_flags;
/* Mask of supported hdmi deep color modes */ u8 edid_hdmi_dc_modes;
The drivers current default configuration drives the pixel data on rising edge of the pixel clock. However, most display sample data on rising edge... This leads to color shift artefacts visible especially at edges.
This patch changes the relevant defines to be useful and actually set the bits, and changes pixel clock polarity to drive the pixel data on falling edge by default. The patch also adds an explicit pixel clock polarity flag to the display introduced with the driver (NEC WQVGA "nec,nl4827hc19-05b") using the new bus_flags field to retain the initial behavior.
Signed-off-by: Stefan Agner stefan@agner.ch --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 5 +++++ drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 4 ++-- drivers/gpu/drm/panel/panel-simple.c | 5 ++++- 3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c index b36f815..afcbdd8 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c @@ -73,6 +73,7 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; + struct drm_connector *con = &fsl_dev->connector.base; struct drm_display_mode *mode = &crtc->state->mode; unsigned int hbp, hfp, hsw, vbp, vfp, vsw, div, index, pol = 0; unsigned long dcuclk; @@ -89,6 +90,10 @@ static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc) vfp = mode->vsync_start - mode->vdisplay; vsw = mode->vsync_end - mode->vsync_start;
+ /* INV_PXCK as default (most display sample data on rising edge) */ + if (!(con->display_info.bus_flags & DRM_BUS_FLAG_PIXDATA_POSEDGE)) + pol |= DCU_SYN_POL_INV_PXCK; + if (mode->flags & DRM_MODE_FLAG_NHSYNC) pol |= DCU_SYN_POL_INV_HS_LOW;
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h index 6413ac9..af3a707 100644 --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h @@ -47,8 +47,8 @@ #define DCU_VSYN_PARA_FP(x) (x)
#define DCU_SYN_POL 0x0024 -#define DCU_SYN_POL_INV_PXCK_FALL (0 << 6) -#define DCU_SYN_POL_NEG_REMAIN (0 << 5) +#define DCU_SYN_POL_INV_PXCK BIT(6) +#define DCU_SYN_POL_NEG BIT(5) #define DCU_SYN_POL_INV_VS_LOW BIT(1) #define DCU_SYN_POL_INV_HS_LOW BIT(0)
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 2164c99..7fae3d9 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -72,6 +72,7 @@ struct panel_desc { } delay;
u32 bus_format; + u32 bus_flags; };
struct panel_simple { @@ -144,6 +145,7 @@ static int panel_simple_get_fixed_modes(struct panel_simple *panel) if (panel->desc->bus_format) drm_display_info_set_bus_formats(&connector->display_info, &panel->desc->bus_format, 1); + connector->display_info.bus_flags = panel->desc->bus_flags;
return num; } @@ -1027,7 +1029,8 @@ static const struct panel_desc nec_nl4827hc19_05b = { .width = 95, .height = 54, }, - .bus_format = MEDIA_BUS_FMT_RGB888_1X24 + .bus_format = MEDIA_BUS_FMT_RGB888_1X24, + .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE, };
static const struct display_timing okaya_rs800480t_7x0gp_timing = {
Any comments on this?
Also added Manfred, Tomi and Boris to CC which previously attended in similar discussions.
Previous discussions: http://thread.gmane.org/gmane.linux.kernel.api/12830 http://thread.gmane.org/gmane.comp.video.dri.devel/96240/
I think one of the main observation so far was that the pixel clock polarity is not a property of the mode, and therefor does not fit into the DRM_MODE_FLAG. This has been pointed out nicely by Russel: http://thread.gmane.org/gmane.comp.video.dri.devel/96240/focus=96260
Embedded displays connected through parallel bus make use of the bus_formats field in drm_display_mode. This field defines what kind of bus format the display requires. This patch follows that idea and adds bus_flags. bus_flags can be used to define specific bus properties required by the display, such as pixel clock or data enable polarity...
On 2016-02-08 13:57, Stefan Agner wrote:
Hi,
This is a new & split out version of the last patch of my "drm/fsl-dcu: fixes and enhancements" patchset: https://lkml.org/lkml/2015/11/18/949
Instead of using struct drm_display_mode to convey the pixel clock polarity information, this patchset introduces a new field called bus_flags stored in struct drm_display_info.
Note that this solution has been briefly discussed on IRC: https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highli...
-- Stefan
Changes since v1:
- Introduce bus_flags to convey the pixel clock polarity from panel-simple.c to the driver.
Stefan Agner (3): drm/fsl-dcu: use mode flags for hsync/vsync polarity drm: introduce bus_flags in drm_display_info drm/fsl-dcu: use bus_flags for pixel clock polarity
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 16 +++++++++++++--- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 4 ++-- drivers/gpu/drm/panel/panel-simple.c | 6 +++++- include/drm/drm_crtc.h | 9 +++++++++ 4 files changed, 29 insertions(+), 6 deletions(-)
Am Dienstag, den 23.02.2016, 15:30 -0800 schrieb Stefan Agner:
Any comments on this?
None other that I'm all in favor. consider patch 2 Acked-by: Philipp Zabel p.zabel@pengutronix.de
Also added Manfred, Tomi and Boris to CC which previously attended in similar discussions.
Previous discussions: http://thread.gmane.org/gmane.linux.kernel.api/12830 http://thread.gmane.org/gmane.comp.video.dri.devel/96240/
I think one of the main observation so far was that the pixel clock polarity is not a property of the mode, and therefor does not fit into the DRM_MODE_FLAG. This has been pointed out nicely by Russel: http://thread.gmane.org/gmane.comp.video.dri.devel/96240/focus=96260
Embedded displays connected through parallel bus make use of the bus_formats field in drm_display_mode. This field defines what kind of bus format the display requires. This patch follows that idea and adds bus_flags. bus_flags can be used to define specific bus properties required by the display, such as pixel clock or data enable polarity...
regards Philipp
On 2016-02-24 11:28, Philipp Zabel wrote:
Am Dienstag, den 23.02.2016, 15:30 -0800 schrieb Stefan Agner:
Any comments on this?
None other that I'm all in favor. consider patch 2 Acked-by: Philipp Zabel p.zabel@pengutronix.de
Same here! Acked-by: Manfred Schlaegl manfred.schlaegl@gmx.at
regards Manfred
Hi,
On 24/02/16 01:30, Stefan Agner wrote:
Any comments on this?
Also added Manfred, Tomi and Boris to CC which previously attended in similar discussions.
Previous discussions: http://thread.gmane.org/gmane.linux.kernel.api/12830 http://thread.gmane.org/gmane.comp.video.dri.devel/96240/
I think one of the main observation so far was that the pixel clock polarity is not a property of the mode, and therefor does not fit into the DRM_MODE_FLAG. This has been pointed out nicely by Russel: http://thread.gmane.org/gmane.comp.video.dri.devel/96240/focus=96260
Embedded displays connected through parallel bus make use of the bus_formats field in drm_display_mode. This field defines what kind of bus format the display requires. This patch follows that idea and adds bus_flags. bus_flags can be used to define specific bus properties required by the display, such as pixel clock or data enable polarity...
I think it would be good to split the generic and fsl changes to separate patches.
I agree that pixel clock polarity shouldn't be visible to userspace.
I had a look at MIPI DPI spec, and it says "The rising edge of PCLK is used by the display module to capture pixel data.". So, I think that means if the panels are MIPI DPI compatible, they should always sample at rising edge. I'm sure there are exceptions, but that behaviour should probably be the default, then.
I'm also a bit curious on what is "videomode". Why is sync polarity part of it, and settable by the userspace, but not pixel clock polarity? "videomode" is just whatever is in the CEA spec, because DRM originates from the PC world? Is there any reason nowadays for the user to ever set sync polarities?
For MIPI DPI panels, sync polarity is as much a property of the panel as pixel clock polarity: there's only one correct setting for it (usually).
Tomi
On 2016-02-24 03:06, Tomi Valkeinen wrote:
Hi,
On 24/02/16 01:30, Stefan Agner wrote:
Any comments on this?
Also added Manfred, Tomi and Boris to CC which previously attended in similar discussions.
Previous discussions: http://thread.gmane.org/gmane.linux.kernel.api/12830 http://thread.gmane.org/gmane.comp.video.dri.devel/96240/
I think one of the main observation so far was that the pixel clock polarity is not a property of the mode, and therefor does not fit into the DRM_MODE_FLAG. This has been pointed out nicely by Russel: http://thread.gmane.org/gmane.comp.video.dri.devel/96240/focus=96260
Embedded displays connected through parallel bus make use of the bus_formats field in drm_display_mode. This field defines what kind of bus format the display requires. This patch follows that idea and adds bus_flags. bus_flags can be used to define specific bus properties required by the display, such as pixel clock or data enable polarity...
I think it would be good to split the generic and fsl changes to separate patches.
I guess we talk about PATCH 3/3?
This is intentionally in one patch: The problem is that the current default setting of the driver is the correct setting for that one NEC display. The patch changes the default setting of the display controller driver, but to make sure the NEC display continues to work I need to add the polarity flag to the display. Therefor I feel this is one logical change... Also this way the display keeps working between every commit (bisectability...)
The core changes (the flag defines and adding the field to struct drm_display_info) are in a separate patch.
I agree that pixel clock polarity shouldn't be visible to userspace.
I had a look at MIPI DPI spec, and it says "The rising edge of PCLK is used by the display module to capture pixel data.". So, I think that means if the panels are MIPI DPI compatible, they should always sample at rising edge. I'm sure there are exceptions, but that behaviour should probably be the default, then.
Yes, that is the new default of the driver.
I'm also a bit curious on what is "videomode". Why is sync polarity part of it, and settable by the userspace, but not pixel clock polarity? "videomode" is just whatever is in the CEA spec, because DRM originates from the PC world? Is there any reason nowadays for the user to ever set sync polarities?
For MIPI DPI panels, sync polarity is as much a property of the panel as pixel clock polarity: there's only one correct setting for it (usually).
We probably could add the sync properties to the bus_flags too, and those would take precedence over the mode sync polarities...? Certainly something which could be done in a follow up patch.
-- Stefan
On Wed, Feb 24, 2016 at 01:06:39PM +0200, Tomi Valkeinen wrote:
Hi,
On 24/02/16 01:30, Stefan Agner wrote:
Any comments on this?
Also added Manfred, Tomi and Boris to CC which previously attended in similar discussions.
Previous discussions: http://thread.gmane.org/gmane.linux.kernel.api/12830 http://thread.gmane.org/gmane.comp.video.dri.devel/96240/
I think one of the main observation so far was that the pixel clock polarity is not a property of the mode, and therefor does not fit into the DRM_MODE_FLAG. This has been pointed out nicely by Russel: http://thread.gmane.org/gmane.comp.video.dri.devel/96240/focus=96260
Embedded displays connected through parallel bus make use of the bus_formats field in drm_display_mode. This field defines what kind of bus format the display requires. This patch follows that idea and adds bus_flags. bus_flags can be used to define specific bus properties required by the display, such as pixel clock or data enable polarity...
I think it would be good to split the generic and fsl changes to separate patches.
I agree that pixel clock polarity shouldn't be visible to userspace.
I had a look at MIPI DPI spec, and it says "The rising edge of PCLK is used by the display module to capture pixel data.". So, I think that means if the panels are MIPI DPI compatible, they should always sample at rising edge. I'm sure there are exceptions, but that behaviour should probably be the default, then.
I'm also a bit curious on what is "videomode". Why is sync polarity part of it, and settable by the userspace, but not pixel clock polarity? "videomode" is just whatever is in the CEA spec, because DRM originates from the PC world? Is there any reason nowadays for the user to ever set sync polarities?
Yes, the EDID/something will tell us the modes the display claims to use including the sync polarities (and note that they are not the same for every listed mode), but the way the kms API works is that the user specifies the full timings anyway. So if we didn't have sync polarities as part of the mode, we wouldn't know what to output unless we went trawling through the mode list looking for a match, which may not even be present if the user specified a custom mode.
For MIPI DPI panels, sync polarity is as much a property of the panel as pixel clock polarity: there's only one correct setting for it (usually).
In i915 we ignore the user requested timings almost entirely when dealing with LVDS/eDP/DSI. The only think we keep is that size of the active video portion, and we use that as the input size for the panel fitter (== scaler). All the actual timings used to drive the display come from EDID or the VBT (video BIOS table).
For external displays we do respect what the user has requested, including the sync polarities.
dri-devel@lists.freedesktop.org
-
Manfred Schlaegl -
Philipp Zabel -
Stefan Agner -
Thierry Reding -
Tomi Valkeinen -
Ville Syrjälä