From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752049AbdHDH1p (ORCPT ); Fri, 4 Aug 2017 03:27:45 -0400 Received: from smtp.domeneshop.no ([194.63.252.55]:55404 "EHLO smtp.domeneshop.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752011AbdHDH1n (ORCPT ); Fri, 4 Aug 2017 03:27:43 -0400 Subject: Re: [PATCH v3 2/6] drm/tinydrm: generalize tinydrm_xrgb8888_to_gray8() To: David Lechner , dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org Cc: Daniel Vetter , David Airlie , Rob Herring , Mark Rutland , Sekhar Nori , Kevin Hilman , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org References: <1501799630-1650-1-git-send-email-david@lechnology.com> <1501799630-1650-3-git-send-email-david@lechnology.com> From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= Message-ID: Date: Fri, 4 Aug 2017 09:27:17 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1501799630-1650-3-git-send-email-david@lechnology.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Den 04.08.2017 00.33, skrev David Lechner: > This adds parameters for vaddr and clip to tinydrm_xrgb8888_to_gray8() to > make it more generic. > > dma_buf_{begin,end}_cpu_access() are moved out to the repaper driver. > > Signed-off-by: David Lechner > --- > drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 35 ++++++++++---------------- > drivers/gpu/drm/tinydrm/repaper.c | 21 +++++++++++++++- > include/drm/tinydrm/tinydrm-helpers.h | 3 ++- > 3 files changed, 35 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c > index 75808bb..5915ba8 100644 > --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c > +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c > @@ -185,7 +185,9 @@ EXPORT_SYMBOL(tinydrm_xrgb8888_to_rgb565); > /** > * tinydrm_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale > * @dst: 8-bit grayscale destination buffer > + * @vaddr: XRGB8888 source buffer > * @fb: DRM framebuffer > + * @clip: Clip rectangle area to copy > * > * Drm doesn't have native monochrome or grayscale support. > * Such drivers can announce the commonly supported XR24 format to userspace > @@ -199,12 +201,11 @@ EXPORT_SYMBOL(tinydrm_xrgb8888_to_rgb565); > * Returns: > * Zero on success, negative error code on failure. > */ > -int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb) > +int tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, > + struct drm_clip_rect *clip) > { > - struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); > - struct dma_buf_attachment *import_attach = cma_obj->base.import_attach; > - unsigned int x, y, pitch = fb->pitches[0]; > - int ret = 0; > + unsigned int len = (clip->x2 - clip->x1) * sizeof(u32); > + unsigned int x, y; > void *buf; > u32 *src; > > @@ -214,22 +215,16 @@ int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb) > * The cma memory is write-combined so reads are uncached. > * Speed up by fetching one line at a time. > */ > - buf = kmalloc(pitch, GFP_KERNEL); > + buf = kmalloc(len, GFP_KERNEL); > if (!buf) > return -ENOMEM; > > - if (import_attach) { > - ret = dma_buf_begin_cpu_access(import_attach->dmabuf, > - DMA_FROM_DEVICE); > - if (ret) > - goto err_free; > - } > - > - for (y = 0; y < fb->height; y++) { > - src = cma_obj->vaddr + (y * pitch); > - memcpy(buf, src, pitch); > + for (y = clip->y1; y < clip->y2; y++) { > + src = vaddr + (y * fb->pitches[0]); > + src += clip->x1; > + memcpy(buf, src, len); > src = buf; > - for (x = 0; x < fb->width; x++) { > + for (x = clip->x1; x < clip->x2; x++) { > u8 r = (*src & 0x00ff0000) >> 16; > u8 g = (*src & 0x0000ff00) >> 8; > u8 b = *src & 0x000000ff; > @@ -240,13 +235,9 @@ int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb) > } > } > > - if (import_attach) > - ret = dma_buf_end_cpu_access(import_attach->dmabuf, > - DMA_FROM_DEVICE); > -err_free: > kfree(buf); > > - return ret; > + return 0; > } > EXPORT_SYMBOL(tinydrm_xrgb8888_to_gray8); > > diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c > index 3343d3f..d34cd9b 100644 > --- a/drivers/gpu/drm/tinydrm/repaper.c > +++ b/drivers/gpu/drm/tinydrm/repaper.c > @@ -18,6 +18,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -525,11 +526,20 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, > struct drm_clip_rect *clips, > unsigned int num_clips) > { > + struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); > + struct dma_buf_attachment *import_attach = cma_obj->base.import_attach; > struct tinydrm_device *tdev = fb->dev->dev_private; > struct repaper_epd *epd = epd_from_tinydrm(tdev); > + struct drm_clip_rect clip; > u8 *buf = NULL; > int ret = 0; > > + /* repaper can't do partial updates */ > + clip.x1 = 0; > + clip.x2 = fb->width; > + clip.y1 = 0; > + clip.y2 = fb->height; > + > mutex_lock(&tdev->dirty_lock); > > if (!epd->enabled) > @@ -550,7 +560,16 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, > goto out_unlock; > } > > - ret = tinydrm_xrgb8888_to_gray8(buf, fb); > + if (import_attach) { > + ret = dma_buf_begin_cpu_access(import_attach->dmabuf, > + DMA_FROM_DEVICE); > + if (ret) > + goto out_unlock; > + } > + > + ret = tinydrm_xrgb8888_to_gray8(buf, cma_obj->vaddr, fb, &clip); > + if (import_attach) > + dma_buf_end_cpu_access(import_attach->dmabuf, DMA_FROM_DEVICE); I think we can make tinydrm_xrgb8888_to_gray8() return void like the other copy functions. We loose the error if the line buffer can't be allocated, but that's no big deal. If we can't allocate <4k of memory, then we have much bigger problems elsewhere. This simplifies returning an error from dma_buf_end_cpu_access(). The error is propagated to userspace who called the dirty ioctl. When I have switched from cma to shmem buffers backing the framebuffer (ongoing work), we don't need the line buffer copy to speed up the uncached reads from write combined memory mappings. But we will probably need it on buffers imported with PRIME, since we don't know the memory mapping. At least I don't think we can find that out. Noralf. > if (ret) > goto out_unlock; > > diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h > index a6c387f..9e13ef5 100644 > --- a/include/drm/tinydrm/tinydrm-helpers.h > +++ b/include/drm/tinydrm/tinydrm-helpers.h > @@ -43,7 +43,8 @@ void tinydrm_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb, > void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr, > struct drm_framebuffer *fb, > struct drm_clip_rect *clip, bool swap); > -int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb); > +int tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, > + struct drm_clip_rect *clip); > > struct backlight_device *tinydrm_of_find_backlight(struct device *dev); > int tinydrm_enable_backlight(struct backlight_device *backlight); From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= Subject: Re: [PATCH v3 2/6] drm/tinydrm: generalize tinydrm_xrgb8888_to_gray8() Date: Fri, 4 Aug 2017 09:27:17 +0200 Message-ID: References: <1501799630-1650-1-git-send-email-david@lechnology.com> <1501799630-1650-3-git-send-email-david@lechnology.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1501799630-1650-3-git-send-email-david@lechnology.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: David Lechner , dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org Cc: Mark Rutland , Kevin Hilman , Sekhar Nori , linux-kernel@vger.kernel.org, Rob Herring , linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org CkRlbiAwNC4wOC4yMDE3IDAwLjMzLCBza3JldiBEYXZpZCBMZWNobmVyOgo+IFRoaXMgYWRkcyBw YXJhbWV0ZXJzIGZvciB2YWRkciBhbmQgY2xpcCB0byB0aW55ZHJtX3hyZ2I4ODg4X3RvX2dyYXk4 KCkgdG8KPiBtYWtlIGl0IG1vcmUgZ2VuZXJpYy4KPgo+IGRtYV9idWZfe2JlZ2luLGVuZH1fY3B1 X2FjY2VzcygpIGFyZSBtb3ZlZCBvdXQgdG8gdGhlIHJlcGFwZXIgZHJpdmVyLgo+Cj4gU2lnbmVk LW9mZi1ieTogRGF2aWQgTGVjaG5lciA8ZGF2aWRAbGVjaG5vbG9neS5jb20+Cj4gLS0tCj4gICBk cml2ZXJzL2dwdS9kcm0vdGlueWRybS9jb3JlL3Rpbnlkcm0taGVscGVycy5jIHwgMzUgKysrKysr KysrKy0tLS0tLS0tLS0tLS0tLS0KPiAgIGRyaXZlcnMvZ3B1L2RybS90aW55ZHJtL3JlcGFwZXIu YyAgICAgICAgICAgICAgfCAyMSArKysrKysrKysrKysrKystCj4gICBpbmNsdWRlL2RybS90aW55 ZHJtL3Rpbnlkcm0taGVscGVycy5oICAgICAgICAgIHwgIDMgKystCj4gICAzIGZpbGVzIGNoYW5n ZWQsIDM1IGluc2VydGlvbnMoKyksIDI0IGRlbGV0aW9ucygtKQo+Cj4gZGlmZiAtLWdpdCBhL2Ry aXZlcnMvZ3B1L2RybS90aW55ZHJtL2NvcmUvdGlueWRybS1oZWxwZXJzLmMgYi9kcml2ZXJzL2dw dS9kcm0vdGlueWRybS9jb3JlL3Rpbnlkcm0taGVscGVycy5jCj4gaW5kZXggNzU4MDhiYi4uNTkx NWJhOCAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vdGlueWRybS9jb3JlL3Rpbnlkcm0t aGVscGVycy5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL3Rpbnlkcm0vY29yZS90aW55ZHJtLWhl bHBlcnMuYwo+IEBAIC0xODUsNyArMTg1LDkgQEAgRVhQT1JUX1NZTUJPTCh0aW55ZHJtX3hyZ2I4 ODg4X3RvX3JnYjU2NSk7Cj4gICAvKioKPiAgICAqIHRpbnlkcm1feHJnYjg4ODhfdG9fZ3JheTgg LSBDb252ZXJ0IFhSR0I4ODg4IHRvIGdyYXlzY2FsZQo+ICAgICogQGRzdDogOC1iaXQgZ3JheXNj YWxlIGRlc3RpbmF0aW9uIGJ1ZmZlcgo+ICsgKiBAdmFkZHI6IFhSR0I4ODg4IHNvdXJjZSBidWZm ZXIKPiAgICAqIEBmYjogRFJNIGZyYW1lYnVmZmVyCj4gKyAqIEBjbGlwOiBDbGlwIHJlY3Rhbmds ZSBhcmVhIHRvIGNvcHkKPiAgICAqCj4gICAgKiBEcm0gZG9lc24ndCBoYXZlIG5hdGl2ZSBtb25v Y2hyb21lIG9yIGdyYXlzY2FsZSBzdXBwb3J0Lgo+ICAgICogU3VjaCBkcml2ZXJzIGNhbiBhbm5v dW5jZSB0aGUgY29tbW9ubHkgc3VwcG9ydGVkIFhSMjQgZm9ybWF0IHRvIHVzZXJzcGFjZQo+IEBA IC0xOTksMTIgKzIwMSwxMSBAQCBFWFBPUlRfU1lNQk9MKHRpbnlkcm1feHJnYjg4ODhfdG9fcmdi NTY1KTsKPiAgICAqIFJldHVybnM6Cj4gICAgKiBaZXJvIG9uIHN1Y2Nlc3MsIG5lZ2F0aXZlIGVy cm9yIGNvZGUgb24gZmFpbHVyZS4KPiAgICAqLwo+IC1pbnQgdGlueWRybV94cmdiODg4OF90b19n cmF5OCh1OCAqZHN0LCBzdHJ1Y3QgZHJtX2ZyYW1lYnVmZmVyICpmYikKPiAraW50IHRpbnlkcm1f eHJnYjg4ODhfdG9fZ3JheTgodTggKmRzdCwgdm9pZCAqdmFkZHIsIHN0cnVjdCBkcm1fZnJhbWVi dWZmZXIgKmZiLAo+ICsJCQkgICAgICBzdHJ1Y3QgZHJtX2NsaXBfcmVjdCAqY2xpcCkKPiAgIHsK PiAtCXN0cnVjdCBkcm1fZ2VtX2NtYV9vYmplY3QgKmNtYV9vYmogPSBkcm1fZmJfY21hX2dldF9n ZW1fb2JqKGZiLCAwKTsKPiAtCXN0cnVjdCBkbWFfYnVmX2F0dGFjaG1lbnQgKmltcG9ydF9hdHRh Y2ggPSBjbWFfb2JqLT5iYXNlLmltcG9ydF9hdHRhY2g7Cj4gLQl1bnNpZ25lZCBpbnQgeCwgeSwg cGl0Y2ggPSBmYi0+cGl0Y2hlc1swXTsKPiAtCWludCByZXQgPSAwOwo+ICsJdW5zaWduZWQgaW50 IGxlbiA9IChjbGlwLT54MiAtIGNsaXAtPngxKSAqIHNpemVvZih1MzIpOwo+ICsJdW5zaWduZWQg aW50IHgsIHk7Cj4gICAJdm9pZCAqYnVmOwo+ICAgCXUzMiAqc3JjOwo+ICAgCj4gQEAgLTIxNCwy MiArMjE1LDE2IEBAIGludCB0aW55ZHJtX3hyZ2I4ODg4X3RvX2dyYXk4KHU4ICpkc3QsIHN0cnVj dCBkcm1fZnJhbWVidWZmZXIgKmZiKQo+ICAgCSAqIFRoZSBjbWEgbWVtb3J5IGlzIHdyaXRlLWNv bWJpbmVkIHNvIHJlYWRzIGFyZSB1bmNhY2hlZC4KPiAgIAkgKiBTcGVlZCB1cCBieSBmZXRjaGlu ZyBvbmUgbGluZSBhdCBhIHRpbWUuCj4gICAJICovCj4gLQlidWYgPSBrbWFsbG9jKHBpdGNoLCBH RlBfS0VSTkVMKTsKPiArCWJ1ZiA9IGttYWxsb2MobGVuLCBHRlBfS0VSTkVMKTsKPiAgIAlpZiAo IWJ1ZikKPiAgIAkJcmV0dXJuIC1FTk9NRU07Cj4gICAKPiAtCWlmIChpbXBvcnRfYXR0YWNoKSB7 Cj4gLQkJcmV0ID0gZG1hX2J1Zl9iZWdpbl9jcHVfYWNjZXNzKGltcG9ydF9hdHRhY2gtPmRtYWJ1 ZiwKPiAtCQkJCQkgICAgICAgRE1BX0ZST01fREVWSUNFKTsKPiAtCQlpZiAocmV0KQo+IC0JCQln b3RvIGVycl9mcmVlOwo+IC0JfQo+IC0KPiAtCWZvciAoeSA9IDA7IHkgPCBmYi0+aGVpZ2h0OyB5 KyspIHsKPiAtCQlzcmMgPSBjbWFfb2JqLT52YWRkciArICh5ICogcGl0Y2gpOwo+IC0JCW1lbWNw eShidWYsIHNyYywgcGl0Y2gpOwo+ICsJZm9yICh5ID0gY2xpcC0+eTE7IHkgPCBjbGlwLT55Mjsg eSsrKSB7Cj4gKwkJc3JjID0gdmFkZHIgKyAoeSAqIGZiLT5waXRjaGVzWzBdKTsKPiArCQlzcmMg Kz0gY2xpcC0+eDE7Cj4gKwkJbWVtY3B5KGJ1Ziwgc3JjLCBsZW4pOwo+ICAgCQlzcmMgPSBidWY7 Cj4gLQkJZm9yICh4ID0gMDsgeCA8IGZiLT53aWR0aDsgeCsrKSB7Cj4gKwkJZm9yICh4ID0gY2xp cC0+eDE7IHggPCBjbGlwLT54MjsgeCsrKSB7Cj4gICAJCQl1OCByID0gKCpzcmMgJiAweDAwZmYw MDAwKSA+PiAxNjsKPiAgIAkJCXU4IGcgPSAoKnNyYyAmIDB4MDAwMGZmMDApID4+IDg7Cj4gICAJ CQl1OCBiID0gICpzcmMgJiAweDAwMDAwMGZmOwo+IEBAIC0yNDAsMTMgKzIzNSw5IEBAIGludCB0 aW55ZHJtX3hyZ2I4ODg4X3RvX2dyYXk4KHU4ICpkc3QsIHN0cnVjdCBkcm1fZnJhbWVidWZmZXIg KmZiKQo+ICAgCQl9Cj4gICAJfQo+ICAgCj4gLQlpZiAoaW1wb3J0X2F0dGFjaCkKPiAtCQlyZXQg PSBkbWFfYnVmX2VuZF9jcHVfYWNjZXNzKGltcG9ydF9hdHRhY2gtPmRtYWJ1ZiwKPiAtCQkJCQkg ICAgIERNQV9GUk9NX0RFVklDRSk7Cj4gLWVycl9mcmVlOgo+ICAgCWtmcmVlKGJ1Zik7Cj4gICAK PiAtCXJldHVybiByZXQ7Cj4gKwlyZXR1cm4gMDsKPiAgIH0KPiAgIEVYUE9SVF9TWU1CT0wodGlu eWRybV94cmdiODg4OF90b19ncmF5OCk7Cj4gICAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUv ZHJtL3Rpbnlkcm0vcmVwYXBlci5jIGIvZHJpdmVycy9ncHUvZHJtL3Rpbnlkcm0vcmVwYXBlci5j Cj4gaW5kZXggMzM0M2QzZi4uZDM0Y2Q5YiAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0v dGlueWRybS9yZXBhcGVyLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vdGlueWRybS9yZXBhcGVy LmMKPiBAQCAtMTgsNiArMTgsNyBAQAo+ICAgICovCj4gICAKPiAgICNpbmNsdWRlIDxsaW51eC9k ZWxheS5oPgo+ICsjaW5jbHVkZSA8bGludXgvZG1hLWJ1Zi5oPgo+ICAgI2luY2x1ZGUgPGxpbnV4 L2dwaW8vY29uc3VtZXIuaD4KPiAgICNpbmNsdWRlIDxsaW51eC9tb2R1bGUuaD4KPiAgICNpbmNs dWRlIDxsaW51eC9vZl9kZXZpY2UuaD4KPiBAQCAtNTI1LDExICs1MjYsMjAgQEAgc3RhdGljIGlu dCByZXBhcGVyX2ZiX2RpcnR5KHN0cnVjdCBkcm1fZnJhbWVidWZmZXIgKmZiLAo+ICAgCQkJICAg IHN0cnVjdCBkcm1fY2xpcF9yZWN0ICpjbGlwcywKPiAgIAkJCSAgICB1bnNpZ25lZCBpbnQgbnVt X2NsaXBzKQo+ICAgewo+ICsJc3RydWN0IGRybV9nZW1fY21hX29iamVjdCAqY21hX29iaiA9IGRy bV9mYl9jbWFfZ2V0X2dlbV9vYmooZmIsIDApOwo+ICsJc3RydWN0IGRtYV9idWZfYXR0YWNobWVu dCAqaW1wb3J0X2F0dGFjaCA9IGNtYV9vYmotPmJhc2UuaW1wb3J0X2F0dGFjaDsKPiAgIAlzdHJ1 Y3QgdGlueWRybV9kZXZpY2UgKnRkZXYgPSBmYi0+ZGV2LT5kZXZfcHJpdmF0ZTsKPiAgIAlzdHJ1 Y3QgcmVwYXBlcl9lcGQgKmVwZCA9IGVwZF9mcm9tX3Rpbnlkcm0odGRldik7Cj4gKwlzdHJ1Y3Qg ZHJtX2NsaXBfcmVjdCBjbGlwOwo+ICAgCXU4ICpidWYgPSBOVUxMOwo+ICAgCWludCByZXQgPSAw Owo+ICAgCj4gKwkvKiByZXBhcGVyIGNhbid0IGRvIHBhcnRpYWwgdXBkYXRlcyAqLwo+ICsJY2xp cC54MSA9IDA7Cj4gKwljbGlwLngyID0gZmItPndpZHRoOwo+ICsJY2xpcC55MSA9IDA7Cj4gKwlj bGlwLnkyID0gZmItPmhlaWdodDsKPiArCj4gICAJbXV0ZXhfbG9jaygmdGRldi0+ZGlydHlfbG9j ayk7Cj4gICAKPiAgIAlpZiAoIWVwZC0+ZW5hYmxlZCkKPiBAQCAtNTUwLDcgKzU2MCwxNiBAQCBz dGF0aWMgaW50IHJlcGFwZXJfZmJfZGlydHkoc3RydWN0IGRybV9mcmFtZWJ1ZmZlciAqZmIsCj4g ICAJCWdvdG8gb3V0X3VubG9jazsKPiAgIAl9Cj4gICAKPiAtCXJldCA9IHRpbnlkcm1feHJnYjg4 ODhfdG9fZ3JheTgoYnVmLCBmYik7Cj4gKwlpZiAoaW1wb3J0X2F0dGFjaCkgewo+ICsJCXJldCA9 IGRtYV9idWZfYmVnaW5fY3B1X2FjY2VzcyhpbXBvcnRfYXR0YWNoLT5kbWFidWYsCj4gKwkJCQkJ ICAgICAgIERNQV9GUk9NX0RFVklDRSk7Cj4gKwkJaWYgKHJldCkKPiArCQkJZ290byBvdXRfdW5s b2NrOwo+ICsJfQo+ICsKPiArCXJldCA9IHRpbnlkcm1feHJnYjg4ODhfdG9fZ3JheTgoYnVmLCBj bWFfb2JqLT52YWRkciwgZmIsICZjbGlwKTsKPiArCWlmIChpbXBvcnRfYXR0YWNoKQo+ICsJCWRt YV9idWZfZW5kX2NwdV9hY2Nlc3MoaW1wb3J0X2F0dGFjaC0+ZG1hYnVmLCBETUFfRlJPTV9ERVZJ Q0UpOwoKSSB0aGluayB3ZSBjYW4gbWFrZSB0aW55ZHJtX3hyZ2I4ODg4X3RvX2dyYXk4KCkgcmV0 dXJuIHZvaWQgbGlrZSB0aGUKb3RoZXIgY29weSBmdW5jdGlvbnMuIFdlIGxvb3NlIHRoZSBlcnJv ciBpZiB0aGUgbGluZSBidWZmZXIgY2FuJ3QgYmUKYWxsb2NhdGVkLCBidXQgdGhhdCdzIG5vIGJp ZyBkZWFsLiBJZiB3ZSBjYW4ndCBhbGxvY2F0ZSA8NGsgb2YgbWVtb3J5LAp0aGVuIHdlIGhhdmUg bXVjaCBiaWdnZXIgcHJvYmxlbXMgZWxzZXdoZXJlLgpUaGlzIHNpbXBsaWZpZXMgcmV0dXJuaW5n IGFuIGVycm9yIGZyb20gZG1hX2J1Zl9lbmRfY3B1X2FjY2VzcygpLgpUaGUgZXJyb3IgaXMgcHJv cGFnYXRlZCB0byB1c2Vyc3BhY2Ugd2hvIGNhbGxlZCB0aGUgZGlydHkgaW9jdGwuCgpXaGVuIEkg aGF2ZSBzd2l0Y2hlZCBmcm9tIGNtYSB0byBzaG1lbSBidWZmZXJzIGJhY2tpbmcgdGhlIGZyYW1l YnVmZmVyCihvbmdvaW5nIHdvcmspLCB3ZSBkb24ndCBuZWVkIHRoZSBsaW5lIGJ1ZmZlciBjb3B5 IHRvIHNwZWVkIHVwIHRoZQp1bmNhY2hlZCByZWFkcyBmcm9tIHdyaXRlIGNvbWJpbmVkIG1lbW9y eSBtYXBwaW5ncy4gQnV0IHdlIHdpbGwKcHJvYmFibHkgbmVlZCBpdCBvbiBidWZmZXJzIGltcG9y dGVkIHdpdGggUFJJTUUsIHNpbmNlIHdlIGRvbid0IGtub3cKdGhlIG1lbW9yeSBtYXBwaW5nLiBB dCBsZWFzdCBJIGRvbid0IHRoaW5rIHdlIGNhbiBmaW5kIHRoYXQgb3V0LgoKTm9yYWxmLgoKPiAg IAlpZiAocmV0KQo+ICAgCQlnb3RvIG91dF91bmxvY2s7Cj4gICAKPiBkaWZmIC0tZ2l0IGEvaW5j bHVkZS9kcm0vdGlueWRybS90aW55ZHJtLWhlbHBlcnMuaCBiL2luY2x1ZGUvZHJtL3Rpbnlkcm0v dGlueWRybS1oZWxwZXJzLmgKPiBpbmRleCBhNmMzODdmLi45ZTEzZWY1IDEwMDY0NAo+IC0tLSBh L2luY2x1ZGUvZHJtL3Rpbnlkcm0vdGlueWRybS1oZWxwZXJzLmgKPiArKysgYi9pbmNsdWRlL2Ry bS90aW55ZHJtL3Rpbnlkcm0taGVscGVycy5oCj4gQEAgLTQzLDcgKzQzLDggQEAgdm9pZCB0aW55 ZHJtX3N3YWIxNih1MTYgKmRzdCwgdm9pZCAqdmFkZHIsIHN0cnVjdCBkcm1fZnJhbWVidWZmZXIg KmZiLAo+ICAgdm9pZCB0aW55ZHJtX3hyZ2I4ODg4X3RvX3JnYjU2NSh1MTYgKmRzdCwgdm9pZCAq dmFkZHIsCj4gICAJCQkJc3RydWN0IGRybV9mcmFtZWJ1ZmZlciAqZmIsCj4gICAJCQkJc3RydWN0 IGRybV9jbGlwX3JlY3QgKmNsaXAsIGJvb2wgc3dhcCk7Cj4gLWludCB0aW55ZHJtX3hyZ2I4ODg4 X3RvX2dyYXk4KHU4ICpkc3QsIHN0cnVjdCBkcm1fZnJhbWVidWZmZXIgKmZiKTsKPiAraW50IHRp bnlkcm1feHJnYjg4ODhfdG9fZ3JheTgodTggKmRzdCwgdm9pZCAqdmFkZHIsIHN0cnVjdCBkcm1f ZnJhbWVidWZmZXIgKmZiLAo+ICsJCQkgICAgICBzdHJ1Y3QgZHJtX2NsaXBfcmVjdCAqY2xpcCk7 Cj4gICAKPiAgIHN0cnVjdCBiYWNrbGlnaHRfZGV2aWNlICp0aW55ZHJtX29mX2ZpbmRfYmFja2xp Z2h0KHN0cnVjdCBkZXZpY2UgKmRldik7Cj4gICBpbnQgdGlueWRybV9lbmFibGVfYmFja2xpZ2h0 KHN0cnVjdCBiYWNrbGlnaHRfZGV2aWNlICpiYWNrbGlnaHQpOwoKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmkt ZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3Jn L21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: noralf@tronnes.org (=?UTF-8?Q?Noralf_Tr=c3=b8nnes?=) Date: Fri, 4 Aug 2017 09:27:17 +0200 Subject: [PATCH v3 2/6] drm/tinydrm: generalize tinydrm_xrgb8888_to_gray8() In-Reply-To: <1501799630-1650-3-git-send-email-david@lechnology.com> References: <1501799630-1650-1-git-send-email-david@lechnology.com> <1501799630-1650-3-git-send-email-david@lechnology.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Den 04.08.2017 00.33, skrev David Lechner: > This adds parameters for vaddr and clip to tinydrm_xrgb8888_to_gray8() to > make it more generic. > > dma_buf_{begin,end}_cpu_access() are moved out to the repaper driver. > > Signed-off-by: David Lechner > --- > drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 35 ++++++++++---------------- > drivers/gpu/drm/tinydrm/repaper.c | 21 +++++++++++++++- > include/drm/tinydrm/tinydrm-helpers.h | 3 ++- > 3 files changed, 35 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c > index 75808bb..5915ba8 100644 > --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c > +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c > @@ -185,7 +185,9 @@ EXPORT_SYMBOL(tinydrm_xrgb8888_to_rgb565); > /** > * tinydrm_xrgb8888_to_gray8 - Convert XRGB8888 to grayscale > * @dst: 8-bit grayscale destination buffer > + * @vaddr: XRGB8888 source buffer > * @fb: DRM framebuffer > + * @clip: Clip rectangle area to copy > * > * Drm doesn't have native monochrome or grayscale support. > * Such drivers can announce the commonly supported XR24 format to userspace > @@ -199,12 +201,11 @@ EXPORT_SYMBOL(tinydrm_xrgb8888_to_rgb565); > * Returns: > * Zero on success, negative error code on failure. > */ > -int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb) > +int tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, > + struct drm_clip_rect *clip) > { > - struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); > - struct dma_buf_attachment *import_attach = cma_obj->base.import_attach; > - unsigned int x, y, pitch = fb->pitches[0]; > - int ret = 0; > + unsigned int len = (clip->x2 - clip->x1) * sizeof(u32); > + unsigned int x, y; > void *buf; > u32 *src; > > @@ -214,22 +215,16 @@ int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb) > * The cma memory is write-combined so reads are uncached. > * Speed up by fetching one line at a time. > */ > - buf = kmalloc(pitch, GFP_KERNEL); > + buf = kmalloc(len, GFP_KERNEL); > if (!buf) > return -ENOMEM; > > - if (import_attach) { > - ret = dma_buf_begin_cpu_access(import_attach->dmabuf, > - DMA_FROM_DEVICE); > - if (ret) > - goto err_free; > - } > - > - for (y = 0; y < fb->height; y++) { > - src = cma_obj->vaddr + (y * pitch); > - memcpy(buf, src, pitch); > + for (y = clip->y1; y < clip->y2; y++) { > + src = vaddr + (y * fb->pitches[0]); > + src += clip->x1; > + memcpy(buf, src, len); > src = buf; > - for (x = 0; x < fb->width; x++) { > + for (x = clip->x1; x < clip->x2; x++) { > u8 r = (*src & 0x00ff0000) >> 16; > u8 g = (*src & 0x0000ff00) >> 8; > u8 b = *src & 0x000000ff; > @@ -240,13 +235,9 @@ int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb) > } > } > > - if (import_attach) > - ret = dma_buf_end_cpu_access(import_attach->dmabuf, > - DMA_FROM_DEVICE); > -err_free: > kfree(buf); > > - return ret; > + return 0; > } > EXPORT_SYMBOL(tinydrm_xrgb8888_to_gray8); > > diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c > index 3343d3f..d34cd9b 100644 > --- a/drivers/gpu/drm/tinydrm/repaper.c > +++ b/drivers/gpu/drm/tinydrm/repaper.c > @@ -18,6 +18,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -525,11 +526,20 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, > struct drm_clip_rect *clips, > unsigned int num_clips) > { > + struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0); > + struct dma_buf_attachment *import_attach = cma_obj->base.import_attach; > struct tinydrm_device *tdev = fb->dev->dev_private; > struct repaper_epd *epd = epd_from_tinydrm(tdev); > + struct drm_clip_rect clip; > u8 *buf = NULL; > int ret = 0; > > + /* repaper can't do partial updates */ > + clip.x1 = 0; > + clip.x2 = fb->width; > + clip.y1 = 0; > + clip.y2 = fb->height; > + > mutex_lock(&tdev->dirty_lock); > > if (!epd->enabled) > @@ -550,7 +560,16 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, > goto out_unlock; > } > > - ret = tinydrm_xrgb8888_to_gray8(buf, fb); > + if (import_attach) { > + ret = dma_buf_begin_cpu_access(import_attach->dmabuf, > + DMA_FROM_DEVICE); > + if (ret) > + goto out_unlock; > + } > + > + ret = tinydrm_xrgb8888_to_gray8(buf, cma_obj->vaddr, fb, &clip); > + if (import_attach) > + dma_buf_end_cpu_access(import_attach->dmabuf, DMA_FROM_DEVICE); I think we can make tinydrm_xrgb8888_to_gray8() return void like the other copy functions. We loose the error if the line buffer can't be allocated, but that's no big deal. If we can't allocate <4k of memory, then we have much bigger problems elsewhere. This simplifies returning an error from dma_buf_end_cpu_access(). The error is propagated to userspace who called the dirty ioctl. When I have switched from cma to shmem buffers backing the framebuffer (ongoing work), we don't need the line buffer copy to speed up the uncached reads from write combined memory mappings. But we will probably need it on buffers imported with PRIME, since we don't know the memory mapping. At least I don't think we can find that out. Noralf. > if (ret) > goto out_unlock; > > diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h > index a6c387f..9e13ef5 100644 > --- a/include/drm/tinydrm/tinydrm-helpers.h > +++ b/include/drm/tinydrm/tinydrm-helpers.h > @@ -43,7 +43,8 @@ void tinydrm_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb, > void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr, > struct drm_framebuffer *fb, > struct drm_clip_rect *clip, bool swap); > -int tinydrm_xrgb8888_to_gray8(u8 *dst, struct drm_framebuffer *fb); > +int tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb, > + struct drm_clip_rect *clip); > > struct backlight_device *tinydrm_of_find_backlight(struct device *dev); > int tinydrm_enable_backlight(struct backlight_device *backlight);