diff mbox

[2/2] drm: Check the fb size against the adjusted v/hdisplay for stereo modes

Message ID 1379948260-21846-3-git-send-email-damien.lespiau@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lespiau, Damien Sept. 23, 2013, 2:57 p.m. UTC
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(+)

Comments

Ville Syrjälä Sept. 23, 2013, 6:39 p.m. UTC | #1
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>

> +
>  	if (crtc->invert_dimensions)
>  		swap(hdisplay, vdisplay);
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Sept. 24, 2013, 9:18 a.m. UTC | #2
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
diff mbox

Patch

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;
+	}
+
 	if (crtc->invert_dimensions)
 		swap(hdisplay, vdisplay);