diff mbox series

[8/9] drm/ast: Add cursor plane

Message ID 20191028154928.4058-9-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/ast: Convert to atomic modesetting | expand

Commit Message

Thomas Zimmermann Oct. 28, 2019, 3:49 p.m. UTC
The cursor plane uses an internal format of ARGB4444. To userspace, we
announce ARGB8888 and do the transformation internally.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.h  |   1 +
 drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
 2 files changed, 161 insertions(+), 1 deletion(-)

Comments

Gerd Hoffmann Nov. 5, 2019, 9:52 a.m. UTC | #1
On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
> The cursor plane uses an internal format of ARGB4444. To userspace, we
> announce ARGB8888 and do the transformation internally.

Same comments as on patch 6/9 (primary plane ...)

cheers,
  Gerd
Daniel Vetter Nov. 5, 2019, 9:55 a.m. UTC | #2
On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
> The cursor plane uses an internal format of ARGB4444. To userspace, we
> announce ARGB8888 and do the transformation internally.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Hm, might be fun to also expose the ARGB4444 directly. Not that anyone
will actually use it :-/
-Daniel

> ---
>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
>  drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
>  2 files changed, 161 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 13560622f22a..49557a73390f 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -122,6 +122,7 @@ struct ast_private {
>  	} cursor;
>  
>  	struct drm_plane primary_plane;
> +	struct drm_plane cursor_plane;
>  
>  	bool support_wide_screen;
>  	enum {
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 7667f4502eb9..f5f73200e8e4 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -54,6 +54,16 @@ static int ast_cursor_move(struct drm_crtc *crtc,
>  			   int x, int y);
>  
>  
> +static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
> +static int ast_cursor_update(void *dst, void *src, unsigned int width,
> +			     unsigned int height);
> +static void ast_cursor_set_base(struct ast_private *ast, u64 address);
> +static int ast_show_cursor(struct drm_crtc *crtc, void *src,
> +			   unsigned int width, unsigned int height);
> +static void ast_hide_cursor(struct drm_crtc *crtc);
> +static int ast_cursor_move(struct drm_crtc *crtc,
> +			   int x, int y);
> +
>  static inline void ast_load_palette_index(struct ast_private *ast,
>  				     u8 index, u8 red, u8 green,
>  				     u8 blue)
> @@ -594,6 +604,139 @@ static const struct drm_plane_funcs ast_primary_plane_funcs = {
>  	.format_mod_supported = NULL,
>  };
>  
> +/*
> + * Cursor plane
> + */
> +
> +static int
> +ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
> +				   struct drm_plane_state *new_state)
> +{
> +	struct drm_framebuffer *fb = new_state->fb;
> +	struct drm_crtc *crtc = new_state->crtc;
> +	struct drm_gem_vram_object *gbo;
> +	struct ast_private *ast;
> +	int ret;
> +	void *src, *dst;
> +
> +	if (!crtc || !fb)
> +		return 0;
> +
> +	if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
> +		return -EINVAL;
> +
> +	ast = crtc->dev->dev_private;
> +
> +	gbo = drm_gem_vram_of_gem(fb->obj[0]);
> +	src = drm_gem_vram_vmap(gbo);
> +	if (IS_ERR(src)) {
> +		ret = PTR_ERR(src);
> +		goto err_drm_gem_vram_unpin;
> +	}
> +
> +	dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
> +	if (IS_ERR(dst)) {
> +		ret = PTR_ERR(dst);
> +		goto err_drm_gem_vram_vunmap_src;
> +	}
> +
> +	ret = ast_cursor_update(dst, src, fb->width, fb->height);
> +	if (ret)
> +		goto err_drm_gem_vram_vunmap_dst;
> +
> +	/* Always unmap buffers here. Destination buffers are
> +	 * perma-pinned while the driver is active. We're only
> +	 * changing ref-counters here.
> +	 */
> +	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
> +	drm_gem_vram_vunmap(gbo, src);
> +
> +	return 0;
> +
> +err_drm_gem_vram_vunmap_dst:
> +	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
> +err_drm_gem_vram_vunmap_src:
> +	drm_gem_vram_vunmap(gbo, src);
> +err_drm_gem_vram_unpin:
> +	drm_gem_vram_unpin(gbo);
> +	return ret;
> +}
> +
> +static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
> +						struct drm_plane_state *state)
> +{
> +	return 0;
> +}
> +
> +static void
> +ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
> +				      struct drm_plane_state *old_state)
> +{
> +	struct drm_plane_state *state = plane->state;
> +	struct drm_crtc *crtc = state->crtc;
> +	struct drm_framebuffer *fb = state->fb;
> +	struct ast_private *ast = plane->dev->dev_private;
> +	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> +	struct drm_gem_vram_object *gbo;
> +	s64 off;
> +	u8 jreg;
> +
> +	ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
> +	ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
> +
> +	if (state->fb != old_state->fb) {
> +		/* A new cursor image was installed. */
> +		gbo = ast->cursor.gbo[ast->cursor.next_index];
> +		off = drm_gem_vram_offset(gbo);
> +		if (WARN_ON_ONCE(off < 0))
> +			return; /* Bug: we didn't pin cursor HW BO to VRAM. */
> +		ast_cursor_set_base(ast, off);
> +
> +		++ast->cursor.next_index;
> +		ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
> +	}
> +
> +	ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
> +
> +	jreg = 0x2;
> +	/* enable ARGB cursor */
> +	jreg |= 1;
> +	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
> +}
> +
> +static void
> +ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
> +				       struct drm_plane_state *old_state)
> +{
> +	struct ast_private *ast = plane->dev->dev_private;
> +
> +	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
> +}
> +
> +static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
> +	.prepare_fb = ast_cursor_plane_helper_prepare_fb,
> +	.cleanup_fb = NULL, /* not required for cursor plane */
> +	.atomic_check = ast_cursor_plane_helper_atomic_check,
> +	.atomic_update = ast_cursor_plane_helper_atomic_update,
> +	.atomic_disable = ast_cursor_plane_helper_atomic_disable,
> +};
> +
> +static const struct drm_plane_funcs ast_cursor_plane_funcs = {
> +	.update_plane = drm_atomic_helper_update_plane,
> +	.disable_plane = drm_atomic_helper_disable_plane,
> +	.destroy = drm_plane_cleanup,
> +	.reset = drm_atomic_helper_plane_reset,
> +	.set_property = NULL,
> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> +	.atomic_set_property = NULL,
> +	.atomic_get_property = NULL,
> +	.late_register = NULL,
> +	.early_unregister = NULL,
> +	.atomic_print_state = NULL,
> +	.format_mod_supported = NULL,
> +};
> +
>  /*
>   * CRTC
>   */
> @@ -883,7 +1026,8 @@ static int ast_crtc_init(struct drm_device *dev)
>  		return -ENOMEM;
>  
>  	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
> -					NULL, &ast_crtc_funcs, NULL);
> +					&ast->cursor_plane, &ast_crtc_funcs,
> +					NULL);
>  	if (ret)
>  		goto err_kfree;
>  
> @@ -1153,6 +1297,9 @@ int ast_mode_init(struct drm_device *dev)
>  		DRM_FORMAT_RGB565,
>  		DRM_FORMAT_C8,
>  	};
> +	static const uint32_t cursor_plane_formats[] = {
> +		DRM_FORMAT_ARGB8888,
> +	};
>  
>  	struct ast_private *ast = dev->dev_private;
>  	int ret;
> @@ -1170,6 +1317,18 @@ int ast_mode_init(struct drm_device *dev)
>  	drm_plane_helper_add(&ast->primary_plane,
>  			     &ast_primary_plane_helper_funcs);
>  
> +	ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
> +				       &ast_cursor_plane_funcs,
> +				       cursor_plane_formats,
> +				       ARRAY_SIZE(cursor_plane_formats),
> +				       NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> +	if (ret) {
> +		DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
> +		return ret;
> +	}
> +	drm_plane_helper_add(&ast->cursor_plane,
> +			     &ast_cursor_plane_helper_funcs);
> +
>  	ast_cursor_init(dev);
>  	ast_crtc_init(dev);
>  	ast_encoder_init(dev);
> -- 
> 2.23.0
>
Thomas Zimmermann Nov. 6, 2019, 8:31 a.m. UTC | #3
Hi

Am 05.11.19 um 10:55 schrieb Daniel Vetter:
> On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
>> The cursor plane uses an internal format of ARGB4444. To userspace, we
>> announce ARGB8888 and do the transformation internally.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Hm, might be fun to also expose the ARGB4444 directly. Not that anyone
> will actually use it :-/

Is that a serious proposal? I thought about ARGB4444 and quickly
dismissed it because no one will ever support it anyway.

Best regards
Thomas

> -Daniel
> 
>> ---
>>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
>>  drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
>>  2 files changed, 161 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
>> index 13560622f22a..49557a73390f 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -122,6 +122,7 @@ struct ast_private {
>>  	} cursor;
>>  
>>  	struct drm_plane primary_plane;
>> +	struct drm_plane cursor_plane;
>>  
>>  	bool support_wide_screen;
>>  	enum {
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index 7667f4502eb9..f5f73200e8e4 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -54,6 +54,16 @@ static int ast_cursor_move(struct drm_crtc *crtc,
>>  			   int x, int y);
>>  
>>  
>> +static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
>> +static int ast_cursor_update(void *dst, void *src, unsigned int width,
>> +			     unsigned int height);
>> +static void ast_cursor_set_base(struct ast_private *ast, u64 address);
>> +static int ast_show_cursor(struct drm_crtc *crtc, void *src,
>> +			   unsigned int width, unsigned int height);
>> +static void ast_hide_cursor(struct drm_crtc *crtc);
>> +static int ast_cursor_move(struct drm_crtc *crtc,
>> +			   int x, int y);
>> +
>>  static inline void ast_load_palette_index(struct ast_private *ast,
>>  				     u8 index, u8 red, u8 green,
>>  				     u8 blue)
>> @@ -594,6 +604,139 @@ static const struct drm_plane_funcs ast_primary_plane_funcs = {
>>  	.format_mod_supported = NULL,
>>  };
>>  
>> +/*
>> + * Cursor plane
>> + */
>> +
>> +static int
>> +ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
>> +				   struct drm_plane_state *new_state)
>> +{
>> +	struct drm_framebuffer *fb = new_state->fb;
>> +	struct drm_crtc *crtc = new_state->crtc;
>> +	struct drm_gem_vram_object *gbo;
>> +	struct ast_private *ast;
>> +	int ret;
>> +	void *src, *dst;
>> +
>> +	if (!crtc || !fb)
>> +		return 0;
>> +
>> +	if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
>> +		return -EINVAL;
>> +
>> +	ast = crtc->dev->dev_private;
>> +
>> +	gbo = drm_gem_vram_of_gem(fb->obj[0]);
>> +	src = drm_gem_vram_vmap(gbo);
>> +	if (IS_ERR(src)) {
>> +		ret = PTR_ERR(src);
>> +		goto err_drm_gem_vram_unpin;
>> +	}
>> +
>> +	dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
>> +	if (IS_ERR(dst)) {
>> +		ret = PTR_ERR(dst);
>> +		goto err_drm_gem_vram_vunmap_src;
>> +	}
>> +
>> +	ret = ast_cursor_update(dst, src, fb->width, fb->height);
>> +	if (ret)
>> +		goto err_drm_gem_vram_vunmap_dst;
>> +
>> +	/* Always unmap buffers here. Destination buffers are
>> +	 * perma-pinned while the driver is active. We're only
>> +	 * changing ref-counters here.
>> +	 */
>> +	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
>> +	drm_gem_vram_vunmap(gbo, src);
>> +
>> +	return 0;
>> +
>> +err_drm_gem_vram_vunmap_dst:
>> +	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
>> +err_drm_gem_vram_vunmap_src:
>> +	drm_gem_vram_vunmap(gbo, src);
>> +err_drm_gem_vram_unpin:
>> +	drm_gem_vram_unpin(gbo);
>> +	return ret;
>> +}
>> +
>> +static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
>> +						struct drm_plane_state *state)
>> +{
>> +	return 0;
>> +}
>> +
>> +static void
>> +ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
>> +				      struct drm_plane_state *old_state)
>> +{
>> +	struct drm_plane_state *state = plane->state;
>> +	struct drm_crtc *crtc = state->crtc;
>> +	struct drm_framebuffer *fb = state->fb;
>> +	struct ast_private *ast = plane->dev->dev_private;
>> +	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
>> +	struct drm_gem_vram_object *gbo;
>> +	s64 off;
>> +	u8 jreg;
>> +
>> +	ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
>> +	ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
>> +
>> +	if (state->fb != old_state->fb) {
>> +		/* A new cursor image was installed. */
>> +		gbo = ast->cursor.gbo[ast->cursor.next_index];
>> +		off = drm_gem_vram_offset(gbo);
>> +		if (WARN_ON_ONCE(off < 0))
>> +			return; /* Bug: we didn't pin cursor HW BO to VRAM. */
>> +		ast_cursor_set_base(ast, off);
>> +
>> +		++ast->cursor.next_index;
>> +		ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
>> +	}
>> +
>> +	ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
>> +
>> +	jreg = 0x2;
>> +	/* enable ARGB cursor */
>> +	jreg |= 1;
>> +	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
>> +}
>> +
>> +static void
>> +ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
>> +				       struct drm_plane_state *old_state)
>> +{
>> +	struct ast_private *ast = plane->dev->dev_private;
>> +
>> +	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
>> +}
>> +
>> +static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
>> +	.prepare_fb = ast_cursor_plane_helper_prepare_fb,
>> +	.cleanup_fb = NULL, /* not required for cursor plane */
>> +	.atomic_check = ast_cursor_plane_helper_atomic_check,
>> +	.atomic_update = ast_cursor_plane_helper_atomic_update,
>> +	.atomic_disable = ast_cursor_plane_helper_atomic_disable,
>> +};
>> +
>> +static const struct drm_plane_funcs ast_cursor_plane_funcs = {
>> +	.update_plane = drm_atomic_helper_update_plane,
>> +	.disable_plane = drm_atomic_helper_disable_plane,
>> +	.destroy = drm_plane_cleanup,
>> +	.reset = drm_atomic_helper_plane_reset,
>> +	.set_property = NULL,
>> +	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>> +	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>> +	.atomic_set_property = NULL,
>> +	.atomic_get_property = NULL,
>> +	.late_register = NULL,
>> +	.early_unregister = NULL,
>> +	.atomic_print_state = NULL,
>> +	.format_mod_supported = NULL,
>> +};
>> +
>>  /*
>>   * CRTC
>>   */
>> @@ -883,7 +1026,8 @@ static int ast_crtc_init(struct drm_device *dev)
>>  		return -ENOMEM;
>>  
>>  	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
>> -					NULL, &ast_crtc_funcs, NULL);
>> +					&ast->cursor_plane, &ast_crtc_funcs,
>> +					NULL);
>>  	if (ret)
>>  		goto err_kfree;
>>  
>> @@ -1153,6 +1297,9 @@ int ast_mode_init(struct drm_device *dev)
>>  		DRM_FORMAT_RGB565,
>>  		DRM_FORMAT_C8,
>>  	};
>> +	static const uint32_t cursor_plane_formats[] = {
>> +		DRM_FORMAT_ARGB8888,
>> +	};
>>  
>>  	struct ast_private *ast = dev->dev_private;
>>  	int ret;
>> @@ -1170,6 +1317,18 @@ int ast_mode_init(struct drm_device *dev)
>>  	drm_plane_helper_add(&ast->primary_plane,
>>  			     &ast_primary_plane_helper_funcs);
>>  
>> +	ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
>> +				       &ast_cursor_plane_funcs,
>> +				       cursor_plane_formats,
>> +				       ARRAY_SIZE(cursor_plane_formats),
>> +				       NULL, DRM_PLANE_TYPE_CURSOR, NULL);
>> +	if (ret) {
>> +		DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
>> +		return ret;
>> +	}
>> +	drm_plane_helper_add(&ast->cursor_plane,
>> +			     &ast_cursor_plane_helper_funcs);
>> +
>>  	ast_cursor_init(dev);
>>  	ast_crtc_init(dev);
>>  	ast_encoder_init(dev);
>> -- 
>> 2.23.0
>>
>
Daniel Vetter Nov. 6, 2019, 9:05 a.m. UTC | #4
On Wed, Nov 6, 2019 at 9:31 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 05.11.19 um 10:55 schrieb Daniel Vetter:
> > On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
> >> The cursor plane uses an internal format of ARGB4444. To userspace, we
> >> announce ARGB8888 and do the transformation internally.
> >>
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >
> > Hm, might be fun to also expose the ARGB4444 directly. Not that anyone
> > will actually use it :-/
>
> Is that a serious proposal? I thought about ARGB4444 and quickly
> dismissed it because no one will ever support it anyway.

For cursor maybe not, but in other cases where we added the
RGB8888->native format hack because userspace, we did make sure that
the driver exposes the native formats too. Up to you really.
-Daniel

>
> Best regards
> Thomas
>
> > -Daniel
> >
> >> ---
> >>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
> >>  drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
> >>  2 files changed, 161 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> >> index 13560622f22a..49557a73390f 100644
> >> --- a/drivers/gpu/drm/ast/ast_drv.h
> >> +++ b/drivers/gpu/drm/ast/ast_drv.h
> >> @@ -122,6 +122,7 @@ struct ast_private {
> >>      } cursor;
> >>
> >>      struct drm_plane primary_plane;
> >> +    struct drm_plane cursor_plane;
> >>
> >>      bool support_wide_screen;
> >>      enum {
> >> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> >> index 7667f4502eb9..f5f73200e8e4 100644
> >> --- a/drivers/gpu/drm/ast/ast_mode.c
> >> +++ b/drivers/gpu/drm/ast/ast_mode.c
> >> @@ -54,6 +54,16 @@ static int ast_cursor_move(struct drm_crtc *crtc,
> >>                         int x, int y);
> >>
> >>
> >> +static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
> >> +static int ast_cursor_update(void *dst, void *src, unsigned int width,
> >> +                         unsigned int height);
> >> +static void ast_cursor_set_base(struct ast_private *ast, u64 address);
> >> +static int ast_show_cursor(struct drm_crtc *crtc, void *src,
> >> +                       unsigned int width, unsigned int height);
> >> +static void ast_hide_cursor(struct drm_crtc *crtc);
> >> +static int ast_cursor_move(struct drm_crtc *crtc,
> >> +                       int x, int y);
> >> +
> >>  static inline void ast_load_palette_index(struct ast_private *ast,
> >>                                   u8 index, u8 red, u8 green,
> >>                                   u8 blue)
> >> @@ -594,6 +604,139 @@ static const struct drm_plane_funcs ast_primary_plane_funcs = {
> >>      .format_mod_supported = NULL,
> >>  };
> >>
> >> +/*
> >> + * Cursor plane
> >> + */
> >> +
> >> +static int
> >> +ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
> >> +                               struct drm_plane_state *new_state)
> >> +{
> >> +    struct drm_framebuffer *fb = new_state->fb;
> >> +    struct drm_crtc *crtc = new_state->crtc;
> >> +    struct drm_gem_vram_object *gbo;
> >> +    struct ast_private *ast;
> >> +    int ret;
> >> +    void *src, *dst;
> >> +
> >> +    if (!crtc || !fb)
> >> +            return 0;
> >> +
> >> +    if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
> >> +            return -EINVAL;
> >> +
> >> +    ast = crtc->dev->dev_private;
> >> +
> >> +    gbo = drm_gem_vram_of_gem(fb->obj[0]);
> >> +    src = drm_gem_vram_vmap(gbo);
> >> +    if (IS_ERR(src)) {
> >> +            ret = PTR_ERR(src);
> >> +            goto err_drm_gem_vram_unpin;
> >> +    }
> >> +
> >> +    dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
> >> +    if (IS_ERR(dst)) {
> >> +            ret = PTR_ERR(dst);
> >> +            goto err_drm_gem_vram_vunmap_src;
> >> +    }
> >> +
> >> +    ret = ast_cursor_update(dst, src, fb->width, fb->height);
> >> +    if (ret)
> >> +            goto err_drm_gem_vram_vunmap_dst;
> >> +
> >> +    /* Always unmap buffers here. Destination buffers are
> >> +     * perma-pinned while the driver is active. We're only
> >> +     * changing ref-counters here.
> >> +     */
> >> +    drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
> >> +    drm_gem_vram_vunmap(gbo, src);
> >> +
> >> +    return 0;
> >> +
> >> +err_drm_gem_vram_vunmap_dst:
> >> +    drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
> >> +err_drm_gem_vram_vunmap_src:
> >> +    drm_gem_vram_vunmap(gbo, src);
> >> +err_drm_gem_vram_unpin:
> >> +    drm_gem_vram_unpin(gbo);
> >> +    return ret;
> >> +}
> >> +
> >> +static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
> >> +                                            struct drm_plane_state *state)
> >> +{
> >> +    return 0;
> >> +}
> >> +
> >> +static void
> >> +ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
> >> +                                  struct drm_plane_state *old_state)
> >> +{
> >> +    struct drm_plane_state *state = plane->state;
> >> +    struct drm_crtc *crtc = state->crtc;
> >> +    struct drm_framebuffer *fb = state->fb;
> >> +    struct ast_private *ast = plane->dev->dev_private;
> >> +    struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> >> +    struct drm_gem_vram_object *gbo;
> >> +    s64 off;
> >> +    u8 jreg;
> >> +
> >> +    ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
> >> +    ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
> >> +
> >> +    if (state->fb != old_state->fb) {
> >> +            /* A new cursor image was installed. */
> >> +            gbo = ast->cursor.gbo[ast->cursor.next_index];
> >> +            off = drm_gem_vram_offset(gbo);
> >> +            if (WARN_ON_ONCE(off < 0))
> >> +                    return; /* Bug: we didn't pin cursor HW BO to VRAM. */
> >> +            ast_cursor_set_base(ast, off);
> >> +
> >> +            ++ast->cursor.next_index;
> >> +            ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
> >> +    }
> >> +
> >> +    ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
> >> +
> >> +    jreg = 0x2;
> >> +    /* enable ARGB cursor */
> >> +    jreg |= 1;
> >> +    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
> >> +}
> >> +
> >> +static void
> >> +ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
> >> +                                   struct drm_plane_state *old_state)
> >> +{
> >> +    struct ast_private *ast = plane->dev->dev_private;
> >> +
> >> +    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
> >> +}
> >> +
> >> +static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
> >> +    .prepare_fb = ast_cursor_plane_helper_prepare_fb,
> >> +    .cleanup_fb = NULL, /* not required for cursor plane */
> >> +    .atomic_check = ast_cursor_plane_helper_atomic_check,
> >> +    .atomic_update = ast_cursor_plane_helper_atomic_update,
> >> +    .atomic_disable = ast_cursor_plane_helper_atomic_disable,
> >> +};
> >> +
> >> +static const struct drm_plane_funcs ast_cursor_plane_funcs = {
> >> +    .update_plane = drm_atomic_helper_update_plane,
> >> +    .disable_plane = drm_atomic_helper_disable_plane,
> >> +    .destroy = drm_plane_cleanup,
> >> +    .reset = drm_atomic_helper_plane_reset,
> >> +    .set_property = NULL,
> >> +    .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> >> +    .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
> >> +    .atomic_set_property = NULL,
> >> +    .atomic_get_property = NULL,
> >> +    .late_register = NULL,
> >> +    .early_unregister = NULL,
> >> +    .atomic_print_state = NULL,
> >> +    .format_mod_supported = NULL,
> >> +};
> >> +
> >>  /*
> >>   * CRTC
> >>   */
> >> @@ -883,7 +1026,8 @@ static int ast_crtc_init(struct drm_device *dev)
> >>              return -ENOMEM;
> >>
> >>      ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
> >> -                                    NULL, &ast_crtc_funcs, NULL);
> >> +                                    &ast->cursor_plane, &ast_crtc_funcs,
> >> +                                    NULL);
> >>      if (ret)
> >>              goto err_kfree;
> >>
> >> @@ -1153,6 +1297,9 @@ int ast_mode_init(struct drm_device *dev)
> >>              DRM_FORMAT_RGB565,
> >>              DRM_FORMAT_C8,
> >>      };
> >> +    static const uint32_t cursor_plane_formats[] = {
> >> +            DRM_FORMAT_ARGB8888,
> >> +    };
> >>
> >>      struct ast_private *ast = dev->dev_private;
> >>      int ret;
> >> @@ -1170,6 +1317,18 @@ int ast_mode_init(struct drm_device *dev)
> >>      drm_plane_helper_add(&ast->primary_plane,
> >>                           &ast_primary_plane_helper_funcs);
> >>
> >> +    ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
> >> +                                   &ast_cursor_plane_funcs,
> >> +                                   cursor_plane_formats,
> >> +                                   ARRAY_SIZE(cursor_plane_formats),
> >> +                                   NULL, DRM_PLANE_TYPE_CURSOR, NULL);
> >> +    if (ret) {
> >> +            DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
> >> +            return ret;
> >> +    }
> >> +    drm_plane_helper_add(&ast->cursor_plane,
> >> +                         &ast_cursor_plane_helper_funcs);
> >> +
> >>      ast_cursor_init(dev);
> >>      ast_crtc_init(dev);
> >>      ast_encoder_init(dev);
> >> --
> >> 2.23.0
> >>
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
Thomas Zimmermann Nov. 6, 2019, 9:46 a.m. UTC | #5
Hi

Am 06.11.19 um 10:05 schrieb Daniel Vetter:
> On Wed, Nov 6, 2019 at 9:31 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 05.11.19 um 10:55 schrieb Daniel Vetter:
>>> On Mon, Oct 28, 2019 at 04:49:27PM +0100, Thomas Zimmermann wrote:
>>>> The cursor plane uses an internal format of ARGB4444. To userspace, we
>>>> announce ARGB8888 and do the transformation internally.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>
>>> Hm, might be fun to also expose the ARGB4444 directly. Not that anyone
>>> will actually use it :-/
>>
>> Is that a serious proposal? I thought about ARGB4444 and quickly
>> dismissed it because no one will ever support it anyway.
> 
> For cursor maybe not, but in other cases where we added the
> RGB8888->native format hack because userspace, we did make sure that
> the driver exposes the native formats too. Up to you really.

I see. In that case, I'd like to continue with the patchset as-is, but
provide ARGB4444 in a later patch.

Best regards
Thomas

> -Daniel
> 
>>
>> Best regards
>> Thomas
>>
>>> -Daniel
>>>
>>>> ---
>>>>  drivers/gpu/drm/ast/ast_drv.h  |   1 +
>>>>  drivers/gpu/drm/ast/ast_mode.c | 161 ++++++++++++++++++++++++++++++++-
>>>>  2 files changed, 161 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
>>>> index 13560622f22a..49557a73390f 100644
>>>> --- a/drivers/gpu/drm/ast/ast_drv.h
>>>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>>>> @@ -122,6 +122,7 @@ struct ast_private {
>>>>      } cursor;
>>>>
>>>>      struct drm_plane primary_plane;
>>>> +    struct drm_plane cursor_plane;
>>>>
>>>>      bool support_wide_screen;
>>>>      enum {
>>>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>>>> index 7667f4502eb9..f5f73200e8e4 100644
>>>> --- a/drivers/gpu/drm/ast/ast_mode.c
>>>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>>>> @@ -54,6 +54,16 @@ static int ast_cursor_move(struct drm_crtc *crtc,
>>>>                         int x, int y);
>>>>
>>>>
>>>> +static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
>>>> +static int ast_cursor_update(void *dst, void *src, unsigned int width,
>>>> +                         unsigned int height);
>>>> +static void ast_cursor_set_base(struct ast_private *ast, u64 address);
>>>> +static int ast_show_cursor(struct drm_crtc *crtc, void *src,
>>>> +                       unsigned int width, unsigned int height);
>>>> +static void ast_hide_cursor(struct drm_crtc *crtc);
>>>> +static int ast_cursor_move(struct drm_crtc *crtc,
>>>> +                       int x, int y);
>>>> +
>>>>  static inline void ast_load_palette_index(struct ast_private *ast,
>>>>                                   u8 index, u8 red, u8 green,
>>>>                                   u8 blue)
>>>> @@ -594,6 +604,139 @@ static const struct drm_plane_funcs ast_primary_plane_funcs = {
>>>>      .format_mod_supported = NULL,
>>>>  };
>>>>
>>>> +/*
>>>> + * Cursor plane
>>>> + */
>>>> +
>>>> +static int
>>>> +ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
>>>> +                               struct drm_plane_state *new_state)
>>>> +{
>>>> +    struct drm_framebuffer *fb = new_state->fb;
>>>> +    struct drm_crtc *crtc = new_state->crtc;
>>>> +    struct drm_gem_vram_object *gbo;
>>>> +    struct ast_private *ast;
>>>> +    int ret;
>>>> +    void *src, *dst;
>>>> +
>>>> +    if (!crtc || !fb)
>>>> +            return 0;
>>>> +
>>>> +    if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
>>>> +            return -EINVAL;
>>>> +
>>>> +    ast = crtc->dev->dev_private;
>>>> +
>>>> +    gbo = drm_gem_vram_of_gem(fb->obj[0]);
>>>> +    src = drm_gem_vram_vmap(gbo);
>>>> +    if (IS_ERR(src)) {
>>>> +            ret = PTR_ERR(src);
>>>> +            goto err_drm_gem_vram_unpin;
>>>> +    }
>>>> +
>>>> +    dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
>>>> +    if (IS_ERR(dst)) {
>>>> +            ret = PTR_ERR(dst);
>>>> +            goto err_drm_gem_vram_vunmap_src;
>>>> +    }
>>>> +
>>>> +    ret = ast_cursor_update(dst, src, fb->width, fb->height);
>>>> +    if (ret)
>>>> +            goto err_drm_gem_vram_vunmap_dst;
>>>> +
>>>> +    /* Always unmap buffers here. Destination buffers are
>>>> +     * perma-pinned while the driver is active. We're only
>>>> +     * changing ref-counters here.
>>>> +     */
>>>> +    drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
>>>> +    drm_gem_vram_vunmap(gbo, src);
>>>> +
>>>> +    return 0;
>>>> +
>>>> +err_drm_gem_vram_vunmap_dst:
>>>> +    drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
>>>> +err_drm_gem_vram_vunmap_src:
>>>> +    drm_gem_vram_vunmap(gbo, src);
>>>> +err_drm_gem_vram_unpin:
>>>> +    drm_gem_vram_unpin(gbo);
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
>>>> +                                            struct drm_plane_state *state)
>>>> +{
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void
>>>> +ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
>>>> +                                  struct drm_plane_state *old_state)
>>>> +{
>>>> +    struct drm_plane_state *state = plane->state;
>>>> +    struct drm_crtc *crtc = state->crtc;
>>>> +    struct drm_framebuffer *fb = state->fb;
>>>> +    struct ast_private *ast = plane->dev->dev_private;
>>>> +    struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
>>>> +    struct drm_gem_vram_object *gbo;
>>>> +    s64 off;
>>>> +    u8 jreg;
>>>> +
>>>> +    ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
>>>> +    ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
>>>> +
>>>> +    if (state->fb != old_state->fb) {
>>>> +            /* A new cursor image was installed. */
>>>> +            gbo = ast->cursor.gbo[ast->cursor.next_index];
>>>> +            off = drm_gem_vram_offset(gbo);
>>>> +            if (WARN_ON_ONCE(off < 0))
>>>> +                    return; /* Bug: we didn't pin cursor HW BO to VRAM. */
>>>> +            ast_cursor_set_base(ast, off);
>>>> +
>>>> +            ++ast->cursor.next_index;
>>>> +            ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
>>>> +    }
>>>> +
>>>> +    ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
>>>> +
>>>> +    jreg = 0x2;
>>>> +    /* enable ARGB cursor */
>>>> +    jreg |= 1;
>>>> +    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
>>>> +}
>>>> +
>>>> +static void
>>>> +ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
>>>> +                                   struct drm_plane_state *old_state)
>>>> +{
>>>> +    struct ast_private *ast = plane->dev->dev_private;
>>>> +
>>>> +    ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
>>>> +}
>>>> +
>>>> +static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
>>>> +    .prepare_fb = ast_cursor_plane_helper_prepare_fb,
>>>> +    .cleanup_fb = NULL, /* not required for cursor plane */
>>>> +    .atomic_check = ast_cursor_plane_helper_atomic_check,
>>>> +    .atomic_update = ast_cursor_plane_helper_atomic_update,
>>>> +    .atomic_disable = ast_cursor_plane_helper_atomic_disable,
>>>> +};
>>>> +
>>>> +static const struct drm_plane_funcs ast_cursor_plane_funcs = {
>>>> +    .update_plane = drm_atomic_helper_update_plane,
>>>> +    .disable_plane = drm_atomic_helper_disable_plane,
>>>> +    .destroy = drm_plane_cleanup,
>>>> +    .reset = drm_atomic_helper_plane_reset,
>>>> +    .set_property = NULL,
>>>> +    .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>>>> +    .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>>>> +    .atomic_set_property = NULL,
>>>> +    .atomic_get_property = NULL,
>>>> +    .late_register = NULL,
>>>> +    .early_unregister = NULL,
>>>> +    .atomic_print_state = NULL,
>>>> +    .format_mod_supported = NULL,
>>>> +};
>>>> +
>>>>  /*
>>>>   * CRTC
>>>>   */
>>>> @@ -883,7 +1026,8 @@ static int ast_crtc_init(struct drm_device *dev)
>>>>              return -ENOMEM;
>>>>
>>>>      ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
>>>> -                                    NULL, &ast_crtc_funcs, NULL);
>>>> +                                    &ast->cursor_plane, &ast_crtc_funcs,
>>>> +                                    NULL);
>>>>      if (ret)
>>>>              goto err_kfree;
>>>>
>>>> @@ -1153,6 +1297,9 @@ int ast_mode_init(struct drm_device *dev)
>>>>              DRM_FORMAT_RGB565,
>>>>              DRM_FORMAT_C8,
>>>>      };
>>>> +    static const uint32_t cursor_plane_formats[] = {
>>>> +            DRM_FORMAT_ARGB8888,
>>>> +    };
>>>>
>>>>      struct ast_private *ast = dev->dev_private;
>>>>      int ret;
>>>> @@ -1170,6 +1317,18 @@ int ast_mode_init(struct drm_device *dev)
>>>>      drm_plane_helper_add(&ast->primary_plane,
>>>>                           &ast_primary_plane_helper_funcs);
>>>>
>>>> +    ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
>>>> +                                   &ast_cursor_plane_funcs,
>>>> +                                   cursor_plane_formats,
>>>> +                                   ARRAY_SIZE(cursor_plane_formats),
>>>> +                                   NULL, DRM_PLANE_TYPE_CURSOR, NULL);
>>>> +    if (ret) {
>>>> +            DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
>>>> +            return ret;
>>>> +    }
>>>> +    drm_plane_helper_add(&ast->cursor_plane,
>>>> +                         &ast_cursor_plane_helper_funcs);
>>>> +
>>>>      ast_cursor_init(dev);
>>>>      ast_crtc_init(dev);
>>>>      ast_encoder_init(dev);
>>>> --
>>>> 2.23.0
>>>>
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
> 
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 13560622f22a..49557a73390f 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -122,6 +122,7 @@  struct ast_private {
 	} cursor;
 
 	struct drm_plane primary_plane;
+	struct drm_plane cursor_plane;
 
 	bool support_wide_screen;
 	enum {
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 7667f4502eb9..f5f73200e8e4 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -54,6 +54,16 @@  static int ast_cursor_move(struct drm_crtc *crtc,
 			   int x, int y);
 
 
+static u32 copy_cursor_image(u8 *src, u8 *dst, int width, int height);
+static int ast_cursor_update(void *dst, void *src, unsigned int width,
+			     unsigned int height);
+static void ast_cursor_set_base(struct ast_private *ast, u64 address);
+static int ast_show_cursor(struct drm_crtc *crtc, void *src,
+			   unsigned int width, unsigned int height);
+static void ast_hide_cursor(struct drm_crtc *crtc);
+static int ast_cursor_move(struct drm_crtc *crtc,
+			   int x, int y);
+
 static inline void ast_load_palette_index(struct ast_private *ast,
 				     u8 index, u8 red, u8 green,
 				     u8 blue)
@@ -594,6 +604,139 @@  static const struct drm_plane_funcs ast_primary_plane_funcs = {
 	.format_mod_supported = NULL,
 };
 
+/*
+ * Cursor plane
+ */
+
+static int
+ast_cursor_plane_helper_prepare_fb(struct drm_plane *plane,
+				   struct drm_plane_state *new_state)
+{
+	struct drm_framebuffer *fb = new_state->fb;
+	struct drm_crtc *crtc = new_state->crtc;
+	struct drm_gem_vram_object *gbo;
+	struct ast_private *ast;
+	int ret;
+	void *src, *dst;
+
+	if (!crtc || !fb)
+		return 0;
+
+	if (fb->width > AST_MAX_HWC_WIDTH || fb->height > AST_MAX_HWC_HEIGHT)
+		return -EINVAL;
+
+	ast = crtc->dev->dev_private;
+
+	gbo = drm_gem_vram_of_gem(fb->obj[0]);
+	src = drm_gem_vram_vmap(gbo);
+	if (IS_ERR(src)) {
+		ret = PTR_ERR(src);
+		goto err_drm_gem_vram_unpin;
+	}
+
+	dst = drm_gem_vram_vmap(ast->cursor.gbo[ast->cursor.next_index]);
+	if (IS_ERR(dst)) {
+		ret = PTR_ERR(dst);
+		goto err_drm_gem_vram_vunmap_src;
+	}
+
+	ret = ast_cursor_update(dst, src, fb->width, fb->height);
+	if (ret)
+		goto err_drm_gem_vram_vunmap_dst;
+
+	/* Always unmap buffers here. Destination buffers are
+	 * perma-pinned while the driver is active. We're only
+	 * changing ref-counters here.
+	 */
+	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
+	drm_gem_vram_vunmap(gbo, src);
+
+	return 0;
+
+err_drm_gem_vram_vunmap_dst:
+	drm_gem_vram_vunmap(ast->cursor.gbo[ast->cursor.next_index], dst);
+err_drm_gem_vram_vunmap_src:
+	drm_gem_vram_vunmap(gbo, src);
+err_drm_gem_vram_unpin:
+	drm_gem_vram_unpin(gbo);
+	return ret;
+}
+
+static int ast_cursor_plane_helper_atomic_check(struct drm_plane *plane,
+						struct drm_plane_state *state)
+{
+	return 0;
+}
+
+static void
+ast_cursor_plane_helper_atomic_update(struct drm_plane *plane,
+				      struct drm_plane_state *old_state)
+{
+	struct drm_plane_state *state = plane->state;
+	struct drm_crtc *crtc = state->crtc;
+	struct drm_framebuffer *fb = state->fb;
+	struct ast_private *ast = plane->dev->dev_private;
+	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
+	struct drm_gem_vram_object *gbo;
+	s64 off;
+	u8 jreg;
+
+	ast_crtc->offset_x = AST_MAX_HWC_WIDTH - fb->width;
+	ast_crtc->offset_y = AST_MAX_HWC_WIDTH - fb->height;
+
+	if (state->fb != old_state->fb) {
+		/* A new cursor image was installed. */
+		gbo = ast->cursor.gbo[ast->cursor.next_index];
+		off = drm_gem_vram_offset(gbo);
+		if (WARN_ON_ONCE(off < 0))
+			return; /* Bug: we didn't pin cursor HW BO to VRAM. */
+		ast_cursor_set_base(ast, off);
+
+		++ast->cursor.next_index;
+		ast->cursor.next_index %= ARRAY_SIZE(ast->cursor.gbo);
+	}
+
+	ast_cursor_move(crtc, state->crtc_x, state->crtc_y);
+
+	jreg = 0x2;
+	/* enable ARGB cursor */
+	jreg |= 1;
+	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, jreg);
+}
+
+static void
+ast_cursor_plane_helper_atomic_disable(struct drm_plane *plane,
+				       struct drm_plane_state *old_state)
+{
+	struct ast_private *ast = plane->dev->dev_private;
+
+	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xcb, 0xfc, 0x00);
+}
+
+static const struct drm_plane_helper_funcs ast_cursor_plane_helper_funcs = {
+	.prepare_fb = ast_cursor_plane_helper_prepare_fb,
+	.cleanup_fb = NULL, /* not required for cursor plane */
+	.atomic_check = ast_cursor_plane_helper_atomic_check,
+	.atomic_update = ast_cursor_plane_helper_atomic_update,
+	.atomic_disable = ast_cursor_plane_helper_atomic_disable,
+};
+
+static const struct drm_plane_funcs ast_cursor_plane_funcs = {
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.destroy = drm_plane_cleanup,
+	.reset = drm_atomic_helper_plane_reset,
+	.set_property = NULL,
+	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+	.atomic_set_property = NULL,
+	.atomic_get_property = NULL,
+	.late_register = NULL,
+	.early_unregister = NULL,
+	.atomic_print_state = NULL,
+	.format_mod_supported = NULL,
+};
+
 /*
  * CRTC
  */
@@ -883,7 +1026,8 @@  static int ast_crtc_init(struct drm_device *dev)
 		return -ENOMEM;
 
 	ret = drm_crtc_init_with_planes(dev, &crtc->base, &ast->primary_plane,
-					NULL, &ast_crtc_funcs, NULL);
+					&ast->cursor_plane, &ast_crtc_funcs,
+					NULL);
 	if (ret)
 		goto err_kfree;
 
@@ -1153,6 +1297,9 @@  int ast_mode_init(struct drm_device *dev)
 		DRM_FORMAT_RGB565,
 		DRM_FORMAT_C8,
 	};
+	static const uint32_t cursor_plane_formats[] = {
+		DRM_FORMAT_ARGB8888,
+	};
 
 	struct ast_private *ast = dev->dev_private;
 	int ret;
@@ -1170,6 +1317,18 @@  int ast_mode_init(struct drm_device *dev)
 	drm_plane_helper_add(&ast->primary_plane,
 			     &ast_primary_plane_helper_funcs);
 
+	ret = drm_universal_plane_init(dev, &ast->cursor_plane, 0x01,
+				       &ast_cursor_plane_funcs,
+				       cursor_plane_formats,
+				       ARRAY_SIZE(cursor_plane_formats),
+				       NULL, DRM_PLANE_TYPE_CURSOR, NULL);
+	if (ret) {
+		DRM_ERROR("drm_universal_plane_failed(): %d\n", ret);
+		return ret;
+	}
+	drm_plane_helper_add(&ast->cursor_plane,
+			     &ast_cursor_plane_helper_funcs);
+
 	ast_cursor_init(dev);
 	ast_crtc_init(dev);
 	ast_encoder_init(dev);