Re: [PATCH weston v2 1/3] compositor-drm: refactor destroy drm_fb function

[Vincent ABRIOU]

Looks good to me.

Reviewed-by: Vincent Abriou 

On 10/03/2016 07:28 PM, Derek Foreman wrote:
> On 30/09/16 04:28 AM, Tomohito Esaki wrote:
>> The drm_fb destroy callback to mostly the same thing regardless of
>> whether the buffer is a dumb buffer or gbm buffer. This patch refactors
>> the common parts into a new function that can be called for both cases.
>>
>> Signed-off-by: Tomohito Esaki 
>> ---
>>  libweston/compositor-drm.c | 31 ---
>>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> Looks functionally identical and a little simpler, so:
> Reviewed-by: Derek Foreman 
>
> Thanks,
> Derek
>
>> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
>> index 8319d7c..a707fc4 100644
>> --- a/libweston/compositor-drm.c
>> +++ b/libweston/compositor-drm.c
>> @@ -261,17 +261,23 @@ drm_sprite_crtc_supported(struct drm_output *output, 
>> uint32_t supported)
>>  }
>>
>>  static void
>> -drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
>> +drm_fb_destroy(struct drm_fb *fb, int fd)
>>  {
>> -struct drm_fb *fb = data;
>> -struct gbm_device *gbm = gbm_bo_get_device(bo);
>> -
>>  if (fb->fb_id)
>> -drmModeRmFB(gbm_device_get_fd(gbm), fb->fb_id);
>> +drmModeRmFB(fd, fb->fb_id);
>>
>>  weston_buffer_reference(>buffer_ref, NULL);
>>
>> -free(data);
>> +free(fb);
>> +}
>> +
>> +static void
>> +drm_fb_destroy_gbm_fb(struct gbm_bo *bo, void *data)
>> +{
>> +struct drm_fb *fb = data;
>> +struct gbm_device *gbm = gbm_bo_get_device(bo);
>> +
>> +drm_fb_destroy(fb, gbm_device_get_fd(gbm));
>>  }
>>
>>  static struct drm_fb *
>> @@ -370,22 +376,17 @@ static void
>>  drm_fb_destroy_dumb(struct drm_fb *fb)
>>  {
>>  struct drm_mode_destroy_dumb destroy_arg;
>> +int fd = fb->fd;
>>
>>  if (!fb->map)
>>  return;
>>
>> -if (fb->fb_id)
>> -drmModeRmFB(fb->fd, fb->fb_id);
>> -
>> -weston_buffer_reference(>buffer_ref, NULL);
>> -
>>  munmap(fb->map, fb->size);
>> -
>>  memset(_arg, 0, sizeof(destroy_arg));
>>  destroy_arg.handle = fb->handle;
>> -drmIoctl(fb->fd, DRM_IOCTL_MODE_DESTROY_DUMB, _arg);
>>
>> -free(fb);
>> +drm_fb_destroy(fb, fd);
>> +drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, _arg);
>>  }
>>
>>  static struct drm_fb *
>> @@ -446,7 +447,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo,
>>  goto err_free;
>>  }
>>
>> -gbm_bo_set_user_data(bo, fb, drm_fb_destroy_callback);
>> +gbm_bo_set_user_data(bo, fb, drm_fb_destroy_gbm_fb);
>>
>>  return fb;
>>
>>
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 1/3] compositor-drm: refactor destroy drm_fb function

[Derek Foreman]

On 30/09/16 04:28 AM, Tomohito Esaki wrote:

The drm_fb destroy callback to mostly the same thing regardless of
whether the buffer is a dumb buffer or gbm buffer. This patch refactors
the common parts into a new function that can be called for both cases.

Signed-off-by: Tomohito Esaki 
---
 libweston/compositor-drm.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)


Looks functionally identical and a little simpler, so:
Reviewed-by: Derek Foreman 

Thanks,
Derek


diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 8319d7c..a707fc4 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -261,17 +261,23 @@ drm_sprite_crtc_supported(struct drm_output *output, 
uint32_t supported)
 }

 static void
-drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
+drm_fb_destroy(struct drm_fb *fb, int fd)
 {
-   struct drm_fb *fb = data;
-   struct gbm_device *gbm = gbm_bo_get_device(bo);
-
if (fb->fb_id)
-   drmModeRmFB(gbm_device_get_fd(gbm), fb->fb_id);
+   drmModeRmFB(fd, fb->fb_id);

weston_buffer_reference(>buffer_ref, NULL);

-   free(data);
+   free(fb);
+}
+
+static void
+drm_fb_destroy_gbm_fb(struct gbm_bo *bo, void *data)
+{
+   struct drm_fb *fb = data;
+   struct gbm_device *gbm = gbm_bo_get_device(bo);
+
+   drm_fb_destroy(fb, gbm_device_get_fd(gbm));
 }

 static struct drm_fb *
@@ -370,22 +376,17 @@ static void
 drm_fb_destroy_dumb(struct drm_fb *fb)
 {
struct drm_mode_destroy_dumb destroy_arg;
+   int fd = fb->fd;

if (!fb->map)
return;

-   if (fb->fb_id)
-   drmModeRmFB(fb->fd, fb->fb_id);
-
-   weston_buffer_reference(>buffer_ref, NULL);
-
munmap(fb->map, fb->size);
-
memset(_arg, 0, sizeof(destroy_arg));
destroy_arg.handle = fb->handle;
-   drmIoctl(fb->fd, DRM_IOCTL_MODE_DESTROY_DUMB, _arg);

-   free(fb);
+   drm_fb_destroy(fb, fd);
+   drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, _arg);
 }

 static struct drm_fb *
@@ -446,7 +447,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo,
goto err_free;
}

-   gbm_bo_set_user_data(bo, fb, drm_fb_destroy_callback);
+   gbm_bo_set_user_data(bo, fb, drm_fb_destroy_gbm_fb);

return fb;




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

Re: [PATCH weston v2 1/3] compositor-drm: refactor destroy drm_fb function

[Eric Engestrom]

On Fri, Sep 30, 2016 at 06:28:51PM +0900, Tomohito Esaki wrote:
> The drm_fb destroy callback to mostly the same thing regardless of
> whether the buffer is a dumb buffer or gbm buffer. This patch refactors
> the common parts into a new function that can be called for both cases.
> 
> Signed-off-by: Tomohito Esaki 

Reviewed-by: Eric Engestrom 

> ---
>  libweston/compositor-drm.c | 31 ---
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 8319d7c..a707fc4 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -261,17 +261,23 @@ drm_sprite_crtc_supported(struct drm_output *output, 
> uint32_t supported)
>  }
>  
>  static void
> -drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
> +drm_fb_destroy(struct drm_fb *fb, int fd)
>  {
> - struct drm_fb *fb = data;
> - struct gbm_device *gbm = gbm_bo_get_device(bo);
> -
>   if (fb->fb_id)
> - drmModeRmFB(gbm_device_get_fd(gbm), fb->fb_id);
> + drmModeRmFB(fd, fb->fb_id);
>  
>   weston_buffer_reference(>buffer_ref, NULL);
>  
> - free(data);
> + free(fb);
> +}
> +
> +static void
> +drm_fb_destroy_gbm_fb(struct gbm_bo *bo, void *data)
> +{
> + struct drm_fb *fb = data;
> + struct gbm_device *gbm = gbm_bo_get_device(bo);
> +
> + drm_fb_destroy(fb, gbm_device_get_fd(gbm));
>  }
>  
>  static struct drm_fb *
> @@ -370,22 +376,17 @@ static void
>  drm_fb_destroy_dumb(struct drm_fb *fb)
>  {
>   struct drm_mode_destroy_dumb destroy_arg;
> + int fd = fb->fd;
>  
>   if (!fb->map)
>   return;
>  
> - if (fb->fb_id)
> - drmModeRmFB(fb->fd, fb->fb_id);
> -
> - weston_buffer_reference(>buffer_ref, NULL);
> -
>   munmap(fb->map, fb->size);
> -
>   memset(_arg, 0, sizeof(destroy_arg));
>   destroy_arg.handle = fb->handle;
> - drmIoctl(fb->fd, DRM_IOCTL_MODE_DESTROY_DUMB, _arg);
>  
> - free(fb);
> + drm_fb_destroy(fb, fd);
> + drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, _arg);
>  }
>  
>  static struct drm_fb *
> @@ -446,7 +447,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo,
>   goto err_free;
>   }
>  
> - gbm_bo_set_user_data(bo, fb, drm_fb_destroy_callback);
> + gbm_bo_set_user_data(bo, fb, drm_fb_destroy_gbm_fb);
>  
>   return fb;
>  
> -- 
> 2.7.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel