EDID 1.4 digital displays report the color spaces they support in the features block. Add support for grabbing this data and stuffing it into the display_info struct for driver use.
Signed-off-by: Jesse Barnes jbarnes@virtuousgeek.org --- drivers/gpu/drm/drm_edid.c | 10 ++++++++++ include/drm/drm_crtc.h | 6 +++++- 2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index fb5ebd9..64f8b7f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1429,6 +1429,7 @@ static void drm_add_display_info(struct edid *edid,
/* driver figures it out in this case */ info->bpp = 0; + info->color_formats = 0;
/* Only defined for 1.4 with digital displays */ if (edid->revision < 4) @@ -1461,6 +1462,15 @@ static void drm_add_display_info(struct edid *edid, info->bpp = 0; break; } + + if (edid->features & DRM_EDID_FEATURE_RGB) + info->color_formats = DRM_COLOR_FORMAT_RGB444; + else if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB444) + info->color_formats = DRM_COLOR_FORMAT_RGB444 | + DRM_COLOR_FORMAT_YCRCB444; + else if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB) + info->color_formats = DRM_COLOR_FORMAT_RGB444 | + DRM_COLOR_FORMAT_YCRCB444 | DRM_COLOR_FORMAT_YCRCB422; }
/** diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index aaec097..5cc7008 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -183,7 +183,9 @@ enum subpixel_order { SubPixelNone, };
- +#define DRM_COLOR_FORMAT_RGB444 (1<<0) +#define DRM_COLOR_FORMAT_YCRCB444 (1<<1) +#define DRM_COLOR_FORMAT_YCRCB422 (1<<2) /* * Describes a given display (e.g. CRT or flat panel) and its limitations. */ @@ -198,8 +200,10 @@ struct drm_display_info { unsigned int min_vfreq, max_vfreq; unsigned int min_hfreq, max_hfreq; unsigned int pixel_clock; + unsigned int bpp;
enum subpixel_order subpixel_order; + unsigned long color_formats;
char *raw_edid; /* if any */ };
On 4/15/11 2:40 PM, Jesse Barnes wrote:
@@ -1461,6 +1462,15 @@ static void drm_add_display_info(struct edid *edid, info->bpp = 0; break; }
- if (edid->features & DRM_EDID_FEATURE_RGB)
info->color_formats = DRM_COLOR_FORMAT_RGB444;- else if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB444)
info->color_formats = DRM_COLOR_FORMAT_RGB444 |DRM_COLOR_FORMAT_YCRCB444;- else if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB)
info->color_formats = DRM_COLOR_FORMAT_RGB444 |DRM_COLOR_FORMAT_YCRCB444 | DRM_COLOR_FORMAT_YCRCB422;
Overcomplicated (RGB is numerically 0, YCRCB is just two other values or'd together) and wrong (doesn't handle YCbCr 4:2:2 alone). You want:
info->color_formats = DRM_COLOR_FORMAT_RGB444; if (edid->features & DRM_EDID_FEATURE_YCRCB444) info->color_formats |= DRM_COLOR_FORMAT_YCBCR444; if (edid->features & DRM_EDID_FEATURE_YCRCB422) info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
... which is corrected to not include RGB uselessly in the DRM_EDID_FEATURE_* tokens. I should have noticed that in your first patch, whoops.
- ajax
On Fri, 15 Apr 2011 15:13:02 -0400 Adam Jackson ajax@redhat.com wrote:
On 4/15/11 2:40 PM, Jesse Barnes wrote:
@@ -1461,6 +1462,15 @@ static void drm_add_display_info(struct edid *edid, info->bpp = 0; break; }
- if (edid->features & DRM_EDID_FEATURE_RGB)
info->color_formats = DRM_COLOR_FORMAT_RGB444;- else if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB444)
info->color_formats = DRM_COLOR_FORMAT_RGB444 |DRM_COLOR_FORMAT_YCRCB444;- else if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB)
info->color_formats = DRM_COLOR_FORMAT_RGB444 |DRM_COLOR_FORMAT_YCRCB444 | DRM_COLOR_FORMAT_YCRCB422;Overcomplicated (RGB is numerically 0, YCRCB is just two other values or'd together) and wrong (doesn't handle YCbCr 4:2:2 alone). You want:
Arg, of course I have to mask out the value first, I'll fix that (my current test display conveniently sets RGB_YCRCB so I missed this in testing).
info->color_formats = DRM_COLOR_FORMAT_RGB444; if (edid->features & DRM_EDID_FEATURE_YCRCB444) info->color_formats |= DRM_COLOR_FORMAT_YCBCR444; if (edid->features & DRM_EDID_FEATURE_YCRCB422) info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
... which is corrected to not include RGB uselessly in the DRM_EDID_FEATURE_* tokens. I should have noticed that in your first patch, whoops.
I don't think EDID supports that? The docs I have here imply that either RGB, RGB + YCrCb444 or RGB + YCrCb444 + YCrCb422 are the only things we can report.
Or is there a CEA block extension that allows for more granularity?
On Fri, 15 Apr 2011 12:19:31 -0700 Jesse Barnes jbarnes@virtuousgeek.org wrote:
On Fri, 15 Apr 2011 15:13:02 -0400 Adam Jackson ajax@redhat.com wrote:
info->color_formats = DRM_COLOR_FORMAT_RGB444; if (edid->features & DRM_EDID_FEATURE_YCRCB444) info->color_formats |= DRM_COLOR_FORMAT_YCBCR444; if (edid->features & DRM_EDID_FEATURE_YCRCB422) info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;
... which is corrected to not include RGB uselessly in the DRM_EDID_FEATURE_* tokens. I should have noticed that in your first patch, whoops.
I don't think EDID supports that? The docs I have here imply that either RGB, RGB + YCrCb444 or RGB + YCrCb444 + YCrCb422 are the only things we can report.
Or is there a CEA block extension that allows for more granularity?
Nevermind, I even had the define correct, I just missed it when adding the conditionals. Will fix things to look like the above as it's nicer than doing the switch.
Thanks,
On 4/15/11 3:19 PM, Jesse Barnes wrote:
Or is there a CEA block extension that allows for more granularity?
CEA has bits for the two YCbCr formats too, which we should also parse since there's plenty of 1.3+CEA blocks in the world thanks to HDMI. For CEA blocks version 2 and up (version number in byte 2 of the CEA block, zero-based indexing), (1 << 5) of byte 3 is 4:4:4 and (1 << 4) is 4:2:2. Same byte as what we already check for EDID_BASIC_AUDIO, if that's any clearer. CEA spec contains the same language about always supporting RGB though.
I don't have a good answer for what to do if you have a 1.4+CEA block and the two bitfields are inconsistent, besides violence against the monitor vendor.
- ajax
On Fri, 15 Apr 2011 15:36:22 -0400 Adam Jackson ajax@redhat.com wrote:
On 4/15/11 3:19 PM, Jesse Barnes wrote:
Or is there a CEA block extension that allows for more granularity?
CEA has bits for the two YCbCr formats too, which we should also parse since there's plenty of 1.3+CEA blocks in the world thanks to HDMI. For CEA blocks version 2 and up (version number in byte 2 of the CEA block, zero-based indexing), (1 << 5) of byte 3 is 4:4:4 and (1 << 4) is 4:2:2. Same byte as what we already check for EDID_BASIC_AUDIO, if that's any clearer. CEA spec contains the same language about always supporting RGB though.
Ok, sounds good. I can add that a separate function that runs after we fill out display_info from the input & feature bits.
I don't have a good answer for what to do if you have a 1.4+CEA block and the two bitfields are inconsistent, besides violence against the monitor vendor.
Hm I guess we'd take the CEA block instead as it's probably fresher at least.
dri-devel@lists.freedesktop.org
-
Adam Jackson -
Jesse Barnes