From mboxrd@z Thu Jan 1 00:00:00 1970 From: zourongrong@huawei.com (Rongrong Zou) Date: Wed, 26 Oct 2016 17:19:31 +0800 Subject: [PATCH v5 3/9] drm/hisilicon/hibmc: Add support for frame buffer In-Reply-To: <20161026055648.bipks7wthiu4gmlf@phenom.ffwll.local> References: <1477449426-69018-1-git-send-email-zourongrong@gmail.com> <1477449426-69018-4-git-send-email-zourongrong@gmail.com> <20161026055648.bipks7wthiu4gmlf@phenom.ffwll.local> Message-ID: <58107523.20409@huawei.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 > > 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 >> +#include >> #include >> #include >> >> +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 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rongrong Zou Subject: Re: [PATCH v5 3/9] drm/hisilicon/hibmc: Add support for frame buffer Date: Wed, 26 Oct 2016 17:19:31 +0800 Message-ID: <58107523.20409@huawei.com> References: <1477449426-69018-1-git-send-email-zourongrong@gmail.com> <1477449426-69018-4-git-send-email-zourongrong@gmail.com> <20161026055648.bipks7wthiu4gmlf@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20161026055648.bipks7wthiu4gmlf@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Daniel Vetter , Rongrong Zou Cc: mark.rutland@arm.com, architt@codeaurora.org, lijianhua@huawei.com, shenhui@huawei.com, tomeu.vizoso@collabora.com, corbet@lwn.net, airlied@linux.ie, catalin.marinas@arm.com, emil.l.velikov@gmail.com, linuxarm@huawei.com, dri-devel@lists.freedesktop.org, xinliang.liu@linaro.org, james.xiong@huawei.com, benjamin.gaignard@linaro.org, will.deacon@arm.com, daniel@fooishbar.org, linux-arm-kernel@lists.infradead.org List-Id: dri-devel@lists.freedesktop.org SGkgRGFuaWVsLAoKVGhhbnNrIGZvciByZXZpZXdpbmchCgrlnKggMjAxNi8xMC8yNiAxMzo1Niwg RGFuaWVsIFZldHRlciDlhpnpgZM6Cj4gT24gV2VkLCBPY3QgMjYsIDIwMTYgYXQgMTA6Mzc6MDBB TSArMDgwMCwgUm9uZ3JvbmcgWm91IHdyb3RlOgo+PiBBZGQgc3VwcG9ydCBmb3IgZmJkZXYgYW5k IGttcyBmYiBtYW5hZ2VtZW50Lgo+Pgo+PiBTaWduZWQtb2ZmLWJ5OiBSb25ncm9uZyBab3UgPHpv dXJvbmdyb25nQGdtYWlsLmNvbT4KPgo+IFNtYWxsIGRyaXZlLWJ5IGNvbW1lbnQgYmVsb3cuCj4K Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9oaXNpbGljb24vaGlibWMvaGlibWNfZHJt X2Rydi5oIGIvZHJpdmVycy9ncHUvZHJtL2hpc2lsaWNvbi9oaWJtYy9oaWJtY19kcm1fZHJ2LmgK Pj4gaW5kZXggZGI4ZDgwZS4uZDQxMTM4YSAxMDA2NDQKPj4gLS0tIGEvZHJpdmVycy9ncHUvZHJt L2hpc2lsaWNvbi9oaWJtYy9oaWJtY19kcm1fZHJ2LmgKPj4gKysrIGIvZHJpdmVycy9ncHUvZHJt L2hpc2lsaWNvbi9oaWJtYy9oaWJtY19kcm1fZHJ2LmgKPj4gQEAgLTIwLDkgKzIwLDIzIEBACj4+ ICAgI2RlZmluZSBISUJNQ19EUk1fRFJWX0gKPj4KPj4gICAjaW5jbHVkZSA8ZHJtL2RybVAuaD4K Pj4gKyNpbmNsdWRlIDxkcm0vZHJtX2ZiX2hlbHBlci5oPgo+PiAgICNpbmNsdWRlIDxkcm0vdHRt L3R0bV9ib19kcml2ZXIuaD4KPj4gICAjaW5jbHVkZSA8ZHJtL2RybV9nZW0uaD4KPj4KPj4gK3N0 cnVjdCBoaWJtY19mcmFtZWJ1ZmZlciB7Cj4+ICsJc3RydWN0IGRybV9mcmFtZWJ1ZmZlciBmYjsK Pj4gKwlzdHJ1Y3QgZHJtX2dlbV9vYmplY3QgKm9iajsKPj4gKwlib29sIGlzX2ZiZGV2X2ZiOwo+ PiArfTsKPj4gKwo+PiArc3RydWN0IGhpYm1jX2ZiZGV2IHsKPj4gKwlzdHJ1Y3QgZHJtX2ZiX2hl bHBlciBoZWxwZXI7Cj4+ICsJc3RydWN0IGhpYm1jX2ZyYW1lYnVmZmVyIGZiOwo+Cj4gSSB3b3Vs ZG4ndCBlbWJlZCB0aGUgc2luZ2xlIGZyYW1lYnVmZmVyIGhlcmUsIGJ1dCBpbnN0ZWFkIGhhdmUg YSBwb2ludGVyCj4gYW5kIGp1c3QgcmVmY291bnQgaXQuIFRoaXMgaGVyZSBpcyBhIHBhdHRlcm4g dGhhdCBwcmVkYXRlcyBmcmFtZWJ1ZmZlcgo+IHJlZmNvdW50aW5nLCBhbmQgaXQgbGVhZHMgdG8g cGxlbnR5IG9mIHN1cnByaXNlcy4KCndpbGwgYWxsb2NhdGUgZmJkZXYgaW4gbmV4dCBwYXRjaCB2 ZXJzaW9uLCB0aGFua3MuCgo+Cj4gTWF5YmUgd2Ugc2hvdWxkIHVwZGF0ZSB0aGUgZG9jdW1lbnRh dGlvbiBvZgo+IGRybV9mcmFtZWJ1ZmZlcl91bnJlZ2lzdGVyX3ByaXZhdGUoKSB0byBtZW50aW9u IHRoYXQgaXQgaXMgZGVwcmVjYXRlZD8gVGhlCj4gb3ZlcnZpZXcgZG9jIGluIGRybV9mcmFtZWJ1 ZmZlci5jIGFscmVhZHkgZXhwbGFpbnMgdGhhdCwgYnV0IEkgZ3Vlc3MKPiB0aGF0J3Mgbm90IG9i dmlvdXMgZW5vdWdoLgoKSnVzdCByZXBsYWNlIGRybV9mcmFtZWJ1ZmZlcl91bnJlZ2lzdGVyX3By aXZhdGUoKSB3aXRoCmRybV9mcmFtZWJ1ZmZlcl9yZW1vdmUoKT8KCkkgZm91bmQgbWFueSBvdGhl ciBkcml2ZXJzIHVzZSB0aGUgZm9sbG93aW5nIHR3byBmdW5jdGlvbnMgaW4gdGhlaXIKb3duIHh4 eF9mYmRldl9kZXN0cm95KCk6Cglkcm1fZnJhbWVidWZmZXJfdW5yZWdpc3Rlcl9wcml2YXRlKGZi ZGV2LT5mYik7Cglkcm1fZnJhbWVidWZmZXJfcmVtb3ZlKGZiZGV2LT5mYik7CnNvIHRoZSBmb3Jt ZXIgaXMgcmVkdW5kYW50IGFuZCBjYW4gYmUgcmVtb3ZlZCBkaXJlY3RseT8KCj4KPiBDYW4geW91 IHBscyBkbyB0aGF0IHBhdGNoPyBBbmQgcGxzIG1ha2Ugc3VyZSBpdCBhbGwgbG9va3MgcHJldHR5 IHdoZW4KPiBidWlsZGluZyB0aGUgZG9jcyB3aXRoCgpObyBwcm9ibGVtLCBpJ2xsIHNlbmQgYW5v dGhlciBwYXRjaCBsYXRlci4KClJlZ2FyZHMsClJvbmdyb25nCgo+Cj4gJCBtYWtlIGh0bWxkb2Nz Cj4KPiBUaGFua3MsIERhbmllbAo+CgoKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0t a2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFp bG1hbi9saXN0aW5mby9saW51eC1hcm0ta2VybmVsCg==