Re: [PATCH v4 3/7] drm/format-helper: Pass format-conversion state to helpers

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3968 bytes --]

Hi

Am 05.10.23 um 13:10 schrieb Noralf Trønnes:
> 
> 
> On 10/5/23 11:04, Thomas Zimmermann wrote:
>> Pass an instance of struct drm_format_conv_state to DRM's format
>> conversion helpers. Update all callers.
>>
>> Most drivers can use the format-conversion state from their shadow-
>> plane state. The shadow plane's destroy function releases the
>> allocated buffer. Drivers will later be able to allocate a buffer
>> of appropriate size in their plane's atomic_check code.
>>
>> The gud driver uses a separate thread for committing updates. For
>> now, the update worker contains its own format-conversion state.
>>
>> Images in the format-helper tests are small. The tests preallocate
>> a static page for the temporary buffer. Unloading the module releases
>> the memory.
>>
>> v3:
>> 	* store buffer in shadow-plane state (Javier, Maxime)
>> 	* replace ARRAY_SIZE() with sizeof() (Jani)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Noralf Trønnes <noralf@tronnes.org>
>> Cc: Javier Martinez Canillas <javierm@redhat.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: David Lechner <david@lechnology.com>
>> ---
> 
>> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
>> index 37c499ae4fe4f..b9b3dadf7b5f8 100644
>> --- a/drivers/gpu/drm/drm_format_helper.c
>> +++ b/drivers/gpu/drm/drm_format_helper.c
> 
>> @@ -328,6 +324,7 @@ static void drm_fb_swab32_line(void *dbuf, const void *sbuf, unsigned int pixels
>>    * @fb: DRM framebuffer
>>    * @clip: Clip rectangle area to copy
>>    * @cached: Source buffer is mapped cached (eg. not write-combined)
>> + * @xfrm: Transform and conversion state
> 
> Here and throughout the patch: xfrm does not match the argument name.

Thanks! I should be more careful when renaming.

> 
>>    *
>>    * This function copies parts of a framebuffer to display memory and swaps per-pixel
>>    * bytes during the process. Destination and framebuffer formats must match. The
>> @@ -342,7 +339,8 @@ static void drm_fb_swab32_line(void *dbuf, const void *sbuf, unsigned int pixels
>>    */
>>   void drm_fb_swab(struct iosys_map *dst, const unsigned int *dst_pitch,
>>   		 const struct iosys_map *src, const struct drm_framebuffer *fb,
>> -		 const struct drm_rect *clip, bool cached)
>> +		 const struct drm_rect *clip, bool cached,
>> +		 struct drm_format_conv_state *state)
>>   {
>>   	const struct drm_format_info *format = fb->format;
>>   	u8 cpp = DIV_ROUND_UP(drm_format_info_bpp(format, 0), 8);
> 
> 
>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
>> index 73dd4f4289c20..826fb20dbbf0d 100644
>> --- a/drivers/gpu/drm/tiny/repaper.c
>> +++ b/drivers/gpu/drm/tiny/repaper.c
> 
>> @@ -830,13 +831,14 @@ static void repaper_pipe_update(struct drm_simple_display_pipe *pipe,
>>   				struct drm_plane_state *old_state)
>>   {
>>   	struct drm_plane_state *state = pipe->plane.state;
>> +	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
>>   	struct drm_rect rect;
>>   
>>   	if (!pipe->crtc.state->active)
>>   		return;
>>   
>>   	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
>> -		repaper_fb_dirty(state->fb);
>> +		repaper_fb_dirty(state->fb, &shadow_plane_state->fmtcnv_state);
> 
> This won't work since repaper doesn't use the shadow plane helper.

Indeed. It can use a local buffer. But I'm a bit surprised to find that 
there are no shadow-plane helpers here. That sounds like a useful 
follow-up patch.

Best regards
Thomas

> 
> Noralf.
> 
>>   }
>>   
>>   static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]