Bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=105331
received a potential fix that was backported to stable. While that patch itself is correct for treating DP video sinks with "unknown color depth", it uncovered some lack in our general EDID 1.3 handling, and in how we treat DP->DVI/VGA, causing the fall back of Intel DP to 6 bpc / 18 bpp in cases where it shouldn't fall back. That leads to unhappy neuroscience/medical users of Intel gpus which need their DP->DVI or DP->VGA display devices to operate at at least 8 bpc without dithering.
The following three patches try to improve our EDID handling and Intel DP to try harder to detect the proper bpc to avoid these regressions for DP-DVI and DP-VGA. The third patch tries to fix FDO bug 105331 without causing general unhappiness of other users.
thanks, -mario
According to E-EDID spec 1.3, table 3.9, a digital video sink with the "DFP 1.x compliant TMDS" bit set is "signal compatible with VESA DFP 1.x TMDS CRGB, 1 pixel / clock, up to 8 bits / color MSB aligned".
For such displays, the DFP spec 1.0, section 3.10 "EDID support" says:
"If the DFP monitor only supports EDID 1.X (1.1, 1.2, etc.) without extensions, the host will make the following assumptions:
1. 24-bit MSB-aligned RGB TFT 2. DE polarity is active high 3. H and V syncs are active high 4. Established CRT timings will be used 5. Dithering will not be enabled on the host"
So if we don't know the bit depth of the display from additional colorimetry info we should assume 8 bpc / 24 bpp with no dithering by default.
This patch adds info->bpc = 8 assignement for that case.
Now the DVI 1.0 spec (section 2.2.11.2 "Monitor data format support") mandates that "...If the monitor implements the EDID 2.0, 1.3 or newer data structure the monitor may specify any data format that is definable within the EDID data structure used. In all cases the monitor must support the 24-bit MSB aligned RGB TFT data format as a minimum."
So any DVI display with EDID 1.3 should also have at least info->bpc = 8, even if the "DFP 1.x compliant" bit isn't set, but in our EDID handling we don't know if the EDID comes from a DVI display or something else. I therefore don't know if it is safe to always assume 8 bpc for digital inputs?
Most of my tested DVI sinks set the DFP bit, but one tested Dell panel doesn't.
Lack of handling this correctly was exposed by commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown") which assumes that the sink capability is unknown if our edid handling reports 0 bpc.
As we return bpc = 0 for such displays, that patch will cause a degradation of output precison to 6 bpc for such displays on Intel hw if they are connected via an active DP->dual-link DVI converter and thereby treated as Displayport.
As that patch was backported to stable we should include this one also in stable to fix the regression in color depth for such panels.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: Jani Nikula jani.nikula@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org --- drivers/gpu/drm/drm_edid.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 414d7f6..ff28815 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3810,8 +3810,12 @@ static void drm_add_display_info(struct edid *edid, if (edid->revision < 3) return;
- if (!(edid->input & DRM_EDID_INPUT_DIGITAL)) + if (!(edid->input & DRM_EDID_INPUT_DIGITAL)) { + /* Analog sinks = infinite bpc, but driver decides */ + DRM_DEBUG("%s: Assigning analog sink color depth as %d bpc.\n", + connector->name, info->bpc); return; + }
/* Get data from CEA blocks if present */ edid_ext = drm_find_cea_extension(edid); @@ -3829,6 +3833,31 @@ static void drm_add_display_info(struct edid *edid, /* HDMI deep color modes supported? Assign to info, if so */ drm_assign_hdmi_deep_color_info(edid, info, connector);
+ /* + * Digital sink with "DFP 1.x compliant TMDS" according to EDID 1.3? + * + * For such displays, the DFP spec 1.0, section 3.10 "EDID support" + * says: + * + * "If the DFP monitor only supports EDID 1.X (1.1, 1.2, etc.) + * without extensions, the host will make the following assumptions: + * + * 1. 24-bit MSB-aligned RGB TFT + * 2. DE polarity is active high + * 3. H and V syncs are active high + * 4. Established CRT timings will be used + * 5. Dithering will not be enabled on the host" + * + * So we use 8 bpc in this case and "no dithering" will hopefully + * follow. + */ + if ((info->bpc == 0) && (edid->revision < 4) && + (edid->input & DRM_EDID_DIGITAL_TYPE_DVI)) { + info->bpc = 8; + DRM_DEBUG("%s: Assigning DFP/DVI sink color depth as %d bpc.\n", + connector->name, info->bpc); + } + /* Only defined for 1.4 with digital displays */ if (edid->revision < 4) return;
-------- Forwarded Message -------- Subject: [PATCH 1/3] drm/edid: Set 8 bpc color depth for displays with "DFP 1.x compliant TMDS". Date: Mon, 28 Mar 2016 01:52:45 +0200 From: Mario Kleiner mario.kleiner.de@gmail.com To: dri-devel@lists.freedesktop.org CC: mario.kleiner.de@gmail.com, Jani Nikula jani.nikula@intel.com, Ville Syrjälä ville.syrjala@linux.intel.com, stable@vger.kernel.org
According to E-EDID spec 1.3, table 3.9, a digital video sink with the "DFP 1.x compliant TMDS" bit set is "signal compatible with VESA DFP 1.x TMDS CRGB, 1 pixel / clock, up to 8 bits / color MSB aligned".
For such displays, the DFP spec 1.0, section 3.10 "EDID support" says:
"If the DFP monitor only supports EDID 1.X (1.1, 1.2, etc.) without extensions, the host will make the following assumptions:
1. 24-bit MSB-aligned RGB TFT 2. DE polarity is active high 3. H and V syncs are active high 4. Established CRT timings will be used 5. Dithering will not be enabled on the host"
So if we don't know the bit depth of the display from additional colorimetry info we should assume 8 bpc / 24 bpp with no dithering by default.
This patch adds info->bpc = 8 assignement for that case.
Now the DVI 1.0 spec (section 2.2.11.2 "Monitor data format support") mandates that "...If the monitor implements the EDID 2.0, 1.3 or newer data structure the monitor may specify any data format that is definable within the EDID data structure used. In all cases the monitor must support the 24-bit MSB aligned RGB TFT data format as a minimum."
So any DVI display with EDID 1.3 should also have at least info->bpc = 8, even if the "DFP 1.x compliant" bit isn't set, but in our EDID handling we don't know if the EDID comes from a DVI display or something else. I therefore don't know if it is safe to always assume 8 bpc for digital inputs?
Most of my tested DVI sinks set the DFP bit, but one tested Dell panel doesn't.
Lack of handling this correctly was exposed by commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown") which assumes that the sink capability is unknown if our edid handling reports 0 bpc.
As we return bpc = 0 for such displays, that patch will cause a degradation of output precison to 6 bpc for such displays on Intel hw if they are connected via an active DP->dual-link DVI converter and thereby treated as Displayport.
As that patch was backported to stable we should include this one also in stable to fix the regression in color depth for such panels.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: Jani Nikula jani.nikula@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: stable@vger.kernel.org --- drivers/gpu/drm/drm_edid.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 414d7f6..ff28815 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3810,8 +3810,12 @@ static void drm_add_display_info(struct edid *edid, if (edid->revision < 3) return;
- if (!(edid->input & DRM_EDID_INPUT_DIGITAL)) + if (!(edid->input & DRM_EDID_INPUT_DIGITAL)) { + /* Analog sinks = infinite bpc, but driver decides */ + DRM_DEBUG("%s: Assigning analog sink color depth as %d bpc.\n", + connector->name, info->bpc); return; + }
/* Get data from CEA blocks if present */ edid_ext = drm_find_cea_extension(edid); @@ -3829,6 +3833,31 @@ static void drm_add_display_info(struct edid *edid, /* HDMI deep color modes supported? Assign to info, if so */ drm_assign_hdmi_deep_color_info(edid, info, connector);
+ /* + * Digital sink with "DFP 1.x compliant TMDS" according to EDID 1.3? + * + * For such displays, the DFP spec 1.0, section 3.10 "EDID support" + * says: + * + * "If the DFP monitor only supports EDID 1.X (1.1, 1.2, etc.) + * without extensions, the host will make the following assumptions: + * + * 1. 24-bit MSB-aligned RGB TFT + * 2. DE polarity is active high + * 3. H and V syncs are active high + * 4. Established CRT timings will be used + * 5. Dithering will not be enabled on the host" + * + * So we use 8 bpc in this case and "no dithering" will hopefully + * follow. + */ + if ((info->bpc == 0) && (edid->revision < 4) && + (edid->input & DRM_EDID_DIGITAL_TYPE_DVI)) { + info->bpc = 8; + DRM_DEBUG("%s: Assigning DFP/DVI sink color depth as %d bpc.\n", + connector->name, info->bpc); + } + /* Only defined for 1.4 with digital displays */ if (edid->revision < 4) return;
This fixes a regression in output precision for DVI and VGA video sinks connected to Intel hw via active DisplayPort->DVI/VGA converters.
The regression was indirectly introduced by commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown").
Our current drm edid 1.3 handling can't reliably assign a proper minimum supported display depth of 8 bpc to all DVI sinks, as mandated by DVI 1.0 spec, section 2.2.11.2 "Monitor data format support", but returns 0 bpc = "Don't know" instead. For analog VGA sinks it also returns 0 bpc, although those sinks themselves have infinite color depth, only restricted by the DAC resolution of the encoder.
If a VGA or dual-link DVI display is connected via DisplayPort connector then due to above commit the driver would fall back to only 6 bpc, which would cause degradation for DVI and VGA displays, annoying in general, but especially harmful for application of display devices used in neuroscience research and for medical diagnosic which absolutely need native non-dithered 8 bpc at a minimum to operate correctly.
For DP connectors with bpc == 0 according to EDID, fix this problem by checking the dpcd data to find out if a DP->legacy converter is connected. If the converter is DP->DVI/HDMI assume 8 bpc depth. If the converter is DP->VGA assume at least 8 bpc, but try to get a more accurate value (8, 10, 12 or 16 bpc) if the converter exposes this info.
As the "fall back to 18 bpp" patch was backported to stable we should include this one also into stable to fix the regression in color depth for such panels.
Tested with MiniDP->DP adapter, MiniDP->HDMI adapter, MiniDP->single-link DVI adapter, MiniDP->dual-link DVI active adapter, and Apple MiniDP->VGA active adapter.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: Jani Nikula jani.nikula@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: stable@vger.kernel.org --- drivers/gpu/drm/i915/intel_display.c | 12 ++++++++ drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6e0d828..9903949 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12018,12 +12018,24 @@ connected_sink_compute_bpp(struct intel_connector *connector, if (connector->base.display_info.bpc == 0) { int type = connector->base.connector_type; int clamp_bpp = 24; + int legacy_bpc = 0;
/* Fall back to 18 bpp when DP sink capability is unknown. */ if (type == DRM_MODE_CONNECTOR_DisplayPort || type == DRM_MODE_CONNECTOR_eDP) clamp_bpp = 18;
+ /* On DP to legacy converter, try harder to find sink bpc */ + if (type == DRM_MODE_CONNECTOR_DisplayPort) { + legacy_bpc = intel_dp_legacy_bpc(&connector->base); + + if (legacy_bpc) { + DRM_DEBUG_KMS("DP to VGA/DVI/HDMI converter with bpc %d\n", + legacy_bpc); + clamp_bpp = 3 * legacy_bpc; + } + } + if (bpp > clamp_bpp) { DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit of %d\n", bpp, clamp_bpp); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index f069a82..963a8f8 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6117,3 +6117,59 @@ void intel_dp_mst_resume(struct drm_device *dev) } } } + +/* XXX Needs work for more than 1 downstream port */ +int intel_dp_legacy_bpc(struct drm_connector *connector) +{ + struct intel_dp *intel_dp = intel_attached_dp(connector); + uint8_t *dpcd = intel_dp->dpcd; + uint8_t type; + int bpc = 0; + + /* if there's no downstream port, then this can't be DP->legacy */ + if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) + return bpc; + + /* Basic type of downstream ports */ + type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & + DP_DWN_STRM_PORT_TYPE_MASK; + + /* + * Lacking other info, 8 bpc is a reasonable start for analog out. + * E.g., Apple MiniDP->VGA adaptors don't provide more info than + * that. Despite having DP_DPCD_REV == 0x11 their downstream_ports + * descriptor is empty - all zeros. + */ + if (type == DP_DWN_STRM_PORT_TYPE_ANALOG) + bpc = 8; + + if (intel_dp->dpcd[DP_DPCD_REV] < 0x11) + return bpc; + + /* Rev 1.1+. More specific downstream port type available */ + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; + + /* VGA, DVI and HDMI support at least 8 bpc */ + if (type == DP_DS_PORT_TYPE_VGA || type == DP_DS_PORT_TYPE_DVI || + type == DP_DS_PORT_TYPE_HDMI) + bpc = 8; + + /* As of DP interop v1.1a only VGA defines additional detail */ + if (type != DP_DS_PORT_TYPE_VGA || + !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE)) + return bpc; + + /* VGA with DP_DETAILED_CAP_INFO_AVAILABLE provides bpc info */ + switch (intel_dp->downstream_ports[2] & DP_DS_VGA_MAX_BPC_MASK) { + case DP_DS_VGA_8BPC: + return 8; + case DP_DS_VGA_10BPC: + return 10; + case DP_DS_VGA_12BPC: + return 12; + case DP_DS_VGA_16BPC: + return 16; + } + + return bpc; +} diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4c027d6..6692788 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1253,6 +1253,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp); void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector); void intel_dp_mst_suspend(struct drm_device *dev); void intel_dp_mst_resume(struct drm_device *dev); +int intel_dp_legacy_bpc(struct drm_connector *connector); int intel_dp_max_link_rate(struct intel_dp *intel_dp); int intel_dp_rate_select(struct intel_dp *intel_dp, int rate); void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
-------- Forwarded Message -------- Subject: [PATCH 2/3] drm/i915/dp: Try to find proper bpc for DP->legacy converters. Date: Mon, 28 Mar 2016 01:52:46 +0200 From: Mario Kleiner mario.kleiner.de@gmail.com To: dri-devel@lists.freedesktop.org CC: mario.kleiner.de@gmail.com, Jani Nikula jani.nikula@intel.com, Ville Syrjälä ville.syrjala@linux.intel.com, Daniel Vetter daniel.vetter@ffwll.ch, stable@vger.kernel.org
This fixes a regression in output precision for DVI and VGA video sinks connected to Intel hw via active DisplayPort->DVI/VGA converters.
The regression was indirectly introduced by commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown").
Our current drm edid 1.3 handling can't reliably assign a proper minimum supported display depth of 8 bpc to all DVI sinks, as mandated by DVI 1.0 spec, section 2.2.11.2 "Monitor data format support", but returns 0 bpc = "Don't know" instead. For analog VGA sinks it also returns 0 bpc, although those sinks themselves have infinite color depth, only restricted by the DAC resolution of the encoder.
If a VGA or dual-link DVI display is connected via DisplayPort connector then due to above commit the driver would fall back to only 6 bpc, which would cause degradation for DVI and VGA displays, annoying in general, but especially harmful for application of display devices used in neuroscience research and for medical diagnosic which absolutely need native non-dithered 8 bpc at a minimum to operate correctly.
For DP connectors with bpc == 0 according to EDID, fix this problem by checking the dpcd data to find out if a DP->legacy converter is connected. If the converter is DP->DVI/HDMI assume 8 bpc depth. If the converter is DP->VGA assume at least 8 bpc, but try to get a more accurate value (8, 10, 12 or 16 bpc) if the converter exposes this info.
As the "fall back to 18 bpp" patch was backported to stable we should include this one also into stable to fix the regression in color depth for such panels.
Tested with MiniDP->DP adapter, MiniDP->HDMI adapter, MiniDP->single-link DVI adapter, MiniDP->dual-link DVI active adapter, and Apple MiniDP->VGA active adapter.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: Jani Nikula jani.nikula@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: stable@vger.kernel.org --- drivers/gpu/drm/i915/intel_display.c | 12 ++++++++ drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6e0d828..9903949 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12018,12 +12018,24 @@ connected_sink_compute_bpp(struct intel_connector *connector, if (connector->base.display_info.bpc == 0) { int type = connector->base.connector_type; int clamp_bpp = 24; + int legacy_bpc = 0;
/* Fall back to 18 bpp when DP sink capability is unknown. */ if (type == DRM_MODE_CONNECTOR_DisplayPort || type == DRM_MODE_CONNECTOR_eDP) clamp_bpp = 18;
+ /* On DP to legacy converter, try harder to find sink bpc */ + if (type == DRM_MODE_CONNECTOR_DisplayPort) { + legacy_bpc = intel_dp_legacy_bpc(&connector->base); + + if (legacy_bpc) { + DRM_DEBUG_KMS("DP to VGA/DVI/HDMI converter with bpc %d\n", + legacy_bpc); + clamp_bpp = 3 * legacy_bpc; + } + } + if (bpp > clamp_bpp) { DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit of %d\n", bpp, clamp_bpp); diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index f069a82..963a8f8 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6117,3 +6117,59 @@ void intel_dp_mst_resume(struct drm_device *dev) } } } + +/* XXX Needs work for more than 1 downstream port */ +int intel_dp_legacy_bpc(struct drm_connector *connector) +{ + struct intel_dp *intel_dp = intel_attached_dp(connector); + uint8_t *dpcd = intel_dp->dpcd; + uint8_t type; + int bpc = 0; + + /* if there's no downstream port, then this can't be DP->legacy */ + if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) + return bpc; + + /* Basic type of downstream ports */ + type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & + DP_DWN_STRM_PORT_TYPE_MASK; + + /* + * Lacking other info, 8 bpc is a reasonable start for analog out. + * E.g., Apple MiniDP->VGA adaptors don't provide more info than + * that. Despite having DP_DPCD_REV == 0x11 their downstream_ports + * descriptor is empty - all zeros. + */ + if (type == DP_DWN_STRM_PORT_TYPE_ANALOG) + bpc = 8; + + if (intel_dp->dpcd[DP_DPCD_REV] < 0x11) + return bpc; + + /* Rev 1.1+. More specific downstream port type available */ + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; + + /* VGA, DVI and HDMI support at least 8 bpc */ + if (type == DP_DS_PORT_TYPE_VGA || type == DP_DS_PORT_TYPE_DVI || + type == DP_DS_PORT_TYPE_HDMI) + bpc = 8; + + /* As of DP interop v1.1a only VGA defines additional detail */ + if (type != DP_DS_PORT_TYPE_VGA || + !(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DETAILED_CAP_INFO_AVAILABLE)) + return bpc; + + /* VGA with DP_DETAILED_CAP_INFO_AVAILABLE provides bpc info */ + switch (intel_dp->downstream_ports[2] & DP_DS_VGA_MAX_BPC_MASK) { + case DP_DS_VGA_8BPC: + return 8; + case DP_DS_VGA_10BPC: + return 10; + case DP_DS_VGA_12BPC: + return 12; + case DP_DS_VGA_16BPC: + return 16; + } + + return bpc; +} diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4c027d6..6692788 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1253,6 +1253,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp); void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector); void intel_dp_mst_suspend(struct drm_device *dev); void intel_dp_mst_resume(struct drm_device *dev); +int intel_dp_legacy_bpc(struct drm_connector *connector); int intel_dp_max_link_rate(struct intel_dp *intel_dp); int intel_dp_rate_select(struct intel_dp *intel_dp, int rate); void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
Bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=105331 reports that the "AEO model 0" display is driven with 8 bpc without dithering by default, which looks bad because that panel is apparently a 6 bpc panel.
A fix for this was made by commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown").
However, that commit, while correct in itself, as a side effect triggers new regressions in precision for DisplayPort->DVI and DP->VGA displays. Patches are out to fix those regressions, but they will revert video output for the AEO model 0 panel to 8 bpc without dithering.
The EDID 1.3 of that panel, as decoded from the xrandr output attached to that bugzilla bug report, is somewhat faulty, and beyond other problems also sets the "DFP 1.x compliant TMDS" bit, which according to DFP spec means to drive the panel with 8 bpc and no dithering in absence of other colorimetry information.
Try to make the original bug reporter happy despite the faulty EDID by adding a quirk to mark that panel as 6 bpc, so 6 bpc output with dithering creates a nice picture.
As the original patch was backported to stable and the fixes for regressions caused by that patch are aimed at stable, this patch should also go to stable to make everybody happy again.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: Jani Nikula jani.nikula@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: stable@vger.kernel.org --- drivers/gpu/drm/drm_edid.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index ff28815..7d10537 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -74,6 +74,8 @@ #define EDID_QUIRK_FORCE_8BPC (1 << 8) /* Force 12bpc */ #define EDID_QUIRK_FORCE_12BPC (1 << 9) +/* Force 6bpc */ +#define EDID_QUIRK_FORCE_6BPC (1 << 10)
struct detailed_mode_closure { struct drm_connector *connector; @@ -100,6 +102,9 @@ static struct edid_quirk { /* Unknown Acer */ { "ACR", 2423, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+ /* AEO model 0 reports 8 bpc, but is a 6 bpc panel */ + { "AEO", 0, EDID_QUIRK_FORCE_6BPC }, + /* Belinea 10 15 55 */ { "MAX", 1516, EDID_QUIRK_PREFER_LARGE_60 }, { "MAX", 0x77e, EDID_QUIRK_PREFER_LARGE_60 }, @@ -3950,6 +3955,9 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
drm_add_display_info(edid, &connector->display_info, connector);
+ if (quirks & EDID_QUIRK_FORCE_6BPC) + connector->display_info.bpc = 6; + if (quirks & EDID_QUIRK_FORCE_8BPC) connector->display_info.bpc = 8;
-------- Forwarded Message -------- Subject: [PATCH 3/3] drm/edid: Add 6 bpc quirk for display AEO model 0. Date: Mon, 28 Mar 2016 01:52:47 +0200 From: Mario Kleiner mario.kleiner.de@gmail.com To: dri-devel@lists.freedesktop.org CC: mario.kleiner.de@gmail.com, Jani Nikula jani.nikula@intel.com, Ville Syrjälä ville.syrjala@linux.intel.com, Daniel Vetter daniel.vetter@ffwll.ch, stable@vger.kernel.org
Bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=105331 reports that the "AEO model 0" display is driven with 8 bpc without dithering by default, which looks bad because that panel is apparently a 6 bpc panel.
A fix for this was made by commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown").
However, that commit, while correct in itself, as a side effect triggers new regressions in precision for DisplayPort->DVI and DP->VGA displays. Patches are out to fix those regressions, but they will revert video output for the AEO model 0 panel to 8 bpc without dithering.
The EDID 1.3 of that panel, as decoded from the xrandr output attached to that bugzilla bug report, is somewhat faulty, and beyond other problems also sets the "DFP 1.x compliant TMDS" bit, which according to DFP spec means to drive the panel with 8 bpc and no dithering in absence of other colorimetry information.
Try to make the original bug reporter happy despite the faulty EDID by adding a quirk to mark that panel as 6 bpc, so 6 bpc output with dithering creates a nice picture.
As the original patch was backported to stable and the fixes for regressions caused by that patch are aimed at stable, this patch should also go to stable to make everybody happy again.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: Jani Nikula jani.nikula@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: stable@vger.kernel.org --- drivers/gpu/drm/drm_edid.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index ff28815..7d10537 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -74,6 +74,8 @@ #define EDID_QUIRK_FORCE_8BPC (1 << 8) /* Force 12bpc */ #define EDID_QUIRK_FORCE_12BPC (1 << 9) +/* Force 6bpc */ +#define EDID_QUIRK_FORCE_6BPC (1 << 10)
struct detailed_mode_closure { struct drm_connector *connector; @@ -100,6 +102,9 @@ static struct edid_quirk { /* Unknown Acer */ { "ACR", 2423, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+ /* AEO model 0 reports 8 bpc, but is a 6 bpc panel */ + { "AEO", 0, EDID_QUIRK_FORCE_6BPC }, + /* Belinea 10 15 55 */ { "MAX", 1516, EDID_QUIRK_PREFER_LARGE_60 }, { "MAX", 0x77e, EDID_QUIRK_PREFER_LARGE_60 }, @@ -3950,6 +3955,9 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
drm_add_display_info(edid, &connector->display_info, connector);
+ if (quirks & EDID_QUIRK_FORCE_6BPC) + connector->display_info.bpc = 6; + if (quirks & EDID_QUIRK_FORCE_8BPC) connector->display_info.bpc = 8;
Ping? Could somebody give this a review? Would be good to get the associated regressions in DVI/VGA output precision fixed in intel hw, also for stable kernels which regressed.
Resending the three patches...
thanks, -mario
On 03/28/2016 01:52 AM, Mario Kleiner wrote:
Bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=105331
received a potential fix that was backported to stable. While that patch itself is correct for treating DP video sinks with "unknown color depth", it uncovered some lack in our general EDID 1.3 handling, and in how we treat DP->DVI/VGA, causing the fall back of Intel DP to 6 bpc / 18 bpp in cases where it shouldn't fall back. That leads to unhappy neuroscience/medical users of Intel gpus which need their DP->DVI or DP->VGA display devices to operate at at least 8 bpc without dithering.
The following three patches try to improve our EDID handling and Intel DP to try harder to detect the proper bpc to avoid these regressions for DP-DVI and DP-VGA. The third patch tries to fix FDO bug 105331 without causing general unhappiness of other users.
thanks, -mario
On Mon, Mar 28, 2016 at 01:52:44AM +0200, Mario Kleiner wrote:
Bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=105331
received a potential fix that was backported to stable. While that patch itself is correct for treating DP video sinks with "unknown color depth", it uncovered some lack in our general EDID 1.3 handling, and in how we treat DP->DVI/VGA, causing the fall back of Intel DP to 6 bpc / 18 bpp in cases where it shouldn't fall back. That leads to unhappy neuroscience/medical users of Intel gpus which need their DP->DVI or DP->VGA display devices to operate at at least 8 bpc without dithering.
The following three patches try to improve our EDID handling and Intel DP to try harder to detect the proper bpc to avoid these regressions for DP-DVI and DP-VGA. The third patch tries to fix FDO bug 105331 without causing general unhappiness of other users.
It would seem simpler to me to move the 18bpp fallback into intel_dp.c and only do it for native DP sinks/downstream ports. That way we should avoid the need for any EDID quirks IIUC.
The downstream port caps we should still check I suppose. Later versions of the spec extend the information for pretty much all port types. I started to write something similar [1] a while back, and by the looks of things I was probably basing that on the DP 1.2 spec since 1.3 has even more stuff there. Anyways we should put that logic into the DP helper so that other drivers migth use it as well.
[1] git://github.com/vsyrjala/linux.git dp_downstream_ports
On 05/06/2016 08:27 PM, Ville Syrjälä wrote:
On Mon, Mar 28, 2016 at 01:52:44AM +0200, Mario Kleiner wrote:
Bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=105331
received a potential fix that was backported to stable. While that patch itself is correct for treating DP video sinks with "unknown color depth", it uncovered some lack in our general EDID 1.3 handling, and in how we treat DP->DVI/VGA, causing the fall back of Intel DP to 6 bpc / 18 bpp in cases where it shouldn't fall back. That leads to unhappy neuroscience/medical users of Intel gpus which need their DP->DVI or DP->VGA display devices to operate at at least 8 bpc without dithering.
The following three patches try to improve our EDID handling and Intel DP to try harder to detect the proper bpc to avoid these regressions for DP-DVI and DP-VGA. The third patch tries to fix FDO bug 105331 without causing general unhappiness of other users.
Thanks for the feedback.
It would seem simpler to me to move the 18bpp fallback into intel_dp.c and only do it for native DP sinks/downstream ports. That way we should avoid the need for any EDID quirks IIUC.
I think that specific EDID quirk in patch 3/3 for that FDO bug we'd always need, because that specific panels EDID reports 8 bpc capability by setting the "DFP 1.x compliant TMDS" bit in its EDID 1.3, but according to the FDO bug it needs to be driven with 6 bpc + dithering for good results.
Do you agree with patch 1/3? That would avoid kms drivers needing to work out DFP compliant displays.I think we could probably make the assumption that anything that has EDID 1.3 is 8 bpc capable? DVI spec seems to require that for anything DVI, and i'd assume any VGA DAC manufactured in the last 20 years would have at least 8 bpc?
Wrt. 18 bpp fallback you mean putting it into intel_dp_legacy_bpc() from patch 2/3 or similar and checking that the sink is really not an active DVI or VGA converter?
I tried to keep these patches relatively simple/conservative to allow safe backporting to stable kernels that are affected by the regression.
The downstream port caps we should still check I suppose. Later versions of the spec extend the information for pretty much all port types. I started to write something similar [1] a while back, and by the looks of things I was probably basing that on the DP 1.2 spec since 1.3 has even more stuff there. Anyways we should put that logic into the DP helper so that other drivers migth use it as well.
[1] git://github.com/vsyrjala/linux.git dp_downstream_ports
Have to look at that. I don't have official access to the latest specs, just to whatever i could find floating in the internet.
thanks, -mario
On Fri, May 06, 2016 at 10:03:06PM +0200, Mario Kleiner wrote:
On 05/06/2016 08:27 PM, Ville Syrjälä wrote:
On Mon, Mar 28, 2016 at 01:52:44AM +0200, Mario Kleiner wrote:
Bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=105331
received a potential fix that was backported to stable. While that patch itself is correct for treating DP video sinks with "unknown color depth", it uncovered some lack in our general EDID 1.3 handling, and in how we treat DP->DVI/VGA, causing the fall back of Intel DP to 6 bpc / 18 bpp in cases where it shouldn't fall back. That leads to unhappy neuroscience/medical users of Intel gpus which need their DP->DVI or DP->VGA display devices to operate at at least 8 bpc without dithering.
The following three patches try to improve our EDID handling and Intel DP to try harder to detect the proper bpc to avoid these regressions for DP-DVI and DP-VGA. The third patch tries to fix FDO bug 105331 without causing general unhappiness of other users.
Thanks for the feedback.
It would seem simpler to me to move the 18bpp fallback into intel_dp.c and only do it for native DP sinks/downstream ports. That way we should avoid the need for any EDID quirks IIUC.
I think that specific EDID quirk in patch 3/3 for that FDO bug we'd always need, because that specific panels EDID reports 8 bpc capability by setting the "DFP 1.x compliant TMDS" bit in its EDID 1.3, but according to the FDO bug it needs to be driven with 6 bpc + dithering for good results.
If we just do the fallback for DP, then I don;t think we need any changes to the EDID parser, and hence no quirk either.
Do you agree with patch 1/3? That would avoid kms drivers needing to work out DFP compliant displays.I think we could probably make the assumption that anything that has EDID 1.3 is 8 bpc capable? DVI spec seems to require that for anything DVI, and i'd assume any VGA DAC manufactured in the last 20 years would have at least 8 bpc?
Wrt. 18 bpp fallback you mean putting it into intel_dp_legacy_bpc() from patch 2/3 or similar and checking that the sink is really not an active DVI or VGA converter?
Yeah, I'd just check the downstream port type, and do the 18bpp fallback only for native DP if the sink didn't give us a bpc. For everything else 8bpc seems like a reasonable default (unless the port caps say otherwise, of course).
I tried to keep these patches relatively simple/conservative to allow safe backporting to stable kernels that are affected by the regression.
The downstream port caps we should still check I suppose. Later versions of the spec extend the information for pretty much all port types. I started to write something similar [1] a while back, and by the looks of things I was probably basing that on the DP 1.2 spec since 1.3 has even more stuff there. Anyways we should put that logic into the DP helper so that other drivers migth use it as well.
[1] git://github.com/vsyrjala/linux.git dp_downstream_ports
Have to look at that. I don't have official access to the latest specs, just to whatever i could find floating in the internet.
thanks, -mario
On 05/07/2016 08:15 PM, Ville Syrjälä wrote:
On Fri, May 06, 2016 at 10:03:06PM +0200, Mario Kleiner wrote:
On 05/06/2016 08:27 PM, Ville Syrjälä wrote:
On Mon, Mar 28, 2016 at 01:52:44AM +0200, Mario Kleiner wrote:
Bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=105331
received a potential fix that was backported to stable. While that patch itself is correct for treating DP video sinks with "unknown color depth", it uncovered some lack in our general EDID 1.3 handling, and in how we treat DP->DVI/VGA, causing the fall back of Intel DP to 6 bpc / 18 bpp in cases where it shouldn't fall back. That leads to unhappy neuroscience/medical users of Intel gpus which need their DP->DVI or DP->VGA display devices to operate at at least 8 bpc without dithering.
The following three patches try to improve our EDID handling and Intel DP to try harder to detect the proper bpc to avoid these regressions for DP-DVI and DP-VGA. The third patch tries to fix FDO bug 105331 without causing general unhappiness of other users.
Thanks for the feedback.
It would seem simpler to me to move the 18bpp fallback into intel_dp.c and only do it for native DP sinks/downstream ports. That way we should avoid the need for any EDID quirks IIUC.
I think that specific EDID quirk in patch 3/3 for that FDO bug we'd always need, because that specific panels EDID reports 8 bpc capability by setting the "DFP 1.x compliant TMDS" bit in its EDID 1.3, but according to the FDO bug it needs to be driven with 6 bpc + dithering for good results.
If we just do the fallback for DP, then I don;t think we need any changes to the EDID parser, and hence no quirk either.
You are probably right. Rereading the bug report it seems to be a native DP panel without EDID color info, so the 6 bpc fallback only for native DP sinks should handle that.
I just sent out a new patch, also for stable, which folds the 6 bpc fallback into my DP bpc detection code.
I wonder if it would still make sense to apply the DFP 1.x bit handling in the shared EDID parser, just so we take full advantage of what info a EDID 1.3 provides? Although then we'd need a quirk for that panel again, as it does have a somewhat broken EDID.
thanks, -mario
Do you agree with patch 1/3? That would avoid kms drivers needing to work out DFP compliant displays.I think we could probably make the assumption that anything that has EDID 1.3 is 8 bpc capable? DVI spec seems to require that for anything DVI, and i'd assume any VGA DAC manufactured in the last 20 years would have at least 8 bpc?
Wrt. 18 bpp fallback you mean putting it into intel_dp_legacy_bpc() from patch 2/3 or similar and checking that the sink is really not an active DVI or VGA converter?
Yeah, I'd just check the downstream port type, and do the 18bpp fallback only for native DP if the sink didn't give us a bpc. For everything else 8bpc seems like a reasonable default (unless the port caps say otherwise, of course).
I tried to keep these patches relatively simple/conservative to allow safe backporting to stable kernels that are affected by the regression.
The downstream port caps we should still check I suppose. Later versions of the spec extend the information for pretty much all port types. I started to write something similar [1] a while back, and by the looks of things I was probably basing that on the DP 1.2 spec since 1.3 has even more stuff there. Anyways we should put that logic into the DP helper so that other drivers migth use it as well.
[1] git://github.com/vsyrjala/linux.git dp_downstream_ports
Have to look at that. I don't have official access to the latest specs, just to whatever i could find floating in the internet.
thanks, -mario
dri-devel@lists.freedesktop.org
-
Mario Kleiner -
Ville Syrjälä