On 12/11/19 5:08 PM, Ville Syrjälä wrote:
> On Wed, Dec 11, 2019 at 10:48:42AM +0100, Johan Korsnes wrote:
>> When logging the AVI InfoFrame, clearly indicate whether or not
>> attributes are active/"in effect". The specification is given in
>> CTA-861-G Section 6.4: Format of Version 2, 3 & 4 AVI InfoFrames.
>>
>> Attribute Bytes Requirement
>> Ext. Colorimetry EC0..EC2 Colorimetry (C0,C1) set to Extended.
>> IT Contents Type CN0,CN1 IT Content (ITC) set to True.
>> RGB Quant. Range Q0,Q1 Color Space (Y0..Y2) set to RGB.
>> YCC Quant. Range YQ0,YQ1 Color space (Y0..Y2) set to YCbCr.
>>
>> Example log output with patch applied:
>> HDMI infoframe: Auxiliary Video Information (AVI), version 2, length 13
>> colorspace: RGB
>> scan mode: No Data
>> colorimetry: ITU709
>> picture aspect: 16:9
>> active aspect: Same as Picture
>> itc: IT Content
>> extended colorimetry: N/A (0x0)
>> quantization range: Full
>> nups: Unknown Non-uniform Scaling
>> video code: 16
>> ycc quantization range: N/A (0x0)
>> hdmi content type: Graphics
>> pixel repeat: 0
>> bar top 0, bottom 0, left 0, right 0
>>
>> Signed-off-by: Johan Korsnes <jkorsnes@xxxxxxxxx>
>> Cc: Hans Verkuil <hansverk@xxxxxxxxx>
>> Cc: Martin Bugge <marbugge@xxxxxxxxx>
>>
>> ---
>> v1 -> v2:
>> * Indicate applicability not only for ext. colorimetry
>> ---
>> drivers/video/hdmi.c | 40 ++++++++++++++++++++++++++++++++--------
>> 1 file changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
>> index 9c82e2a0a411..491a77b28a9b 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -1209,16 +1209,40 @@ static void hdmi_avi_infoframe_log(const char *level,
>> hdmi_log(" active aspect: %s\n",
>> hdmi_active_aspect_get_name(frame->active_aspect));
>> hdmi_log(" itc: %s\n", frame->itc ? "IT Content" : "No Data");
>> - hdmi_log(" extended colorimetry: %s\n",
>> - hdmi_extended_colorimetry_get_name(frame->extended_colorimetry));
>> - hdmi_log(" quantization range: %s\n",
>> - hdmi_quantization_range_get_name(frame->quantization_range));
>> +
>> + if (frame->colorimetry == HDMI_COLORIMETRY_EXTENDED)
>> + hdmi_log(" extended colorimetry: %s\n",
>> + hdmi_extended_colorimetry_get_name(frame->extended_colorimetry));
>> + else
>> + hdmi_log(" extended colorimetry: N/A (0x%x)\n",
>> + frame->extended_colorimetry);
>> +
>> + if (frame->colorspace == HDMI_COLORSPACE_RGB)
>> + hdmi_log(" quantization range: %s\n",
>> + hdmi_quantization_range_get_name(frame->quantization_range));
>> + else
>> + hdmi_log(" quantization range: N/A (0x%x)\n",
>> + frame->quantization_range);
>> +
>> hdmi_log(" nups: %s\n", hdmi_nups_get_name(frame->nups));
>> hdmi_log(" video code: %u\n", frame->video_code);
>> - hdmi_log(" ycc quantization range: %s\n",
>> - hdmi_ycc_quantization_range_get_name(frame->ycc_quantization_range));
>> - hdmi_log(" hdmi content type: %s\n",
>> - hdmi_content_type_get_name(frame->content_type));
>> +
>> + if (frame->colorspace == HDMI_COLORSPACE_YUV422 ||
>> + frame->colorspace == HDMI_COLORSPACE_YUV444 ||
>> + frame->colorspace == HDMI_COLORSPACE_YUV420)
>
> ocd nit: order 444||422||420 or 420||422||444
>
Should I send a v3 with your Reviewed-by tag added after fixing this?
>> + hdmi_log(" ycc quantization range: %s\n",
>> + hdmi_ycc_quantization_range_get_name(frame->ycc_quantization_range));
>> + else
>> + hdmi_log(" ycc quantization range: N/A (0x%x)\n",
>> + frame->ycc_quantization_range);
>
> CTA-861-G does recommend that we set YQ to match Q when trasmitting
> RGB. So not sure "N/A" is entirely accurate here. However we also
> found out that following that recommendation did break some crappy
> sinks which get confused when they see RGB + YQ!=0. So now we follow
> that recommendation only for HDMI 2.0+ sinks. Anyways, as long as the
> raw value is present I guess we can stil spot such cases from the logs.
>
Good observation. I don't have any strong opinion about this, so unless
you have any objections I won't change this in v3.
Johan
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
>> +
>> + if (frame->itc)
>> + hdmi_log(" hdmi content type: %s\n",
>> + hdmi_content_type_get_name(frame->content_type));
>> + else
>> + hdmi_log(" hdmi content type: N/A (0x%x)\n",
>> + frame->content_type);
>> +
>> hdmi_log(" pixel repeat: %u\n", frame->pixel_repeat);
>> hdmi_log(" bar top %u, bottom %u, left %u, right %u\n",
>> frame->top_bar, frame->bottom_bar,
>> --
>> 2.23.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
