On Tue, 2016-07-26 at 19:07 +0300, ville.syrjala at linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala at linux.intel.com> > > Replace the use of drm_plane_helper_check_update() with > drm_plane_helper_check_state() since we have a plane state. > > This also eliminates the double clipping the driver was doing > in both check and commit phases). And it should fix src coordinate > addr adjustement. Previously the driver was expecting negative dst > coordinates after clipping, which is not going happen, so any clipping > induced addr adjustment simply didn't happen. Neither did the driver > respect any user configured src coordinates, so panning and such would > have been totally broken. It should be all good now. > > Cc: CK Hu <ck.hu at mediatek.com> > Cc: linux-mediatek at lists.infradead.org > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
This patch looks fine to me, thanks for your patch. Reviewed-by: Bibby Hsieh <bibby.hsieh at mediatek.com> Tested-by: Bibby Hsieh <bibby.hsieh at mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 72 > +++++++++----------------------- > 1 file changed, 20 insertions(+), 52 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > index 3995765a90dc..5f2516fca079 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > @@ -30,15 +30,20 @@ static const u32 formats[] = { > DRM_FORMAT_RGB565, > }; > > -static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, bool enable, > - dma_addr_t addr, struct drm_rect *dest) > +static void mtk_plane_enable(struct mtk_drm_plane *mtk_plane, > + dma_addr_t addr) > { > struct drm_plane *plane = &mtk_plane->base; > struct mtk_plane_state *state = to_mtk_plane_state(plane->state); > unsigned int pitch, format; > - int x, y; > + bool enable; > > - if (WARN_ON(!plane->state || (enable && !plane->state->fb))) > + if (WARN_ON(!plane->state)) > + return; > + > + enable = state->base.visible; > + > + if (WARN_ON(enable && !plane->state->fb)) > return; > > if (plane->state->fb) { > @@ -49,27 +54,17 @@ static void mtk_plane_enable(struct mtk_drm_plane > *mtk_plane, bool enable, > format = DRM_FORMAT_RGBA8888; > } > > - x = plane->state->crtc_x; > - y = plane->state->crtc_y; > - > - if (x < 0) { > - addr -= x * 4; > - x = 0; > - } > - > - if (y < 0) { > - addr -= y * pitch; > - y = 0; > - } > + addr += (state->base.src.x1 >> 16) * 4; > + addr += (state->base.src.y1 >> 16) * pitch; > > state->pending.enable = enable; > state->pending.pitch = pitch; > state->pending.format = format; > state->pending.addr = addr; > - state->pending.x = x; > - state->pending.y = y; > - state->pending.width = dest->x2 - dest->x1; > - state->pending.height = dest->y2 - dest->y1; > + state->pending.x = state->base.dst.x1; > + state->pending.y = state->base.dst.y1; > + state->pending.width = drm_rect_width(&state->base.dst); > + state->pending.height = drm_rect_height(&state->base.dst); > wmb(); /* Make sure the above parameters are set before update */ > state->pending.dirty = true; > } > @@ -134,20 +129,6 @@ static int mtk_plane_atomic_check(struct drm_plane > *plane, > { > struct drm_framebuffer *fb = state->fb; > struct drm_crtc_state *crtc_state; > - bool visible; > - struct drm_rect dest = { > - .x1 = state->crtc_x, > - .y1 = state->crtc_y, > - .x2 = state->crtc_x + state->crtc_w, > - .y2 = state->crtc_y + state->crtc_h, > - }; > - struct drm_rect src = { > - /* 16.16 fixed point */ > - .x1 = state->src_x, > - .y1 = state->src_y, > - .x2 = state->src_x + state->src_w, > - .y2 = state->src_y + state->src_h, > - }; > struct drm_rect clip = { 0, }; > > if (!fb) > @@ -168,12 +149,10 @@ static int mtk_plane_atomic_check(struct drm_plane > *plane, > clip.x2 = crtc_state->mode.hdisplay; > clip.y2 = crtc_state->mode.vdisplay; > > - return drm_plane_helper_check_update(plane, state->crtc, fb, > - &src, &dest, &clip, > - state->rotation, > - DRM_PLANE_HELPER_NO_SCALING, > - DRM_PLANE_HELPER_NO_SCALING, > - true, true, &visible); > + return drm_plane_helper_check_state(state, &clip, > + DRM_PLANE_HELPER_NO_SCALING, > + DRM_PLANE_HELPER_NO_SCALING, > + true, true); > } > > static void mtk_plane_atomic_update(struct drm_plane *plane, > @@ -184,24 +163,13 @@ static void mtk_plane_atomic_update(struct drm_plane > *plane, > struct drm_gem_object *gem; > struct mtk_drm_gem_obj *mtk_gem; > struct mtk_drm_plane *mtk_plane = to_mtk_plane(plane); > - struct drm_rect dest = { > - .x1 = state->base.crtc_x, > - .y1 = state->base.crtc_y, > - .x2 = state->base.crtc_x + state->base.crtc_w, > - .y2 = state->base.crtc_y + state->base.crtc_h, > - }; > - struct drm_rect clip = { 0, }; > > if (!crtc) > return; > > - clip.x2 = state->base.crtc->state->mode.hdisplay; > - clip.y2 = state->base.crtc->state->mode.vdisplay; > - drm_rect_intersect(&dest, &clip); > - > gem = mtk_fb_get_gem_obj(state->base.fb); > mtk_gem = to_mtk_gem_obj(gem); > - mtk_plane_enable(mtk_plane, true, mtk_gem->dma_addr, &dest); > + mtk_plane_enable(mtk_plane, mtk_gem->dma_addr); > } > > static void mtk_plane_atomic_disable(struct drm_plane *plane, -- Bibby