Re: [PATCH v2 2/4] drm/vc4: Take underscan setup into account when updating planes

[Boris Brezillon <boris.brezillon-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>]

On Fri, 11 May 2018 18:34:50 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Fri, May 11, 2018 at 04:59:17PM +0200, Boris Brezillon wrote:
> > Applying an underscan setup is just a matter of scaling all planes
> > appropriately and adjusting the CRTC X/Y offset to account for the
> > horizontal and vertical border.
> > 
> > Create an vc4_plane_underscan_adj() function doing that and call it from
> > vc4_plane_setup_clipping_and_scaling() so that we are ready to attach
> > underscan properties to the HDMI connector.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> > ---
> > Changes in v2:
> > - Take changes on hborder/vborder meaning into account
> > ---
> >  drivers/gpu/drm/vc4/vc4_plane.c | 49 ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 48 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> > index 71d44c357d35..61ed60841cd6 100644
> > --- a/drivers/gpu/drm/vc4/vc4_plane.c
> > +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> > @@ -258,6 +258,49 @@ static u32 vc4_get_scl_field(struct drm_plane_state *state, int plane)
> >  	}
> >  }
> >  
> > +static int vc4_plane_underscan_adj(struct drm_plane_state *pstate)
> > +{
> > +	struct vc4_plane_state *vc4_pstate = to_vc4_plane_state(pstate);
> > +	struct drm_connector_state *conn_state = NULL;
> > +	struct drm_connector *conn;
> > +	struct drm_crtc_state *crtc_state;
> > +	int i;
> > +
> > +	for_each_new_connector_in_state(pstate->state, conn, conn_state, i) {
> > +		if (conn_state->crtc == pstate->crtc)
> > +			break;
> > +	}
> > +
> > +	if (i == pstate->state->num_connector)
> > +		return 0;
> > +
> > +	if (conn_state->underscan.mode != DRM_UNDERSCAN_ON)
> > +		return 0;
> > +
> > +	crtc_state = drm_atomic_get_new_crtc_state(pstate->state,
> > +						   pstate->crtc);
> > +
> > +	if (conn_state->underscan.hborder >= crtc_state->mode.hdisplay ||
> > +	    conn_state->underscan.vborder >= crtc_state->mode.vdisplay)
> > +		return -EINVAL;  
> 
> border * 2 ?

Oops, indeed. I'll fix that.

> 
> > +
> > +	vc4_pstate->crtc_x += conn_state->underscan.hborder;
> > +	vc4_pstate->crtc_y += conn_state->underscan.vborder;
> > +	vc4_pstate->crtc_w = (vc4_pstate->crtc_w *
> > +			      (crtc_state->mode.hdisplay -
> > +			       (conn_state->underscan.hborder * 2))) /
> > +			     crtc_state->mode.hdisplay;
> > +	vc4_pstate->crtc_h = (vc4_pstate->crtc_h *
> > +			      (crtc_state->mode.vdisplay -
> > +			       (conn_state->underscan.vborder * 2))) /
> > +			     crtc_state->mode.vdisplay;  
> 
> So you're now scaling all planes? The code seems to reject scaling for
> the cursor plane, how are you dealing with that? Or just no cursor
> allowed when underscanning?

No, I just didn't test with a cursor plane. We should probably avoid
scaling the cursor plane and just adjust its position. Eric any opinion
on that?

> 
> I'm also wondering if there's any way we can reconcile these border
> props with the scaling mode prop, should we ever wish to expose
> these props on connectors that already have the scaling mode prop.

Nouveau seems to expose both, and I don't see why we couldn't.

> Maybe we just have to require the scaling mode to be set to fullscreen 
> to allow borders to be specified explictly?
> 
> > +
> > +	if (!vc4_pstate->crtc_w || !vc4_pstate->crtc_h)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> >  static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state)
> >  {
> >  	struct drm_plane *plane = state->plane;
> > @@ -269,7 +312,7 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state)
> >  	int num_planes = fb->format->num_planes;
> >  	u32 h_subsample = 1;
> >  	u32 v_subsample = 1;
> > -	int i;
> > +	int i, ret;
> >  
> >  	for (i = 0; i < num_planes; i++)
> >  		vc4_state->offsets[i] = bo->paddr + fb->offsets[i];
> > @@ -292,6 +335,10 @@ static int vc4_plane_setup_clipping_and_scaling(struct drm_plane_state *state)
> >  	vc4_state->crtc_w = state->crtc_w;
> >  	vc4_state->crtc_h = state->crtc_h;
> >  
> > +	ret = vc4_plane_underscan_adj(state);
> > +	if (ret)
> > +		return ret;
> > +
> >  	vc4_state->x_scaling[0] = vc4_get_scaling_mode(vc4_state->src_w[0],
> >  						       vc4_state->crtc_w);
> >  	vc4_state->y_scaling[0] = vc4_get_scaling_mode(vc4_state->src_h[0],
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel  
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx