| Message ID | 1464084653-16684-4-git-send-email-gnuiyl@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am Dienstag, den 24.05.2016, 18:10 +0800 schrieb Liu Ying: > The IPUv3 primary plane doesn't support partial off screen. > So, this patch separates plane check logics for primary plane and overlay > plane and adds more limitations on the primary plane. > > Signed-off-by: Liu Ying <gnuiyl@gmail.com> > --- > drivers/gpu/drm/imx/ipuv3-plane.c | 72 ++++++++++++++++++++++----------------- > 1 file changed, 41 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c > index e6ec8eb..8f91b2e 100644 > --- a/drivers/gpu/drm/imx/ipuv3-plane.c > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c > @@ -190,44 +190,54 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc, > uint32_t src_x, uint32_t src_y, > uint32_t src_w, uint32_t src_h, bool interlaced) > { > - struct device *dev = ipu_plane->base.dev->dev; > + struct drm_plane plane = ipu_plane->base; Why make a copy of the drm_plane here? > + struct device *dev = plane.dev->dev; > int ret; > > /* no scaling */ > if (src_w != crtc_w || src_h != crtc_h) > return -EINVAL; > > - /* clip to crtc bounds */ > - if (crtc_x < 0) { > - if (-crtc_x > crtc_w) > + if (plane.type == DRM_PLANE_TYPE_PRIMARY) { > + /* full plane doesn't support partial off screen */ > + if (crtc_x || crtc_y || crtc_w != mode->hdisplay || > + crtc_h != mode->vdisplay) As long as the requested plane is large enough to cover the whole base plane, we can fix the crtc_x/y/w/h up by clipping to the base plane boundaries. There is no need to return -EINVAL here as long as the IDMAC is capable to start reading at src_x/y = -crtc_x/y. regards Philipp
On Tue, May 24, 2016 at 10:20 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > Am Dienstag, den 24.05.2016, 18:10 +0800 schrieb Liu Ying: >> The IPUv3 primary plane doesn't support partial off screen. >> So, this patch separates plane check logics for primary plane and overlay >> plane and adds more limitations on the primary plane. >> >> Signed-off-by: Liu Ying <gnuiyl@gmail.com> >> --- >> drivers/gpu/drm/imx/ipuv3-plane.c | 72 ++++++++++++++++++++++----------------- >> 1 file changed, 41 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c >> index e6ec8eb..8f91b2e 100644 >> --- a/drivers/gpu/drm/imx/ipuv3-plane.c >> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c >> @@ -190,44 +190,54 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc, >> uint32_t src_x, uint32_t src_y, >> uint32_t src_w, uint32_t src_h, bool interlaced) >> { >> - struct device *dev = ipu_plane->base.dev->dev; >> + struct drm_plane plane = ipu_plane->base; > > Why make a copy of the drm_plane here? > >> + struct device *dev = plane.dev->dev; >> int ret; >> >> /* no scaling */ >> if (src_w != crtc_w || src_h != crtc_h) >> return -EINVAL; >> >> - /* clip to crtc bounds */ >> - if (crtc_x < 0) { >> - if (-crtc_x > crtc_w) >> + if (plane.type == DRM_PLANE_TYPE_PRIMARY) { >> + /* full plane doesn't support partial off screen */ >> + if (crtc_x || crtc_y || crtc_w != mode->hdisplay || >> + crtc_h != mode->vdisplay) > > As long as the requested plane is large enough to cover the whole base > plane, we can fix the crtc_x/y/w/h up by clipping to the base plane > boundaries. There is no need to return -EINVAL here as long as the IDMAC > is capable to start reading at src_x/y = -crtc_x/y. For the primary plane, we really need to look at mode->v/hdisplay to make sure no partial off screen happens(plane partial off CRTC). The clip is tricky, so for primary plane, I just take the simple way to avoid the clip. Eventually, we'll use atomic check to simply accept or reject the user's request instead of changing the state to do clip. This makes the driver simpler. Regards, Liu Ying > > regards > Philipp >
On Tue, May 24, 2016 at 10:20 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > Am Dienstag, den 24.05.2016, 18:10 +0800 schrieb Liu Ying: >> The IPUv3 primary plane doesn't support partial off screen. >> So, this patch separates plane check logics for primary plane and overlay >> plane and adds more limitations on the primary plane. >> >> Signed-off-by: Liu Ying <gnuiyl@gmail.com> >> --- >> drivers/gpu/drm/imx/ipuv3-plane.c | 72 ++++++++++++++++++++++----------------- >> 1 file changed, 41 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c >> index e6ec8eb..8f91b2e 100644 >> --- a/drivers/gpu/drm/imx/ipuv3-plane.c >> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c >> @@ -190,44 +190,54 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc, >> uint32_t src_x, uint32_t src_y, >> uint32_t src_w, uint32_t src_h, bool interlaced) >> { >> - struct device *dev = ipu_plane->base.dev->dev; >> + struct drm_plane plane = ipu_plane->base; > > Why make a copy of the drm_plane here? Will remove that copy. Regards, Liu Ying > >> + struct device *dev = plane.dev->dev; >> int ret; >> >> /* no scaling */ >> if (src_w != crtc_w || src_h != crtc_h) >> return -EINVAL; >> >> - /* clip to crtc bounds */ >> - if (crtc_x < 0) { >> - if (-crtc_x > crtc_w) >> + if (plane.type == DRM_PLANE_TYPE_PRIMARY) { >> + /* full plane doesn't support partial off screen */ >> + if (crtc_x || crtc_y || crtc_w != mode->hdisplay || >> + crtc_h != mode->vdisplay) > > As long as the requested plane is large enough to cover the whole base > plane, we can fix the crtc_x/y/w/h up by clipping to the base plane > boundaries. There is no need to return -EINVAL here as long as the IDMAC > is capable to start reading at src_x/y = -crtc_x/y. > > regards > Philipp >
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index e6ec8eb..8f91b2e 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -190,44 +190,54 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc, uint32_t src_x, uint32_t src_y, uint32_t src_w, uint32_t src_h, bool interlaced) { - struct device *dev = ipu_plane->base.dev->dev; + struct drm_plane plane = ipu_plane->base; + struct device *dev = plane.dev->dev; int ret; /* no scaling */ if (src_w != crtc_w || src_h != crtc_h) return -EINVAL; - /* clip to crtc bounds */ - if (crtc_x < 0) { - if (-crtc_x > crtc_w) + if (plane.type == DRM_PLANE_TYPE_PRIMARY) { + /* full plane doesn't support partial off screen */ + if (crtc_x || crtc_y || crtc_w != mode->hdisplay || + crtc_h != mode->vdisplay) return -EINVAL; - src_x += -crtc_x; - src_w -= -crtc_x; - crtc_w -= -crtc_x; - crtc_x = 0; - } - if (crtc_y < 0) { - if (-crtc_y > crtc_h) - return -EINVAL; - src_y += -crtc_y; - src_h -= -crtc_y; - crtc_h -= -crtc_y; - crtc_y = 0; - } - if (crtc_x + crtc_w > mode->hdisplay) { - if (crtc_x > mode->hdisplay) - return -EINVAL; - crtc_w = mode->hdisplay - crtc_x; - src_w = crtc_w; - } - if (crtc_y + crtc_h > mode->vdisplay) { - if (crtc_y > mode->vdisplay) + + /* full plane minimum width is 13 pixels */ + if (crtc_w < 13) return -EINVAL; - crtc_h = mode->vdisplay - crtc_y; - src_h = crtc_h; - } - /* full plane minimum width is 13 pixels */ - if (crtc_w < 13 && (ipu_plane->dp_flow != IPU_DP_FLOW_SYNC_FG)) + } else if (plane.type == DRM_PLANE_TYPE_OVERLAY) { + /* clip to crtc bounds */ + if (crtc_x < 0) { + if (-crtc_x > crtc_w) + return -EINVAL; + src_x += -crtc_x; + src_w -= -crtc_x; + crtc_w -= -crtc_x; + crtc_x = 0; + } + if (crtc_y < 0) { + if (-crtc_y > crtc_h) + return -EINVAL; + src_y += -crtc_y; + src_h -= -crtc_y; + crtc_h -= -crtc_y; + crtc_y = 0; + } + if (crtc_x + crtc_w > mode->hdisplay) { + if (crtc_x > mode->hdisplay) + return -EINVAL; + crtc_w = mode->hdisplay - crtc_x; + src_w = crtc_w; + } + if (crtc_y + crtc_h > mode->vdisplay) { + if (crtc_y > mode->vdisplay) + return -EINVAL; + crtc_h = mode->vdisplay - crtc_y; + src_h = crtc_h; + } + } else return -EINVAL; if (crtc_h < 2) return -EINVAL; @@ -238,7 +248,7 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc, */ if (ipu_plane->enabled) { if (src_w != ipu_plane->w || src_h != ipu_plane->h || - fb->pixel_format != ipu_plane->base.fb->pixel_format) + fb->pixel_format != plane.fb->pixel_format) return -EINVAL; return ipu_plane_set_base(ipu_plane, fb, src_x, src_y);
The IPUv3 primary plane doesn't support partial off screen. So, this patch separates plane check logics for primary plane and overlay plane and adds more limitations on the primary plane. Signed-off-by: Liu Ying <gnuiyl@gmail.com> --- drivers/gpu/drm/imx/ipuv3-plane.c | 72 ++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 31 deletions(-)