| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190827154810.mr5jt2rkaorrry3t@uno.localdomain>
Date: Tue, 27 Aug 2019 17:48:10 +0200
From: Jacopo Mondi <jacopo@...ndi.org>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Jacopo Mondi <jacopo+renesas@...ndi.org>,
kieran.bingham+renesas@...asonboard.com, geert@...ux-m68k.org,
horms@...ge.net.au, uli@...nd.eu, airlied@...ux.ie,
daniel@...ll.ch, koji.matsuoka.xm@...esas.com, muroya@....co.jp,
VenkataRajesh.Kalakodima@...bosch.com,
Harsha.ManjulaMallikarjun@...bosch.com,
linux-renesas-soc@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 08/14] drm: rcar-du: Add support for CMM
On Tue, Aug 27, 2019 at 04:56:19PM +0200, Jacopo Mondi wrote:
> Hi Laurent,
>
> On Tue, Aug 27, 2019 at 03:24:22AM +0300, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > On Sun, Aug 25, 2019 at 03:51:48PM +0200, Jacopo Mondi wrote:
> > > Add a driver for the R-Car Display Unit Color Correction Module.
> > >
> > > In most of Gen3 SoCs, each DU output channel is provided with a CMM unit
> > > to perform image enhancement and color correction.
> > >
> > > Add support for CMM through a driver that supports configuration of
> > > the 1-dimensional LUT table. More advanced CMM feature will be
> > > implemented on top of this basic one.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@...ndi.org>
> > > ---
> > > drivers/gpu/drm/rcar-du/Kconfig | 7 +
> > > drivers/gpu/drm/rcar-du/Makefile | 1 +
> > > drivers/gpu/drm/rcar-du/rcar_cmm.c | 262 +++++++++++++++++++++++++++++
> > > drivers/gpu/drm/rcar-du/rcar_cmm.h | 38 +++++
> > > 4 files changed, 308 insertions(+)
> > > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
> > > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> > >
> > > diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> > > index 1529849e217e..539d232790d1 100644
> > > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > > @@ -13,6 +13,13 @@ config DRM_RCAR_DU
> > > Choose this option if you have an R-Car chipset.
> > > If M is selected the module will be called rcar-du-drm.
> > >
> > > +config DRM_RCAR_CMM
> > > + bool "R-Car DU Color Management Module (CMM) Support"
> > > + depends on DRM && OF
> > > + depends on DRM_RCAR_DU
> > > + help
> > > + Enable support for R-Car Color Management Module (CMM).
> > > +
> > > config DRM_RCAR_DW_HDMI
> > > tristate "R-Car DU Gen3 HDMI Encoder Support"
> > > depends on DRM && OF
> > > diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile
> > > index 6c2ed9c46467..4d1187ccc3e5 100644
> > > --- a/drivers/gpu/drm/rcar-du/Makefile
> > > +++ b/drivers/gpu/drm/rcar-du/Makefile
> > > @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS) += rcar_du_of.o \
> > > rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
> > > rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
> > >
> > > +obj-$(CONFIG_DRM_RCAR_CMM) += rcar_cmm.o
> > > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du-drm.o
> > > obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o
> > > obj-$(CONFIG_DRM_RCAR_LVDS) += rcar_lvds.o
> > > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > > new file mode 100644
> > > index 000000000000..55361f5701e8
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > > @@ -0,0 +1,262 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> > > + *
> > > + * Copyright (C) 2019 Jacopo Mondi <jacopo+renesas@...ndi.org>
> > > + */
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/pm_runtime.h>
> > > +
> > > +#include <drm/drm_color_mgmt.h>
> > > +
> > > +#include "rcar_cmm.h"
> > > +
> > > +#define CM2_LUT_CTRL 0x0000
> > > +#define CM2_LUT_CTRL_LUT_EN BIT(0)
> > > +#define CM2_LUT_TBL_BASE 0x0600
> > > +#define CM2_LUT_TBL(__i) (CM2_LUT_TBL_BASE + (__i) * 4)
> > > +
> > > +struct rcar_cmm {
> > > + void __iomem *base;
> > > + bool enabled;
> > > +
> > > + /*
> > > + * @lut: 1D-LUT status
> > > + * @lut.enabled: 1D-LUT enabled flag
> > > + * @lut.size: Number of entries in the LUT table
> >
> > Please see my review of patch 13/14, I wonder if we could drop this
> > field.
> >
> > > + * @lut.table: Table of 1D-LUT entries scaled to HW support
> > > + * precision (8-bits per color component)
> > > + */
> > > + struct {
> > > + bool enabled;
> > > + unsigned int size;
> > > + u32 table[CMM_GAMMA_LUT_SIZE];
> > > + } lut;
> > > +};
> > > +
> > > +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> > > +{
> > > + return ioread32(rcmm->base + reg);
> > > +}
> > > +
> > > +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> > > +{
> > > + iowrite32(data, rcmm->base + reg);
> > > +}
> > > +
> > > +/*
> > > + * rcar_cmm_lut_extract() - Scale down to hw precision the DRM LUT table
> >
> > s/hw/hardware/ (and below too)
> >
> > > + * entries and store them.
> > > + * @rcmm: Pointer to the CMM device
> > > + * @size: Number of entries in the table
> > > + * @drm_lut: DRM LUT table
> > > + */
> > > +static void rcar_cmm_lut_extract(struct rcar_cmm *rcmm, size_t size,
> > > + const struct drm_color_lut *drm_lut)
> > > +{
> > > + unsigned int i;
> > > +
> > > + for (i = 0; i < size; ++i) {
> > > + const struct drm_color_lut *lut = &drm_lut[i];
> > > +
> > > + rcmm->lut.table[i] = drm_color_lut_extract(lut->red, 8) << 16
> > > + | drm_color_lut_extract(lut->green, 8) << 8
> > > + | drm_color_lut_extract(lut->blue, 8);
> > > + }
> > > +
> > > + rcmm->lut.size = size;
> > > +}
> > > +
> > > +/*
> > > + * rcar_cmm_lut_load() - Write to hw the LUT table entries from the local table.
> > > + *
> >
> > No need for a blank line
> >
> > > + * @rcmm: Pointer to the CMM device
> > > + */
> > > +static void rcar_cmm_lut_load(struct rcar_cmm *rcmm)
> >
> > I would name this rcar_cmm_lut_write().
> >
>
> I won't, as I would like to convey the LUT tables is loaded from the
Sorry, I meant "I wouldn't"... "I won't" is really harsh, sorry about
it :p
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists