Hi Daniel,

After applying this patch there are some slight differences
in the effective behavior of the code.

I can't tell if they are important, please see below.

Andrzej

W dniu 15.11.2019 o 10:21, Daniel Vetter pisze:
If rockchip would switch over to the generic fbdev setup we could
grabage collect even more of all this code (all of the remaining fb
handling code really).

Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
Cc: Sandy Huang <h...@rock-chips.com>
Cc: "Heiko Stübner" <he...@sntech.de>
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-rockc...@lists.infradead.org
---
  drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 54 +---------------------
  1 file changed, 1 insertion(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index ca01234c037c..081dbdaa0b07 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -53,64 +53,12 @@ rockchip_fb_alloc(struct drm_device *dev, const struct 
drm_mode_fb_cmd2 *mode_cm
        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)
-{
-       const struct drm_format_info *info = drm_get_format_info(dev,
-                                                                mode_cmd);
-       struct drm_framebuffer *fb;
-       struct drm_gem_object *objs[ROCKCHIP_MAX_FB_BUFFER];
-       struct drm_gem_object *obj;
-       int num_planes = min_t(int, info->num_planes, ROCKCHIP_MAX_FB_BUFFER);
-       int ret;
-       int i;
-
-       for (i = 0; i < num_planes; i++) {

drm_gem_fb_create_with_funcs(), if no error happens,
iterates exactly info->num_planes times,
but the function being removed here iterates
min_t(int, info->num_planes, 3) times.

Is it ensured earlier elsewhere that info->num_planes does not exceed 3?

-               unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
-               unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
-               unsigned int min_size;
-
-               obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
-               if (!obj) {
-                       DRM_DEV_ERROR(dev->dev,
-                                     "Failed to lookup GEM object\n");
-                       ret = -ENXIO;
-                       goto err_gem_object_unreference;
-               }
-
-               min_size = (height - 1) * mode_cmd->pitches[i] +
-                       mode_cmd->offsets[i] +
-                       width * info->cpp[i];

This computation in drm_gem_fb_create_with_funcs looks like this:

                min_size = (height - 1) * mode_cmd->pitches[i]
                         + drm_format_info_min_pitch(info, i, width)
                         + mode_cmd->offsets[i];

Perhaps that's actually the same thing?

-
-               if (obj->size < min_size) {
-                       drm_gem_object_put_unlocked(obj);
-                       ret = -EINVAL;
-                       goto err_gem_object_unreference;
-               }
-               objs[i] = obj;
-       }
-
-       fb = rockchip_fb_alloc(dev, mode_cmd, objs, i);
-       if (IS_ERR(fb)) {
-               ret = PTR_ERR(fb);
-               goto err_gem_object_unreference;
-       }
-
-       return fb;
-
-err_gem_object_unreference:
-       for (i--; i >= 0; i--)
-               drm_gem_object_put_unlocked(objs[i]);
-       return ERR_PTR(ret);
-}
-
  static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers 
= {
        .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
  };
static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = {
-       .fb_create = rockchip_user_fb_create,
+       .fb_create = drm_gem_fb_create,

That way you leave out the ->dirty() callback from
static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs

I'd say instead:

struct drm_framebuffer *
rockchip_gem_fb_create(struct drm_device *dev, struct drm_file *file,
                       const struct drm_mode_fb_cmd2 *mode_cmd)
{
        return drm_gem_fb_create_with_funcs(dev, file, mode_cmd,
                                            &rockchip_drm_fb_funcs);
}

and then

+       .fb_create = rockchip_gem_fb_create,

        .output_poll_changed = drm_fb_helper_output_poll_changed,
        .atomic_check = drm_atomic_helper_check,
        .atomic_commit = drm_atomic_helper_commit,


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to