diff mbox series

[05/18] video/hdmi: Add an enum for HDMI packet types

Message ID 20180920185145.1912-6-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Infoframe precompute/check | expand

Commit Message

Ville Syrjälä Sept. 20, 2018, 6:51 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We'll be wanting to send more than just infoframes over HDMI. So add an
enum for other packet types.

TODO: Maybe just include the infoframe types in the packet type enum
      and get rid of the infoframe type enum?

Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: linux-media@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/linux/hdmi.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Hans Verkuil (hansverk) Sept. 21, 2018, 8:41 a.m. UTC | #1 
On 09/20/18 20:51, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We'll be wanting to send more than just infoframes over HDMI. So add an
> enum for other packet types.
> 
> TODO: Maybe just include the infoframe types in the packet type enum
>       and get rid of the infoframe type enum?

I think that's better, IMHO. With a comment that the types starting with
0x81 are defined in CTA-861-G.

It's really the same byte that is being checked, so having two enums is
a bit misleading. The main difference is really which standard defines
the packet types.

Regards,

	Hans

> 
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  include/linux/hdmi.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index c76b50a48e48..80521d9591a1 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -27,6 +27,21 @@
>  #include <linux/types.h>
>  #include <linux/device.h>
>  
> +enum hdmi_packet_type {
> +	HDMI_PACKET_TYPE_NULL = 0x00,
> +	HDMI_PACKET_TYPE_AUDIO_CLOCK_REGEN = 0x01,
> +	HDMI_PACKET_TYPE_AUDIO_SAMPLE = 0x02,
> +	HDMI_PACKET_TYPE_GENERAL_CONTROL = 0x03,
> +	HDMI_PACKET_TYPE_AUDIO_CP = 0x04,
> +	HDMI_PACKET_TYPE_ISRC1 = 0x05,
> +	HDMI_PACKET_TYPE_ISRC2 = 0x06,
> +	HDMI_PACKET_TYPE_ONE_BIT_AUDIO_SAMPLE = 0x07,
> +	HDMI_PACKET_TYPE_DST_AUDIO = 0x08,
> +	HDMI_PACKET_TYPE_HBR_AUDIO_STREAM = 0x09,
> +	HDMI_PACKET_TYPE_GAMUT_METADATA = 0x0a,
> +	/* + enum hdmi_infoframe_type */
> +};
> +
>  enum hdmi_infoframe_type {
>  	HDMI_INFOFRAME_TYPE_VENDOR = 0x81,
>  	HDMI_INFOFRAME_TYPE_AVI = 0x82,
>
Ville Syrjälä Sept. 21, 2018, 2:01 p.m. UTC | #2 
On Fri, Sep 21, 2018 at 10:41:46AM +0200, Hans Verkuil wrote:
> On 09/20/18 20:51, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We'll be wanting to send more than just infoframes over HDMI. So add an
> > enum for other packet types.
> > 
> > TODO: Maybe just include the infoframe types in the packet type enum
> >       and get rid of the infoframe type enum?
> 
> I think that's better, IMHO. With a comment that the types starting with
> 0x81 are defined in CTA-861-G.
> 
> It's really the same byte that is being checked, so having two enums is
> a bit misleading. The main difference is really which standard defines
> the packet types.

Right. The only slight annoyance is that we'll get a bunch of warnings
from the compiler if we don't handle all the enum valus in the switch
statements. If we want to avoid that I guess I could limit this
to just the null, gcp and gamut metadata packets initially and try to
write some actual code for them. Those three are the only ones we
care about in i915 at the moment.

> 
> Regards,
> 
> 	Hans
> 
> > 
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Hans Verkuil <hans.verkuil@cisco.com>
> > Cc: linux-media@vger.kernel.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  include/linux/hdmi.h | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> > index c76b50a48e48..80521d9591a1 100644
> > --- a/include/linux/hdmi.h
> > +++ b/include/linux/hdmi.h
> > @@ -27,6 +27,21 @@
> >  #include <linux/types.h>
> >  #include <linux/device.h>
> >  
> > +enum hdmi_packet_type {
> > +	HDMI_PACKET_TYPE_NULL = 0x00,
> > +	HDMI_PACKET_TYPE_AUDIO_CLOCK_REGEN = 0x01,
> > +	HDMI_PACKET_TYPE_AUDIO_SAMPLE = 0x02,
> > +	HDMI_PACKET_TYPE_GENERAL_CONTROL = 0x03,
> > +	HDMI_PACKET_TYPE_AUDIO_CP = 0x04,
> > +	HDMI_PACKET_TYPE_ISRC1 = 0x05,
> > +	HDMI_PACKET_TYPE_ISRC2 = 0x06,
> > +	HDMI_PACKET_TYPE_ONE_BIT_AUDIO_SAMPLE = 0x07,
> > +	HDMI_PACKET_TYPE_DST_AUDIO = 0x08,
> > +	HDMI_PACKET_TYPE_HBR_AUDIO_STREAM = 0x09,
> > +	HDMI_PACKET_TYPE_GAMUT_METADATA = 0x0a,
> > +	/* + enum hdmi_infoframe_type */
> > +};
> > +
> >  enum hdmi_infoframe_type {
> >  	HDMI_INFOFRAME_TYPE_VENDOR = 0x81,
> >  	HDMI_INFOFRAME_TYPE_AVI = 0x82,
> >
Hans Verkuil (hansverk) Sept. 21, 2018, 2:12 p.m. UTC | #3 
On 09/21/18 16:01, Ville Syrjälä wrote:
> On Fri, Sep 21, 2018 at 10:41:46AM +0200, Hans Verkuil wrote:
>> On 09/20/18 20:51, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> We'll be wanting to send more than just infoframes over HDMI. So add an
>>> enum for other packet types.
>>>
>>> TODO: Maybe just include the infoframe types in the packet type enum
>>>       and get rid of the infoframe type enum?
>>
>> I think that's better, IMHO. With a comment that the types starting with
>> 0x81 are defined in CTA-861-G.
>>
>> It's really the same byte that is being checked, so having two enums is
>> a bit misleading. The main difference is really which standard defines
>> the packet types.
> 
> Right. The only slight annoyance is that we'll get a bunch of warnings
> from the compiler if we don't handle all the enum valus in the switch
> statements. If we want to avoid that I guess I could limit this
> to just the null, gcp and gamut metadata packets initially and try to
> write some actual code for them. Those three are the only ones we
> care about in i915 at the moment.

Note that I don't have a terribly strong opinion on this, so if using
one enum instead of two causes more problems than it is worth, then
that's fine with me as well.

But you asked, and given a choice with all other things being equal,
then one enum has my preference.

Regards,

	Hans

> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
>>> Cc: linux-media@vger.kernel.org
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>  include/linux/hdmi.h | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
>>> index c76b50a48e48..80521d9591a1 100644
>>> --- a/include/linux/hdmi.h
>>> +++ b/include/linux/hdmi.h
>>> @@ -27,6 +27,21 @@
>>>  #include <linux/types.h>
>>>  #include <linux/device.h>
>>>  
>>> +enum hdmi_packet_type {
>>> +	HDMI_PACKET_TYPE_NULL = 0x00,
>>> +	HDMI_PACKET_TYPE_AUDIO_CLOCK_REGEN = 0x01,
>>> +	HDMI_PACKET_TYPE_AUDIO_SAMPLE = 0x02,
>>> +	HDMI_PACKET_TYPE_GENERAL_CONTROL = 0x03,
>>> +	HDMI_PACKET_TYPE_AUDIO_CP = 0x04,
>>> +	HDMI_PACKET_TYPE_ISRC1 = 0x05,
>>> +	HDMI_PACKET_TYPE_ISRC2 = 0x06,
>>> +	HDMI_PACKET_TYPE_ONE_BIT_AUDIO_SAMPLE = 0x07,
>>> +	HDMI_PACKET_TYPE_DST_AUDIO = 0x08,
>>> +	HDMI_PACKET_TYPE_HBR_AUDIO_STREAM = 0x09,
>>> +	HDMI_PACKET_TYPE_GAMUT_METADATA = 0x0a,
>>> +	/* + enum hdmi_infoframe_type */
>>> +};
>>> +
>>>  enum hdmi_infoframe_type {
>>>  	HDMI_INFOFRAME_TYPE_VENDOR = 0x81,
>>>  	HDMI_INFOFRAME_TYPE_AVI = 0x82,
>>>
>
Ville Syrjälä Sept. 21, 2018, 3:07 p.m. UTC | #4 
On Fri, Sep 21, 2018 at 04:12:36PM +0200, Hans Verkuil wrote:
> On 09/21/18 16:01, Ville Syrjälä wrote:
> > On Fri, Sep 21, 2018 at 10:41:46AM +0200, Hans Verkuil wrote:
> >> On 09/20/18 20:51, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> We'll be wanting to send more than just infoframes over HDMI. So add an
> >>> enum for other packet types.
> >>>
> >>> TODO: Maybe just include the infoframe types in the packet type enum
> >>>       and get rid of the infoframe type enum?
> >>
> >> I think that's better, IMHO. With a comment that the types starting with
> >> 0x81 are defined in CTA-861-G.
> >>
> >> It's really the same byte that is being checked, so having two enums is
> >> a bit misleading. The main difference is really which standard defines
> >> the packet types.
> > 
> > Right. The only slight annoyance is that we'll get a bunch of warnings
> > from the compiler if we don't handle all the enum valus in the switch
> > statements. If we want to avoid that I guess I could limit this
> > to just the null, gcp and gamut metadata packets initially and try to
> > write some actual code for them. Those three are the only ones we
> > care about in i915 at the moment.
> 
> Note that I don't have a terribly strong opinion on this, so if using
> one enum instead of two causes more problems than it is worth, then
> that's fine with me as well.
> 
> But you asked, and given a choice with all other things being equal,
> then one enum has my preference.

I do agree that it would seem nicer.

But I'm a bit busy with other things at the moment so I might want
to leave it like this for now and revisit the topic in the
hopefully-not-too-distant future.

> 
> Regards,
> 
> 	Hans
> 
> > 
> >>
> >> Regards,
> >>
> >> 	Hans
> >>
> >>>
> >>> Cc: Thierry Reding <thierry.reding@gmail.com>
> >>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> >>> Cc: linux-media@vger.kernel.org
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>  include/linux/hdmi.h | 15 +++++++++++++++
> >>>  1 file changed, 15 insertions(+)
> >>>
> >>> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> >>> index c76b50a48e48..80521d9591a1 100644
> >>> --- a/include/linux/hdmi.h
> >>> +++ b/include/linux/hdmi.h
> >>> @@ -27,6 +27,21 @@
> >>>  #include <linux/types.h>
> >>>  #include <linux/device.h>
> >>>  
> >>> +enum hdmi_packet_type {
> >>> +	HDMI_PACKET_TYPE_NULL = 0x00,
> >>> +	HDMI_PACKET_TYPE_AUDIO_CLOCK_REGEN = 0x01,
> >>> +	HDMI_PACKET_TYPE_AUDIO_SAMPLE = 0x02,
> >>> +	HDMI_PACKET_TYPE_GENERAL_CONTROL = 0x03,
> >>> +	HDMI_PACKET_TYPE_AUDIO_CP = 0x04,
> >>> +	HDMI_PACKET_TYPE_ISRC1 = 0x05,
> >>> +	HDMI_PACKET_TYPE_ISRC2 = 0x06,
> >>> +	HDMI_PACKET_TYPE_ONE_BIT_AUDIO_SAMPLE = 0x07,
> >>> +	HDMI_PACKET_TYPE_DST_AUDIO = 0x08,
> >>> +	HDMI_PACKET_TYPE_HBR_AUDIO_STREAM = 0x09,
> >>> +	HDMI_PACKET_TYPE_GAMUT_METADATA = 0x0a,
> >>> +	/* + enum hdmi_infoframe_type */
> >>> +};
> >>> +
> >>>  enum hdmi_infoframe_type {
> >>>  	HDMI_INFOFRAME_TYPE_VENDOR = 0x81,
> >>>  	HDMI_INFOFRAME_TYPE_AVI = 0x82,
> >>>
> >
diff mbox series

Patch

diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
index c76b50a48e48..80521d9591a1 100644
--- a/include/linux/hdmi.h
+++ b/include/linux/hdmi.h
@@ -27,6 +27,21 @@ 
 #include <linux/types.h>
 #include <linux/device.h>
 
+enum hdmi_packet_type {
+	HDMI_PACKET_TYPE_NULL = 0x00,
+	HDMI_PACKET_TYPE_AUDIO_CLOCK_REGEN = 0x01,
+	HDMI_PACKET_TYPE_AUDIO_SAMPLE = 0x02,
+	HDMI_PACKET_TYPE_GENERAL_CONTROL = 0x03,
+	HDMI_PACKET_TYPE_AUDIO_CP = 0x04,
+	HDMI_PACKET_TYPE_ISRC1 = 0x05,
+	HDMI_PACKET_TYPE_ISRC2 = 0x06,
+	HDMI_PACKET_TYPE_ONE_BIT_AUDIO_SAMPLE = 0x07,
+	HDMI_PACKET_TYPE_DST_AUDIO = 0x08,
+	HDMI_PACKET_TYPE_HBR_AUDIO_STREAM = 0x09,
+	HDMI_PACKET_TYPE_GAMUT_METADATA = 0x0a,
+	/* + enum hdmi_infoframe_type */
+};
+
 enum hdmi_infoframe_type {
 	HDMI_INFOFRAME_TYPE_VENDOR = 0x81,
 	HDMI_INFOFRAME_TYPE_AVI = 0x82,