Re: [RFC 2/7] drm: Add GEM backed framebuffer library

["Noralf Trønnes" <noralf@tronnes.org>]

Den 20.07.2017 10.10, skrev Daniel Vetter:
> On Tue, Jul 18, 2017 at 05:42:28PM +0200, Noralf Trønnes wrote:
>> Den 12.07.2017 15.46, skrev Noralf Trønnes:
>>> Add a library for drivers that can use a simple representation
>>> of a GEM backed framebuffer.
>>>
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>> ---
>> This patch adds a gem backed drm_framebuffer like this:
>>
>> struct drm_fb_gem {
>>      /**
>>       * @base: Base DRM framebuffer
>>       */
>>      struct drm_framebuffer base;
>>      /**
>>       * @obj: GEM object array backing the framebuffer. One object per
>>       * plane.
>>       */
>>      struct drm_gem_object *obj[4];
>> };
>>
>> Now I wonder if it would be better to extend drm_framebuffer instead:
>>
>>   struct drm_framebuffer {
>> +    /**
>> +     * @obj: GEM objects backing the framebuffer, one per plane (optional).
>> +     */
>> +    struct drm_gem_object *obj[4];
>>   };
> I think we can directly embedd the gem obj pointers into the
> drm_framebuffer. Again the idea that there would be anything else than gem
> kinda didn't pan out, except for vmwgfx.
>
> We could then have a gem version of drm_helper_mode_fill_fb_struct(),
> which also does the gem bo lookups.

How about this, I've copied from drm_fb_cma_create_with_funcs():

/**
  * drm_helper_mode_fill_fb_gem - fill out framebuffer metadata and 
lookup GEM
  * @dev: DRM device
  * @file_priv: DRM file to lookup buffers from
  * @fb: drm_framebuffer object to fill out
  * @mode_cmd: metadata from the userspace fb creation request
  *
  * This helper can be used in a drivers fb_create callback to pre-fill 
the fb's
  * metadata fields and lookup the GEM buffer object(s).
  *
  * Returns:
  * 0 on success or a negative error code on failure.
  */
int drm_helper_mode_fill_fb_gem(struct drm_device *dev,
                 struct drm_file *file_priv,
                 struct drm_framebuffer *fb,
                 const struct drm_mode_fb_cmd2 *mode_cmd)
{
     int i, ret;

     drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
     if (!fb->format)
         return -EINVAL;

     for (i = 0; i < fb->format->num_planes; i++) {
         unsigned int width = mode_cmd->width /
                      (i ? fb->format->hsub : 1);
         unsigned int height = mode_cmd->height /
                       (i ? fb->format->vsub : 1);
         unsigned int min_size;

         fb->objs[i] = drm_gem_object_lookup(file_priv,
                             mode_cmd->handles[i]);
         if (!fb->objs[i]) {
             ret = -ENOENT;
             goto err_gem_object_put;
         }

         min_size = (height - 1) * mode_cmd->pitches[i] +
                width * fb->format->cpp[i] + mode_cmd->offsets[i];

         if (fb->objs[i]->size < min_size) {
             drm_gem_object_put_unlocked(fb->objs[i]);
             fb->objs[i] = NULL;
             ret = -EINVAL;
             goto err_gem_object_put;
         }
     }

err_gem_object_put:
     for (i--; i >= 0; i--) {
         drm_gem_object_put_unlocked(fb->objs[i]);
         fb->objs[i] = NULL;
     }

     return ret;
}
EXPORT_SYMBOL(drm_helper_mode_fill_fb_gem);

>> The reason for this, is that I have looked through all drivers that
>> subclass drm_framebuffer and found this:
>> - 11 drivers just adds drm_gem_object
> Usually that's because they don't support yuv, so only need one plane.
>
>> - 2 drivers adds drm_gem_object and some more
>> - 6 drivers adds drm_gem_object indirectly through their subclassed
>>    drm_gem_object
>> - 3 drivers doesn't add drm_gem_object
> I think for easier conversion we can leave all the driver-specific stuff
> intact, and just provide helpers that work on the core bits.
>
> I guess this would also help a lot with unifying the cma fb helpers by
> essentially making them fully generic gem fb helpers?

Yes both cma and shmem library can now use these helpers. But pushing
the helpers all the way out to the cma drivers will be too much for me.

Any chance the fbdev helper could get a struct drm_file?
It would be nice to use drm_driver.dumb_create and
drm_mode_config_funcs.fb_create to get a
framebuffer for fbdev.

Noralf.

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