diff mbox

[1/2] drm/i915/dp: Revert "drm/i915/dp: fall back to 18 bpp when sink capability is unknown"

Message ID 1464273544-23834-2-git-send-email-mario.kleiner.de@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mario Kleiner May 26, 2016, 2:39 p.m. UTC
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(-)

Comments

Daniel Vetter June 14, 2016, 11:05 a.m. UTC | #1
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
Mario Kleiner June 14, 2016, 2:12 p.m. UTC | #2
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
Mario Kleiner June 21, 2016, 3:48 p.m. UTC | #3
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
diff mbox

Patch

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;
 	}
 }