From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [Intel-gfx] [PATCH 2/2] drm: Check the fb size against the adjusted v/hdisplay for stereo modes Date: Tue, 24 Sep 2013 11:18:28 +0200 Message-ID: <20130924091827.GG13668@phenom.ffwll.local> References: <1379948260-21846-1-git-send-email-damien.lespiau@intel.com> <1379948260-21846-3-git-send-email-damien.lespiau@intel.com> <20130923183928.GX4531@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20130923183928.GX4531@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Sep 23, 2013 at 09:39:28PM +0300, Ville Syrj=E4l=E4 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 > > --- > > 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 =3D mode->hdisplay; > > vdisplay =3D mode->vdisplay; > > = > > + if (drm_mode_is_stereo(mode)) { > > + struct drm_display_mode adjusted =3D *mode; > > + > > + drm_mode_set_crtcinfo(&adjusted, CRTC_STEREO_DOUBLE); > > + hdisplay =3D adjusted.crtc_hdisplay; > > + vdisplay =3D 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=E4l=E4 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