diff mbox series

edid-decode: Cannot support NonMixed MS without MS

Message ID 20240529100301.127652-1-sebastian.wick@redhat.com (mailing list archive)
State Changes Requested
Headers
Series edid-decode: Cannot support NonMixed MS without MS |

Commit Message

Sebastian Wick May 29, 2024, 10:02 a.m. UTC 
  When `Max Stream Count` is zero, the sink does not support multistream
and thus cannot support NonMixed MS.

An EDID with Max Stream Count = 0 and Non Mixed MS = 1 can be found in
linuxhw/EDID ./Digital/TCL/TCL5655/1723FF2DC6D1 at commit cff7fe4d44.

Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
---
 parse-cta-block.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Hans Verkuil May 29, 2024, 2:07 p.m. UTC | #1 
Hi Sebastian,

On 29/05/2024 12:02, Sebastian Wick wrote:
> When `Max Stream Count` is zero, the sink does not support multistream
> and thus cannot support NonMixed MS.
> 
> An EDID with Max Stream Count = 0 and Non Mixed MS = 1 can be found in
> linuxhw/EDID ./Digital/TCL/TCL5655/1723FF2DC6D1 at commit cff7fe4d44.
> 
> Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
> ---
>  parse-cta-block.cpp | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git ./parse-cta-block.cpp ../parse-cta-block.cpp
> index 4d2afc6..7cd7a3a 100644
> --- ./parse-cta-block.cpp
> +++ ../parse-cta-block.cpp
> @@ -2498,10 +2498,11 @@ static void cta_hdmi_audio_block(const unsigned char *x, unsigned length)
>  		fail("Empty Data Block with length %u.\n", length);
>  		return;
>  	}
> -	if (x[0] & 3)
> +	if (x[0] & 3) {
>  		printf("    Max Stream Count: %u\n", (x[0] & 3) + 1);
> -	if (x[0] & 4)
> -		printf("    Supports MS NonMixed\n");
> +	    if (x[0] & 4)
> +		    printf("    Supports MS NonMixed\n");
> +	}

I would actually leave this as-is, but instead add a fail() message
if MS NonMixed is set, but Max Stream Count == 0.

It's really an EDID conformity failure, and it should be reported as such.

Regards,

	Hans

>  
>  	num_descs = x[1] & 7;
>  	if (num_descs == 0)
Sebastian Wick June 4, 2024, 4:57 p.m. UTC | #2 
On Wed, May 29, 2024 at 4:16 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> Hi Sebastian,
>
> On 29/05/2024 12:02, Sebastian Wick wrote:
> > When `Max Stream Count` is zero, the sink does not support multistream
> > and thus cannot support NonMixed MS.
> >
> > An EDID with Max Stream Count = 0 and Non Mixed MS = 1 can be found in
> > linuxhw/EDID ./Digital/TCL/TCL5655/1723FF2DC6D1 at commit cff7fe4d44.
> >
> > Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
> > ---
> >  parse-cta-block.cpp | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git ./parse-cta-block.cpp ../parse-cta-block.cpp
> > index 4d2afc6..7cd7a3a 100644
> > --- ./parse-cta-block.cpp
> > +++ ../parse-cta-block.cpp
> > @@ -2498,10 +2498,11 @@ static void cta_hdmi_audio_block(const unsigned char *x, unsigned length)
> >               fail("Empty Data Block with length %u.\n", length);
> >               return;
> >       }
> > -     if (x[0] & 3)
> > +     if (x[0] & 3) {
> >               printf("    Max Stream Count: %u\n", (x[0] & 3) + 1);
> > -     if (x[0] & 4)
> > -             printf("    Supports MS NonMixed\n");
> > +         if (x[0] & 4)
> > +                 printf("    Supports MS NonMixed\n");
> > +     }
>
> I would actually leave this as-is, but instead add a fail() message
> if MS NonMixed is set, but Max Stream Count == 0.
>
> It's really an EDID conformity failure, and it should be reported as such.

I'm all for adding a fail() message but I'm afraid our implementation
which tries to be compatible with the output here won't have access to
the MS NonMixed bit when Max Stream Count == 0.

Would this work for you?

    if (x[0] & 3) {
        printf("    Max Stream Count: %u\n", (x[0] & 3) + 1);
        if (x[0] & 4)
            printf("    Supports MS NonMixed\n");
        else
                fail("NonMixed MS support indicated but MS is unsupported\n");
    }

>
> Regards,
>
>         Hans
>
> >
> >       num_descs = x[1] & 7;
> >       if (num_descs == 0)
>
Hans Verkuil June 5, 2024, 6 a.m. UTC | #3 
On 04/06/2024 18:57, Sebastian Wick wrote:
> On Wed, May 29, 2024 at 4:16 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> Hi Sebastian,
>>
>> On 29/05/2024 12:02, Sebastian Wick wrote:
>>> When `Max Stream Count` is zero, the sink does not support multistream
>>> and thus cannot support NonMixed MS.
>>>
>>> An EDID with Max Stream Count = 0 and Non Mixed MS = 1 can be found in
>>> linuxhw/EDID ./Digital/TCL/TCL5655/1723FF2DC6D1 at commit cff7fe4d44.
>>>
>>> Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
>>> ---
>>>  parse-cta-block.cpp | 7 ++++---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git ./parse-cta-block.cpp ../parse-cta-block.cpp
>>> index 4d2afc6..7cd7a3a 100644
>>> --- ./parse-cta-block.cpp
>>> +++ ../parse-cta-block.cpp
>>> @@ -2498,10 +2498,11 @@ static void cta_hdmi_audio_block(const unsigned char *x, unsigned length)
>>>               fail("Empty Data Block with length %u.\n", length);
>>>               return;
>>>       }
>>> -     if (x[0] & 3)
>>> +     if (x[0] & 3) {
>>>               printf("    Max Stream Count: %u\n", (x[0] & 3) + 1);
>>> -     if (x[0] & 4)
>>> -             printf("    Supports MS NonMixed\n");
>>> +         if (x[0] & 4)
>>> +                 printf("    Supports MS NonMixed\n");
>>> +     }
>>
>> I would actually leave this as-is, but instead add a fail() message
>> if MS NonMixed is set, but Max Stream Count == 0.
>>
>> It's really an EDID conformity failure, and it should be reported as such.
> 
> I'm all for adding a fail() message but I'm afraid our implementation
> which tries to be compatible with the output here won't have access to
> the MS NonMixed bit when Max Stream Count == 0.
> 
> Would this work for you?
> 
>     if (x[0] & 3) {
>         printf("    Max Stream Count: %u\n", (x[0] & 3) + 1);
>         if (x[0] & 4)
>             printf("    Supports MS NonMixed\n");
>         else
>                 fail("NonMixed MS support indicated but MS is unsupported\n");
>     }

Correcting the logic that would be this:

	if (x[0] & 3) {
        	printf("    Max Stream Count: %u\n", (x[0] & 3) + 1);
		if (x[0] & 4)
			printf("    Supports MS NonMixed\n");
	} else if (x[0] & 4) {
		fail("MS NonMixed support indicated but Max Stream Count == 0.\n");
	}

And that works for me.

Regards,

	Hans

> 
>>
>> Regards,
>>
>>         Hans
>>
>>>
>>>       num_descs = x[1] & 7;
>>>       if (num_descs == 0)
>>
>
diff mbox series

Patch

diff --git ./parse-cta-block.cpp ../parse-cta-block.cpp
index 4d2afc6..7cd7a3a 100644
--- ./parse-cta-block.cpp
+++ ../parse-cta-block.cpp
@@ -2498,10 +2498,11 @@  static void cta_hdmi_audio_block(const unsigned char *x, unsigned length)
 		fail("Empty Data Block with length %u.\n", length);
 		return;
 	}
-	if (x[0] & 3)
+	if (x[0] & 3) {
 		printf("    Max Stream Count: %u\n", (x[0] & 3) + 1);
-	if (x[0] & 4)
-		printf("    Supports MS NonMixed\n");
+	    if (x[0] & 4)
+		    printf("    Supports MS NonMixed\n");
+	}
 
 	num_descs = x[1] & 7;
 	if (num_descs == 0)