lkml.org 
[lkml]   [2019]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
Date
On Tue, 03 Dec 2019, <allen.chen@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@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@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@ite.com.tw>
>
>> Reported-by: kbuild test robot <lkp@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@01D5A9EA.9B7364D0]
>
>
>
> [cid:image005.jpg@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@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.
>
>
>
> BR,
>
> Jani.
>
>
>
>
>
>> }
>
>>
>
>> return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
>
>
>
> --
>
> Jani Nikula, Intel Open Source Graphics Center

--
Jani Nikula, Intel Open Source Graphics Center

\
 
 \ /
  Last update: 2019-12-03 09:02    [W:0.020 / U:3.724 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site