From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sam Ravnborg Date: Mon, 14 Oct 2019 20:30:07 +0000 Subject: Re: [PATCH v2 05/15] drm/fbconv: Add DRM <-> fbdev pixel-format conversion Message-Id: <20191014203007.GA4373@ravnborg.org> List-Id: References: <20191014140416.28517-1-tzimmermann@suse.de> <20191014140416.28517-6-tzimmermann@suse.de> In-Reply-To: <20191014140416.28517-6-tzimmermann@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Thomas Zimmermann Cc: linux-fbdev@vger.kernel.org, b.zolnierkie@samsung.com, airlied@linux.ie, gregkh@linuxfoundation.org, michel@daenzer.net, corbet@lwn.net, malat@debian.org, dri-devel@lists.freedesktop.org, sean@poorly.run Hi Thomas. On Mon, Oct 14, 2019 at 04:04:06PM +0200, Thomas Zimmermann wrote: > DRM uses FOURCC constants to describe pixel formats, fbdev uses a > per-component bitfield structure. The functions in this patch convert > between the two. > A few nits below. > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/drm_fbconv_helper.c | 435 ++++++++++++++++++++++++++++ > include/drm/drm_fbconv_helper.h | 23 ++ > 2 files changed, 458 insertions(+) > create mode 100644 include/drm/drm_fbconv_helper.h > > diff --git a/drivers/gpu/drm/drm_fbconv_helper.c b/drivers/gpu/drm/drm_fbconv_helper.c > index 0cb46d2c98c3..af45358a156a 100644 > --- a/drivers/gpu/drm/drm_fbconv_helper.c > +++ b/drivers/gpu/drm/drm_fbconv_helper.c > @@ -1 +1,436 @@ > // SPDX-License-Identifier: GPL-2.0-or-later > + > +#include > + > +#include after So we in this way pick the more general include file first. > + > +struct format_map { > + bool (*is_format)(const struct fb_var_screeninfo *fb_var); > + uint32_t format; > +}; We are in the kernel - where I think u32 is preferred over the longer uint32_t. If I grep in drm/* then they seems be be equally popular, so feel free to ignore this comment. > +static void set_fb_bitfield(struct fb_bitfield *bits, __u32 offset, > + __u32 length) This is not uapi - so u32 is preferred. Both comments apply to the whole file. I did not see that this was wired into the kernel-doc in Documentation/ but maybe I just missed it. With my comments considered you can add: Acked-by: Sam Ravnborg All code looks sane, but as I have not grasped the bigger picture this can hardly be a review. Sam From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sam Ravnborg Subject: Re: [PATCH v2 05/15] drm/fbconv: Add DRM <-> fbdev pixel-format conversion Date: Mon, 14 Oct 2019 22:30:07 +0200 Message-ID: <20191014203007.GA4373@ravnborg.org> References: <20191014140416.28517-1-tzimmermann@suse.de> <20191014140416.28517-6-tzimmermann@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from asavdk4.altibox.net (asavdk4.altibox.net [109.247.116.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id EE3316E59D for ; Mon, 14 Oct 2019 20:30:14 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20191014140416.28517-6-tzimmermann@suse.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Thomas Zimmermann Cc: linux-fbdev@vger.kernel.org, b.zolnierkie@samsung.com, airlied@linux.ie, gregkh@linuxfoundation.org, michel@daenzer.net, corbet@lwn.net, malat@debian.org, dri-devel@lists.freedesktop.org, sean@poorly.run List-Id: dri-devel@lists.freedesktop.org SGkgVGhvbWFzLgoKT24gTW9uLCBPY3QgMTQsIDIwMTkgYXQgMDQ6MDQ6MDZQTSArMDIwMCwgVGhv bWFzIFppbW1lcm1hbm4gd3JvdGU6Cj4gRFJNIHVzZXMgRk9VUkNDIGNvbnN0YW50cyB0byBkZXNj cmliZSBwaXhlbCBmb3JtYXRzLCBmYmRldiB1c2VzIGEKPiBwZXItY29tcG9uZW50IGJpdGZpZWxk IHN0cnVjdHVyZS4gVGhlIGZ1bmN0aW9ucyBpbiB0aGlzIHBhdGNoIGNvbnZlcnQKPiBiZXR3ZWVu IHRoZSB0d28uCj4gCgpBIGZldyBuaXRzIGJlbG93LgoKCj4gU2lnbmVkLW9mZi1ieTogVGhvbWFz IFppbW1lcm1hbm4gPHR6aW1tZXJtYW5uQHN1c2UuZGU+Cj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2Ry bS9kcm1fZmJjb252X2hlbHBlci5jIHwgNDM1ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysK PiAgaW5jbHVkZS9kcm0vZHJtX2ZiY29udl9oZWxwZXIuaCAgICAgfCAgMjMgKysKPiAgMiBmaWxl cyBjaGFuZ2VkLCA0NTggaW5zZXJ0aW9ucygrKQo+ICBjcmVhdGUgbW9kZSAxMDA2NDQgaW5jbHVk ZS9kcm0vZHJtX2ZiY29udl9oZWxwZXIuaAo+IAo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9k cm0vZHJtX2ZiY29udl9oZWxwZXIuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fZmJjb252X2hlbHBl ci5jCj4gaW5kZXggMGNiNDZkMmM5OGMzLi5hZjQ1MzU4YTE1NmEgMTAwNjQ0Cj4gLS0tIGEvZHJp dmVycy9ncHUvZHJtL2RybV9mYmNvbnZfaGVscGVyLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0v ZHJtX2ZiY29udl9oZWxwZXIuYwo+IEBAIC0xICsxLDQzNiBAQAo+ICAvLyBTUERYLUxpY2Vuc2Ut SWRlbnRpZmllcjogR1BMLTIuMC1vci1sYXRlcgo+ICsKPiArI2luY2x1ZGUgPGFzbS9ieXRlb3Jk ZXIuaD4KPiArCj4gKyNpbmNsdWRlIDxsaW51eC9mYi5oPgoKPGFzbS8qPiBhZnRlciA8bGludXgv Kj4KU28gd2UgaW4gdGhpcyB3YXkgcGljayB0aGUgbW9yZSBnZW5lcmFsIGluY2x1ZGUgZmlsZSBm aXJzdC4KCj4gKwo+ICtzdHJ1Y3QgZm9ybWF0X21hcCB7Cj4gKwlib29sICgqaXNfZm9ybWF0KShj b25zdCBzdHJ1Y3QgZmJfdmFyX3NjcmVlbmluZm8gKmZiX3Zhcik7Cj4gKwl1aW50MzJfdCBmb3Jt YXQ7Cj4gK307CldlIGFyZSBpbiB0aGUga2VybmVsIC0gd2hlcmUgSSB0aGluayB1MzIgaXMgcHJl ZmVycmVkIG92ZXIgdGhlIGxvbmdlcgp1aW50MzJfdC4KSWYgSSBncmVwIGluIGRybS8qIHRoZW4g dGhleSBzZWVtcyBiZSBiZSBlcXVhbGx5IHBvcHVsYXIsIHNvIGZlZWwgZnJlZQp0byBpZ25vcmUg dGhpcyBjb21tZW50LgoKCj4gK3N0YXRpYyB2b2lkIHNldF9mYl9iaXRmaWVsZChzdHJ1Y3QgZmJf Yml0ZmllbGQgKmJpdHMsIF9fdTMyIG9mZnNldCwKPiArCQkJICAgIF9fdTMyIGxlbmd0aCkKClRo aXMgaXMgbm90IHVhcGkgLSBzbyB1MzIgaXMgcHJlZmVycmVkLgoKQm90aCBjb21tZW50cyBhcHBs eSB0byB0aGUgd2hvbGUgZmlsZS4KCkkgZGlkIG5vdCBzZWUgdGhhdCB0aGlzIHdhcyB3aXJlZCBp bnRvIHRoZSBrZXJuZWwtZG9jIGluIERvY3VtZW50YXRpb24vCmJ1dCBtYXliZSBJIGp1c3QgbWlz c2VkIGl0LgoKV2l0aCBteSBjb21tZW50cyBjb25zaWRlcmVkIHlvdSBjYW4gYWRkOgpBY2tlZC1i eTogU2FtIFJhdm5ib3JnIDxzYW1AcmF2bmJvcmcub3JnPgoKQWxsIGNvZGUgbG9va3Mgc2FuZSwg YnV0IGFzIEkgaGF2ZSBub3QgZ3Jhc3BlZCB0aGUgYmlnZ2VyIHBpY3R1cmUKdGhpcyBjYW4gaGFy ZGx5IGJlIGEgcmV2aWV3LgoKCVNhbQpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVl ZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5m by9kcmktZGV2ZWw=