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" <shashank.sha...@intel.com>
> >
> > 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 <shashank.sha...@intel.com>
> > Signed-off-by: Lin, Jia <lin.a....@intel.com>
> > Signed-off-by: Akashdeep Sharma <akashdeep.sha...@intel.com>
> > Reviewed-by: Jim Bride <jim.br...@linux.intel.com> (V2)
> > Reviewed-by: Jose Abreu <jose.ab...@synopsys.com> (V4)
> >
> > Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> > Cc: Jim Bride <jim.br...@linux.intel.com>
> > Cc: Jose Abreu <jose.ab...@synopsys.com>
> > Cc: Ankit Nautiyal <ankit.k.nauti...@intel.com>
> >
> > 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, &fb_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:
> > +           out->flags |= DRM_MODE_FLAG_PIC_AR_16_9;
> > +           break;
> > +   case HDMI_PICTURE_ASPECT_RESERVED:
> > +   default:
> > +           out->flags |= DRM_MODE_FLAG_PIC_AR_NONE;
> > +           break;
> > +   }
> > +
> >     strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
> >     out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
> >   }
> > @@ -1693,6 +1708,24 @@ int drm_mode_convert_umode(struct drm_device *dev,
> >     strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
> >     out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
> >   
> > +   /* Clearing picture aspect ratio bits from out flags,
> > +    * as the aspect-ratio information is not stored in
> > +    * flags for kernel-mode, but in picture_aspect_ratio.
> > +    */
> > +   out->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK;
> > +
> > +   switch (in->flags & DRM_MODE_FLAG_PIC_AR_MASK) {
> > +   case DRM_MODE_FLAG_PIC_AR_4_3:
> > +           out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3;
> > +           break;
> > +   case DRM_MODE_FLAG_PIC_AR_16_9:
> > +           out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9;
> > +           break;
> > +   default:
> > +           out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> > +           break;
> > +   }
> > +
> >     out->status = drm_mode_validate_driver(dev, out);
> >     if (out->status != MODE_OK)
> >             return -EINVAL;
> 

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to