RE: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
Hi Jani
Thanks for your help and I will follow your suggestion to modify the patch.
-Original Message-
From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
Sent: Tuesday, December 10, 2019 7:10 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 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
> Reported-by: kbuild test robot
> ---
> 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_DMT0
> #define LEVEL_GTF1
> #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)) {
I'll try to explain my original comment again.
Consider edid->features & BIT(0). It remains unchanged across the
iteration. The code will only change anything if edid->features &
BIT(0).
> + closure->valid_support_rb = true;
> + closure->support_rb = (r[15] & 0x10) ? true : false;
You could combine these to e.g. a single int.
if (r[10] == BIT(2)) {
int *ret = data;
*ret = !!(r[15] & 0x10);
}
> + }
> + }
> }
>
> /* 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, );
> - return ret;
> + drm_for_each_detailed_block((u8 *)edid, is_rb, );
> + if (closure.valid_support_rb)
> + return closure.support_rb;
Here, you'd do:
if (edid->features & BIT(0)) {
int ret = -1;
drm_for_each_detailed_block((u8 *)edid, is_rb, );
if (ret != -1)
return ret;
}
> }
>
> return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
On Tue, 26 Nov 2019, allen 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
> Reported-by: kbuild test robot
> ---
> 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_DMT0
> #define LEVEL_GTF1
> #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)) {
I'll try to explain my original comment again.
Consider edid->features & BIT(0). It remains unchanged across the
iteration. The code will only change anything if edid->features &
BIT(0).
> + closure->valid_support_rb = true;
> + closure->support_rb = (r[15] & 0x10) ? true : false;
You could combine these to e.g. a single int.
if (r[10] == BIT(2)) {
int *ret = data;
*ret = !!(r[15] & 0x10);
}
> + }
> + }
> }
>
> /* 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, );
> - return ret;
> + drm_for_each_detailed_block((u8 *)edid, is_rb, );
> + if (closure.valid_support_rb)
> + return closure.support_rb;
Here, you'd do:
if (edid->features & BIT(0)) {
int ret = -1;
drm_for_each_detailed_block((u8 *)edid, is_rb, );
if (ret != -1)
return ret;
}
> }
>
> return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
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.rip...@bootlin.com;
linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org;
airl...@linux.ie; pih...@chromium.org; s...@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.nik...@linux.intel.com]
Sent: Tuesday, December 03, 2019 4:02 PM
To: Allen Chen (陳柏宇)
Cc: Jau-Chih Tseng (曾昭智); maxime.rip...@bootlin.com;
linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org;
airl...@linux.ie; pih...@chromium.org; s...@poorly.run
Subject: RE: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking
timings logic
On Tue, 03 Dec 2019, 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.nik...@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 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
>
>> Reported-by: kbuild test robot
>
>> ---
>
>> 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;
>
>> + }
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.nik...@linux.intel.com]
Sent: Tuesday, December 03, 2019 4:02 PM
To: Allen Chen (陳柏宇)
Cc: Jau-Chih Tseng (曾昭智); maxime.rip...@bootlin.com;
linux-ker...@vger.kernel.org; dri-devel@lists.freedesktop.org;
airl...@linux.ie; pih...@chromium.org; s...@poorly.run
Subject: RE: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking
timings logic
On Tue, 03 Dec 2019, 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.nik...@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 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
>
>> Reported-by: kbuild test robot
>
>> ---
>
>> 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,
>
>> + };
>
>> +
>
>>
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 two comments below.
-Original Message-
From: Jani Nikula [mailto:jani.nik...@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 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
> Reported-by: kbuild test robot
> ---
> 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, );
> -return ret;
> + drm_for_each_detailed_block((u8 *)edid, is_rb, );
> + 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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
On Tue, 03 Dec 2019, 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.nik...@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 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
>
>> Reported-by: kbuild test robot
>
>> ---
>
>> 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, );
>
>> -return ret;
>
>> + drm_for_each_detailed_block((u8 *)edid, is_rb, );
>
>> + 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@01D
Re: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
On Tue, 26 Nov 2019, allen 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
> Reported-by: kbuild test robot
> ---
> 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_DMT0
> #define LEVEL_GTF1
> #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, );
> - return ret;
> + drm_for_each_detailed_block((u8 *)edid, is_rb, );
> + 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, 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.
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
On Mon, Nov 11, 2019 at 01:43:52AM +, allen.c...@ite.com.tw wrote:
> Hi Ville Syrjälä
>
> Thanks for your suggestion and I have replied two comments below.
>
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Thursday, November 07, 2019 11:42 PM
> To: Allen Chen (陳柏宇)
> Cc: Jau-Chih Tseng (曾昭智); Maxime Ripard; 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 Mon, Nov 04, 2019 at 04:42:49PM +0800, allen 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 byte0 = 0,
> > byte1 = 0, byte2 = 0 and byte3 = 0xFD
>
> That should probably be some new function:
> bool is_display_descriptor(const u8 *desc, u8 tag);
> is_display_descriptor(EDID_DETAIL_MONITOR_RANGE)
> or something along those lines
>
> We don't seem to check that in most places so should be rolled out all
> over. The usage of struct detailed_timing all over also makes everything
> rather confusing.
>
> > Second: read detailed timing descriptor byte10 = 0x04 and
> > EDID byte18h bit0 = 1
>
> Indicates CVT support. Should give these things real names so
> one wouldn't have to decode by hand.
>
> > Third: if EDID byte18h bit0 == 1 && byte10 == 0x04,
> > then we can check byte15, if byte15 bit4 =1 is support RB
> > if EDID byte18h bit0 != 1 || byte10 != 0x04,
> > then byte15 can not be used
> >
> > The linux code is_rb function not follow the VESA's rule
> >
> > EDID 1.3
> > LCD flat panels do not require long blanking intervals as a retrace
> > period so default support reduced-blanking timings.
> >
> > Signed-off-by: Allen Chen
> > Reported-by: kbuild test robot
> > ---
> > drivers/gpu/drm/drm_edid.c | 28 +---
> > 1 file changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index e5e7e65..9b67b80 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -93,6 +93,11 @@ struct detailed_mode_closure {
> > int modes;
> > };
> >
> > +struct edid_support_rb_closure {
> > + struct edid *edid;
> > + s8 support_rb;
>
> bool
>
> ==> ITE: If use bool, we could not return EDID1.3 when EDID1.4 logic can not
> be applied
Hmm. Could use two bools then.
> > +};
> > +
> > #define LEVEL_DMT 0
> > #define LEVEL_GTF 1
> > #define LEVEL_GTF2 2
> > @@ -2018,22 +2023,31 @@ struct drm_display_mode *drm_mode_find_dmt(struct
> > drm_device *dev,
> > 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 (!r[0] && !r[1] && !r[2] && r[3] == EDID_DETAIL_MONITOR_RANGE) {
> > + if (edid->features & BIT(0) && r[10] == BIT(2))
> > + closure->support_rb = (r[15] & 0x10) ? 1 : 0;
>
> With the bool the ternary operator is not needed. Also should maybe
> be |= in case we have multiple range descriptors? Not sure that is
> legal.
>
> > + }
> > }
> >
> > /* 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,
> > + .support_rb = -1,
> > + };
> > +
> > if (edid->revision >= 4) {
> > - bool ret = false;
> > - drm_for_each_detailed_block((u8 *)edid, is_rb, );
> > - return ret;
> > + drm_for_each_detailed_block((u8 *)edid, is_rb, );
> > + if (closure.support_rb >= 0)
> > + return closure.support_rb;
> > }
> >
> > - return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
> > + return true;
>
> Why are we now assuming rb for all pre 1.4 EDIDs?
>
> ==> ITE: Today, most of the monitor are LCD and LCD monitor do not require
> long blanking intervals as a retrace period so default support
> reduced-blanking timings.
You can't assume such things. Someone out there is surely still using
something that doesn't do reduced blanking.
>
> > }
> >
> > static void
> > --
> > 1.9.1
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
Hi Ville Syrjälä
Thanks for your suggestion and I have replied two comments below.
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Thursday, November 07, 2019 11:42 PM
To: Allen Chen (陳柏宇)
Cc: Jau-Chih Tseng (曾昭智); Maxime Ripard; 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 Mon, Nov 04, 2019 at 04:42:49PM +0800, allen 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 byte0 = 0,
> byte1 = 0, byte2 = 0 and byte3 = 0xFD
That should probably be some new function:
bool is_display_descriptor(const u8 *desc, u8 tag);
is_display_descriptor(EDID_DETAIL_MONITOR_RANGE)
or something along those lines
We don't seem to check that in most places so should be rolled out all
over. The usage of struct detailed_timing all over also makes everything
rather confusing.
> Second: read detailed timing descriptor byte10 = 0x04 and
> EDID byte18h bit0 = 1
Indicates CVT support. Should give these things real names so
one wouldn't have to decode by hand.
> Third: if EDID byte18h bit0 == 1 && byte10 == 0x04,
> then we can check byte15, if byte15 bit4 =1 is support RB
> if EDID byte18h bit0 != 1 || byte10 != 0x04,
> then byte15 can not be used
>
> The linux code is_rb function not follow the VESA's rule
>
> EDID 1.3
> LCD flat panels do not require long blanking intervals as a retrace
> period so default support reduced-blanking timings.
>
> Signed-off-by: Allen Chen
> Reported-by: kbuild test robot
> ---
> drivers/gpu/drm/drm_edid.c | 28 +---
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index e5e7e65..9b67b80 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -93,6 +93,11 @@ struct detailed_mode_closure {
> int modes;
> };
>
> +struct edid_support_rb_closure {
> + struct edid *edid;
> + s8 support_rb;
bool
==> ITE: If use bool, we could not return EDID1.3 when EDID1.4 logic can not
be applied
> +};
> +
> #define LEVEL_DMT0
> #define LEVEL_GTF1
> #define LEVEL_GTF2 2
> @@ -2018,22 +2023,31 @@ struct drm_display_mode *drm_mode_find_dmt(struct
> drm_device *dev,
> 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 (!r[0] && !r[1] && !r[2] && r[3] == EDID_DETAIL_MONITOR_RANGE) {
> + if (edid->features & BIT(0) && r[10] == BIT(2))
> + closure->support_rb = (r[15] & 0x10) ? 1 : 0;
With the bool the ternary operator is not needed. Also should maybe
be |= in case we have multiple range descriptors? Not sure that is
legal.
> + }
> }
>
> /* 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,
> + .support_rb = -1,
> + };
> +
> if (edid->revision >= 4) {
> - bool ret = false;
> - drm_for_each_detailed_block((u8 *)edid, is_rb, );
> - return ret;
> + drm_for_each_detailed_block((u8 *)edid, is_rb, );
> + if (closure.support_rb >= 0)
> + return closure.support_rb;
> }
>
> - return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
> + return true;
Why are we now assuming rb for all pre 1.4 EDIDs?
==> ITE: Today, most of the monitor are LCD and LCD monitor do not require long
blanking intervals as a retrace period so default support reduced-blanking
timings.
> }
>
> static void
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
On Mon, Nov 04, 2019 at 04:42:49PM +0800, allen 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 byte0 = 0,
> byte1 = 0, byte2 = 0 and byte3 = 0xFD
That should probably be some new function:
bool is_display_descriptor(const u8 *desc, u8 tag);
is_display_descriptor(EDID_DETAIL_MONITOR_RANGE)
or something along those lines
We don't seem to check that in most places so should be rolled out all
over. The usage of struct detailed_timing all over also makes everything
rather confusing.
> Second: read detailed timing descriptor byte10 = 0x04 and
> EDID byte18h bit0 = 1
Indicates CVT support. Should give these things real names so
one wouldn't have to decode by hand.
> Third: if EDID byte18h bit0 == 1 && byte10 == 0x04,
> then we can check byte15, if byte15 bit4 =1 is support RB
> if EDID byte18h bit0 != 1 || byte10 != 0x04,
> then byte15 can not be used
>
> The linux code is_rb function not follow the VESA's rule
>
> EDID 1.3
> LCD flat panels do not require long blanking intervals as a retrace
> period so default support reduced-blanking timings.
>
> Signed-off-by: Allen Chen
> Reported-by: kbuild test robot
> ---
> drivers/gpu/drm/drm_edid.c | 28 +---
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index e5e7e65..9b67b80 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -93,6 +93,11 @@ struct detailed_mode_closure {
> int modes;
> };
>
> +struct edid_support_rb_closure {
> + struct edid *edid;
> + s8 support_rb;
bool
> +};
> +
> #define LEVEL_DMT0
> #define LEVEL_GTF1
> #define LEVEL_GTF2 2
> @@ -2018,22 +2023,31 @@ struct drm_display_mode *drm_mode_find_dmt(struct
> drm_device *dev,
> 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 (!r[0] && !r[1] && !r[2] && r[3] == EDID_DETAIL_MONITOR_RANGE) {
> + if (edid->features & BIT(0) && r[10] == BIT(2))
> + closure->support_rb = (r[15] & 0x10) ? 1 : 0;
With the bool the ternary operator is not needed. Also should maybe
be |= in case we have multiple range descriptors? Not sure that is
legal.
> + }
> }
>
> /* 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,
> + .support_rb = -1,
> + };
> +
> if (edid->revision >= 4) {
> - bool ret = false;
> - drm_for_each_detailed_block((u8 *)edid, is_rb, );
> - return ret;
> + drm_for_each_detailed_block((u8 *)edid, is_rb, );
> + if (closure.support_rb >= 0)
> + return closure.support_rb;
> }
>
> - return ((edid->input & DRM_EDID_INPUT_DIGITAL) != 0);
> + return true;
Why are we now assuming rb for all pre 1.4 EDIDs?
> }
>
> static void
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/edid: fixup EDID 1.3 and 1.4 judge reduced-blanking timings logic
Hi allen,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.4-rc5 next-20191031]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/allen/drm-edid-fixup-EDID-1-3-and-1-4-judge-reduced-blanking-timings-logic/20191102-200357
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
1204c70d9dcba31164f78ad5d8c88c42335d51f8
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
smatch warnings:
drivers/gpu/drm/drm_edid.c:2042 drm_monitor_supports_rb() warn: always true
condition '(closure.support_rb >= 0) => (0-255 >= 0)'
vim +2042 drivers/gpu/drm/drm_edid.c
2030
2031 /* EDID 1.4 defines this explicitly. For EDID 1.3, we guess, badly. */
2032 static bool
2033 drm_monitor_supports_rb(struct edid *edid)
2034 {
2035 struct edid_support_rb_closure closure = {
2036 .edid = edid,
2037 .support_rb = -1,
2038 };
2039
2040 if (edid->revision >= 4) {
2041 drm_for_each_detailed_block((u8 *)edid, is_rb,
);
> 2042 if (closure.support_rb >= 0)
2043 return closure.support_rb;
2044 }
2045
2046 return true;
2047 }
2048
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
