[PATCH 2/2] drm/edid: Add 6 bpc quirk for display AEO model 0.

[Mario Kleiner]

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 at gmail.com>
>> Cc: stable at vger.kernel.org
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter at 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
>