Re: [PATCH 05/24] drm/rockchip: rockchip_drm_fb -> drm_framebuffer

2018-05-17 Thread Thierry Reding 
On Fri, Mar 30, 2018 at 03:11:19PM +0100, Daniel Stone wrote:
> Now that rockchip_drm_fb is just a wrapper around drm_framebuffer, we
> can remove it.
> 
> Signed-off-by: Daniel Stone 
> Cc: Sandy Huang 
> Cc: Heiko Stübner 
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 54 
> ++---
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.h  |  3 --
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  4 +--
>  3 files changed, 21 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index d7083c07c294..a4d0a00abcd9 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -25,21 +25,6 @@
>  #include "rockchip_drm_gem.h"
>  #include "rockchip_drm_psr.h"
>  
> -#define to_rockchip_fb(x) container_of(x, struct rockchip_drm_fb, fb)
> -
> -struct rockchip_drm_fb {
> - struct drm_framebuffer fb;
> -};
> -
> -struct drm_gem_object *rockchip_fb_get_gem_obj(struct drm_framebuffer *fb,
> -unsigned int plane)
> -{
> - if (plane >= ROCKCHIP_MAX_FB_BUFFER)
> - return NULL;
> -
> - return fb->obj[plane];
> -}
> -
>  static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb,
>struct drm_file *file,
>unsigned int flags, unsigned int color,
> @@ -56,41 +41,40 @@ static const struct drm_framebuffer_funcs 
> rockchip_drm_fb_funcs = {
>   .dirty = rockchip_drm_fb_dirty,
>  };
>  
> -static struct rockchip_drm_fb *
> +static struct drm_framebuffer *
>  rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 
> *mode_cmd,
> struct drm_gem_object **obj, unsigned int num_planes)
>  {
> - struct rockchip_drm_fb *rockchip_fb;
> + struct drm_framebuffer *fb;
>   int ret;
>   int i;
>  
> - rockchip_fb = kzalloc(sizeof(*rockchip_fb), GFP_KERNEL);
> - if (!rockchip_fb)
> + fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> + if (!fb)
>   return ERR_PTR(-ENOMEM);
>  
> - drm_helper_mode_fill_fb_struct(dev, _fb->fb, mode_cmd);
> + drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
>  
>   for (i = 0; i < num_planes; i++)
> - rockchip_fb->fb.obj[i] = obj[i];
> + fb->obj[i] = obj[i];
>  
> - ret = drm_framebuffer_init(dev, _fb->fb,
> -_drm_fb_funcs);
> + ret = drm_framebuffer_init(dev, fb, _drm_fb_funcs);
>   if (ret) {
>   DRM_DEV_ERROR(dev->dev,
> "Failed to initialize framebuffer: %d\n",
> ret);
> - kfree(rockchip_fb);
> + kfree(fb);
>   return ERR_PTR(ret);
>   }
>  
> - return rockchip_fb;
> + return fb;
>  }
>  
>  static struct drm_framebuffer *
>  rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
>   const struct drm_mode_fb_cmd2 *mode_cmd)
>  {
> - struct rockchip_drm_fb *rockchip_fb;
> + struct drm_framebuffer *fb;
>   struct drm_gem_object *objs[ROCKCHIP_MAX_FB_BUFFER];

The use of the ROCKCHIP_MAX_FB_BUFFER seems somewhat misguided here.
What if a given pixel format requires 4 planes? This function will just
silently ignore the last plane. Not sure that's a good idea. Currently I
don't know of any formats that require 4 planes, so this is probably not
an issue, but there's also no reason to limit this to anything less than
the 4 planes that's baked into the KMS ABI everywhere.

Anyway, just a random comment and it doesn't have anything to do with
this series, so:

Reviewed-by: Thierry Reding 


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

Re: [PATCH 05/24] drm/rockchip: rockchip_drm_fb -> drm_framebuffer

2018-05-17 Thread Daniel Stone 
Hi Heiko,

On 17 May 2018 at 14:42, Heiko Stübner  wrote:
> Am Donnerstag, 17. Mai 2018, 15:08:15 CEST schrieb Daniel Stone:
>> On 30 March 2018 at 15:11, Daniel Stone  wrote:
>> > Now that rockchip_drm_fb is just a wrapper around drm_framebuffer, we
>> > can remove it.
>> >
>> > Signed-off-by: Daniel Stone 
>> > Cc: Sandy Huang 
>> > Cc: Heiko Stübner 
>>
>> Ping?
>
> I only see the cover-letter (not listing all patches of the series)
> plus patches 4+5 of the series, nothing else.
>
> Both patches seem to reference other previous patches (1-3?)
> so could you point me at mboxes [patchwork] for those?

The series is available at:
https://patchwork.freedesktop.org/series/40944/

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

Re: [PATCH 05/24] drm/rockchip: rockchip_drm_fb -> drm_framebuffer

2018-05-17 Thread Sean Paul 
On Fri, Mar 30, 2018 at 03:11:19PM +0100, Daniel Stone wrote:
> Now that rockchip_drm_fb is just a wrapper around drm_framebuffer, we
> can remove it.
> 
> Signed-off-by: Daniel Stone 

Reviewed-by: Sean Paul 

> Cc: Sandy Huang 
> Cc: Heiko Stübner 
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 54 
> ++---
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.h  |  3 --
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  4 +--
>  3 files changed, 21 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index d7083c07c294..a4d0a00abcd9 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -25,21 +25,6 @@
>  #include "rockchip_drm_gem.h"
>  #include "rockchip_drm_psr.h"
>  
> -#define to_rockchip_fb(x) container_of(x, struct rockchip_drm_fb, fb)
> -
> -struct rockchip_drm_fb {
> - struct drm_framebuffer fb;
> -};
> -
> -struct drm_gem_object *rockchip_fb_get_gem_obj(struct drm_framebuffer *fb,
> -unsigned int plane)
> -{
> - if (plane >= ROCKCHIP_MAX_FB_BUFFER)
> - return NULL;
> -
> - return fb->obj[plane];
> -}
> -
>  static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb,
>struct drm_file *file,
>unsigned int flags, unsigned int color,
> @@ -56,41 +41,40 @@ static const struct drm_framebuffer_funcs 
> rockchip_drm_fb_funcs = {
>   .dirty = rockchip_drm_fb_dirty,
>  };
>  
> -static struct rockchip_drm_fb *
> +static struct drm_framebuffer *
>  rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 
> *mode_cmd,
> struct drm_gem_object **obj, unsigned int num_planes)
>  {
> - struct rockchip_drm_fb *rockchip_fb;
> + struct drm_framebuffer *fb;
>   int ret;
>   int i;
>  
> - rockchip_fb = kzalloc(sizeof(*rockchip_fb), GFP_KERNEL);
> - if (!rockchip_fb)
> + fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> + if (!fb)
>   return ERR_PTR(-ENOMEM);
>  
> - drm_helper_mode_fill_fb_struct(dev, _fb->fb, mode_cmd);
> + drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
>  
>   for (i = 0; i < num_planes; i++)
> - rockchip_fb->fb.obj[i] = obj[i];
> + fb->obj[i] = obj[i];
>  
> - ret = drm_framebuffer_init(dev, _fb->fb,
> -_drm_fb_funcs);
> + ret = drm_framebuffer_init(dev, fb, _drm_fb_funcs);
>   if (ret) {
>   DRM_DEV_ERROR(dev->dev,
> "Failed to initialize framebuffer: %d\n",
> ret);
> - kfree(rockchip_fb);
> + kfree(fb);
>   return ERR_PTR(ret);
>   }
>  
> - return rockchip_fb;
> + return fb;
>  }
>  
>  static struct drm_framebuffer *
>  rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
>   const struct drm_mode_fb_cmd2 *mode_cmd)
>  {
> - struct rockchip_drm_fb *rockchip_fb;
> + struct drm_framebuffer *fb;
>   struct drm_gem_object *objs[ROCKCHIP_MAX_FB_BUFFER];
>   struct drm_gem_object *obj;
>   unsigned int hsub;
> @@ -129,13 +113,13 @@ rockchip_user_fb_create(struct drm_device *dev, struct 
> drm_file *file_priv,
>   objs[i] = obj;
>   }
>  
> - rockchip_fb = rockchip_fb_alloc(dev, mode_cmd, objs, i);
> - if (IS_ERR(rockchip_fb)) {
> - ret = PTR_ERR(rockchip_fb);
> + fb = rockchip_fb_alloc(dev, mode_cmd, objs, i);
> + if (IS_ERR(fb)) {
> + ret = PTR_ERR(fb);
>   goto err_gem_object_unreference;
>   }
>  
> - return _fb->fb;
> + return fb;
>  
>  err_gem_object_unreference:
>   for (i--; i >= 0; i--)
> @@ -159,13 +143,13 @@ rockchip_drm_framebuffer_init(struct drm_device *dev,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> struct drm_gem_object *obj)
>  {
> - struct rockchip_drm_fb *rockchip_fb;
> + struct drm_framebuffer *fb;
>  
> - rockchip_fb = rockchip_fb_alloc(dev, mode_cmd, , 1);
> - if (IS_ERR(rockchip_fb))
> - return ERR_CAST(rockchip_fb);
> + fb = rockchip_fb_alloc(dev, mode_cmd, , 1);
> + if (IS_ERR(fb))
> + return ERR_CAST(fb);
>  
> - return _fb->fb;
> + return fb;
>  }
>  
>  void rockchip_drm_mode_config_init(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.h 
> b/drivers/gpu/drm/rockchip/rockchip_drm_fb.h
> index 2fe47f1ee98f..f1265cb1aee8 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.h
> @@ -22,7 +22,4 @@ rockchip_drm_framebuffer_init(struct drm_device *dev,
>  void

Re: [PATCH 05/24] drm/rockchip: rockchip_drm_fb -> drm_framebuffer

2018-05-17 Thread Heiko Stübner 
Hi Daniel,

Am Donnerstag, 17. Mai 2018, 15:08:15 CEST schrieb Daniel Stone:
> On 30 March 2018 at 15:11, Daniel Stone  wrote:
> > Now that rockchip_drm_fb is just a wrapper around drm_framebuffer, we
> > can remove it.
> > 
> > Signed-off-by: Daniel Stone 
> > Cc: Sandy Huang 
> > Cc: Heiko Stübner 
> 
> Ping?

I only see the cover-letter (not listing all patches of the series)
plus patches 4+5 of the series, nothing else.

Both patches seem to reference other previous patches (1-3?)
so could you point me at mboxes [patchwork] for those?


Thanks
Heiko


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

Re: [PATCH 05/24] drm/rockchip: rockchip_drm_fb -> drm_framebuffer

2018-05-17 Thread Daniel Stone 
On 30 March 2018 at 15:11, Daniel Stone  wrote:
> Now that rockchip_drm_fb is just a wrapper around drm_framebuffer, we
> can remove it.
>
> Signed-off-by: Daniel Stone 
> Cc: Sandy Huang 
> Cc: Heiko Stübner 

Ping?

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

[PATCH 05/24] drm/rockchip: rockchip_drm_fb -> drm_framebuffer

2018-03-30 Thread Daniel Stone 
Now that rockchip_drm_fb is just a wrapper around drm_framebuffer, we
can remove it.

Signed-off-by: Daniel Stone 
Cc: Sandy Huang 
Cc: Heiko Stübner 
---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  | 54 ++---
 drivers/gpu/drm/rockchip/rockchip_drm_fb.h  |  3 --
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  4 +--
 3 files changed, 21 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index d7083c07c294..a4d0a00abcd9 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -25,21 +25,6 @@
 #include "rockchip_drm_gem.h"
 #include "rockchip_drm_psr.h"
 
-#define to_rockchip_fb(x) container_of(x, struct rockchip_drm_fb, fb)
-
-struct rockchip_drm_fb {
-   struct drm_framebuffer fb;
-};
-
-struct drm_gem_object *rockchip_fb_get_gem_obj(struct drm_framebuffer *fb,
-  unsigned int plane)
-{
-   if (plane >= ROCKCHIP_MAX_FB_BUFFER)
-   return NULL;
-
-   return fb->obj[plane];
-}
-
 static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb,
 struct drm_file *file,
 unsigned int flags, unsigned int color,
@@ -56,41 +41,40 @@ static const struct drm_framebuffer_funcs 
rockchip_drm_fb_funcs = {
.dirty = rockchip_drm_fb_dirty,
 };
 
-static struct rockchip_drm_fb *
+static struct drm_framebuffer *
 rockchip_fb_alloc(struct drm_device *dev, const struct drm_mode_fb_cmd2 
*mode_cmd,
  struct drm_gem_object **obj, unsigned int num_planes)
 {
-   struct rockchip_drm_fb *rockchip_fb;
+   struct drm_framebuffer *fb;
int ret;
int i;
 
-   rockchip_fb = kzalloc(sizeof(*rockchip_fb), GFP_KERNEL);
-   if (!rockchip_fb)
+   fb = kzalloc(sizeof(*fb), GFP_KERNEL);
+   if (!fb)
return ERR_PTR(-ENOMEM);
 
-   drm_helper_mode_fill_fb_struct(dev, _fb->fb, mode_cmd);
+   drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
 
for (i = 0; i < num_planes; i++)
-   rockchip_fb->fb.obj[i] = obj[i];
+   fb->obj[i] = obj[i];
 
-   ret = drm_framebuffer_init(dev, _fb->fb,
-  _drm_fb_funcs);
+   ret = drm_framebuffer_init(dev, fb, _drm_fb_funcs);
if (ret) {
DRM_DEV_ERROR(dev->dev,
  "Failed to initialize framebuffer: %d\n",
  ret);
-   kfree(rockchip_fb);
+   kfree(fb);
return ERR_PTR(ret);
}
 
-   return rockchip_fb;
+   return fb;
 }
 
 static struct drm_framebuffer *
 rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
const struct drm_mode_fb_cmd2 *mode_cmd)
 {
-   struct rockchip_drm_fb *rockchip_fb;
+   struct drm_framebuffer *fb;
struct drm_gem_object *objs[ROCKCHIP_MAX_FB_BUFFER];
struct drm_gem_object *obj;
unsigned int hsub;
@@ -129,13 +113,13 @@ rockchip_user_fb_create(struct drm_device *dev, struct 
drm_file *file_priv,
objs[i] = obj;
}
 
-   rockchip_fb = rockchip_fb_alloc(dev, mode_cmd, objs, i);
-   if (IS_ERR(rockchip_fb)) {
-   ret = PTR_ERR(rockchip_fb);
+   fb = rockchip_fb_alloc(dev, mode_cmd, objs, i);
+   if (IS_ERR(fb)) {
+   ret = PTR_ERR(fb);
goto err_gem_object_unreference;
}
 
-   return _fb->fb;
+   return fb;
 
 err_gem_object_unreference:
for (i--; i >= 0; i--)
@@ -159,13 +143,13 @@ rockchip_drm_framebuffer_init(struct drm_device *dev,
  const struct drm_mode_fb_cmd2 *mode_cmd,
  struct drm_gem_object *obj)
 {
-   struct rockchip_drm_fb *rockchip_fb;
+   struct drm_framebuffer *fb;
 
-   rockchip_fb = rockchip_fb_alloc(dev, mode_cmd, , 1);
-   if (IS_ERR(rockchip_fb))
-   return ERR_CAST(rockchip_fb);
+   fb = rockchip_fb_alloc(dev, mode_cmd, , 1);
+   if (IS_ERR(fb))
+   return ERR_CAST(fb);
 
-   return _fb->fb;
+   return fb;
 }
 
 void rockchip_drm_mode_config_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.h 
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.h
index 2fe47f1ee98f..f1265cb1aee8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.h
@@ -22,7 +22,4 @@ rockchip_drm_framebuffer_init(struct drm_device *dev,
 void rockchip_drm_framebuffer_fini(struct drm_framebuffer *fb);
 
 void rockchip_drm_mode_config_init(struct drm_device *dev);
-
-struct drm_gem_object *rockchip_fb_get_gem_obj(struct drm_framebuffer *fb,
-  unsigned int plane);
 #endif /*