[PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
allen.chen at ite.com.tw
allen.chen at ite.com.tw
Tue Dec 10 08:01:56 UTC 2019
Hi Jani
Sorry to bother you. May this patch is OK to be upstream?
If have any suggestion, I will try to modify the code to meet the upstream standard.
From: Allen Chen (陳柏宇)
Sent: Thursday, December 05, 2019 9:24 AM
To: 'Jani Nikula'
Cc: Jau-Chih Tseng (曾昭智); maxime.ripard at bootlin.com; linux-kernel at vger.kernel.org; dri-devel at lists.freedesktop.org; airlied at linux.ie; pihsun at chromium.org; sean at poorly.run
Subject: RE: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
Hi Jani Nikula
Thanks for your suggestion and I have replied one comment below.
-----Original Message-----
From: Jani Nikula [mailto:jani.nikula at linux.intel.com]
Sent: Tuesday, December 03, 2019 4:02 PM
To: Allen Chen (陳柏宇)
Cc: Jau-Chih Tseng (曾昭智); maxime.ripard at bootlin.com; linux-kernel at vger.kernel.org; dri-devel at lists.freedesktop.org; airlied at linux.ie; pihsun at chromium.org; sean at poorly.run
Subject: RE: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
On Tue, 03 Dec 2019, <allen.chen at ite.com.tw> wrote:
> Hi Jani Nikula
>
>
>
> Thanks for your suggestion and I have replied two comments below.
Please read my question again.
BR,
Jani.
>
>
>
> -----Original Message-----
> From: Jani Nikula [mailto:jani.nikula at linux.intel.com]
> Sent: Wednesday, November 27, 2019 6:29 PM
> To: Allen Chen (陳柏宇)
> Cc: Jau-Chih Tseng (曾昭智); Maxime Ripard; Allen Chen (陳柏宇); open list; open list:DRM DRIVERS; David Airlie; Pi-Hsun Shih; Sean Paul
> Subject: Re: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
>
>
>
> On Tue, 26 Nov 2019, allen <allen.chen at ite.com.tw> wrote:
>
>> According to VESA ENHANCED EXTENDED DISPLAY IDENTIFICATION DATA STANDARD
>
>> (Defines EDID Structure Version 1, Revision 4) page: 39
>
>> How to determine whether the monitor support RB timing or not?
>
>> EDID 1.4
>
>> First: read detailed timing descriptor and make sure byte 0 = 0x00,
>
>> byte 1 = 0x00, byte 2 = 0x00 and byte 3 = 0xFD
>
>> Second: read EDID bit 0 in feature support byte at address 18h = 1
>
>> and detailed timing descriptor byte 10 = 0x04
>
>> Third: if EDID bit 0 in feature support byte = 1 &&
>
>> detailed timing descriptor byte 10 = 0x04
>
>> then we can check byte 15, if bit 4 in byte 15 = 1 is support RB
>
>> if EDID bit 0 in feature support byte != 1 ||
>
>> detailed timing descriptor byte 10 != 0x04,
>
>> then byte 15 can not be used
>
>>
>
>> The linux code is_rb function not follow the VESA's rule
>
>>
>
>> Signed-off-by: Allen Chen <allen.chen at ite.com.tw>
>
>> Reported-by: kbuild test robot <lkp at intel.com>
>
>> ---
>
>> drivers/gpu/drm/drm_edid.c | 36 ++++++++++++++++++++++++++++++------
>
>> 1 file changed, 30 insertions(+), 6 deletions(-)
>
>>
>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>
>> index f5926bf..e11e585 100644
>
>> --- a/drivers/gpu/drm/drm_edid.c
>
>> +++ b/drivers/gpu/drm/drm_edid.c
>
>> @@ -93,6 +93,12 @@ struct detailed_mode_closure {
>
>> int modes;
>
>> };
>
>>
>
>> +struct edid_support_rb_closure {
>
>> + struct edid *edid;
>
>> + bool valid_support_rb;
>
>> + bool support_rb;
>
>> +};
>
>> +
>
>> #define LEVEL_DMT 0
>
>> #define LEVEL_GTF 1
>
>> #define LEVEL_GTF2 2
>
>> @@ -2017,23 +2023,41 @@ struct drm_display_mode *drm_mode_find_dmt(struct drm_device *dev,
>
>> }
>
>> }
>
>>
>
>> +static bool
>
>> +is_display_descriptor(const u8 *r, u8 tag)
>
>> +{
>
>> + return (!r[0] && !r[1] && !r[2] && r[3] == tag) ? true : false;
>
>> +}
>
>> +
>
>> static void
>
>> is_rb(struct detailed_timing *t, void *data)
>
>> {
>
>> u8 *r = (u8 *)t;
>
>> - if (r[3] == EDID_DETAIL_MONITOR_RANGE)
>
>> - if (r[15] & 0x10)
>
>> - *(bool *)data = true;
>
>> + struct edid_support_rb_closure *closure = data;
>
>> + struct edid *edid = closure->edid;
>
>> +
>
>> + if (is_display_descriptor(r, EDID_DETAIL_MONITOR_RANGE)) {
>
>> + if (edid->features & BIT(0) && r[10] == BIT(2)) {
>
>> + closure->valid_support_rb = true;
>
>> + closure->support_rb = (r[15] & 0x10) ? true : false;
>
>> + }
>
>> + }
>
>> }
>
>>
>
>> /* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */
>
>> static bool
>
>> drm_monitor_supports_rb(struct edid *edid)
>
>> {
>
>> + struct edid_support_rb_closure closure = {
>
>> + .edid = edid,
>
>> + .valid_support_rb = false,
>
>> + .support_rb = false,
>
>> + };
>
>> +
>
>> if (edid->revision >= 4) {
>
>> - bool ret = false;
>
>> - drm_for_each_detailed_block((u8 *)edid, is_rb, &ret);
>
>> - return ret;
>
>> + drm_for_each_detailed_block((u8 *)edid, is_rb, &closure);
>
>> + if (closure.valid_support_rb)
>
>> + return closure.support_rb;
>
>
>
> Are you planning on extending the closure use somehow?
>
>
>
> I did not look up the spec,
>
>
>
> ==> iTE: as the picture below, from VESA E-EDID standard
>
> [cid:image003.jpg at 01D5A9EA.9B7364D0]
>
>
>
> [cid:image005.jpg at 01D5A9EA.9B7364D0]
>
>
>
> if EDID bit 0 in feature support byte = 1 && detailed timing descriptor byte 10 = 0x04 then the CVT timing supported.
>
>
>
> [cid:image009.jpg at 01D5A9EA.9B7364D0]
>
>
>
> If CVT timing supported then we can check byte 15 bit 4 to determine whether the reduced-blanking timings suported or not.
>
> If CVT timing not supported then we can not use byte 15 to judge.
>
> but purely on the code changes alone, you
>
> could just move the edid->features bit check at this level, and not pass
>
> it down, and nothing would change. I.e. don't iterate the EDID at all if
>
> the bit is not set.
>
>
>
> ð iTE: We still have to check whether detailed timing descriptor byte 10 = 0x04 or not, so it is hard to check at this level
>
> You also don't actually use the distinction between valid_support_rb
>
> vs. support_rb for anything, so the closure just adds code.
==> ITE: From Ville Syrjälä <ville.syrjala at linux.intel.com<mailto:ville.syrjala at linux.intel.com>>'s suggestion.
[cid:image001.png at 01D5AF72.CEA3EE00]
>
>
>
> BR,
>
> Jani.
>
>
>
>
>
>> }
>
>>
>
>> return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
>
>
>
> --
>
> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20191210/214aa368/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.png
Type: image/png
Size: 13750 bytes
Desc: image001.png
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20191210/214aa368/attachment-0001.png>
More information about the dri-devel
mailing list