From: zourongrong@huawei•com (Rongrong Zou)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v5 3/9] drm/hisilicon/hibmc: Add support for frame buffer
Date: Wed, 26 Oct 2016 17:19:31 +0800 [thread overview]
Message-ID: <58107523.20409@huawei.com> (raw)
In-Reply-To: <20161026055648.bipks7wthiu4gmlf@phenom.ffwll.local>
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@gmail•com>
>
> 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.
>
> 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?
>
> 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
>
next prev parent reply other threads:[~2016-10-26 9:19 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-26 2:36 [PATCH v5 0/9] Add DRM driver for Hisilicon Hibmc Rongrong Zou
2016-10-26 2:36 ` [PATCH v5 1/9] drm/hisilicon/hibmc: Add hisilicon hibmc drm master driver Rongrong Zou
2016-10-26 2:36 ` [PATCH v5 2/9] drm/hisilicon/hibmc: Add video memory management Rongrong Zou
2016-10-26 2:37 ` [PATCH v5 3/9] drm/hisilicon/hibmc: Add support for frame buffer Rongrong Zou
2016-10-26 5:56 ` Daniel Vetter
2016-10-26 9:19 ` Rongrong Zou [this message]
2016-10-26 12:59 ` Daniel Vetter
2016-10-27 3:01 ` Rongrong Zou
2016-10-26 2:37 ` [PATCH v5 4/9] drm/hisilicon/hibmc: Add plane for DE Rongrong Zou
2016-10-26 2:37 ` [PATCH v5 5/9] drm/hisilicon/hibmc: Add crtc " Rongrong Zou
2016-10-26 2:37 ` [PATCH v5 6/9] drm/hisilicon/hibmc: Add encoder for VDAC Rongrong Zou
2016-10-26 2:37 ` [PATCH v5 7/9] drm/hisilicon/hibmc: Add connector " Rongrong Zou
2016-10-26 2:37 ` [PATCH v5 8/9] drm/hisilicon/hibmc: Add vblank interruput Rongrong Zou
2016-10-26 2:37 ` [PATCH v5 9/9] MAINTAINERS: Update HISILICON DRM entries Rongrong Zou
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=58107523.20409@huawei.com \
--to=zourongrong@huawei$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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