Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

[Nautiyal, Ankit K]

On 4/17/2018 11:17 PM, Ville Syrjälä wrote:

On Tue, Apr 17, 2018 at 10:45:07AM +0530, Nautiyal, Ankit K wrote:

On 4/6/2018 11:14 PM, Ville Syrjälä wrote:

On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:

This patch is causing failure of IGT test kms_3d. The kms_3d test
expects the no. of 3d modes to be 13.

(The test has hard-coded value for expected no. of 3d modes as 13)

But due to the addition of "matching aspect_ratio" in drm_mode_equal in
this patch, the total no. of

modes in the connector modelist is increased by 2, resulting in failure
of assertion 'mode_count==13'.

If kms_3d isn't setting the aspect ratio cap how is it affected by these
changes?

In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
to remove duplicate modes in connector_modes from the
connector->probe_modes.
Earlier, it wasn't matching for aspect-ratio and was discarding two of
the modes with aspect ratio,
as duplicates of other modes in the list.

Later, when we are pruning the modes in drm_mode_get_connector, the
logic there assumes,
that the modes are in a sorted order so that we just match with the last
valid mode for uniqueness.
This isn't the case with the spoofed edid in kms_3d.
Earlier, I was thinking if we should change the no. of expected modes in
kms_3d,
but that's not correct approach.

So finally, The pruning logic needs to be changed, to do away with any
assumption and check
all the modes in the list for duplicates. This however will take more
time to remove duplicates.

Any other suggestions on this?

What are all the modes this EDID gives us? The order in which the
modes are listed in the EDID should not be relevant as we sort
the mode list ourselves, and thus similar modes should appear back to
back on the list. So I don't really understand how we fail to
discard these two modes.


As we know the kms_3d test does not set the aspect ratio cap, the modes 
with aspect-ratios must be pruned,

unless they are unique.
Now the list of mode in kms_3d is something like this:
...
1280x720@60 flags 0x1c005 [3D mode - top and bottom] ->1
1280x720@60 flags 0x4005 [3D mode -frame packing] -> 2
1280x720@60 flags 0x1c005 [3D mode -top and bottom with aspect ratio]->3
1280x720@60 flags 0x4005 [3D mode frame packing with aspect ratio] ->4
...

The first two 3D modes are able to pass and are added in the 
drm_mode_get_connector mode list.
For third mode , the 3d mode with aspect ratio, must be pruned if not 
unique. So we check with the last valid mode in the list (1280x720@60 
flags 0x4005).

Everything matches except the 3d mode flags. So it gets added to the list.
For the fourth again, the mode with aspect ratio must be pruned, (not 
unique), but when we check with the last valid mode,

the 3d flags doesn't match, and we add that also in the list.

had the list been sorted by flag also:
1280x720@60 flags 0x1c005 [3D mode top and bottom]
1280x720@60 flags 0x1c005 [3D mode top and bottom with aspect ratio]
1280x720@60 flags 0x4005 [3D mode frame packing]
1280x720@60 flags 0x4005 [3D mode frame packing aspect ratio]

the aspect-ratio modes would have been pruned.

To avoid these scenarios, the best way is to modify the pruning logic to 
check against the whole list of already added modes.

I will send a patch for that shortly.

Regards,
Ankit




Regards
-Ankit


Perhaps this need to be handled in the test.

-Regards,

Ankit


On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:

From: "Sharma, Shashank" 

Current DRM layer functions don't parse aspect ratio information
while converting a user mode->kernel mode or vice versa. This
causes modeset to pick mode with wrong aspect ratio, eventually
causing failures in HDMI compliance test cases, due to wrong VIC.

This patch adds aspect ratio information in DRM's mode conversion
and mode comparision functions, to make sure kernel picks mode
with right aspect ratio (as per the VIC).

Background:
This patch was once reviewed and merged, and later reverted due to
lack of DRM cap protection. This is a re-spin of this patch, this
time with DRM cap protection, to avoid aspect ratio information, when
the client doesn't request for it.

Review link: https://pw-emeril.freedesktop.org/patch/104068/
Background discussion: https://patchwork.kernel.org/patch/9379057/

Signed-off-by: Shashank Sharma 
Signed-off-by: Lin, Jia 
Signed-off-by: Akashdeep Sharma 
Reviewed-by: Jim Bride  (V2)
Reviewed-by: Jose Abreu  (V4)

Cc: Ville Syrjala 
Cc: Jim Bride 
Cc: Jose Abreu 
Cc: Ankit Nautiyal 

V3: modified the aspect-ratio check in drm_mode_equal as per new flags
   provided by Ville. https://patchwork.freedesktop.org/patch/188043/
V4: rebase
V5: rebase
V6: As recommended by Ville, avoided matching of 

Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

[Ville Syrjälä]

On Tue, Apr 17, 2018 at 10:45:07AM +0530, Nautiyal, Ankit K wrote:
> 
> On 4/6/2018 11:14 PM, Ville Syrjälä wrote:
> > On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
> >> This patch is causing failure of IGT test kms_3d. The kms_3d test
> >> expects the no. of 3d modes to be 13.
> >>
> >> (The test has hard-coded value for expected no. of 3d modes as 13)
> >>
> >> But due to the addition of "matching aspect_ratio" in drm_mode_equal in
> >> this patch, the total no. of
> >>
> >> modes in the connector modelist is increased by 2, resulting in failure
> >> of assertion 'mode_count==13'.
> > If kms_3d isn't setting the aspect ratio cap how is it affected by these
> > changes?
> In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
> to remove duplicate modes in connector_modes from the 
> connector->probe_modes.
> Earlier, it wasn't matching for aspect-ratio and was discarding two of 
> the modes with aspect ratio,
> as duplicates of other modes in the list.
> 
> Later, when we are pruning the modes in drm_mode_get_connector, the 
> logic there assumes,
> that the modes are in a sorted order so that we just match with the last 
> valid mode for uniqueness.
> This isn't the case with the spoofed edid in kms_3d.
> Earlier, I was thinking if we should change the no. of expected modes in 
> kms_3d,
> but that's not correct approach.
> 
> So finally, The pruning logic needs to be changed, to do away with any 
> assumption and check
> all the modes in the list for duplicates. This however will take more 
> time to remove duplicates.
> 
> Any other suggestions on this?

What are all the modes this EDID gives us? The order in which the
modes are listed in the EDID should not be relevant as we sort
the mode list ourselves, and thus similar modes should appear back to
back on the list. So I don't really understand how we fail to
discard these two modes.

> 
> Regards
> -Ankit
> 
> >
> >> Perhaps this need to be handled in the test.
> >>
> >> -Regards,
> >>
> >> Ankit
> >>
> >>
> >> On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> >>> From: "Sharma, Shashank" 
> >>>
> >>> Current DRM layer functions don't parse aspect ratio information
> >>> while converting a user mode->kernel mode or vice versa. This
> >>> causes modeset to pick mode with wrong aspect ratio, eventually
> >>> causing failures in HDMI compliance test cases, due to wrong VIC.
> >>>
> >>> This patch adds aspect ratio information in DRM's mode conversion
> >>> and mode comparision functions, to make sure kernel picks mode
> >>> with right aspect ratio (as per the VIC).
> >>>
> >>> Background:
> >>> This patch was once reviewed and merged, and later reverted due to
> >>> lack of DRM cap protection. This is a re-spin of this patch, this
> >>> time with DRM cap protection, to avoid aspect ratio information, when
> >>> the client doesn't request for it.
> >>>
> >>> Review link: https://pw-emeril.freedesktop.org/patch/104068/
> >>> Background discussion: https://patchwork.kernel.org/patch/9379057/
> >>>
> >>> Signed-off-by: Shashank Sharma 
> >>> Signed-off-by: Lin, Jia 
> >>> Signed-off-by: Akashdeep Sharma 
> >>> Reviewed-by: Jim Bride  (V2)
> >>> Reviewed-by: Jose Abreu  (V4)
> >>>
> >>> Cc: Ville Syrjala 
> >>> Cc: Jim Bride 
> >>> Cc: Jose Abreu 
> >>> Cc: Ankit Nautiyal 
> >>>
> >>> V3: modified the aspect-ratio check in drm_mode_equal as per new flags
> >>>   provided by Ville. https://patchwork.freedesktop.org/patch/188043/
> >>> V4: rebase
> >>> V5: rebase
> >>> V6: As recommended by Ville, avoided matching of aspect-ratio in
> >>>   drm_fb_helper, while trying to find a common mode among connectors
> >>>   for the target clone mode.
> >>> V7: rebase
> >>> V8: rebase
> >>> V9: rebase
> >>> V10: rebase
> >>> ---
> >>>drivers/gpu/drm/drm_fb_helper.c | 12 ++--
> >>>drivers/gpu/drm/drm_modes.c | 35 
> >>> ++-
> >>>2 files changed, 44 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> >>> b/drivers/gpu/drm/drm_fb_helper.c
> >>> index 0646b10..2ee1eaa 100644
> >>> --- a/drivers/gpu/drm/drm_fb_helper.c
> >>> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >>> @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper 
> >>> *fb_helper,
> >>>   for (j = 0; j < i; j++) {
> >>>   if (!enabled[j])
> >>>   continue;
> >>> - if (!drm_mode_equal(modes[j], modes[i]))
> >>> + if (!drm_mode_match(modes[j], modes[i],
> >>> + DRM_MODE_MATCH_TIMINGS |
> >>> + DRM_MODE_MATCH_CLOCK |
> >>> 

Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

[Daniel Vetter]

On Tue, Apr 17, 2018 at 10:45:07AM +0530, Nautiyal, Ankit K wrote:
> 
> On 4/6/2018 11:14 PM, Ville Syrjälä wrote:
> > On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
> > > This patch is causing failure of IGT test kms_3d. The kms_3d test
> > > expects the no. of 3d modes to be 13.
> > > 
> > > (The test has hard-coded value for expected no. of 3d modes as 13)
> > > 
> > > But due to the addition of "matching aspect_ratio" in drm_mode_equal in
> > > this patch, the total no. of
> > > 
> > > modes in the connector modelist is increased by 2, resulting in failure
> > > of assertion 'mode_count==13'.
> > If kms_3d isn't setting the aspect ratio cap how is it affected by these
> > changes?
> In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
> to remove duplicate modes in connector_modes from the
> connector->probe_modes.
> Earlier, it wasn't matching for aspect-ratio and was discarding two of the
> modes with aspect ratio,
> as duplicates of other modes in the list.
> 
> Later, when we are pruning the modes in drm_mode_get_connector, the logic
> there assumes,
> that the modes are in a sorted order so that we just match with the last
> valid mode for uniqueness.
> This isn't the case with the spoofed edid in kms_3d.
> Earlier, I was thinking if we should change the no. of expected modes in
> kms_3d,
> but that's not correct approach.
> 
> So finally, The pruning logic needs to be changed, to do away with any
> assumption and check
> all the modes in the list for duplicates. This however will take more time
> to remove duplicates.

Agreed that the best option looks like just fixing the pruning logic.
Since we're talking about very few modes, and in the probe path (where
reading edid takes 20ms easily) the additional cpu overhead really doesn't
matter I think.
-Daniel


> Any other suggestions on this?
> 
> Regards
> -Ankit
> 
> > 
> > > Perhaps this need to be handled in the test.
> > > 
> > > -Regards,
> > > 
> > > Ankit
> > > 
> > > 
> > > On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> > > > From: "Sharma, Shashank" 
> > > > 
> > > > Current DRM layer functions don't parse aspect ratio information
> > > > while converting a user mode->kernel mode or vice versa. This
> > > > causes modeset to pick mode with wrong aspect ratio, eventually
> > > > causing failures in HDMI compliance test cases, due to wrong VIC.
> > > > 
> > > > This patch adds aspect ratio information in DRM's mode conversion
> > > > and mode comparision functions, to make sure kernel picks mode
> > > > with right aspect ratio (as per the VIC).
> > > > 
> > > > Background:
> > > > This patch was once reviewed and merged, and later reverted due to
> > > > lack of DRM cap protection. This is a re-spin of this patch, this
> > > > time with DRM cap protection, to avoid aspect ratio information, when
> > > > the client doesn't request for it.
> > > > 
> > > > Review link: https://pw-emeril.freedesktop.org/patch/104068/
> > > > Background discussion: https://patchwork.kernel.org/patch/9379057/
> > > > 
> > > > Signed-off-by: Shashank Sharma 
> > > > Signed-off-by: Lin, Jia 
> > > > Signed-off-by: Akashdeep Sharma 
> > > > Reviewed-by: Jim Bride  (V2)
> > > > Reviewed-by: Jose Abreu  (V4)
> > > > 
> > > > Cc: Ville Syrjala 
> > > > Cc: Jim Bride 
> > > > Cc: Jose Abreu 
> > > > Cc: Ankit Nautiyal 
> > > > 
> > > > V3: modified the aspect-ratio check in drm_mode_equal as per new flags
> > > >   provided by Ville. https://patchwork.freedesktop.org/patch/188043/
> > > > V4: rebase
> > > > V5: rebase
> > > > V6: As recommended by Ville, avoided matching of aspect-ratio in
> > > >   drm_fb_helper, while trying to find a common mode among connectors
> > > >   for the target clone mode.
> > > > V7: rebase
> > > > V8: rebase
> > > > V9: rebase
> > > > V10: rebase
> > > > ---
> > > >drivers/gpu/drm/drm_fb_helper.c | 12 ++--
> > > >drivers/gpu/drm/drm_modes.c | 35 
> > > > ++-
> > > >2 files changed, 44 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> > > > b/drivers/gpu/drm/drm_fb_helper.c
> > > > index 0646b10..2ee1eaa 100644
> > > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > > @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct 
> > > > drm_fb_helper *fb_helper,
> > > > for (j = 0; j < i; j++) {
> > > > if (!enabled[j])
> > > > continue;
> > > > -   if (!drm_mode_equal(modes[j], modes[i]))
> > > > +   if (!drm_mode_match(modes[j], modes[i],
> > > > + 

Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

[Nautiyal, Ankit K]

On 4/6/2018 11:14 PM, Ville Syrjälä wrote:

On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:

This patch is causing failure of IGT test kms_3d. The kms_3d test
expects the no. of 3d modes to be 13.

(The test has hard-coded value for expected no. of 3d modes as 13)

But due to the addition of "matching aspect_ratio" in drm_mode_equal in
this patch, the total no. of

modes in the connector modelist is increased by 2, resulting in failure
of assertion 'mode_count==13'.

If kms_3d isn't setting the aspect ratio cap how is it affected by these
changes?

In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
to remove duplicate modes in connector_modes from the 
connector->probe_modes.
Earlier, it wasn't matching for aspect-ratio and was discarding two of 
the modes with aspect ratio,

as duplicates of other modes in the list.

Later, when we are pruning the modes in drm_mode_get_connector, the 
logic there assumes,
that the modes are in a sorted order so that we just match with the last 
valid mode for uniqueness.

This isn't the case with the spoofed edid in kms_3d.
Earlier, I was thinking if we should change the no. of expected modes in 
kms_3d,

but that's not correct approach.

So finally, The pruning logic needs to be changed, to do away with any 
assumption and check
all the modes in the list for duplicates. This however will take more 
time to remove duplicates.


Any other suggestions on this?

Regards
-Ankit




Perhaps this need to be handled in the test.

-Regards,

Ankit


On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:

From: "Sharma, Shashank" 

Current DRM layer functions don't parse aspect ratio information
while converting a user mode->kernel mode or vice versa. This
causes modeset to pick mode with wrong aspect ratio, eventually
causing failures in HDMI compliance test cases, due to wrong VIC.

This patch adds aspect ratio information in DRM's mode conversion
and mode comparision functions, to make sure kernel picks mode
with right aspect ratio (as per the VIC).

Background:
This patch was once reviewed and merged, and later reverted due to
lack of DRM cap protection. This is a re-spin of this patch, this
time with DRM cap protection, to avoid aspect ratio information, when
the client doesn't request for it.

Review link: https://pw-emeril.freedesktop.org/patch/104068/
Background discussion: https://patchwork.kernel.org/patch/9379057/

Signed-off-by: Shashank Sharma 
Signed-off-by: Lin, Jia 
Signed-off-by: Akashdeep Sharma 
Reviewed-by: Jim Bride  (V2)
Reviewed-by: Jose Abreu  (V4)

Cc: Ville Syrjala 
Cc: Jim Bride 
Cc: Jose Abreu 
Cc: Ankit Nautiyal 

V3: modified the aspect-ratio check in drm_mode_equal as per new flags
  provided by Ville. https://patchwork.freedesktop.org/patch/188043/
V4: rebase
V5: rebase
V6: As recommended by Ville, avoided matching of aspect-ratio in
  drm_fb_helper, while trying to find a common mode among connectors
  for the target clone mode.
V7: rebase
V8: rebase
V9: rebase
V10: rebase
---
   drivers/gpu/drm/drm_fb_helper.c | 12 ++--
   drivers/gpu/drm/drm_modes.c | 35 ++-
   2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0646b10..2ee1eaa 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper 
*fb_helper,
for (j = 0; j < i; j++) {
if (!enabled[j])
continue;
-   if (!drm_mode_equal(modes[j], modes[i]))
+   if (!drm_mode_match(modes[j], modes[i],
+   DRM_MODE_MATCH_TIMINGS |
+   DRM_MODE_MATCH_CLOCK |
+   DRM_MODE_MATCH_FLAGS |
+   DRM_MODE_MATCH_3D_FLAGS))
can_clone = false;
}
}
@@ -2203,7 +2207,11 @@ static bool drm_target_cloned(struct drm_fb_helper 
*fb_helper,
   
   		fb_helper_conn = fb_helper->connector_info[i];

list_for_each_entry(mode, _helper_conn->connector->modes, 
head) {
-   if (drm_mode_equal(mode, dmt_mode))
+   if (drm_mode_match(mode, dmt_mode,
+  DRM_MODE_MATCH_TIMINGS |
+  DRM_MODE_MATCH_CLOCK |
+  DRM_MODE_MATCH_FLAGS |
+  DRM_MODE_MATCH_3D_FLAGS))
 

Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

[Ville Syrjälä]

On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:
> This patch is causing failure of IGT test kms_3d. The kms_3d test 
> expects the no. of 3d modes to be 13.
> 
> (The test has hard-coded value for expected no. of 3d modes as 13)
> 
> But due to the addition of "matching aspect_ratio" in drm_mode_equal in 
> this patch, the total no. of
> 
> modes in the connector modelist is increased by 2, resulting in failure 
> of assertion 'mode_count==13'.

If kms_3d isn't setting the aspect ratio cap how is it affected by these
changes?

> 
> Perhaps this need to be handled in the test.
> 
> -Regards,
> 
> Ankit
> 
> 
> On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:
> > From: "Sharma, Shashank" 
> >
> > Current DRM layer functions don't parse aspect ratio information
> > while converting a user mode->kernel mode or vice versa. This
> > causes modeset to pick mode with wrong aspect ratio, eventually
> > causing failures in HDMI compliance test cases, due to wrong VIC.
> >
> > This patch adds aspect ratio information in DRM's mode conversion
> > and mode comparision functions, to make sure kernel picks mode
> > with right aspect ratio (as per the VIC).
> >
> > Background:
> > This patch was once reviewed and merged, and later reverted due to
> > lack of DRM cap protection. This is a re-spin of this patch, this
> > time with DRM cap protection, to avoid aspect ratio information, when
> > the client doesn't request for it.
> >
> > Review link: https://pw-emeril.freedesktop.org/patch/104068/
> > Background discussion: https://patchwork.kernel.org/patch/9379057/
> >
> > Signed-off-by: Shashank Sharma 
> > Signed-off-by: Lin, Jia 
> > Signed-off-by: Akashdeep Sharma 
> > Reviewed-by: Jim Bride  (V2)
> > Reviewed-by: Jose Abreu  (V4)
> >
> > Cc: Ville Syrjala 
> > Cc: Jim Bride 
> > Cc: Jose Abreu 
> > Cc: Ankit Nautiyal 
> >
> > V3: modified the aspect-ratio check in drm_mode_equal as per new flags
> >  provided by Ville. https://patchwork.freedesktop.org/patch/188043/
> > V4: rebase
> > V5: rebase
> > V6: As recommended by Ville, avoided matching of aspect-ratio in
> >  drm_fb_helper, while trying to find a common mode among connectors
> >  for the target clone mode.
> > V7: rebase
> > V8: rebase
> > V9: rebase
> > V10: rebase
> > ---
> >   drivers/gpu/drm/drm_fb_helper.c | 12 ++--
> >   drivers/gpu/drm/drm_modes.c | 35 ++-
> >   2 files changed, 44 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c 
> > b/drivers/gpu/drm/drm_fb_helper.c
> > index 0646b10..2ee1eaa 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper 
> > *fb_helper,
> > for (j = 0; j < i; j++) {
> > if (!enabled[j])
> > continue;
> > -   if (!drm_mode_equal(modes[j], modes[i]))
> > +   if (!drm_mode_match(modes[j], modes[i],
> > +   DRM_MODE_MATCH_TIMINGS |
> > +   DRM_MODE_MATCH_CLOCK |
> > +   DRM_MODE_MATCH_FLAGS |
> > +   DRM_MODE_MATCH_3D_FLAGS))
> > can_clone = false;
> > }
> > }
> > @@ -2203,7 +2207,11 @@ static bool drm_target_cloned(struct drm_fb_helper 
> > *fb_helper,
> >   
> > fb_helper_conn = fb_helper->connector_info[i];
> > list_for_each_entry(mode, _helper_conn->connector->modes, 
> > head) {
> > -   if (drm_mode_equal(mode, dmt_mode))
> > +   if (drm_mode_match(mode, dmt_mode,
> > +  DRM_MODE_MATCH_TIMINGS |
> > +  DRM_MODE_MATCH_CLOCK |
> > +  DRM_MODE_MATCH_FLAGS |
> > +  DRM_MODE_MATCH_3D_FLAGS))
> > modes[i] = mode;
> > }
> > if (!modes[i])
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index d6133e8..454f2ff 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1049,7 +1049,8 @@ bool drm_mode_equal(const struct drm_display_mode 
> > *mode1,
> >   DRM_MODE_MATCH_TIMINGS |
> >   DRM_MODE_MATCH_CLOCK |
> >   DRM_MODE_MATCH_FLAGS |
> > - DRM_MODE_MATCH_3D_FLAGS);
> > + DRM_MODE_MATCH_3D_FLAGS|
> > + 

Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

[Nautiyal, Ankit K]

This patch is causing failure of IGT test kms_3d. The kms_3d test 
expects the no. of 3d modes to be 13.


(The test has hard-coded value for expected no. of 3d modes as 13)

But due to the addition of "matching aspect_ratio" in drm_mode_equal in 
this patch, the total no. of


modes in the connector modelist is increased by 2, resulting in failure 
of assertion 'mode_count==13'.


Perhaps this need to be handled in the test.

-Regards,

Ankit


On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:

From: "Sharma, Shashank" 

Current DRM layer functions don't parse aspect ratio information
while converting a user mode->kernel mode or vice versa. This
causes modeset to pick mode with wrong aspect ratio, eventually
causing failures in HDMI compliance test cases, due to wrong VIC.

This patch adds aspect ratio information in DRM's mode conversion
and mode comparision functions, to make sure kernel picks mode
with right aspect ratio (as per the VIC).

Background:
This patch was once reviewed and merged, and later reverted due to
lack of DRM cap protection. This is a re-spin of this patch, this
time with DRM cap protection, to avoid aspect ratio information, when
the client doesn't request for it.

Review link: https://pw-emeril.freedesktop.org/patch/104068/
Background discussion: https://patchwork.kernel.org/patch/9379057/

Signed-off-by: Shashank Sharma 
Signed-off-by: Lin, Jia 
Signed-off-by: Akashdeep Sharma 
Reviewed-by: Jim Bride  (V2)
Reviewed-by: Jose Abreu  (V4)

Cc: Ville Syrjala 
Cc: Jim Bride 
Cc: Jose Abreu 
Cc: Ankit Nautiyal 

V3: modified the aspect-ratio check in drm_mode_equal as per new flags
 provided by Ville. https://patchwork.freedesktop.org/patch/188043/
V4: rebase
V5: rebase
V6: As recommended by Ville, avoided matching of aspect-ratio in
 drm_fb_helper, while trying to find a common mode among connectors
 for the target clone mode.
V7: rebase
V8: rebase
V9: rebase
V10: rebase
---
  drivers/gpu/drm/drm_fb_helper.c | 12 ++--
  drivers/gpu/drm/drm_modes.c | 35 ++-
  2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0646b10..2ee1eaa 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper 
*fb_helper,
for (j = 0; j < i; j++) {
if (!enabled[j])
continue;
-   if (!drm_mode_equal(modes[j], modes[i]))
+   if (!drm_mode_match(modes[j], modes[i],
+   DRM_MODE_MATCH_TIMINGS |
+   DRM_MODE_MATCH_CLOCK |
+   DRM_MODE_MATCH_FLAGS |
+   DRM_MODE_MATCH_3D_FLAGS))
can_clone = false;
}
}
@@ -2203,7 +2207,11 @@ static bool drm_target_cloned(struct drm_fb_helper 
*fb_helper,
  
  		fb_helper_conn = fb_helper->connector_info[i];

list_for_each_entry(mode, _helper_conn->connector->modes, 
head) {
-   if (drm_mode_equal(mode, dmt_mode))
+   if (drm_mode_match(mode, dmt_mode,
+  DRM_MODE_MATCH_TIMINGS |
+  DRM_MODE_MATCH_CLOCK |
+  DRM_MODE_MATCH_FLAGS |
+  DRM_MODE_MATCH_3D_FLAGS))
modes[i] = mode;
}
if (!modes[i])
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index d6133e8..454f2ff 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1049,7 +1049,8 @@ bool drm_mode_equal(const struct drm_display_mode *mode1,
  DRM_MODE_MATCH_TIMINGS |
  DRM_MODE_MATCH_CLOCK |
  DRM_MODE_MATCH_FLAGS |
- DRM_MODE_MATCH_3D_FLAGS);
+ DRM_MODE_MATCH_3D_FLAGS|
+ DRM_MODE_MATCH_ASPECT_RATIO);
  }
  EXPORT_SYMBOL(drm_mode_equal);
  
@@ -1647,6 +1648,20 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,

out->vrefresh = in->vrefresh;
out->flags = in->flags;
out->type = in->type;
+
+   switch (in->picture_aspect_ratio) {
+   case HDMI_PICTURE_ASPECT_4_3:
+   out->flags |= DRM_MODE_FLAG_PIC_AR_4_3;
+   break;
+   case HDMI_PICTURE_ASPECT_16_9:
+