Re: [RFC 4/4] drm/i915: Switch to unified plane cursor handling

Chris Wilson
2014-05-16T10:39:11Z

On Thu, May 15, 2014 at 06:17:29PM -0700, Matt Roper wrote:
> The DRM core will translate calls to legacy cursor ioctls into universal
> cursor calls automatically, so there's no need to maintain the legacy
> cursor support. This greatly simplifies the transition since we don't
> have to handle reference counting differently depending on which cursor
> interface was called.
>
> The aim here is to transition to the universal plane interface with
> minimal code change. There's a lot of cleanup that can be done (e.g.,
> using state stored in crtc->cursor->fb rather than intel_crtc) that is
> left to future patches.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> +static int
> +intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
> + struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> + unsigned int crtc_w, unsigned int crtc_h,
> + uint32_t src_x, uint32_t src_y,
> + uint32_t src_w, uint32_t src_h)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> + struct drm_i915_gem_object *obj = intel_fb->obj;
> + struct drm_rect dest = {
> + /* integer pixels */
> + .x1 = crtc_x,
> + .y1 = crtc_y,
> + .x2 = crtc_x + crtc_w,
> + .y2 = crtc_y + crtc_h,
> + };
> + struct drm_rect src = {
> + /* 16.16 fixed point */
> + .x1 = src_x,
> + .y1 = src_y,
> + .x2 = src_x + src_w,
> + .y2 = src_y + src_h,
> + };
> + const struct drm_rect clip = {
> + /* integer pixels */
> + .x2 = intel_crtc->config.pipe_src_w,
> + .y2 = intel_crtc->config.pipe_src_h,
> + };
> + int hscale, vscale;
> + bool visible;
> +
> + /* Check scaling */
> + hscale = drm_rect_calc_hscale(&src, &dest,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING);
> + vscale = drm_rect_calc_vscale(&src, &dest,
> + DRM_PLANE_HELPER_NO_SCALING,
> + DRM_PLANE_HELPER_NO_SCALING);
> + if (hscale < 0 || vscale < 0) {
> + DRM_DEBUG_KMS("Invalid scaling of cursor plane\n");
> + return -ERANGE;
> + }
> +
> + /* Check dimensions */
> + if (!((crtc_w == 64 && crtc_h == 64) ||
> + (crtc_w == 128 && crtc_h == 128 && !IS_GEN2(dev)) ||
> + (crtc_w == 256 && crtc_h == 256 && !IS_GEN2(dev)))) {
> + DRM_DEBUG_KMS("Cursor dimension not supported: %dx%d\n",
> + crtc_w, crtc_h);
> + return -EINVAL;
> + }
> +
> + /* Clip to display; if no longer visible after clipping, disable */
> + visible = drm_rect_clip_scaled(&src, &dest, &clip, hscale, vscale);
> +
> + crtc->cursor_x = crtc_x;
> + crtc->cursor_y = crtc_y;
> + if (fb != crtc->cursor->fb) {
> + return intel_crtc_cursor_set_obj(crtc, visible ? obj : NULL,
> + crtc_w, crtc_h);
> + } else {
> + intel_crtc_update_cursor(crtc, true);
> + return 0;
> + }

Does this do the right thing for a cursor that is no longer visible, and
vice versa? It is not immediately clear how clipping now works with
intel_crtc_update_cursor().
-Chris

--
Chris Wilson, Intel Open Source Technology Centre