Re: [PATCH 04/10] drm/client: Add drm_client_framebuffer_flush()

[Noralf Trønnes]

Den 03.05.2020 09.48, skrev Sam Ravnborg:
> Hi Noralf.
> 
> On Wed, Apr 29, 2020 at 02:48:24PM +0200, Noralf Trønnes wrote:
>> Some drivers need explicit flushing of buffer changes, add a function
>> that does that.
> I trust you on this.

All drivers in tiny/ and at least udl need to be told to flush changes.
For userspace this happens either by calling DRM_IOCTL_MODE_DIRTYFB or
doing a page/buffer flip DRM_IOCTL_MODE_PAGE_FLIP or do a
DRM_IOCTL_MODE_ATOMIC (can contain damage report using plane property
FB_DAMAGE_CLIPS). For drivers that use drm_gem_fb_create_with_dirty()
and the drm_damage_helper (all of them now I think) this will result in
an atomic commit. The driver can use drm_atomic_helper_damage_merged()
to get the combined damage rectangle.

Noralf.

> 
>>
>> Signed-off-by: Noralf Trønnes 
> Some bikeshedding below. Either way:
> Reviewed-by: Sam Ravnborg 
> 
>> ---
>>  drivers/gpu/drm/drm_client.c | 31 +++
>>  include/drm/drm_client.h |  1 +
>>  2 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
>> index cb5ee9f1ffaa..8dbc2ecdcaea 100644
>> --- a/drivers/gpu/drm/drm_client.c
>> +++ b/drivers/gpu/drm/drm_client.c
>> @@ -483,6 +483,37 @@ void drm_client_framebuffer_delete(struct 
>> drm_client_buffer *buffer)
>>  }
>>  EXPORT_SYMBOL(drm_client_framebuffer_delete);
>>  
>> +/**
>> + * drm_client_framebuffer_flush - Manually flush client framebuffer
>> + * @buffer: DRM client buffer (can be NULL)
>> + * @rect: Damage rectangle (if NULL flushes all)
>> + *
>> + * This calls _framebuffer_funcs->dirty (if present) to flush buffer 
>> changes
>> + * for drivers that need it.
>> + *
>> + * Returns:
>> + * Zero on success or negative error code on failure.
>> + */
> 
> Alternative proposal - that I think is simpler.
> But both variants works for me.
>> +int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
>> drm_rect *rect)
>> +{
>   struct drm_framebuffer_funcs *funcs;
>   struct drm_clip_rect clip;
>> +
>> +if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
>> +return 0;
>   funcs = buffer->fb->funcs;
>> +
>> +if (rect) {
>> +clip.x1 = rect->x1;
>> +clip.y1 = rect->y1;
>> +clip.x2 = rect->x2;
>> +clip.y2 = rect->y2;
>   return funcs->dirty(buffer->fb, buffer->client->file,
>   0, 0, , 1);
>> +} else {
>   return funcs->dirty(buffer->fb, buffer->client->file,
>   0, 0, NULL, 0);
>   }
> 
>> +
>> +return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
>> +0, 0, clip, clip ? 1 : 0);
>> +}
>> +EXPORT_SYMBOL(drm_client_framebuffer_flush);
>> +
>>  #ifdef CONFIG_DEBUG_FS
>>  static int drm_client_debugfs_internal_clients(struct seq_file *m, void 
>> *data)
>>  {
>> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
>> index bbb5689fa9a8..6ef5364d6dfb 100644
>> --- a/include/drm/drm_client.h
>> +++ b/include/drm/drm_client.h
>> @@ -156,6 +156,7 @@ struct drm_client_buffer {
>>  struct drm_client_buffer *
>>  drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 
>> height, u32 format);
>>  void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
>> +int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
>> drm_rect *rect);
>>  void *drm_client_buffer_vmap(struct drm_client_buffer *buffer);
>>  void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
>>  
>> -- 
>> 2.23.0
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/10] drm/client: Add drm_client_framebuffer_flush()

[Sam Ravnborg]

Hi Noralf.

On Wed, Apr 29, 2020 at 02:48:24PM +0200, Noralf Trønnes wrote:
> Some drivers need explicit flushing of buffer changes, add a function
> that does that.
I trust you on this.

> 
> Signed-off-by: Noralf Trønnes 
Some bikeshedding below. Either way:
Reviewed-by: Sam Ravnborg 

> ---
>  drivers/gpu/drm/drm_client.c | 31 +++
>  include/drm/drm_client.h |  1 +
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index cb5ee9f1ffaa..8dbc2ecdcaea 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -483,6 +483,37 @@ void drm_client_framebuffer_delete(struct 
> drm_client_buffer *buffer)
>  }
>  EXPORT_SYMBOL(drm_client_framebuffer_delete);
>  
> +/**
> + * drm_client_framebuffer_flush - Manually flush client framebuffer
> + * @buffer: DRM client buffer (can be NULL)
> + * @rect: Damage rectangle (if NULL flushes all)
> + *
> + * This calls _framebuffer_funcs->dirty (if present) to flush buffer 
> changes
> + * for drivers that need it.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */

Alternative proposal - that I think is simpler.
But both variants works for me.
> +int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
> drm_rect *rect)
> +{
struct drm_framebuffer_funcs *funcs;
struct drm_clip_rect clip;
> +
> + if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
> + return 0;
funcs = buffer->fb->funcs;
> +
> + if (rect) {
> + clip.x1 = rect->x1;
> + clip.y1 = rect->y1;
> + clip.x2 = rect->x2;
> + clip.y2 = rect->y2;
return funcs->dirty(buffer->fb, buffer->client->file,
0, 0, , 1);
> + } else {
return funcs->dirty(buffer->fb, buffer->client->file,
0, 0, NULL, 0);
}

> +
> + return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
> + 0, 0, clip, clip ? 1 : 0);
> +}
> +EXPORT_SYMBOL(drm_client_framebuffer_flush);
> +
>  #ifdef CONFIG_DEBUG_FS
>  static int drm_client_debugfs_internal_clients(struct seq_file *m, void 
> *data)
>  {
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index bbb5689fa9a8..6ef5364d6dfb 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -156,6 +156,7 @@ struct drm_client_buffer {
>  struct drm_client_buffer *
>  drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 
> height, u32 format);
>  void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
> +int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
> drm_rect *rect);
>  void *drm_client_buffer_vmap(struct drm_client_buffer *buffer);
>  void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
>  
> -- 
> 2.23.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/10] drm/client: Add drm_client_framebuffer_flush()

[Noralf Trønnes]

Some drivers need explicit flushing of buffer changes, add a function
that does that.

Signed-off-by: Noralf Trønnes 
---
 drivers/gpu/drm/drm_client.c | 31 +++
 include/drm/drm_client.h |  1 +
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index cb5ee9f1ffaa..8dbc2ecdcaea 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -483,6 +483,37 @@ void drm_client_framebuffer_delete(struct 
drm_client_buffer *buffer)
 }
 EXPORT_SYMBOL(drm_client_framebuffer_delete);
 
+/**
+ * drm_client_framebuffer_flush - Manually flush client framebuffer
+ * @buffer: DRM client buffer (can be NULL)
+ * @rect: Damage rectangle (if NULL flushes all)
+ *
+ * This calls _framebuffer_funcs->dirty (if present) to flush buffer 
changes
+ * for drivers that need it.
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
drm_rect *rect)
+{
+   struct drm_clip_rect clip_rect, *clip = NULL;
+
+   if (!buffer || !buffer->fb || !buffer->fb->funcs->dirty)
+   return 0;
+
+   if (rect) {
+   clip = _rect;
+   clip->x1 = rect->x1;
+   clip->y1 = rect->y1;
+   clip->x2 = rect->x2;
+   clip->y2 = rect->y2;
+   }
+
+   return buffer->fb->funcs->dirty(buffer->fb, buffer->client->file,
+   0, 0, clip, clip ? 1 : 0);
+}
+EXPORT_SYMBOL(drm_client_framebuffer_flush);
+
 #ifdef CONFIG_DEBUG_FS
 static int drm_client_debugfs_internal_clients(struct seq_file *m, void *data)
 {
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index bbb5689fa9a8..6ef5364d6dfb 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -156,6 +156,7 @@ struct drm_client_buffer {
 struct drm_client_buffer *
 drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 
height, u32 format);
 void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
+int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct 
drm_rect *rect);
 void *drm_client_buffer_vmap(struct drm_client_buffer *buffer);
 void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
 
-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel