Re: [PATCH 05/18] video/hdmi: Add an enum for HDMI packet types
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ä
> >>>
> >>> 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
> >>> Cc: Hans Verkuil
> >>> Cc: linux-me...@vger.kernel.org
> >>> Signed-off-by: Ville Syrjälä
> >>> ---
> >>> 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
> >>> #include
> >>>
> >>> +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ä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/18] video/hdmi: Add an enum for HDMI packet types
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ä
>>>
>>> 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
>>> Cc: Hans Verkuil
>>> Cc: linux-me...@vger.kernel.org
>>> Signed-off-by: Ville Syrjälä
>>> ---
>>> 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
>>> #include
>>>
>>> +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,
>>>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/18] video/hdmi: Add an enum for HDMI packet types
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ä
> >
> > 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
> > Cc: Hans Verkuil
> > Cc: linux-me...@vger.kernel.org
> > Signed-off-by: Ville Syrjälä
> > ---
> > 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
> > #include
> >
> > +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ä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/18] video/hdmi: Add an enum for HDMI packet types
On 09/20/18 20:51, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> 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
> Cc: Hans Verkuil
> Cc: linux-me...@vger.kernel.org
> Signed-off-by: Ville Syrjälä
> ---
> 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
> #include
>
> +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,
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/18] video/hdmi: Add an enum for HDMI packet types
From: Ville Syrjälä
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
Cc: Hans Verkuil
Cc: linux-me...@vger.kernel.org
Signed-off-by: Ville Syrjälä
---
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
#include
+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,
--
2.16.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
