From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 7/8] drm/hibmc: Use drm_gem_fb_create
Date: Fri, 22 Nov 2019 10:12:53 +0100 [thread overview]
Message-ID: <CAKMK7uGR4YZ0h=ctgr+9QfWgu8L2He+YijvHFqSdf7SVmxrHhw@mail.gmail.com> (raw)
In-Reply-To: <282c9881-6134-5d82-c408-ee90986ef123@suse.de>
On Fri, Nov 22, 2019 at 9:16 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 15.11.19 um 10:21 schrieb Daniel Vetter:
> > Again we could delete a lot more if we'd switch over to the generic
> > fbdev stuff.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> There's one comment below. Except for that,
>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> > ---
> > .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 4 +-
> > .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 11 +---
> > .../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 5 +-
> > drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 62 +++++--------------
> > 4 files changed, 19 insertions(+), 63 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> > index 6527a97f68a3..2d0920ec4554 100644
> > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c
> > @@ -99,14 +99,12 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane,
> > s64 gpu_addr = 0;
> > unsigned int line_l;
> > struct hibmc_drm_private *priv = plane->dev->dev_private;
> > - struct hibmc_framebuffer *hibmc_fb;
> > struct drm_gem_vram_object *gbo;
> >
> > if (!state->fb)
> > return;
> >
> > - hibmc_fb = to_hibmc_framebuffer(state->fb);
> > - gbo = drm_gem_vram_of_gem(hibmc_fb->obj);
> > + gbo = drm_gem_vram_of_gem(fb->obj[0]);
>
> There's no fb in this function. You have to use state->fb,
Uh, hibmc is the one driver that doesn't compile on arm32 :-/ I'll
drop my patch here and take a look at your series and try to review
it. Then imo wait 2 weeks or so for somet testing from maintainers,
and push either way. If it works great, if it breaks you have your
tester then :-)
-Daniel
>
> >
> > gpu_addr = drm_gem_vram_offset(gbo);
> > if (WARN_ON_ONCE(gpu_addr < 0))
> > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> > index e58ecd7edcf8..ab5b4a4a2095 100644
> > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> > @@ -20,14 +20,9 @@
> > struct drm_device;
> > struct drm_gem_object;
> >
> > -struct hibmc_framebuffer {
> > - struct drm_framebuffer fb;
> > - struct drm_gem_object *obj;
> > -};
> > -
> > struct hibmc_fbdev {
> > struct drm_fb_helper helper; /* must be first */
> > - struct hibmc_framebuffer *fb;
> > + struct drm_framebuffer *fb;
> > int size;
> > };
> >
> > @@ -47,8 +42,6 @@ struct hibmc_drm_private {
> > struct hibmc_fbdev *fbdev;
> > };
> >
> > -#define to_hibmc_framebuffer(x) container_of(x, struct hibmc_framebuffer, fb)
> > -
> > void hibmc_set_power_mode(struct hibmc_drm_private *priv,
> > unsigned int power_mode);
> > void hibmc_set_current_gate(struct hibmc_drm_private *priv,
> > @@ -61,7 +54,7 @@ void hibmc_fbdev_fini(struct hibmc_drm_private *priv);
> >
> > int hibmc_gem_create(struct drm_device *dev, u32 size, bool iskernel,
> > struct drm_gem_object **obj);
> > -struct hibmc_framebuffer *
> > +struct drm_framebuffer *
> > hibmc_framebuffer_init(struct drm_device *dev,
> > const struct drm_mode_fb_cmd2 *mode_cmd,
> > struct drm_gem_object *obj);
> > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
> > index b4c1cea051e8..446aeedc9e29 100644
> > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
> > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c
> > @@ -141,15 +141,14 @@ static int hibmc_drm_fb_create(struct drm_fb_helper *helper,
> >
> > static void hibmc_fbdev_destroy(struct hibmc_fbdev *fbdev)
> > {
> > - struct hibmc_framebuffer *gfb = fbdev->fb;
> > struct drm_fb_helper *fbh = &fbdev->helper;
> >
> > drm_fb_helper_unregister_fbi(fbh);
> >
> > drm_fb_helper_fini(fbh);
> >
> > - if (gfb)
> > - drm_framebuffer_put(&gfb->fb);
> > + if (fbdev->fb)
> > + drm_framebuffer_put(fbdev->fb);
> > }
> >
> > static const struct drm_fb_helper_funcs hibmc_fbdev_helper_funcs = {
> > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> > index 21b684eab5c9..386033b0d3a2 100644
> > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
> > @@ -16,6 +16,7 @@
> > #include <drm/drm_atomic_helper.h>
> > #include <drm/drm_gem.h>
> > #include <drm/drm_gem_vram_helper.h>
> > +#include <drm/drm_gem_framebuffer_helper.h>
> > #include <drm/drm_print.h>
> >
> > #include "hibmc_drm_drv.h"
> > @@ -97,74 +98,39 @@ int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
> > return 0;
> > }
> >
> > -static void hibmc_user_framebuffer_destroy(struct drm_framebuffer *fb)
> > -{
> > - struct hibmc_framebuffer *hibmc_fb = to_hibmc_framebuffer(fb);
> > -
> > - drm_gem_object_put_unlocked(hibmc_fb->obj);
> > - drm_framebuffer_cleanup(fb);
> > - kfree(hibmc_fb);
> > -}
> > -
> > static const struct drm_framebuffer_funcs hibmc_fb_funcs = {
> > - .destroy = hibmc_user_framebuffer_destroy,
> > + .destroy = drm_gem_fb_destroy,
> > + .create_handle = drm_gem_fb_create_handle,
> > };
> >
> > -struct hibmc_framebuffer *
> > +struct drm_framebuffer *
> > hibmc_framebuffer_init(struct drm_device *dev,
> > const struct drm_mode_fb_cmd2 *mode_cmd,
> > struct drm_gem_object *obj)
> > {
> > - struct hibmc_framebuffer *hibmc_fb;
> > + struct drm_framebuffer *fb;
> > int ret;
> >
> > - hibmc_fb = kzalloc(sizeof(*hibmc_fb), GFP_KERNEL);
> > - if (!hibmc_fb) {
> > - DRM_ERROR("failed to allocate hibmc_fb\n");
> > + fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> > + if (!fb) {
> > + DRM_ERROR("failed to allocate fb\n");
> > return ERR_PTR(-ENOMEM);
> > }
> >
> > - drm_helper_mode_fill_fb_struct(dev, &hibmc_fb->fb, mode_cmd);
> > - hibmc_fb->obj = obj;
> > - ret = drm_framebuffer_init(dev, &hibmc_fb->fb, &hibmc_fb_funcs);
> > + drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
> > + fb->obj[0] = obj;
> > + ret = drm_framebuffer_init(dev, fb, &hibmc_fb_funcs);
> > if (ret) {
> > DRM_ERROR("drm_framebuffer_init failed: %d\n", ret);
> > - kfree(hibmc_fb);
> > + kfree(fb);
> > return ERR_PTR(ret);
> > }
> >
> > - return hibmc_fb;
> > -}
> > -
> > -static struct drm_framebuffer *
> > -hibmc_user_framebuffer_create(struct drm_device *dev,
> > - struct drm_file *filp,
> > - const struct drm_mode_fb_cmd2 *mode_cmd)
> > -{
> > - struct drm_gem_object *obj;
> > - struct hibmc_framebuffer *hibmc_fb;
> > -
> > - DRM_DEBUG_DRIVER("%dx%d, format %c%c%c%c\n",
> > - mode_cmd->width, mode_cmd->height,
> > - (mode_cmd->pixel_format) & 0xff,
> > - (mode_cmd->pixel_format >> 8) & 0xff,
> > - (mode_cmd->pixel_format >> 16) & 0xff,
> > - (mode_cmd->pixel_format >> 24) & 0xff);
> > -
> > - obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
> > - if (!obj)
> > - return ERR_PTR(-ENOENT);
> > -
> > - hibmc_fb = hibmc_framebuffer_init(dev, mode_cmd, obj);
> > - if (IS_ERR(hibmc_fb)) {
> > - drm_gem_object_put_unlocked(obj);
> > - return ERR_PTR((long)hibmc_fb);
> > - }
> > - return &hibmc_fb->fb;
> > + return fb;
> > }
> >
> > const struct drm_mode_config_funcs hibmc_mode_funcs = {
> > .atomic_check = drm_atomic_helper_check,
> > .atomic_commit = drm_atomic_helper_commit,
> > - .fb_create = hibmc_user_framebuffer_create,
> > + .fb_create = drm_gem_fb_create,
> > };
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-11-22 9:13 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-15 9:21 [PATCH 0/8] fb_create drive-through cleanups Daniel Vetter
2019-11-15 9:21 ` Daniel Vetter
2019-11-15 9:21 ` [PATCH 1/8] drm/fb: More paranoia in addfb checks Daniel Vetter
2019-11-15 10:49 ` Pekka Paalanen
2019-11-15 12:44 ` [Intel-gfx] " Ville Syrjälä
2019-11-15 15:26 ` Daniel Vetter
2019-11-15 9:21 ` [PATCH 2/8] drm/atmel: ditch fb_create wrapper Daniel Vetter
2019-11-15 9:21 ` Daniel Vetter
2019-11-15 9:33 ` Boris Brezillon
2019-11-19 21:22 ` Daniel Vetter
2019-11-23 8:49 ` Sam Ravnborg
2019-11-23 8:49 ` Sam Ravnborg
2019-11-15 9:21 ` [PATCH 3/8] drm/mediatek: don't open-code drm_gem_fb_create Daniel Vetter
2019-11-15 9:21 ` Daniel Vetter
2019-11-22 7:42 ` CK Hu
2019-11-22 17:09 ` Daniel Vetter
2019-11-22 17:09 ` Daniel Vetter
2019-11-15 9:21 ` [PATCH 4/8] drm/rockchip: Use drm_gem_fb_create_with_dirty Daniel Vetter
2019-11-15 9:21 ` Daniel Vetter
2019-11-27 11:45 ` Andrzej Pietrasiewicz
2019-11-27 11:45 ` Andrzej Pietrasiewicz
2019-11-27 17:33 ` Andrzej Pietrasiewicz
2019-11-27 17:54 ` Daniel Vetter
2019-11-15 9:21 ` [PATCH 5/8] drm/tilcdc: Drop drm_gem_fb_create wrapper Daniel Vetter
2019-11-15 9:21 ` Daniel Vetter
2019-11-15 13:21 ` Jyri Sarha
2019-11-15 13:21 ` Jyri Sarha
2019-11-19 21:25 ` Daniel Vetter
2019-11-19 21:25 ` Daniel Vetter
2019-11-15 9:21 ` [PATCH 6/8] drm/xen: Simplify fb_create Daniel Vetter
2019-11-15 10:33 ` Oleksandr Andrushchenko
2019-11-15 10:33 ` Oleksandr Andrushchenko
2019-11-19 21:25 ` Daniel Vetter
2019-11-15 9:21 ` [PATCH 7/8] drm/hibmc: Use drm_gem_fb_create Daniel Vetter
2019-11-22 8:09 ` Thomas Zimmermann
2019-11-22 8:16 ` Thomas Zimmermann
2019-11-22 9:12 ` Daniel Vetter [this message]
2019-11-15 9:21 ` [PATCH 8/8] drm/todo: Add entry for fb funcs related cleanups Daniel Vetter
2019-11-15 9:21 ` Daniel Vetter
2019-11-20 10:46 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKMK7uGR4YZ0h=ctgr+9QfWgu8L2He+YijvHFqSdf7SVmxrHhw@mail.gmail.com' \
--to=daniel.vetter@ffwll.ch \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).