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

[Eric Anholt]

Ville Syrjälä  writes:

> On Fri, May 11, 2018 at 09:47:49PM +0200, Boris Brezillon wrote:
>> On Fri, 11 May 2018 20:29:48 +0300
>> Ville Syrjälä  wrote:
>> 
>> > On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
>> > > On Fri, 11 May 2018 19:54:02 +0300
>> > > Ville Syrjälä  wrote:
>> > >   
>> > > > On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:  
>> > > > > On Fri, 11 May 2018 18:34:50 +0300
>> > > > > Ville Syrjälä  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 
>> > > > > > > ---
>> > > > > > > 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 don't think you can just not 

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

[Ville Syrjälä]

On Fri, May 11, 2018 at 09:47:49PM +0200, Boris Brezillon wrote:
> On Fri, 11 May 2018 20:29:48 +0300
> Ville Syrjälä  wrote:
> 
> > On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
> > > On Fri, 11 May 2018 19:54:02 +0300
> > > Ville Syrjälä  wrote:
> > >   
> > > > On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:  
> > > > > On Fri, 11 May 2018 18:34:50 +0300
> > > > > Ville Syrjälä  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 
> > > > > > > ---
> > > > > > > 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 don't think you can just not scale it. The user asked for the cursor
> > > > to be at a specific place with a specific size. Can't just ignore
> > > > that and do something else. Also eg. i915 would definitely scale the
> > > > cursor since we'd just scale the entire crtc instead of scaling
> > > > individual planes. Different drivers doing different things wouldn't
> > > > be good.  
> > > 
> > > Except in our case the scaling takes place before the composition, so
> > > we don't have a choice.  
> > 
> > The choice is to 

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

[Eric Anholt]

Ville Syrjälä  writes:

> On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
>> On Fri, 11 May 2018 19:54:02 +0300
>> Ville Syrjälä  wrote:
>> 
>> > On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
>> > > On Fri, 11 May 2018 18:34:50 +0300
>> > > Ville Syrjälä  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 
>> > > > > ---
>> > > > > 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 don't think you can just not scale it. The user asked for the cursor
>> > to be at a specific place with a specific size. Can't just ignore
>> > that and do something else. Also eg. i915 would definitely scale the
>> > cursor since we'd just scale the entire crtc instead of scaling
>> > individual planes. Different drivers doing different things wouldn't
>> > be good.
>> 
>> Except in our case the scaling takes place before the composition, so
>> we don't have a choice.
>
> The choice is to either do what userspace asked, or return an error.
>
>> 
>> > 
>> > >   
>> > > > 
>> > > > 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.  
>> > 
>> > 

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

[Boris Brezillon]

On Fri, 11 May 2018 20:29:48 +0300
Ville Syrjälä  wrote:

> On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
> > On Fri, 11 May 2018 19:54:02 +0300
> > Ville Syrjälä  wrote:
> >   
> > > On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:  
> > > > On Fri, 11 May 2018 18:34:50 +0300
> > > > Ville Syrjälä  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 
> > > > > > ---
> > > > > > 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 don't think you can just not scale it. The user asked for the cursor
> > > to be at a specific place with a specific size. Can't just ignore
> > > that and do something else. Also eg. i915 would definitely scale the
> > > cursor since we'd just scale the entire crtc instead of scaling
> > > individual planes. Different drivers doing different things wouldn't
> > > be good.  
> > 
> > Except in our case the scaling takes place before the composition, so
> > we don't have a choice.  
> 
> The choice is to either do what userspace asked, or return an error.

Come on! If we can't use underscan when there's a cursor plane enabled
this feature is pretty much useless. But let's take a real use case to
show you how negligible the lack of scaling on the cursor plane 

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

[Ville Syrjälä]

On Fri, May 11, 2018 at 07:12:21PM +0200, Boris Brezillon wrote:
> On Fri, 11 May 2018 19:54:02 +0300
> Ville Syrjälä  wrote:
> 
> > On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
> > > On Fri, 11 May 2018 18:34:50 +0300
> > > Ville Syrjälä  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 
> > > > > ---
> > > > > 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 don't think you can just not scale it. The user asked for the cursor
> > to be at a specific place with a specific size. Can't just ignore
> > that and do something else. Also eg. i915 would definitely scale the
> > cursor since we'd just scale the entire crtc instead of scaling
> > individual planes. Different drivers doing different things wouldn't
> > be good.
> 
> Except in our case the scaling takes place before the composition, so
> we don't have a choice.

The choice is to either do what userspace asked, or return an error.

> 
> > 
> > >   
> > > > 
> > > > 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.  
> > 
> > First of all the interaction of these borders with panels that have
> > a fixed mode would need to be properly specified. Do we apply the
> > 

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

[Boris Brezillon]

On Fri, 11 May 2018 19:54:02 +0300
Ville Syrjälä  wrote:

> On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
> > On Fri, 11 May 2018 18:34:50 +0300
> > Ville Syrjälä  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 
> > > > ---
> > > > 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 don't think you can just not scale it. The user asked for the cursor
> to be at a specific place with a specific size. Can't just ignore
> that and do something else. Also eg. i915 would definitely scale the
> cursor since we'd just scale the entire crtc instead of scaling
> individual planes. Different drivers doing different things wouldn't
> be good.

Except in our case the scaling takes place before the composition, so
we don't have a choice.

> 
> >   
> > > 
> > > 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.  
> 
> First of all the interaction of these borders with panels that have
> a fixed mode would need to be properly specified. Do we apply the
> borders against the user mode or the panel's native mode?

Hm, I'm a bit lost. Isn't crtc_state->mode supposed to contain the mode
that is about to be applied, no matter if it's a usermode or one of the
mode returned by the display? 

> I guess
> the latter would be a bit easier (would also match how the TV 

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

[Ville Syrjälä]

On Fri, May 11, 2018 at 05:52:56PM +0200, Boris Brezillon wrote:
> On Fri, 11 May 2018 18:34:50 +0300
> Ville Syrjälä  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 
> > > ---
> > > 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 don't think you can just not scale it. The user asked for the cursor
to be at a specific place with a specific size. Can't just ignore
that and do something else. Also eg. i915 would definitely scale the
cursor since we'd just scale the entire crtc instead of scaling
individual planes. Different drivers doing different things wouldn't
be good.

> 
> > 
> > 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.

First of all the interaction of these borders with panels that have
a fixed mode would need to be properly specified. Do we apply the
borders against the user mode or the panel's native mode? I guess
the latter would be a bit easier (would also match how the TV margins
behave I suppose). But that does leave open the question of how
would generic userspace know which case it's dealing with? Eg. if it
wants to underscan by some specific percentage it has to calculate
the borders based on the correct mode, otherwise the results aren't
going to match the expectations.

> 
> > 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 
> > > 

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

[Boris Brezillon]

On Fri, 11 May 2018 18:34:50 +0300
Ville Syrjälä  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 
> > ---
> > 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  
> 

___
dri-devel mailing list

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

[Ville Syrjälä]

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 
> ---
> 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 ?

> +
> + 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?

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.
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

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Boris Brezillon]

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 
---
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;
+
+   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;
+
+   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