* [PATCH] drivers: video: hdmi: log ext colorimetry applicability
@ 2019-10-03 7:15 Johan Korsnes
2019-10-03 13:44 ` Ville Syrjälä
2019-10-03 14:59 ` Hans Verkuil
0 siblings, 2 replies; 7+ messages in thread
From: Johan Korsnes @ 2019-10-03 7:15 UTC (permalink / raw)
To: dri-devel; +Cc: linux-media, Johan Korsnes
When logging the AVI InfoFrame, clearly indicate whether or not the
extended colorimetry attribute is active. This is only the case when
the AVI InfoFrame colorimetry attribute is set to extended. [0]
[0] CTA-861-G section 6.4 page 57
Signed-off-by: Johan Korsnes <jkorsnes@cisco.com>
---
drivers/video/hdmi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index f29db728ff29..a709e38a53ca 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -682,8 +682,14 @@ 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",
+
+ 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);
+
hdmi_log(" quantization range: %s\n",
hdmi_quantization_range_get_name(frame->quantization_range));
hdmi_log(" nups: %s\n", hdmi_nups_get_name(frame->nups));
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] drivers: video: hdmi: log ext colorimetry applicability
2019-10-03 7:15 [PATCH] drivers: video: hdmi: log ext colorimetry applicability Johan Korsnes
@ 2019-10-03 13:44 ` Ville Syrjälä
2019-10-03 14:53 ` Johan Korsnes (jkorsnes)
2019-10-03 15:02 ` Hans Verkuil
2019-10-03 14:59 ` Hans Verkuil
1 sibling, 2 replies; 7+ messages in thread
From: Ville Syrjälä @ 2019-10-03 13:44 UTC (permalink / raw)
To: Johan Korsnes; +Cc: dri-devel, linux-media
On Thu, Oct 03, 2019 at 09:15:49AM +0200, Johan Korsnes wrote:
> When logging the AVI InfoFrame, clearly indicate whether or not the
> extended colorimetry attribute is active. This is only the case when
> the AVI InfoFrame colorimetry attribute is set to extended. [0]
>
> [0] CTA-861-G section 6.4 page 57
>
> Signed-off-by: Johan Korsnes <jkorsnes@cisco.com>
> ---
> drivers/video/hdmi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index f29db728ff29..a709e38a53ca 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -682,8 +682,14 @@ 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",
> +
> + 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);
Yeah, seems fine. Might make the logs a bit less confusing at least.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
PS. would be nice it someone were to extend this code to deal with the
ACE bits too. Do you have plans/interest in doing that?
> +
> hdmi_log(" quantization range: %s\n",
> hdmi_quantization_range_get_name(frame->quantization_range));
> hdmi_log(" nups: %s\n", hdmi_nups_get_name(frame->nups));
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drivers: video: hdmi: log ext colorimetry applicability
2019-10-03 13:44 ` Ville Syrjälä
@ 2019-10-03 14:53 ` Johan Korsnes (jkorsnes)
2019-10-03 15:02 ` Hans Verkuil
1 sibling, 0 replies; 7+ messages in thread
From: Johan Korsnes (jkorsnes) @ 2019-10-03 14:53 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: dri-devel, linux-media
On 03/10/2019 15.44, Ville Syrjälä wrote:
> On Thu, Oct 03, 2019 at 09:15:49AM +0200, Johan Korsnes wrote:
>> When logging the AVI InfoFrame, clearly indicate whether or not the
>> extended colorimetry attribute is active. This is only the case when
>> the AVI InfoFrame colorimetry attribute is set to extended. [0]
>>
>> [0] CTA-861-G section 6.4 page 57
>>
>> Signed-off-by: Johan Korsnes <jkorsnes@cisco.com>
>> ---
>> drivers/video/hdmi.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
>> index f29db728ff29..a709e38a53ca 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -682,8 +682,14 @@ 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",
>> +
>> + 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);
>
> Yeah, seems fine. Might make the logs a bit less confusing at least.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> PS. would be nice it someone were to extend this code to deal with the
> ACE bits too. Do you have plans/interest in doing that?
I was actually going to deal with the ACE bits as part of this patch,
but noticed that things seem to be hard coded for AVI InfoFrame v2:
int hdmi_avi_infoframe_init(struct hdmi_avi_infoframe *frame) {
memset(frame, 0, sizeof(*frame));
frame->type = HDMI_INFOFRAME_TYPE_AVI;
frame->version = 2;
frame->length = HDMI_AVI_INFOFRAME_SIZE;
return 0;
}
I have no plans to fix this, for now, unfortunately.
>
>> +
>> hdmi_log(" quantization range: %s\n",
>> hdmi_quantization_range_get_name(frame->quantization_range));
>> hdmi_log(" nups: %s\n", hdmi_nups_get_name(frame->nups));
>> --
>> 2.20.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drivers: video: hdmi: log ext colorimetry applicability
2019-10-03 13:44 ` Ville Syrjälä
2019-10-03 14:53 ` Johan Korsnes (jkorsnes)
@ 2019-10-03 15:02 ` Hans Verkuil
1 sibling, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2019-10-03 15:02 UTC (permalink / raw)
To: Ville Syrjälä, Johan Korsnes; +Cc: dri-devel, linux-media
On 10/3/19 3:44 PM, Ville Syrjälä wrote:
> On Thu, Oct 03, 2019 at 09:15:49AM +0200, Johan Korsnes wrote:
>> When logging the AVI InfoFrame, clearly indicate whether or not the
>> extended colorimetry attribute is active. This is only the case when
>> the AVI InfoFrame colorimetry attribute is set to extended. [0]
>>
>> [0] CTA-861-G section 6.4 page 57
>>
>> Signed-off-by: Johan Korsnes <jkorsnes@cisco.com>
>> ---
>> drivers/video/hdmi.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
>> index f29db728ff29..a709e38a53ca 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -682,8 +682,14 @@ 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",
>> +
>> + 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);
>
> Yeah, seems fine. Might make the logs a bit less confusing at least.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> PS. would be nice it someone were to extend this code to deal with the
> ACE bits too. Do you have plans/interest in doing that?
If we tackle this, then it would be part of a larger effort in updating
this code. There are more fields missing in other InfoFrames as well.
So yes, we have interest in this, but no, there are no plans :-)
Regards,
Hans
>
>> +
>> hdmi_log(" quantization range: %s\n",
>> hdmi_quantization_range_get_name(frame->quantization_range));
>> hdmi_log(" nups: %s\n", hdmi_nups_get_name(frame->nups));
>> --
>> 2.20.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drivers: video: hdmi: log ext colorimetry applicability
2019-10-03 7:15 [PATCH] drivers: video: hdmi: log ext colorimetry applicability Johan Korsnes
2019-10-03 13:44 ` Ville Syrjälä
@ 2019-10-03 14:59 ` Hans Verkuil
2019-10-03 15:03 ` Johan Korsnes (jkorsnes)
1 sibling, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2019-10-03 14:59 UTC (permalink / raw)
To: Johan Korsnes, dri-devel; +Cc: linux-media
On 10/3/19 9:15 AM, Johan Korsnes wrote:
> When logging the AVI InfoFrame, clearly indicate whether or not the
> extended colorimetry attribute is active. This is only the case when
> the AVI InfoFrame colorimetry attribute is set to extended. [0]
>
> [0] CTA-861-G section 6.4 page 57
>
> Signed-off-by: Johan Korsnes <jkorsnes@cisco.com>
> ---
> drivers/video/hdmi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index f29db728ff29..a709e38a53ca 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -682,8 +682,14 @@ 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",
> +
> + 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);
> +
> hdmi_log(" quantization range: %s\n",
> hdmi_quantization_range_get_name(frame->quantization_range));
> hdmi_log(" nups: %s\n", hdmi_nups_get_name(frame->nups));
>
I just realized that there are more fields like that:
content_type is only valid if itc == true
quantization_range is only valid if colorspace is RGB
ycc_quantization_range is only valid if colorspace is YCC
Can you make a v2 where these fields are handled in the same way?
That would really help reduce the confusion when logging the
AVI InfoFrame.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] drivers: video: hdmi: log ext colorimetry applicability
2019-10-03 14:59 ` Hans Verkuil
@ 2019-10-03 15:03 ` Johan Korsnes (jkorsnes)
0 siblings, 0 replies; 7+ messages in thread
From: Johan Korsnes (jkorsnes) @ 2019-10-03 15:03 UTC (permalink / raw)
To: Hans Verkuil, dri-devel; +Cc: linux-media
On 03/10/2019 16.59, Hans Verkuil wrote:
> On 10/3/19 9:15 AM, Johan Korsnes wrote:
>> When logging the AVI InfoFrame, clearly indicate whether or not the
>> extended colorimetry attribute is active. This is only the case when
>> the AVI InfoFrame colorimetry attribute is set to extended. [0]
>>
>> [0] CTA-861-G section 6.4 page 57
>>
>> Signed-off-by: Johan Korsnes <jkorsnes@cisco.com>
>> ---
>> drivers/video/hdmi.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
>> index f29db728ff29..a709e38a53ca 100644
>> --- a/drivers/video/hdmi.c
>> +++ b/drivers/video/hdmi.c
>> @@ -682,8 +682,14 @@ 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",
>> +
>> + 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);
>> +
>> hdmi_log(" quantization range: %s\n",
>> hdmi_quantization_range_get_name(frame->quantization_range));
>> hdmi_log(" nups: %s\n", hdmi_nups_get_name(frame->nups));
>>
>
> I just realized that there are more fields like that:
>
> content_type is only valid if itc == true
>
> quantization_range is only valid if colorspace is RGB
>
> ycc_quantization_range is only valid if colorspace is YCC
>
> Can you make a v2 where these fields are handled in the same way?
> That would really help reduce the confusion when logging the
> AVI InfoFrame.
I will make a v2 handling these cases as well; thanks for pointing it
out.
Best regards,
Johan
>
> Regards,
>
> Hans
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] drivers: video: hdmi: log ext colorimetry applicability
@ 2019-10-02 12:42 Johan Korsnes
0 siblings, 0 replies; 7+ messages in thread
From: Johan Korsnes @ 2019-10-02 12:42 UTC (permalink / raw)
To: dri-devel; +Cc: linux-media, Johan Korsnes
When logging the AVI InfoFrame, clearly indicate whether or not the
extended colorimetry attribute is active. This is only the case when
the AVI InfoFrame colorimetry attribute is set to extended. [0]
[0] CTA-861-G section 6.4 page 57
Signed-off-by: Johan Korsnes <jkorsnes@cisco.com>
---
drivers/video/hdmi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index f29db728ff29..a709e38a53ca 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -682,8 +682,14 @@ 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",
+
+ 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);
+
hdmi_log(" quantization range: %s\n",
hdmi_quantization_range_get_name(frame->quantization_range));
hdmi_log(" nups: %s\n", hdmi_nups_get_name(frame->nups));
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-03 15:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-03 7:15 [PATCH] drivers: video: hdmi: log ext colorimetry applicability Johan Korsnes
2019-10-03 13:44 ` Ville Syrjälä
2019-10-03 14:53 ` Johan Korsnes (jkorsnes)
2019-10-03 15:02 ` Hans Verkuil
2019-10-03 14:59 ` Hans Verkuil
2019-10-03 15:03 ` Johan Korsnes (jkorsnes)
-- strict thread matches above, loose matches on Subject: below --
2019-10-02 12:42 Johan Korsnes
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).