From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mario Kleiner Subject: Re: [PATCH 2/2] drm/edid: Add 6 bpc quirk for display AEO model 0. Date: Tue, 14 Jun 2016 16:05:05 +0200 Message-ID: <57600F11.6000803@gmail.com> References: <1464273544-23834-1-git-send-email-mario.kleiner.de@gmail.com> <1464273544-23834-3-git-send-email-mario.kleiner.de@gmail.com> <20160614104449.GS4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20160614104449.GS4329@intel.com> Sender: stable-owner@vger.kernel.org To: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= Cc: dri-devel@lists.freedesktop.org, stable@vger.kernel.org, Jani Nikula , Daniel Vetter , Mario Kleiner List-Id: dri-devel@lists.freedesktop.org On 06/14/2016 12:44 PM, Ville Syrj=E4l=E4 wrote: > On Thu, May 26, 2016 at 04:39:04PM +0200, Mario Kleiner wrote: >> Bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=3D105331 >> 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")= =2E >> >> 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 >> Cc: stable@vger.kernel.org >> Cc: Jani Nikula >> Cc: Ville Syrj=E4l=E4 >> Cc: Daniel Vetter > > 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=20 cleaned up, merged and hooked up. But as Jani advised me as well, that code might be a bit too much for=20 backporting to stable kernels, and i really also need a fix for stable=20 kernels. So plan is: 1. Get patch 1/2 into drm-next or drm-fixes, which is a revert of Jani'= s=20 patch and thereby fixes the regression i care about. Easily=20 back-portable to stable. 2. Because that would cause mishandling of that panel again, get this=20 patch 2/2 in as well, backport to stable, so owners of that panel stay=20 happy on LTS kernels. I personally don't care much about patch 2/2,=20 because that doesn't fix a kernel bug, but a bug in that panels edid.=20 Given how easy it is to fix with an edid quirk, it would still be nice=20 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=20 helper for Linux 4.8 to fix this properly. I tested this patch against the edid from the fdo bug to make sure it=20 does the right thing. But i don't really care if we keep or drop it. Th= e=20 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 *c= onnector, struct edid *edid) >> >> drm_add_display_info(edid, &connector->display_info, connector); >> >> + if (quirks & EDID_QUIRK_FORCE_6BPC) >> + connector->display_info.bpc =3D 6; >> + >> if (quirks & EDID_QUIRK_FORCE_8BPC) >> connector->display_info.bpc =3D 8; >> >> -- >> 2.7.0 >