Following Jani's advice, this series just fixes the stable kernel regressions for DP color precision on Intel by reverting Jani's commit and adding the edid 6 bpc quirk for the AEO 0 panel to fix the original fdo bug that triggered this. Minimal patches to allow easy backporting to affected stable kernels.
I'll send out the patches with improved DP sink precision handling on top of these in a separate series later, once these have landed.
thanks, -mario
This reverts commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown")
This commit introduced a regression into stable kernels, as it reduces output color depth to 6 bpc for any video sink connected to a Displayport connector if that sink doesn't report a specific color depth via EDID, or if our EDID parser doesn't actually recognize the proper bpc from EDID.
Affected are active DisplayPort->VGA converters and active DisplayPort->DVI converters. Both should be able to handle 8 bpc, but are degraded to 6 bpc with this patch.
The reverted commit was meant to fix Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=105331
A followup patch implements a fix for that specific bug, which is caused by a faulty EDID of the affected DP panel by adding a new EDID quirk for that panel.
DP 18 bpp fallback handling and other improvements to DP sink bpc detection will be handled for future kernels in a separate series of patches.
Please backport to stable.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Acked-by: Jani Nikula jani.nikula@intel.com Cc: stable@vger.kernel.org Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 46f9be3..3a57fa1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12066,21 +12066,11 @@ connected_sink_compute_bpp(struct intel_connector *connector, pipe_config->pipe_bpp = connector->base.display_info.bpc*3; }
- /* Clamp bpp to default limit on screens without EDID 1.4 */ - if (connector->base.display_info.bpc == 0) { - int type = connector->base.connector_type; - int clamp_bpp = 24; - - /* 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; - - if (bpp > clamp_bpp) { - DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit of %d\n", - bpp, clamp_bpp); - pipe_config->pipe_bpp = clamp_bpp; - } + /* Clamp bpp to 8 on screens without EDID 1.4 */ + if (connector->base.display_info.bpc == 0 && bpp > 24) { + DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit of 24\n", + bpp); + pipe_config->pipe_bpp = 24; } }
On Thu, May 26, 2016 at 4:39 PM, Mario Kleiner mario.kleiner.de@gmail.com wrote:
This reverts commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown")
This commit introduced a regression into stable kernels, as it reduces output color depth to 6 bpc for any video sink connected to a Displayport connector if that sink doesn't report a specific color depth via EDID, or if our EDID parser doesn't actually recognize the proper bpc from EDID.
Affected are active DisplayPort->VGA converters and active DisplayPort->DVI converters. Both should be able to handle 8 bpc, but are degraded to 6 bpc with this patch.
The reverted commit was meant to fix Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=105331
A followup patch implements a fix for that specific bug, which is caused by a faulty EDID of the affected DP panel by adding a new EDID quirk for that panel.
DP 18 bpp fallback handling and other improvements to DP sink bpc detection will be handled for future kernels in a separate series of patches.
Please backport to stable.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Acked-by: Jani Nikula jani.nikula@intel.com Cc: stable@vger.kernel.org Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
I wonder whether we shouldn't just move this into the DP code, and instead of looking at the edid (which is just pass-through for dp->vga dongles) we should only look at dpcd values? Or maybe only look at the edid value if the sink is native DP, and not when it's a dongle.
That would probably also avoid the quirk, and that quirk seems a bit fishy. -Daniel
On 06/14/2016 01:05 PM, Daniel Vetter wrote:
On Thu, May 26, 2016 at 4:39 PM, Mario Kleiner mario.kleiner.de@gmail.com wrote:
This reverts commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown")
This commit introduced a regression into stable kernels, as it reduces output color depth to 6 bpc for any video sink connected to a Displayport connector if that sink doesn't report a specific color depth via EDID, or if our EDID parser doesn't actually recognize the proper bpc from EDID.
Affected are active DisplayPort->VGA converters and active DisplayPort->DVI converters. Both should be able to handle 8 bpc, but are degraded to 6 bpc with this patch.
The reverted commit was meant to fix Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=105331
A followup patch implements a fix for that specific bug, which is caused by a faulty EDID of the affected DP panel by adding a new EDID quirk for that panel.
DP 18 bpp fallback handling and other improvements to DP sink bpc detection will be handled for future kernels in a separate series of patches.
Please backport to stable.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Acked-by: Jani Nikula jani.nikula@intel.com Cc: stable@vger.kernel.org Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
I wonder whether we shouldn't just move this into the DP code, and instead of looking at the edid (which is just pass-through for dp->vga dongles) we should only look at dpcd values? Or maybe only look at the edid value if the sink is native DP, and not when it's a dongle.
That would probably also avoid the quirk, and that quirk seems a bit fishy. -Daniel
This patch is just a simple fix for the color depth regression which affects stable kernels. It can be back-ported easily to affected stable kernels, as Jani advised me.
I wanted to clean up and resubmit that DP helper function which looks at dpcd values and might be a bit too much for stable, once this fix is in.
-mario
Any news on this one?
Thanks a bunch, -mario
On 06/14/2016 04:12 PM, Mario Kleiner wrote:
On 06/14/2016 01:05 PM, Daniel Vetter wrote:
On Thu, May 26, 2016 at 4:39 PM, Mario Kleiner mario.kleiner.de@gmail.com wrote:
This reverts commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown")
This commit introduced a regression into stable kernels, as it reduces output color depth to 6 bpc for any video sink connected to a Displayport connector if that sink doesn't report a specific color depth via EDID, or if our EDID parser doesn't actually recognize the proper bpc from EDID.
Affected are active DisplayPort->VGA converters and active DisplayPort->DVI converters. Both should be able to handle 8 bpc, but are degraded to 6 bpc with this patch.
The reverted commit was meant to fix Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=105331
A followup patch implements a fix for that specific bug, which is caused by a faulty EDID of the affected DP panel by adding a new EDID quirk for that panel.
DP 18 bpp fallback handling and other improvements to DP sink bpc detection will be handled for future kernels in a separate series of patches.
Please backport to stable.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Acked-by: Jani Nikula jani.nikula@intel.com Cc: stable@vger.kernel.org Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
I wonder whether we shouldn't just move this into the DP code, and instead of looking at the edid (which is just pass-through for dp->vga dongles) we should only look at dpcd values? Or maybe only look at the edid value if the sink is native DP, and not when it's a dongle.
That would probably also avoid the quirk, and that quirk seems a bit fishy. -Daniel
This patch is just a simple fix for the color depth regression which affects stable kernels. It can be back-ported easily to affected stable kernels, as Jani advised me.
I wanted to clean up and resubmit that DP helper function which looks at dpcd values and might be a bit too much for stable, once this fix is in.
-mario
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 DP panel with faulty EDID.
A fix for this was made by commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown").
That commit triggers new regressions in precision for DP->DVI and DP->VGA displays. A patch is out to revert that commit, but it 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.
Tested by injecting the edid from the fdo bug into a DP connector via drm_kms_helper.edid_firmware and verifying the 6 bpc + dithering is selected.
This patch should be backported to stable.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: stable@vger.kernel.org Cc: Jani Nikula jani.nikula@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch --- 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 7df26d4..2cb472b 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 }, @@ -4082,6 +4087,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;
On Thu, May 26, 2016 at 04:39:04PM +0200, Mario Kleiner wrote:
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 DP panel with faulty EDID.
A fix for this was made by commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown").
That commit triggers new regressions in precision for DP->DVI and DP->VGA displays. A patch is out to revert that commit, but it 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.
Tested by injecting the edid from the fdo bug into a DP connector via drm_kms_helper.edid_firmware and verifying the 6 bpc + dithering is selected.
This patch should be backported to stable.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: stable@vger.kernel.org Cc: Jani Nikula jani.nikula@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
Now I'm confused. I thought we didn't need any quirks?
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 7df26d4..2cb472b 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 },
@@ -4082,6 +4087,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;
-- 2.7.0
On 06/14/2016 12:44 PM, Ville Syrjälä wrote:
On Thu, May 26, 2016 at 04:39:04PM +0200, Mario Kleiner wrote:
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 DP panel with faulty EDID.
A fix for this was made by commit 013dd9e03872 ("drm/i915/dp: fall back to 18 bpp when sink capability is unknown").
That commit triggers new regressions in precision for DP->DVI and DP->VGA displays. A patch is out to revert that commit, but it 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.
Tested by injecting the edid from the fdo bug into a DP connector via drm_kms_helper.edid_firmware and verifying the 6 bpc + dithering is selected.
This patch should be backported to stable.
Signed-off-by: Mario Kleiner mario.kleiner.de@gmail.com Cc: stable@vger.kernel.org Cc: Jani Nikula jani.nikula@intel.com Cc: Ville Syrjälä ville.syrjala@linux.intel.com Cc: Daniel Vetter daniel.vetter@ffwll.ch
Now I'm confused. I thought we didn't need any quirks?
We don't need a quirk for this, once the DP sink bpc helper i wrote is cleaned up, merged and hooked up.
But as Jani advised me as well, that code might be a bit too much for backporting to stable kernels, and i really also need a fix for stable kernels.
So plan is:
1. Get patch 1/2 into drm-next or drm-fixes, which is a revert of Jani's patch and thereby fixes the regression i care about. Easily back-portable to stable.
2. Because that would cause mishandling of that panel again, get this patch 2/2 in as well, backport to stable, so owners of that panel stay happy on LTS kernels. I personally don't care much about patch 2/2, because that doesn't fix a kernel bug, but a bug in that panels edid. Given how easy it is to fix with an edid quirk, it would still be nice to apply, so stable kernels can deal better with that panel.
3. On top of 1 i'll then resubmit a cleaned up version of a new DP helper for Linux 4.8 to fix this properly.
I tested this patch against the edid from the fdo bug to make sure it does the right thing. But i don't really care if we keep or drop it. The important one is 1/2 for fixing the stable regression.
thanks, -mario
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 7df26d4..2cb472b 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 },
@@ -4082,6 +4087,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;
-- 2.7.0
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Mario Kleiner -
Ville Syrjälä