Re: [Intel-gfx] [PATCH 2/2] drm: Check the fb size against the adjusted v/hdisplay for stereo modes

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Sep 23, 2013 at 09:39:28PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 23, 2013 at 03:57:40PM +0100, Damien Lespiau wrote:
> > Some stereo modes, like frame packing, need a larger CRTC viewport than
> > the "natural" underlying 2D mode and thus drm_crtc_check_viewport()
> > needs to query the adjusted mode to use the correct h/vdisplay.
> > 
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/drm_crtc.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index db05864..807309f 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -2078,6 +2078,14 @@ static int drm_crtc_check_viewport(const struct drm_crtc *crtc,
> >  	hdisplay = mode->hdisplay;
> >  	vdisplay = mode->vdisplay;
> >  
> > +	if (drm_mode_is_stereo(mode)) {
> > +		struct drm_display_mode adjusted = *mode;
> > +
> > +		drm_mode_set_crtcinfo(&adjusted, CRTC_STEREO_DOUBLE);
> > +		hdisplay = adjusted.crtc_hdisplay;
> > +		vdisplay = adjusted.crtc_vdisplay;
> > +	}
> 
> Yeah, I think this is as good as we can do in generic code. Normally
> the driver is free to adjust the non-visible portion of the timings,
> but with frame packing the non-visible part also affects the viewport
> size, so drivers supporting frame packing simply have to be more
> careful how they adjust the timings, or they need to do another check
> after the adjustments are done.
> 
> For the series:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Depending upon how the hw handles separate left/right framebuffers this
might be the wrong thing to do: If the hw wants double-up timings, but two
framebuffer pointers then we'd check the wrong thing.

But since this is all internal we can easily replace the mode argument
with an explicit h/vdisplay for drm_crtc_check_viewport and shovel the
handling down into drivers. So I think we can postpone this until we have
the separate left/right buffer stuff working.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch