[PATCH 2/3] drm/ast: Store image size in HW cursor info

Thomas Zimmermann tzimmermann at suse.de
Mon Jul 27 11:37:39 UTC 2020


Hi

Am 27.07.20 um 12:42 schrieb daniel at ffwll.ch:
> On Mon, Jul 27, 2020 at 09:37:06AM +0200, Thomas Zimmermann wrote:
>> Store the image size as part of the HW cursor info, so that the
>> cursor show function doesn't require the information from the
>> caller. No functional changes.
> 
> Uh just pass the state structure and done? All these "store random stuff
> in private structures" (they're not even atomic state structures, it's the
> driver private thing even!) is very non-atomic. And I see zero reasons why
> you have to do this, the cursor stays around.

It's not random stuff. Ast cannot use ARGB8888 for cursors. Anything in
ast_private.cursor represents cursor hardware state (not DRM state);
duplicated for double buffering.

 * gbo: two perma-pinned GEM objects at the end of VRAM. It's the HW
cursor buffer in ARGB4444 format. The userspace's cursor image is
converted to ARGB4444 and copied into the current backbuffer.

 * vaddr: A mapping of the gbo's into kernel address space. We don't
want to map the gbo on each update, so they are mapped once and the
kernel address is stored in vaddr.

 * size: the size of each HW buffer. We could use the value in the fb,
but storing this as well makes the cursor code self-contained.

Best regards
Thomas

> -Daniel
> 
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> Fixes: 4961eb60f145 ("drm/ast: Enable atomic modesetting")
>> Cc: Thomas Zimmermann <tzimmermann at suse.de>
>> Cc: Gerd Hoffmann <kraxel at redhat.com>
>> Cc: Dave Airlie <airlied at redhat.com>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Cc: Sam Ravnborg <sam at ravnborg.org>
>> Cc: Emil Velikov <emil.l.velikov at gmail.com>
>> Cc: "Y.C. Chen" <yc_chen at aspeedtech.com>
>> Cc: <stable at vger.kernel.org> # v5.6+
>> ---
>>  drivers/gpu/drm/ast/ast_cursor.c | 13 +++++++++++--
>>  drivers/gpu/drm/ast/ast_drv.h    |  7 +++++--
>>  drivers/gpu/drm/ast/ast_mode.c   |  8 +-------
>>  3 files changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_cursor.c b/drivers/gpu/drm/ast/ast_cursor.c
>> index acf0d23514e8..8642a0ce9da6 100644
>> --- a/drivers/gpu/drm/ast/ast_cursor.c
>> +++ b/drivers/gpu/drm/ast/ast_cursor.c
>> @@ -87,6 +87,8 @@ int ast_cursor_init(struct ast_private *ast)
>>  
>>  		ast->cursor.gbo[i] = gbo;
>>  		ast->cursor.vaddr[i] = vaddr;
>> +		ast->cursor.size[i].width = 0;
>> +		ast->cursor.size[i].height = 0;
>>  	}
>>  
>>  	return drmm_add_action_or_reset(dev, ast_cursor_release, NULL);
>> @@ -194,6 +196,9 @@ int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb)
>>  	/* do data transfer to cursor BO */
>>  	update_cursor_image(dst, src, fb->width, fb->height);
>>  
>> +	ast->cursor.size[ast->cursor.next_index].width = fb->width;
>> +	ast->cursor.size[ast->cursor.next_index].height = fb->height;
>> +
>>  	drm_gem_vram_vunmap(gbo, src);
>>  	drm_gem_vram_unpin(gbo);
>>  
>> @@ -249,14 +254,18 @@ static void ast_cursor_set_location(struct ast_private *ast, u16 x, u16 y,
>>  	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xc7, y1);
>>  }
>>  
>> -void ast_cursor_show(struct ast_private *ast, int x, int y,
>> -		     unsigned int offset_x, unsigned int offset_y)
>> +void ast_cursor_show(struct ast_private *ast, int x, int y)
>>  {
>> +	unsigned int offset_x, offset_y;
>>  	u8 x_offset, y_offset;
>>  	u8 __iomem *dst, __iomem *sig;
>>  	u8 jreg;
>>  
>>  	dst = ast->cursor.vaddr[ast->cursor.next_index];
>> +	offset_x = AST_MAX_HWC_WIDTH -
>> +		   ast->cursor.size[ast->cursor.next_index].width;
>> +	offset_y = AST_MAX_HWC_HEIGHT -
>> +		   ast->cursor.size[ast->cursor.next_index].height;
>>  
>>  	sig = dst + AST_HWC_SIZE;
>>  	writel(x, sig + AST_HWC_SIGNATURE_X);
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
>> index e3a264ac7ee2..57414b429db3 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -116,6 +116,10 @@ struct ast_private {
>>  	struct {
>>  		struct drm_gem_vram_object *gbo[AST_DEFAULT_HWC_NUM];
>>  		void __iomem *vaddr[AST_DEFAULT_HWC_NUM];
>> +		struct {
>> +			unsigned int width;
>> +			unsigned int height;
>> +		} size[AST_DEFAULT_HWC_NUM];
>>  		unsigned int next_index;
>>  	} cursor;
>>  
>> @@ -311,8 +315,7 @@ void ast_release_firmware(struct drm_device *dev);
>>  int ast_cursor_init(struct ast_private *ast);
>>  int ast_cursor_blit(struct ast_private *ast, struct drm_framebuffer *fb);
>>  void ast_cursor_page_flip(struct ast_private *ast);
>> -void ast_cursor_show(struct ast_private *ast, int x, int y,
>> -		     unsigned int offset_x, unsigned int offset_y);
>> +void ast_cursor_show(struct ast_private *ast, int x, int y);
>>  void ast_cursor_hide(struct ast_private *ast);
>>  
>>  #endif
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index 3680a000b812..5b2b39c93033 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -671,20 +671,14 @@ 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_framebuffer *fb = state->fb;
>>  	struct ast_private *ast = plane->dev->dev_private;
>> -	unsigned int offset_x, offset_y;
>> -
>> -	offset_x = AST_MAX_HWC_WIDTH - fb->width;
>> -	offset_y = AST_MAX_HWC_WIDTH - fb->height;
>>  
>>  	if (state->fb != old_state->fb) {
>>  		/* A new cursor image was installed. */
>>  		ast_cursor_page_flip(ast);
>>  	}
>>  
>> -	ast_cursor_show(ast, state->crtc_x, state->crtc_y,
>> -			offset_x, offset_y);
>> +	ast_cursor_show(ast, state->crtc_x, state->crtc_y);
>>  }
>>  
>>  static void
>> -- 
>> 2.27.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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 516 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20200727/9478e9e8/attachment.sig>


More information about the dri-devel mailing list