From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-arm-kernel@lists.infradead.org,
David Airlie <airlied@linux.ie>, Chen-Yu Tsai <wens@csie.org>,
Daniel Vetter <daniel.vetter@intel.com>,
Gustavo Padovan <gustavo@padovan.org>,
Sean Paul <seanpaul@chromium.org>
Subject: Re: [PATCH 06/10] drm/sun4i: Move and extend format-related helpers and tables
Date: Tue, 27 Mar 2018 10:27:44 +0200 [thread overview]
Message-ID: <1522139264.1110.28.camel@bootlin.com> (raw)
In-Reply-To: <20180323101338.z422gpxcsdre5hgj@flea>
[-- Attachment #1: Type: text/plain, Size: 12631 bytes --]
Hi,
On Fri, 2018-03-23 at 11:13 +0100, Maxime Ripard wrote:
> On Wed, Mar 21, 2018 at 04:29:00PM +0100, Paul Kocialkowski wrote:
> > This moves the various helpers and tables related to format
> > detection
> > and conversion to a dedicated file, while adding a bunch of new
> > helpers
> > (especially for YUV and tiling support) along the way.
>
> The addition of new helpers should be in a separate patch.
Fair enough.
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> > drivers/gpu/drm/sun4i/Makefile | 1 +
> > drivers/gpu/drm/sun4i/sun4i_backend.c | 54 ++--------
> > drivers/gpu/drm/sun4i/sun4i_format.c | 193
> > ++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/sun4i/sun4i_format.h | 35 ++++++
> > 4 files changed, 235 insertions(+), 48 deletions(-)
> > create mode 100644 drivers/gpu/drm/sun4i/sun4i_format.c
> > create mode 100644 drivers/gpu/drm/sun4i/sun4i_format.h
> >
> > diff --git a/drivers/gpu/drm/sun4i/Makefile
> > b/drivers/gpu/drm/sun4i/Makefile
> > index 582607c0c488..c89c42ff803e 100644
> > --- a/drivers/gpu/drm/sun4i/Makefile
> > +++ b/drivers/gpu/drm/sun4i/Makefile
> > @@ -4,6 +4,7 @@ sun4i-frontend-y += sun4i_frontend.o
> >
> > sun4i-drm-y += sun4i_drv.o
> > sun4i-drm-y += sun4i_framebuffer.o
> > +sun4i-drm-y += sun4i_format.o
> >
> > sun4i-drm-hdmi-y += sun4i_hdmi_ddc_clk.o
> > sun4i-drm-hdmi-y += sun4i_hdmi_enc.o
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c
> > b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > index 1fad0714c70e..e8af9f3cf20b 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > @@ -29,6 +29,7 @@
> > #include "sun4i_drv.h"
> > #include "sun4i_frontend.h"
> > #include "sun4i_layer.h"
> > +#include "sun4i_format.h"
> > #include "sunxi_engine.h"
> >
> > struct sun4i_backend_quirks {
> > @@ -36,50 +37,6 @@ struct sun4i_backend_quirks {
> > bool needs_output_muxing;
> > };
> >
> > -static const u32 sunxi_rgb2yuv_coef[12] = {
> > - 0x00000107, 0x00000204, 0x00000064, 0x00000108,
> > - 0x00003f69, 0x00003ed6, 0x000001c1, 0x00000808,
> > - 0x000001c1, 0x00003e88, 0x00003fb8, 0x00000808
> > -};
> > -
> > -static const u32 sunxi_bt601_yuv2rgb_coef[12] = {
> > - 0x000004a7, 0x00001e6f, 0x00001cbf, 0x00000877,
> > - 0x000004a7, 0x00000000, 0x00000662, 0x00003211,
> > - 0x000004a7, 0x00000812, 0x00000000, 0x00002eb1,
> > -};
> > -
> > -static inline bool sun4i_backend_format_is_planar_yuv(uint32_t
> > format)
> > -{
> > - switch (format) {
> > - case DRM_FORMAT_YUV411:
> > - case DRM_FORMAT_YUV422:
> > - case DRM_FORMAT_YUV444:
> > - return true;
> > - default:
> > - return false;
> > - }
> > -}
> > -
> > -static inline bool sun4i_backend_format_is_packed_yuv422(uint32_t
> > format)
> > -{
> > - switch (format) {
> > - case DRM_FORMAT_YUYV:
> > - case DRM_FORMAT_YVYU:
> > - case DRM_FORMAT_UYVY:
> > - case DRM_FORMAT_VYUY:
> > - return true;
> > -
> > - default:
> > - return false;
> > - }
> > -}
> > -
> > -static inline bool sun4i_backend_format_is_yuv(uint32_t format)
> > -{
> > - return sun4i_backend_format_is_planar_yuv(format) ||
> > - sun4i_backend_format_is_packed_yuv422(format);
> > -}
> > -
> > static void sun4i_backend_apply_color_correction(struct
> > sunxi_engine *engine)
> > {
> > int i;
> > @@ -259,7 +216,7 @@ static int
> > sun4i_backend_update_yuv_format(struct sun4i_backend *backend,
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_YUVEN,
> > SUN4I_BACKEND_ATTCTL_REG0_LAY_YUVEN);
> >
> > - if (sun4i_backend_format_is_packed_yuv422(format))
> > + if (sun4i_format_is_packed_yuv422(format))
> > val |= SUN4I_BACKEND_IYUVCTL_FBFMT_PACKED_YUV422;
> > else
> > DRM_DEBUG_DRIVER("Unknown YUV format\n");
> > @@ -310,7 +267,7 @@ int sun4i_backend_update_layer_formats(struct
> > sun4i_backend *backend,
> > DRM_DEBUG_DRIVER("Switching display backend interlaced mode
> > %s\n",
> > interlaced ? "on" : "off");
> >
> > - if (sun4i_backend_format_is_yuv(fb->format->format))
> > + if (sun4i_format_is_yuv(fb->format->format))
> > return sun4i_backend_update_yuv_format(backend,
> > layer, plane);
> >
> > ret = sun4i_backend_drm_format_to_layer(fb->format->format,
> > &val);
> > @@ -404,7 +361,7 @@ int sun4i_backend_update_layer_buffer(struct
> > sun4i_backend *backend,
> > */
> > paddr -= PHYS_OFFSET;
> >
> > - if (sun4i_backend_format_is_yuv(fb->format->format))
> > + if (sun4i_format_is_yuv(fb->format->format))
> > return sun4i_backend_update_yuv_buffer(backend, fb,
> > paddr);
> >
> > /* Write the 32 lower bits of the address (in bits) */
> > @@ -549,10 +506,11 @@ static int sun4i_backend_atomic_check(struct
> > sunxi_engine *engine,
> > DRM_DEBUG_DRIVER("Plane FB format is %s\n",
> > drm_get_format_name(fb->format-
> > >format,
> > &format_name))
> > ;
> > +
> > if (fb->format->has_alpha)
> > num_alpha_planes++;
> >
> > - if (sun4i_backend_format_is_yuv(fb->format-
> > >format)) {
> > + if (sun4i_format_is_yuv(fb->format->format)) {
> > DRM_DEBUG_DRIVER("Plane FB format is
> > YUV\n");
> > num_yuv_planes++;
> > }
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_format.c
> > b/drivers/gpu/drm/sun4i/sun4i_format.c
> > new file mode 100644
> > index 000000000000..3830767e6d5b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sun4i/sun4i_format.c
> > @@ -0,0 +1,193 @@
> > +/*
> > + * Copyright (C) 2015-2018 Free Electrons/Bootlin
> > + *
> > + * Maxime Ripard <maxime.ripard@bootlin.com>
> > + * Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + */
> > +
> > +#include <linux/component.h>
> > +#include <linux/kfifo.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/of_reserved_mem.h>
> > +
> > +#include <drm/drmP.h>
> > +#include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_fb_cma_helper.h>
> > +#include <drm/drm_gem_cma_helper.h>
> > +#include <drm/drm_fb_helper.h>
> > +#include <drm/drm_of.h>
> > +
> > +#include "sun4i_drv.h"
> > +#include "sun4i_format.h"
>
> I guess a lot of these headers are not needed.
Yes indeed, I'll cleanup the list in v2.
> > +
> > +const u32 sunxi_rgb2yuv_coef[12] = {
> > + 0x00000107, 0x00000204, 0x00000064, 0x00000108,
> > + 0x00003f69, 0x00003ed6, 0x000001c1, 0x00000808,
> > + 0x000001c1, 0x00003e88, 0x00003fb8, 0x00000808
> > +};
> > +
> > +const u32 sunxi_bt601_yuv2rgb_coef[12] = {
> > + 0x000004a7, 0x00001e6f, 0x00001cbf, 0x00000877,
> > + 0x000004a7, 0x00000000, 0x00000662, 0x00003211,
> > + 0x000004a7, 0x00000812, 0x00000000, 0x00002eb1,
> > +};
> > +
> > +bool sun4i_format_is_rgb(uint32_t format)
> > +{
> > + switch (format) {
> > + case DRM_FORMAT_ARGB4444:
> > + case DRM_FORMAT_RGBA4444:
> > + case DRM_FORMAT_ARGB1555:
> > + case DRM_FORMAT_RGBA5551:
> > + case DRM_FORMAT_RGB888:
> > + case DRM_FORMAT_RGB565:
> > + case DRM_FORMAT_XRGB8888:
> > + case DRM_FORMAT_ARGB8888:
> > + return true;
> > +
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +bool sun4i_format_is_yuv(uint32_t format)
> > +{
> > + return sun4i_format_is_yuv411(format) ||
> > + sun4i_format_is_yuv420(format) ||
> > + sun4i_format_is_yuv422(format) ||
> > + sun4i_format_is_yuv444(format);
> > +}
> > +
> > +bool sun4i_format_is_yuv411(uint32_t format)
> > +{
> > + switch (format) {
> > + case DRM_FORMAT_YUV411:
> > + case DRM_FORMAT_YVU411:
> > + return true;
> > +
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +bool sun4i_format_is_yuv420(uint32_t format)
> > +{
> > + switch (format) {
> > + case DRM_FORMAT_NV12:
> > + case DRM_FORMAT_NV21:
> > + case DRM_FORMAT_YUV420:
> > + case DRM_FORMAT_YVU420:
> > + return true;
> > +
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +bool sun4i_format_is_yuv422(uint32_t format)
> > +{
> > + switch (format) {
> > + case DRM_FORMAT_YUYV:
> > + case DRM_FORMAT_YVYU:
> > + case DRM_FORMAT_UYVY:
> > + case DRM_FORMAT_VYUY:
> > + case DRM_FORMAT_NV16:
> > + case DRM_FORMAT_NV61:
> > + case DRM_FORMAT_YUV422:
> > + case DRM_FORMAT_YVU422:
> > + return true;
> > +
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +bool sun4i_format_is_yuv444(uint32_t format)
> > +{
> > + switch (format) {
> > + case DRM_FORMAT_YUV444:
> > + case DRM_FORMAT_YVU444:
> > + return true;
> > +
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +bool sun4i_format_is_packed(uint32_t format)
> > +{
> > + if (sun4i_format_is_rgb(format))
> > + return true;
> > +
> > + switch (format) {
> > + case DRM_FORMAT_YUYV:
> > + case DRM_FORMAT_YVYU:
> > + case DRM_FORMAT_UYVY:
> > + case DRM_FORMAT_VYUY:
> > + return true;
> > +
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +bool sun4i_format_is_semiplanar(uint32_t format)
> > +{
> > + switch (format) {
> > + case DRM_FORMAT_NV12:
> > + case DRM_FORMAT_NV21:
> > + case DRM_FORMAT_NV16:
> > + case DRM_FORMAT_NV61:
> > + case DRM_FORMAT_NV24:
> > + case DRM_FORMAT_NV42:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +bool sun4i_format_is_planar(uint32_t format)
> > +{
> > + switch (format) {
> > + case DRM_FORMAT_YUV410:
> > + case DRM_FORMAT_YVU410:
> > + case DRM_FORMAT_YUV411:
> > + case DRM_FORMAT_YVU411:
> > + case DRM_FORMAT_YUV420:
> > + case DRM_FORMAT_YVU420:
> > + case DRM_FORMAT_YUV422:
> > + case DRM_FORMAT_YVU422:
> > + case DRM_FORMAT_YUV444:
> > + case DRM_FORMAT_YVU444:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +bool sun4i_format_supports_tiling(uint32_t format)
> > +{
> > + switch (format) {
> > + /* Semiplanar */
> > + case DRM_FORMAT_NV12:
> > + case DRM_FORMAT_NV21:
> > + case DRM_FORMAT_NV16:
> > + case DRM_FORMAT_NV61:
> > + /* Planar */
> > + case DRM_FORMAT_YUV420:
> > + case DRM_FORMAT_YVU420:
> > + case DRM_FORMAT_YUV422:
> > + case DRM_FORMAT_YVU422:
> > + case DRM_FORMAT_YUV411:
> > + case DRM_FORMAT_YVU411:
> > + return true;
> > +
> > + default:
> > + return false;
> > + }
> > +}
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_format.h
> > b/drivers/gpu/drm/sun4i/sun4i_format.h
> > new file mode 100644
> > index 000000000000..1d6364cd3681
> > --- /dev/null
> > +++ b/drivers/gpu/drm/sun4i/sun4i_format.h
> > @@ -0,0 +1,35 @@
> > +/*
> > + * Copyright (C) 2015-2018 Free Electrons/Bootlin
> > + *
> > + * Maxime Ripard <maxime.ripard@bootlin.com>
> > + * Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 of
> > + * the License, or (at your option) any later version.
> > + */
> > +
> > +#ifndef _SUN4I_FORMAT_H_
> > +#define _SUN4I_FORMAT_H_
> > +
> > +extern const u32 sunxi_rgb2yuv_coef[12];
> > +extern const u32 sunxi_bt601_yuv2rgb_coef[12];
> > +
> > +bool sun4i_format_is_rgb(uint32_t format);
> > +bool sun4i_format_is_yuv(uint32_t format);
> > +bool sun4i_format_is_yuv411(uint32_t format);
> > +bool sun4i_format_is_yuv420(uint32_t format);
> > +bool sun4i_format_is_yuv422(uint32_t format);
> > +bool sun4i_format_is_yuv444(uint32_t format);
> > +bool sun4i_format_is_packed(uint32_t format);
> > +bool sun4i_format_is_semiplanar(uint32_t format);
> > +bool sun4i_format_is_planar(uint32_t format);
> > +bool sun4i_format_supports_tiling(uint32_t format);
>
> If we're going to add so many of them, then we should really consider
> to move them to drm_fourcc.c instead. Every one has some variation of
> some of these functions, we don't really need to duplicate it all the
> time.
Should I try to get that through in this patchset and have sun4i-drm be
their first user? Also, does introducing such a change require
identifying duplicates of these functions in each DRM driver's codebase?
Cheers,
--
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2018-03-27 8:28 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-21 15:28 [PATCH 00/10] drm/sun4i: Frontend YUV and MB32 tile modifier support Paul Kocialkowski
2018-03-21 15:28 ` [PATCH 01/10] drm/sun4i: Disable frontend video channel before enabling a layer Paul Kocialkowski
2018-03-23 9:53 ` Maxime Ripard
2018-03-21 15:28 ` [PATCH 02/10] drm/sun4i: Disable YUV channel when using the frontend and set interlace Paul Kocialkowski
2018-03-23 9:55 ` Maxime Ripard
2018-03-27 8:00 ` Paul Kocialkowski
2018-03-27 8:17 ` Maxime Ripard
2018-03-27 8:44 ` Paul Kocialkowski
2018-03-27 8:48 ` Chen-Yu Tsai
2018-03-27 9:18 ` Maxime Ripard
2018-03-27 9:21 ` Paul Kocialkowski
2018-03-21 15:28 ` [PATCH 03/10] drm/sun4i: Don't pretend to handle ARGB8888 with the frontend Paul Kocialkowski
2018-03-22 6:47 ` Chen-Yu Tsai
2018-03-22 8:23 ` Paul Kocialkowski
2018-03-22 8:37 ` Chen-Yu Tsai
2018-03-22 8:41 ` Paul Kocialkowski
2018-03-22 16:12 ` Maxime Ripard
2018-03-22 16:18 ` Paul Kocialkowski
2018-03-21 15:28 ` [PATCH 04/10] drm/sun4i: Explicitly list and check formats supported by the backend Paul Kocialkowski
2018-03-23 10:03 ` Maxime Ripard
2018-03-27 8:08 ` Paul Kocialkowski
2018-03-29 7:56 ` Maxime Ripard
2018-10-16 13:55 ` Paul Kocialkowski
2018-10-17 15:33 ` Maxime Ripard
2018-03-21 15:28 ` [PATCH 05/10] drm/sun4i: Explicitly list and check formats supported by the frontend Paul Kocialkowski
2018-03-23 10:06 ` Maxime Ripard
2018-03-27 8:24 ` Paul Kocialkowski
2018-03-29 9:03 ` Maxime Ripard
2018-10-16 13:57 ` Paul Kocialkowski
2018-03-21 15:29 ` [PATCH 06/10] drm/sun4i: Move and extend format-related helpers and tables Paul Kocialkowski
2018-03-23 10:13 ` Maxime Ripard
2018-03-27 8:27 ` Paul Kocialkowski [this message]
2018-03-27 14:47 ` Maxime Ripard
2018-03-21 15:29 ` [PATCH 07/10] drm/sun4i: Add support for YUV formats through the frontend Paul Kocialkowski
2018-03-23 10:30 ` Maxime Ripard
2018-03-27 8:39 ` Paul Kocialkowski
2018-03-21 15:29 ` [PATCH 08/10] drm/fourcc: Add definitions for Allwinner vendor and MB32 tiled format Paul Kocialkowski
2018-03-21 16:47 ` Daniel Stone
2018-03-22 8:05 ` Paul Kocialkowski
2018-03-21 15:29 ` [PATCH 09/10] drm/sun4i: Add a dedicated ioctl call for allocating tiled buffers Paul Kocialkowski
2018-03-23 10:48 ` Maxime Ripard
2018-03-27 8:41 ` Paul Kocialkowski
2018-03-27 14:48 ` Maxime Ripard
2018-03-21 15:29 ` [PATCH 10/10] drm/sun4i: Add support for YUV-based formats in MB32 tiles Paul Kocialkowski
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=1522139264.1110.28.camel@bootlin.com \
--to=paul.kocialkowski@bootlin.com \
--cc=airlied@linux.ie \
--cc=daniel.vetter@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gustavo@padovan.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maxime.ripard@bootlin.com \
--cc=seanpaul@chromium.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).