* [PATCH] drm/edid: fix CEA extension byte #3 parsing
@ 2022-03-23 10:04 Jani Nikula
2022-03-23 15:09 ` Ville Syrjälä
0 siblings, 1 reply; 4+ messages in thread
From: Jani Nikula @ 2022-03-23 10:04 UTC (permalink / raw)
To: dri-devel; +Cc: jani.nikula, intel-gfx, Shawn C Lee, stable
Only an EDID CEA extension has byte #3, while the CTA DisplayID Data
Block does not. Don't interpret bogus data for color formats.
For most displays it's probably an unlikely scenario you'd have a CTA
DisplayID Data Block without a CEA extension, but they do exist.
Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
Cc: <stable@vger.kernel.org> # v4.15
Cc: Shawn C Lee <shawn.c.lee@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
commit e28ad544f462 was merged in v5.3, but it has Cc: stable for v4.15.
This is also fixed in my CEA data block iteration series [1], but we'll
want the simple fix for stable first.
Hum, CTA is formerly CEA, I and the code seem to use both, should we use
only one or the other?
[1] https://patchwork.freedesktop.org/series/101659/
---
drivers/gpu/drm/drm_edid.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 561f53831e29..ccf7031a6797 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5187,10 +5187,14 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
/* The existence of a CEA block should imply RGB support */
info->color_formats = DRM_COLOR_FORMAT_RGB444;
- if (edid_ext[3] & EDID_CEA_YCRCB444)
- info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
- if (edid_ext[3] & EDID_CEA_YCRCB422)
- info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
+
+ /* CTA DisplayID Data Block does not have byte #3 */
+ if (edid_ext[0] == CEA_EXT) {
+ if (edid_ext[3] & EDID_CEA_YCRCB444)
+ info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
+ if (edid_ext[3] & EDID_CEA_YCRCB422)
+ info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
+ }
if (cea_db_offsets(edid_ext, &start, &end))
return;
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/edid: fix CEA extension byte #3 parsing
2022-03-23 10:04 [PATCH] drm/edid: fix CEA extension byte #3 parsing Jani Nikula
@ 2022-03-23 15:09 ` Ville Syrjälä
2022-03-23 15:19 ` Jani Nikula
2022-03-24 9:58 ` Jani Nikula
0 siblings, 2 replies; 4+ messages in thread
From: Ville Syrjälä @ 2022-03-23 15:09 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, Shawn C Lee, stable, dri-devel
On Wed, Mar 23, 2022 at 12:04:38PM +0200, Jani Nikula wrote:
> Only an EDID CEA extension has byte #3, while the CTA DisplayID Data
> Block does not. Don't interpret bogus data for color formats.
I think what we might want eventually is a cleaner split between
the CTA data blocks vs. the rest of the EDID CTA ext block. Only
the former is relevant for DisplayID.
But for a bugfix we want to keep it simple.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> For most displays it's probably an unlikely scenario you'd have a CTA
> DisplayID Data Block without a CEA extension, but they do exist.
>
> Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
> Cc: <stable@vger.kernel.org> # v4.15
> Cc: Shawn C Lee <shawn.c.lee@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> ---
>
> commit e28ad544f462 was merged in v5.3, but it has Cc: stable for v4.15.
>
> This is also fixed in my CEA data block iteration series [1], but we'll
> want the simple fix for stable first.
>
> Hum, CTA is formerly CEA, I and the code seem to use both, should we use
> only one or the other?
And before CEA it was called EIA (IIRC). Dunno if we also use that name
somewhere.
If someone cares enough I guess we could rename everything to "cta".
>
> [1] https://patchwork.freedesktop.org/series/101659/
> ---
> drivers/gpu/drm/drm_edid.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 561f53831e29..ccf7031a6797 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5187,10 +5187,14 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>
> /* The existence of a CEA block should imply RGB support */
> info->color_formats = DRM_COLOR_FORMAT_RGB444;
> - if (edid_ext[3] & EDID_CEA_YCRCB444)
> - info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
> - if (edid_ext[3] & EDID_CEA_YCRCB422)
> - info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
> +
> + /* CTA DisplayID Data Block does not have byte #3 */
> + if (edid_ext[0] == CEA_EXT) {
> + if (edid_ext[3] & EDID_CEA_YCRCB444)
> + info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
> + if (edid_ext[3] & EDID_CEA_YCRCB422)
> + info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
> + }
>
> if (cea_db_offsets(edid_ext, &start, &end))
> return;
> --
> 2.30.2
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/edid: fix CEA extension byte #3 parsing
2022-03-23 15:09 ` Ville Syrjälä
@ 2022-03-23 15:19 ` Jani Nikula
2022-03-24 9:58 ` Jani Nikula
1 sibling, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2022-03-23 15:19 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Shawn C Lee, stable, dri-devel
On Wed, 23 Mar 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Mar 23, 2022 at 12:04:38PM +0200, Jani Nikula wrote:
>> Only an EDID CEA extension has byte #3, while the CTA DisplayID Data
>> Block does not. Don't interpret bogus data for color formats.
>
> I think what we might want eventually is a cleaner split between
> the CTA data blocks vs. the rest of the EDID CTA ext block. Only
> the former is relevant for DisplayID.
Well, I just abstracted it all away in the CEA data block iteration
series [1].
It'll be possible to add a different iteration initializer depending on
what you want to iterate and where.
BR,
Jani.
>
> But for a bugfix we want to keep it simple.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>>
>> For most displays it's probably an unlikely scenario you'd have a CTA
>> DisplayID Data Block without a CEA extension, but they do exist.
>>
>> Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
>> Cc: <stable@vger.kernel.org> # v4.15
>> Cc: Shawn C Lee <shawn.c.lee@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> ---
>>
>> commit e28ad544f462 was merged in v5.3, but it has Cc: stable for v4.15.
>>
>> This is also fixed in my CEA data block iteration series [1], but we'll
>> want the simple fix for stable first.
>>
>> Hum, CTA is formerly CEA, I and the code seem to use both, should we use
>> only one or the other?
>
> And before CEA it was called EIA (IIRC). Dunno if we also use that name
> somewhere.
>
> If someone cares enough I guess we could rename everything to "cta".
>
>>
>> [1] https://patchwork.freedesktop.org/series/101659/
>> ---
>> drivers/gpu/drm/drm_edid.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 561f53831e29..ccf7031a6797 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -5187,10 +5187,14 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>
>> /* The existence of a CEA block should imply RGB support */
>> info->color_formats = DRM_COLOR_FORMAT_RGB444;
>> - if (edid_ext[3] & EDID_CEA_YCRCB444)
>> - info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
>> - if (edid_ext[3] & EDID_CEA_YCRCB422)
>> - info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
>> +
>> + /* CTA DisplayID Data Block does not have byte #3 */
>> + if (edid_ext[0] == CEA_EXT) {
>> + if (edid_ext[3] & EDID_CEA_YCRCB444)
>> + info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
>> + if (edid_ext[3] & EDID_CEA_YCRCB422)
>> + info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
>> + }
>>
>> if (cea_db_offsets(edid_ext, &start, &end))
>> return;
>> --
>> 2.30.2
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/edid: fix CEA extension byte #3 parsing
2022-03-23 15:09 ` Ville Syrjälä
2022-03-23 15:19 ` Jani Nikula
@ 2022-03-24 9:58 ` Jani Nikula
1 sibling, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2022-03-24 9:58 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, Shawn C Lee, stable, dri-devel
On Wed, 23 Mar 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Mar 23, 2022 at 12:04:38PM +0200, Jani Nikula wrote:
>> Only an EDID CEA extension has byte #3, while the CTA DisplayID Data
>> Block does not. Don't interpret bogus data for color formats.
>
> I think what we might want eventually is a cleaner split between
> the CTA data blocks vs. the rest of the EDID CTA ext block. Only
> the former is relevant for DisplayID.
>
> But for a bugfix we want to keep it simple.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks, pushed to drm-misc-next-fixes.
BR,
Jani.
>
>>
>> For most displays it's probably an unlikely scenario you'd have a CTA
>> DisplayID Data Block without a CEA extension, but they do exist.
>>
>> Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID")
>> Cc: <stable@vger.kernel.org> # v4.15
>> Cc: Shawn C Lee <shawn.c.lee@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> ---
>>
>> commit e28ad544f462 was merged in v5.3, but it has Cc: stable for v4.15.
>>
>> This is also fixed in my CEA data block iteration series [1], but we'll
>> want the simple fix for stable first.
>>
>> Hum, CTA is formerly CEA, I and the code seem to use both, should we use
>> only one or the other?
>
> And before CEA it was called EIA (IIRC). Dunno if we also use that name
> somewhere.
>
> If someone cares enough I guess we could rename everything to "cta".
>
>>
>> [1] https://patchwork.freedesktop.org/series/101659/
>> ---
>> drivers/gpu/drm/drm_edid.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 561f53831e29..ccf7031a6797 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -5187,10 +5187,14 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
>>
>> /* The existence of a CEA block should imply RGB support */
>> info->color_formats = DRM_COLOR_FORMAT_RGB444;
>> - if (edid_ext[3] & EDID_CEA_YCRCB444)
>> - info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
>> - if (edid_ext[3] & EDID_CEA_YCRCB422)
>> - info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
>> +
>> + /* CTA DisplayID Data Block does not have byte #3 */
>> + if (edid_ext[0] == CEA_EXT) {
>> + if (edid_ext[3] & EDID_CEA_YCRCB444)
>> + info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
>> + if (edid_ext[3] & EDID_CEA_YCRCB422)
>> + info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
>> + }
>>
>> if (cea_db_offsets(edid_ext, &start, &end))
>> return;
>> --
>> 2.30.2
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-24 9:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-23 10:04 [PATCH] drm/edid: fix CEA extension byte #3 parsing Jani Nikula
2022-03-23 15:09 ` Ville Syrjälä
2022-03-23 15:19 ` Jani Nikula
2022-03-24 9:58 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).