在 2016/10/26 20:59, Daniel Vetter 写道:
On Wed, Oct 26, 2016 at 05:19:31PM +0800, Rongrong Zou wrote:
Hi Daniel,
Thansk for reviewing!
在 2016/10/26 13:56, Daniel Vetter 写道:
On Wed, Oct 26, 2016 at 10:37:00AM +0800, Rongrong Zou wrote:
Add support for fbdev and kms fb management.
Signed-off-by: Rongrong Zou <zourongrong@xxxxxxxxx>
Small drive-by comment below.
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index db8d80e..d41138a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -20,9 +20,23 @@
#define HIBMC_DRM_DRV_H
#include <drm/drmP.h>
+#include <drm/drm_fb_helper.h>
#include <drm/ttm/ttm_bo_driver.h>
#include <drm/drm_gem.h>
+struct hibmc_framebuffer {
+ struct drm_framebuffer fb;
+ struct drm_gem_object *obj;
+ bool is_fbdev_fb;
+};
+
+struct hibmc_fbdev {
+ struct drm_fb_helper helper;
+ struct hibmc_framebuffer fb;
I wouldn't embed the single framebuffer here, but instead have a pointer
and just refcount it. This here is a pattern that predates framebuffer
refcounting, and it leads to plenty of surprises.
will allocate fbdev in next patch version, thanks.
Not the fbdev, the hibmc_framebuffer.
Understood, thanks.
Maybe we should update the documentation of
drm_framebuffer_unregister_private() to mention that it is deprecated? The
overview doc in drm_framebuffer.c already explains that, but I guess
that's not obvious enough.
Just replace drm_framebuffer_unregister_private() with
drm_framebuffer_remove()?
I found many other drivers use the following two functions in their
own xxx_fbdev_destroy():
drm_framebuffer_unregister_private(fbdev->fb);
drm_framebuffer_remove(fbdev->fb);
so the former is redundant and can be removed directly?
They all embed the fb instead of having a pointer, because those drivers
are all older than the fb refcounting support. In general good practice is
to look at the most recently merged driver, not at all of them. Only the
most recently one has a good chance to be up-to-date with latest best
practices. The function to call would be drm_framebuffer_unreference(),
and your struct above should be
struct hibmc_fbdev {
struct drm_fb_helper helper;
struct hibmc_framebuffer *fb;
...
};
Note how fb is a pointer here, not an embedded struct.
And in hibmc_user_framebuffer_destroy(), it should call
kfree(hibmc_framebuffer *hibmc_fb) not kfree(drm_framebuffer *fb), thanks.
regards,
Rongrong
-Daniel
Can you pls do that patch? And pls make sure it all looks pretty when
building the docs with
No problem, i'll send another patch later.
Regards,
Rongrong
$ make htmldocs
Thanks, Daniel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
