Re: [Freedreno] [PATCH libdrm] drm: Remove create_handle() drm_framebuffer "virtual".

2017-08-10 Thread Joe Kniss 
On Wed, Aug 9, 2017 at 4:13 PM, Joe Kniss  wrote:
> On Wed, Aug 9, 2017 at 12:14 PM, Noralf Trønnes  wrote:
>>
>> Den 09.08.2017 01.42, skrev Joe Kniss:
>>>
>>> Because all drivers currently use gem objects for framebuffer planes,
>>> the virtual create_handle() is not required.  This change adds a
>>> struct drm_gem_object *gems[4] field to drm_framebuffer and removes
>>> create_handle() function pointer from drm_framebuffer_funcs.  The
>>> corresponding *_create_handle() function is removed from each driver.
>>>
>>> In many cases this change eliminates a struct *_framebuffer object,
>>> as the only need for the derived struct is the addition of the gem
>>> object pointer.
>>>
>>> TESTED: compiled: allyesconfig ARCH=x86,arm platforms:i915, rockchip
>>>
>>> Signed-off-by: Joe Kniss 
>>> ---
>>
>>
>> Hi Joe,
>>
>> I'm also looking into adding gem objs to drm_framebuffer in this patch:
>> [PATCH v2 01/22] drm: Add GEM backed framebuffer library
>> https://lists.freedesktop.org/archives/dri-devel/2017-August/149782.html
>>
>
> Great.  There's only minimal overlap here.  I'll rebase this change on yours
> once it's in.
>
>> [...]
>>
>>> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
>>> b/drivers/gpu/drm/drm_fb_cma_helper.c
>>> index ade319d10e70..f5f011b910b1 100644
>>> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
>>> @@ -31,14 +31,9 @@
>>> #define DEFAULT_FBDEFIO_DELAY_MS 50
>>>   -struct drm_fb_cma {
>>> -   struct drm_framebuffer  fb;
>>> -   struct drm_gem_cma_object   *obj[4];
>>> -};
>>> -
>>>   struct drm_fbdev_cma {
>>> struct drm_fb_helperfb_helper;
>>> -   struct drm_fb_cma   *fb;
>>> +   struct drm_framebuffer  *fb;
>>
>>
>> This fb pointer isn't necessary, since fb_helper already has one.
>>

So, looking deeper into this, it seems that the struct
drm_framebuffer_funcs *fb_funcs is also redundant here?  In which case
this whole struct can go...

>
> I'll remove it... but I am sure when I look deeper there will be more
> of these in the various drivers too.
>
>> Noralf.
>>
>>
>
> -joe
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH libdrm] drm: Remove create_handle() drm_framebuffer "virtual".

2017-08-09 Thread Joe Kniss 
On Wed, Aug 9, 2017 at 12:14 PM, Noralf Trønnes  wrote:
>
> Den 09.08.2017 01.42, skrev Joe Kniss:
>>
>> Because all drivers currently use gem objects for framebuffer planes,
>> the virtual create_handle() is not required.  This change adds a
>> struct drm_gem_object *gems[4] field to drm_framebuffer and removes
>> create_handle() function pointer from drm_framebuffer_funcs.  The
>> corresponding *_create_handle() function is removed from each driver.
>>
>> In many cases this change eliminates a struct *_framebuffer object,
>> as the only need for the derived struct is the addition of the gem
>> object pointer.
>>
>> TESTED: compiled: allyesconfig ARCH=x86,arm platforms:i915, rockchip
>>
>> Signed-off-by: Joe Kniss 
>> ---
>
>
> Hi Joe,
>
> I'm also looking into adding gem objs to drm_framebuffer in this patch:
> [PATCH v2 01/22] drm: Add GEM backed framebuffer library
> https://lists.freedesktop.org/archives/dri-devel/2017-August/149782.html
>

Great.  There's only minimal overlap here.  I'll rebase this change on yours
once it's in.

> [...]
>
>> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c
>> b/drivers/gpu/drm/drm_fb_cma_helper.c
>> index ade319d10e70..f5f011b910b1 100644
>> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
>> @@ -31,14 +31,9 @@
>> #define DEFAULT_FBDEFIO_DELAY_MS 50
>>   -struct drm_fb_cma {
>> -   struct drm_framebuffer  fb;
>> -   struct drm_gem_cma_object   *obj[4];
>> -};
>> -
>>   struct drm_fbdev_cma {
>> struct drm_fb_helperfb_helper;
>> -   struct drm_fb_cma   *fb;
>> +   struct drm_framebuffer  *fb;
>
>
> This fb pointer isn't necessary, since fb_helper already has one.
>

I'll remove it... but I am sure when I look deeper there will be more
of these in the various drivers too.

> Noralf.
>
>

-joe
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH libdrm] drm: Remove create_handle() drm_framebuffer "virtual".

2017-08-09 Thread Noralf Trønnes 


Den 09.08.2017 01.42, skrev Joe Kniss:

Because all drivers currently use gem objects for framebuffer planes,
the virtual create_handle() is not required.  This change adds a
struct drm_gem_object *gems[4] field to drm_framebuffer and removes
create_handle() function pointer from drm_framebuffer_funcs.  The
corresponding *_create_handle() function is removed from each driver.

In many cases this change eliminates a struct *_framebuffer object,
as the only need for the derived struct is the addition of the gem
object pointer.

TESTED: compiled: allyesconfig ARCH=x86,arm platforms:i915, rockchip

Signed-off-by: Joe Kniss 
---


Hi Joe,

I'm also looking into adding gem objs to drm_framebuffer in this patch:
[PATCH v2 01/22] drm: Add GEM backed framebuffer library
https://lists.freedesktop.org/archives/dri-devel/2017-August/149782.html

[...]


diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
b/drivers/gpu/drm/drm_fb_cma_helper.c
index ade319d10e70..f5f011b910b1 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -31,14 +31,9 @@
  
  #define DEFAULT_FBDEFIO_DELAY_MS 50
  
-struct drm_fb_cma {

-   struct drm_framebuffer  fb;
-   struct drm_gem_cma_object   *obj[4];
-};
-
  struct drm_fbdev_cma {
struct drm_fb_helperfb_helper;
-   struct drm_fb_cma   *fb;
+   struct drm_framebuffer  *fb;


This fb pointer isn't necessary, since fb_helper already has one.

Noralf.


const struct drm_framebuffer_funcs *fb_funcs;
  };
  
@@ -72,7 +67,6 @@ struct drm_fbdev_cma {

   *
   * static struct drm_framebuffer_funcs driver_fb_funcs = {
   * .destroy   = drm_fb_cma_destroy,
- * .create_handle = drm_fb_cma_create_handle,
   * .dirty = driver_fb_dirty,
   * };
   *
@@ -90,67 +84,50 @@ static inline struct drm_fbdev_cma *to_fbdev_cma(struct 
drm_fb_helper *helper)
return container_of(helper, struct drm_fbdev_cma, fb_helper);
  }
  
-static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)

-{
-   return container_of(fb, struct drm_fb_cma, fb);
-}
-
  void drm_fb_cma_destroy(struct drm_framebuffer *fb)
  {
-   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
int i;
  
  	for (i = 0; i < 4; i++) {

-   if (fb_cma->obj[i])
-   drm_gem_object_put_unlocked(_cma->obj[i]->base);
+   if (fb->gem_objs[i])
+   drm_gem_object_put_unlocked(fb->gem_objs[i]);
}
  
  	drm_framebuffer_cleanup(fb);

-   kfree(fb_cma);
+   kfree(fb);
  }
  EXPORT_SYMBOL(drm_fb_cma_destroy);
  
-int drm_fb_cma_create_handle(struct drm_framebuffer *fb,

-   struct drm_file *file_priv, unsigned int *handle)
-{
-   struct drm_fb_cma *fb_cma = to_fb_cma(fb);
-
-   return drm_gem_handle_create(file_priv,
-   _cma->obj[0]->base, handle);
-}
-EXPORT_SYMBOL(drm_fb_cma_create_handle);
-
  static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
.destroy= drm_fb_cma_destroy,
-   .create_handle  = drm_fb_cma_create_handle,
  };
  
-static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,

+static struct drm_framebuffer *drm_fb_cma_alloc(struct drm_device *dev,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_cma_object **obj,
unsigned int num_planes, const struct drm_framebuffer_funcs *funcs)
  {
-   struct drm_fb_cma *fb_cma;
+   struct drm_framebuffer *fb;
int ret;
int i;
  
-	fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);

-   if (!fb_cma)
+   fb = kzalloc(sizeof(*fb), GFP_KERNEL);
+   if (!fb)
return ERR_PTR(-ENOMEM);
  
-	drm_helper_mode_fill_fb_struct(dev, _cma->fb, mode_cmd);

+   drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
  
  	for (i = 0; i < num_planes; i++)

-   fb_cma->obj[i] = obj[i];
+   fb->gem_objs[i] = [i]->base;
  
-	ret = drm_framebuffer_init(dev, _cma->fb, funcs);

+   ret = drm_framebuffer_init(dev, fb, funcs);
if (ret) {
dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", 
ret);
-   kfree(fb_cma);
+   kfree(fb);
return ERR_PTR(ret);
}
  
-	return fb_cma;

+   return fb;
  }
  
  /**

@@ -171,7 +148,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct 
drm_device *dev,
const struct drm_framebuffer_funcs *funcs)
  {
const struct drm_format_info *info;
-   struct drm_fb_cma *fb_cma;
+   struct drm_framebuffer *fb;
struct drm_gem_cma_object *objs[4];
struct drm_gem_object *obj;
int ret;
@@ -205,13 +182,13 @@ struct drm_framebuffer 
*drm_fb_cma_create_with_funcs(struct drm_device *dev,
objs[i] = to_drm_gem_cma_obj(obj);
}
  
-	fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs);

-   if (IS_ERR(fb_cma)) {
-   ret =