On Mon, Jan 18, 2016 at 06:14:32PM +0100, Daniel Vetter wrote: > On Fri, Jan 15, 2016 at 08:51:06PM +0200, ville.syrjala at linux.intel.com > wrote: > > From: Ville Syrjälä <ville.syrjala at linux.intel.com> > > > > drm_plane_helper_check_update() needs to account for the plane rotation > > for correct clipping/scaling calculations. Do so. > > > > There was an earlier attempt [1] to add this into > > intel_check_primary_plane() but I requested that it'd be put into the > > helper instead. An updated patch never materialized AFAICS, so I went > > ahead and cooked one up myself. > > > > [1] https://patchwork.freedesktop.org/patch/65177/ > > Cc: Nabendu Maiti <nabendu.bikash.maiti at intel.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com> > > --- > > drivers/gpu/drm/armada/armada_overlay.c | 1 + > > drivers/gpu/drm/drm_plane_helper.c | 28 > > ++++++++++++++++++---------- > > drivers/gpu/drm/i915/intel_display.c | 2 ++ > > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 1 + > > include/drm/drm_plane_helper.h | 1 + > > 5 files changed, 23 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/armada/armada_overlay.c > > b/drivers/gpu/drm/armada/armada_overlay.c > > index 148e8a42b2c6..1ee707ef6b8d 100644 > > --- a/drivers/gpu/drm/armada/armada_overlay.c > > +++ b/drivers/gpu/drm/armada/armada_overlay.c > > @@ -121,6 +121,7 @@ armada_ovl_plane_update(struct drm_plane *plane, struct > > drm_crtc *crtc, > > int ret; > > > > ret = drm_plane_helper_check_update(plane, crtc, fb, &src, &dest, &clip, > > + BIT(DRM_ROTATE_0), > > 0, INT_MAX, true, false, &visible); > > if (ret) > > return ret; > > diff --git a/drivers/gpu/drm/drm_plane_helper.c > > b/drivers/gpu/drm/drm_plane_helper.c > > index 369d2898ff9e..0c12fa1a696f 100644 > > --- a/drivers/gpu/drm/drm_plane_helper.c > > +++ b/drivers/gpu/drm/drm_plane_helper.c > > @@ -115,6 +115,7 @@ static int get_connectors_for_crtc(struct drm_crtc > > *crtc, > > * @src: source coordinates in 16.16 fixed point > > * @dest: integer destination coordinates > > * @clip: integer clipping coordinates > > + * @rotation: plane rotation > > * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point > > * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point > > * @can_position: is it legal to position the plane such that it > > @@ -134,16 +135,17 @@ static int get_connectors_for_crtc(struct drm_crtc > > *crtc, > > * Zero if update appears valid, error code on failure > > */ > > int drm_plane_helper_check_update(struct drm_plane *plane, > > - struct drm_crtc *crtc, > > - struct drm_framebuffer *fb, > > - struct drm_rect *src, > > - struct drm_rect *dest, > > - const struct drm_rect *clip, > > - int min_scale, > > - int max_scale, > > - bool can_position, > > - bool can_update_disabled, > > - bool *visible) > > + struct drm_crtc *crtc, > > + struct drm_framebuffer *fb, > > + struct drm_rect *src, > > + struct drm_rect *dest, > > + const struct drm_rect *clip, > > + unsigned int rotation, > > + int min_scale, > > + int max_scale, > > + bool can_position, > > + bool can_update_disabled, > > + bool *visible) > > { > > int hscale, vscale; > > > > @@ -163,6 +165,8 @@ int drm_plane_helper_check_update(struct drm_plane > > *plane, > > return -EINVAL; > > } > > > > + drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation); > > + > > /* Check scaling */ > > hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale); > > vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale); > > @@ -174,6 +178,9 @@ int drm_plane_helper_check_update(struct drm_plane > > *plane, > > } > > > > *visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale); > > + > > + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation); > > + > > if (!*visible) > > /* > > * Plane isn't visible; some drivers can handle this > > @@ -265,6 +272,7 @@ int drm_primary_helper_update(struct drm_plane *plane, > > struct drm_crtc *crtc, > > > > ret = drm_plane_helper_check_update(plane, crtc, fb, > > &src, &dest, &clip, > > + BIT(DRM_ROTATE_0), > > DRM_PLANE_HELPER_NO_SCALING, > > DRM_PLANE_HELPER_NO_SCALING, > > false, false, &visible); > > With this rotation is essentially unsupported on legacy setups. Shouldn't > we instead wrap this up into an atomic helper which just takes state > structs and fishes out everything it needs on its own? Then there's no > need to change existing drivers all that much, and atomic ones would be > simpler ... > > Or do I miss something?
We'd need to move the various drm_rects to live under drm_plane_state. Right now they live in intel_plane_state. No objection to doing that from me, but requires someone to actually write the patch. > -Daniel > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index c4cd9c94faa6..a726c5e91220 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -14029,6 +14029,7 @@ intel_check_primary_plane(struct drm_plane *plane, > > > > return drm_plane_helper_check_update(plane, crtc, fb, &state->src, > > &state->dst, &state->clip, > > + state->base.rotation, > > min_scale, max_scale, > > can_position, true, > > &state->visible); > > @@ -14192,6 +14193,7 @@ intel_check_cursor_plane(struct drm_plane *plane, > > > > ret = drm_plane_helper_check_update(plane, crtc, fb, &state->src, > > &state->dst, &state->clip, > > + state->base.rotation, > > DRM_PLANE_HELPER_NO_SCALING, > > DRM_PLANE_HELPER_NO_SCALING, > > true, true, &state->visible); > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > index 46c2a8dfd8aa..532df0a8656a 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > @@ -583,6 +583,7 @@ static int vop_plane_atomic_check(struct drm_plane > > *plane, > > > > ret = drm_plane_helper_check_update(plane, crtc, state->fb, > > src, dest, &clip, > > + state->rotation, > > min_scale, > > max_scale, > > true, true, &visible); > > diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h > > index 4421f3f4ca8d..0e0c3573cce0 100644 > > --- a/include/drm/drm_plane_helper.h > > +++ b/include/drm/drm_plane_helper.h > > @@ -46,6 +46,7 @@ int drm_plane_helper_check_update(struct drm_plane *plane, > > struct drm_rect *src, > > struct drm_rect *dest, > > const struct drm_rect *clip, > > + unsigned int rotation, > > int min_scale, > > int max_scale, > > bool can_position, > > -- > > 2.4.10 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel at lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel OTC
