| Message ID | 1470221788-30808-3-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 3, 2016 at 6:56 AM, Shashank Sharma <shashank.sharma@intel.com> wrote: > Current DRM layer functions dont parse aspect ratio information s/dont/don't/ > while converting a user mode->kernel mode or viceversa. This s/viceversa/vice versa/ > causes modeset to pick mode with wrong aspect ratio, eventually > cauing failures in HDMI compliance test cases, due to wrong VIC. s/cauing/causing/ Sorry for the spelling nits. I originally just found this one, but since I was nitpicking one, I figured I should pick at them all. > > 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). > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > Signed-off-by: Lin, Jia <lin.a.jia@intel.com> > Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com> > --- > drivers/gpu/drm/drm_modes.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index fc5040a..e6029e0 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -969,6 +969,7 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, > mode1->vsync_end == mode2->vsync_end && > mode1->vtotal == mode2->vtotal && > mode1->vscan == mode2->vscan && > + mode1->picture_aspect_ratio == mode2->picture_aspect_ratio && > (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) == > (mode2->flags & ~DRM_MODE_FLAG_3D_MASK)) > return true; > @@ -1471,6 +1472,22 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, > out->vrefresh = in->vrefresh; > out->flags = in->flags; > out->type = in->type; > + out->flags &= ~DRM_MODE_FLAG_PARMASK; > + > + switch (in->picture_aspect_ratio) { > + case HDMI_PICTURE_ASPECT_4_3: > + out->flags |= DRM_MODE_FLAG_PAR4_3; > + break; > + case HDMI_PICTURE_ASPECT_16_9: > + out->flags |= DRM_MODE_FLAG_PAR16_9; > + break; > + case HDMI_PICTURE_ASPECT_NONE: > + case HDMI_PICTURE_ASPECT_RESERVED: > + default: No need to specify HDMI_PICTURE_ASPECT_NONE & HDMI_PICTURE_ASPECT_RESERVED if you use default. > + out->flags |= DRM_MODE_FLAG_PARNONE; > + break; > + } > + > strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); > out->name[DRM_DISPLAY_MODE_LEN-1] = 0; > } > @@ -1516,6 +1533,20 @@ int drm_mode_convert_umode(struct drm_display_mode *out, > 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 */ > + out->flags &= ~DRM_MODE_FLAG_PARMASK; > + > + switch (in->flags & DRM_MODE_FLAG_PARMASK) { > + case DRM_MODE_FLAG_PAR4_3: > + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3; > + break; > + case DRM_MODE_FLAG_PAR16_9: > + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9; > + break; > + default: > + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; For safety, break; > + } > + > out->status = drm_mode_validate_basic(out); > if (out->status != MODE_OK) > goto out; > -- > 1.9.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Regards Shashank On 8/3/2016 11:14 PM, Sean Paul wrote: > On Wed, Aug 3, 2016 at 6:56 AM, Shashank Sharma > <shashank.sharma@intel.com> wrote: >> Current DRM layer functions dont parse aspect ratio information > s/dont/don't/ Got it. > >> while converting a user mode->kernel mode or viceversa. This > s/viceversa/vice versa/ Got it. >> causes modeset to pick mode with wrong aspect ratio, eventually >> cauing failures in HDMI compliance test cases, due to wrong VIC. > s/cauing/causing/ Ok. > Sorry for the spelling nits. I originally just found this one, but > since I was nitpicking one, I figured I should pick at them all. Thanks for pointing them out. Will fix this. >> 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). >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> Signed-off-by: Lin, Jia <lin.a.jia@intel.com> >> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com> >> --- >> drivers/gpu/drm/drm_modes.c | 31 +++++++++++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c >> index fc5040a..e6029e0 100644 >> --- a/drivers/gpu/drm/drm_modes.c >> +++ b/drivers/gpu/drm/drm_modes.c >> @@ -969,6 +969,7 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, >> mode1->vsync_end == mode2->vsync_end && >> mode1->vtotal == mode2->vtotal && >> mode1->vscan == mode2->vscan && >> + mode1->picture_aspect_ratio == mode2->picture_aspect_ratio && >> (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) == >> (mode2->flags & ~DRM_MODE_FLAG_3D_MASK)) >> return true; >> @@ -1471,6 +1472,22 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, >> out->vrefresh = in->vrefresh; >> out->flags = in->flags; >> out->type = in->type; >> + out->flags &= ~DRM_MODE_FLAG_PARMASK; >> + >> + switch (in->picture_aspect_ratio) { >> + case HDMI_PICTURE_ASPECT_4_3: >> + out->flags |= DRM_MODE_FLAG_PAR4_3; >> + break; >> + case HDMI_PICTURE_ASPECT_16_9: >> + out->flags |= DRM_MODE_FLAG_PAR16_9; >> + break; >> + case HDMI_PICTURE_ASPECT_NONE: >> + case HDMI_PICTURE_ASPECT_RESERVED: >> + default: > No need to specify HDMI_PICTURE_ASPECT_NONE & > HDMI_PICTURE_ASPECT_RESERVED if you use default. I agree on ASPECT_NONE, but I think we should keep ASPECT_RESERVED, to maintain the convention, do you think so ? >> + out->flags |= DRM_MODE_FLAG_PARNONE; >> + break; >> + } >> + >> strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); >> out->name[DRM_DISPLAY_MODE_LEN-1] = 0; >> } >> @@ -1516,6 +1533,20 @@ int drm_mode_convert_umode(struct drm_display_mode *out, >> 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 */ >> + out->flags &= ~DRM_MODE_FLAG_PARMASK; >> + >> + switch (in->flags & DRM_MODE_FLAG_PARMASK) { >> + case DRM_MODE_FLAG_PAR4_3: >> + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3; >> + break; >> + case DRM_MODE_FLAG_PAR16_9: >> + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9; >> + break; >> + default: >> + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; > For safety, > > break; Sure, Can be done. Regards Shashank > >> + } >> + >> out->status = drm_mode_validate_basic(out); >> if (out->status != MODE_OK) >> goto out; >> -- >> 1.9.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index fc5040a..e6029e0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -969,6 +969,7 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, mode1->vsync_end == mode2->vsync_end && mode1->vtotal == mode2->vtotal && mode1->vscan == mode2->vscan && + mode1->picture_aspect_ratio == mode2->picture_aspect_ratio && (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) == (mode2->flags & ~DRM_MODE_FLAG_3D_MASK)) return true; @@ -1471,6 +1472,22 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, out->vrefresh = in->vrefresh; out->flags = in->flags; out->type = in->type; + out->flags &= ~DRM_MODE_FLAG_PARMASK; + + switch (in->picture_aspect_ratio) { + case HDMI_PICTURE_ASPECT_4_3: + out->flags |= DRM_MODE_FLAG_PAR4_3; + break; + case HDMI_PICTURE_ASPECT_16_9: + out->flags |= DRM_MODE_FLAG_PAR16_9; + break; + case HDMI_PICTURE_ASPECT_NONE: + case HDMI_PICTURE_ASPECT_RESERVED: + default: + out->flags |= DRM_MODE_FLAG_PARNONE; + break; + } + strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); out->name[DRM_DISPLAY_MODE_LEN-1] = 0; } @@ -1516,6 +1533,20 @@ int drm_mode_convert_umode(struct drm_display_mode *out, 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 */ + out->flags &= ~DRM_MODE_FLAG_PARMASK; + + switch (in->flags & DRM_MODE_FLAG_PARMASK) { + case DRM_MODE_FLAG_PAR4_3: + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3; + break; + case DRM_MODE_FLAG_PAR16_9: + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9; + break; + default: + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; + } + out->status = drm_mode_validate_basic(out); if (out->status != MODE_OK) goto out;