| Message ID | 1379353735-4472-12-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 16, 2013 at 06:48:54PM +0100, Damien Lespiau wrote: > When scanning out a stereo mode, the AVI infoframe vic field has to be > the underlyng 2D VIC. Before that commit, we weren't matching the CEA > mode because of the extra stereo flag and then were setting the VIC > field in the AVI infoframe to 0. I think this is where we hit problems. How do we identify the 2D VIC when the timings of the adjusted mode got mangled already to include both eyes? > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > --- > drivers/gpu/drm/drm_edid.c | 4 ++-- > drivers/gpu/drm/drm_modes.c | 18 ++++++++++++------ > include/drm/drm_crtc.h | 2 +- > 3 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 9a36b6e..6b74698 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2401,7 +2401,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) > > if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || > KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && > - drm_mode_equal_no_clocks(to_match, cea_mode)) > + drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode)) > return mode + 1; > } > return 0; > @@ -2450,7 +2450,7 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) > > if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || > KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && > - drm_mode_equal_no_clocks(to_match, hdmi_mode)) > + drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) > return mode + 1; > } > return 0; > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 504a602..76adee0 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -851,12 +851,16 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ > } else if (mode1->clock != mode2->clock) > return false; > > - return drm_mode_equal_no_clocks(mode1, mode2); > + if ((mode1->flags & DRM_MODE_FLAG_3D_MASK) != > + (mode2->flags & DRM_MODE_FLAG_3D_MASK)) > + return false; > + > + return drm_mode_equal_no_clocks_no_stereo(mode1, mode2); > } > EXPORT_SYMBOL(drm_mode_equal); > > /** > - * drm_mode_equal_no_clocks - test modes for equality > + * drm_mode_equal_no_clocks_no_stereo - test modes for equality > * @mode1: first mode > * @mode2: second mode > * > @@ -864,12 +868,13 @@ EXPORT_SYMBOL(drm_mode_equal); > * None. > * > * Check to see if @mode1 and @mode2 are equivalent, but > - * don't check the pixel clocks. > + * don't check the pixel clocks nor the stereo layout. > * > * RETURNS: > * True if the modes are equal, false otherwise. > */ > -bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2) > +bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, > + const struct drm_display_mode *mode2) > { > if (mode1->hdisplay == mode2->hdisplay && > mode1->hsync_start == mode2->hsync_start && > @@ -881,12 +886,13 @@ bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct > mode1->vsync_end == mode2->vsync_end && > mode1->vtotal == mode2->vtotal && > mode1->vscan == mode2->vscan && > - mode1->flags == mode2->flags) > + (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) == > + (mode2->flags & ~DRM_MODE_FLAG_3D_MASK)) > return true; > > return false; > } > -EXPORT_SYMBOL(drm_mode_equal_no_clocks); > +EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo); > > /** > * drm_mode_validate_size - make sure modes adhere to size constraints > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index e685baf..f0c5530 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -943,7 +943,7 @@ extern void drm_mode_config_reset(struct drm_device *dev); > extern void drm_mode_config_cleanup(struct drm_device *dev); > extern void drm_mode_set_name(struct drm_display_mode *mode); > extern bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); > -extern bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); > +extern bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); > extern int drm_mode_width(const struct drm_display_mode *mode); > extern int drm_mode_height(const struct drm_display_mode *mode); > > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Sep 17, 2013 at 11:22:26AM +0300, Ville Syrjälä wrote: > On Mon, Sep 16, 2013 at 06:48:54PM +0100, Damien Lespiau wrote: > > When scanning out a stereo mode, the AVI infoframe vic field has to be > > the underlyng 2D VIC. Before that commit, we weren't matching the CEA > > mode because of the extra stereo flag and then were setting the VIC > > field in the AVI infoframe to 0. > > I think this is where we hit problems. How do we identify the 2D VIC > when the timings of the adjusted mode got mangled already to include > both eyes? Oh, indeed, that wouldn't work. Note that Daniel had the same remark as you regarding the mangling of the modes, "leaking" how we implement the modes (in theory hw could need a different way to be programmed). I wondered about that as well and now that we have a couple of good reasons, in kernel timing adjustment for stereo modes will happen in a v5 of this series.
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 9a36b6e..6b74698 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2401,7 +2401,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && - drm_mode_equal_no_clocks(to_match, cea_mode)) + drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode)) return mode + 1; } return 0; @@ -2450,7 +2450,7 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && - drm_mode_equal_no_clocks(to_match, hdmi_mode)) + drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) return mode + 1; } return 0; diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 504a602..76adee0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -851,12 +851,16 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ } else if (mode1->clock != mode2->clock) return false; - return drm_mode_equal_no_clocks(mode1, mode2); + if ((mode1->flags & DRM_MODE_FLAG_3D_MASK) != + (mode2->flags & DRM_MODE_FLAG_3D_MASK)) + return false; + + return drm_mode_equal_no_clocks_no_stereo(mode1, mode2); } EXPORT_SYMBOL(drm_mode_equal); /** - * drm_mode_equal_no_clocks - test modes for equality + * drm_mode_equal_no_clocks_no_stereo - test modes for equality * @mode1: first mode * @mode2: second mode * @@ -864,12 +868,13 @@ EXPORT_SYMBOL(drm_mode_equal); * None. * * Check to see if @mode1 and @mode2 are equivalent, but - * don't check the pixel clocks. + * don't check the pixel clocks nor the stereo layout. * * RETURNS: * True if the modes are equal, false otherwise. */ -bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2) +bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) { if (mode1->hdisplay == mode2->hdisplay && mode1->hsync_start == mode2->hsync_start && @@ -881,12 +886,13 @@ bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct mode1->vsync_end == mode2->vsync_end && mode1->vtotal == mode2->vtotal && mode1->vscan == mode2->vscan && - mode1->flags == mode2->flags) + (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) == + (mode2->flags & ~DRM_MODE_FLAG_3D_MASK)) return true; return false; } -EXPORT_SYMBOL(drm_mode_equal_no_clocks); +EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo); /** * drm_mode_validate_size - make sure modes adhere to size constraints diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index e685baf..f0c5530 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -943,7 +943,7 @@ extern void drm_mode_config_reset(struct drm_device *dev); extern void drm_mode_config_cleanup(struct drm_device *dev); extern void drm_mode_set_name(struct drm_display_mode *mode); extern bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); -extern bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); +extern bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2); extern int drm_mode_width(const struct drm_display_mode *mode); extern int drm_mode_height(const struct drm_display_mode *mode);
When scanning out a stereo mode, the AVI infoframe vic field has to be the underlyng 2D VIC. Before that commit, we weren't matching the CEA mode because of the extra stereo flag and then were setting the VIC field in the AVI infoframe to 0. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- drivers/gpu/drm/drm_edid.c | 4 ++-- drivers/gpu/drm/drm_modes.c | 18 ++++++++++++------ include/drm/drm_crtc.h | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-)