Hi Ville,
On 10-01-2017 17:21, Ville Syrjälä wrote:
[snip]
>> But we already have color_formats field in drm_display_info
>> struct, right? Shouldn't we instead create for example a helper
>> which returns the best output colorspace? According to what you
>> said it would be something like:
>>
>> if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
>> return YCBCR444;
>> else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
>> return YCBCR422;
>> else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR420
>> && vic_is_420)
>> return YCBCR420;
>> else
>> return RGB444; /* Mandatory by spec */
> Perhaps. But it would have to be more involved than that since there
> might limitations on eg. the max TMDS clock imposed by the source or
> cable/dongle which presumably might require that we pick 4:2:0 over
> 4:4:4.
>
> It would also need to account which formats are actually supported by
> the source.
>
> I guess for bpc it would be enough to just consider 8bpc in such a
> function, and then the driver can bump it up afterwards if possible.
But the max tmds clock will probably be involved in deep color
modes, as 24bpp has always a 1x factor in every YCbCr, except
4:2:0. So, the sink has a max tmds but this gets into account
when the vic list present in the EDID is built, but not
considered in deep color modes, unless the EDID is broken.
>
> As for the RGB vs. YCbCr question, I guess we should just prefer RGB444
> for now. And fall back to YCbCr 4:2:2 or 4:2:0 if necessary. And that
> leaves YCbCr 4:4:4 unsed since it has the same requirements as RGB
> 4:4:4 and thus doesn't provide any benefit as such. We could later add
> a property to let the user choose between RGB vs. YCbCr more explicitly.
>
Hmm, I am trying to implement this but I am facing a difficulty:
how will I fallback to YCbCr? RGB is always supported and the max
tmds only enters in deep color modes. For reference here is a
simple struct i created with the different tmds character rate
factors for the different encodings (I think they are correct,
but cross check please):
#define DRM_CS_DESC(cs, f, b) \
.colorspace = (cs), .factor_to_khz = (f), .bpc = (b)
static const struct drm_mode_colorspace_desc {
u32 colorspace;
u32 factor_to_khz;
u32 bpc;
} drm_mode_colorspace_factors = { /* Ordered by descending
preference */
{ DRM_CS_DESC(DRM_COLOR_FORMAT_RGB444, 2000, 48) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB444, 2000, 48) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_RGB444, 1500, 36) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB444, 1500, 36) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_RGB444, 1250, 30) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB444, 1250, 30) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_RGB444, 1000, 24) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB444, 1000, 24) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB422, 1000, 24) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB422, 1000, 30) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB422, 1000, 36) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB420, 1000, 48) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB420, 750, 36) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB420, 625, 30) },
{ DRM_CS_DESC(DRM_COLOR_FORMAT_YCRCB420, 500, 24) },
Notice how YCbCr 4:4:4 will never get picked: it has the same
factor of RGB 4:4:4 for every bpc. So, the sink must support RGB
4:4:4 and may support YCbCr. What I didn't check was that if it
is possible to have support for deep color YCbCr without having
support for deep color RGB 4:4:4. Do you know if this can happen?
Best regards,
Jose Miguel Abreu
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
