Hi Laurent,
Thanks for the feedback.
> Subject: Re: [RFC 18/28] drm: rcar-du: Add RZ/G2L LCDC Support
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Wed, Jan 12, 2022 at 05:46:02PM +0000, Biju Das wrote:
> > The LCD controller is composed of Frame Compression Processor (FCPVD),
> > Video Signal Processor (VSPD), and Display Unit (DU).
> >
> > It has DPI/DSI interfaces and supports a maximum resolution of 1080p
> > along with 2 rpf's to support blending of two picture layers and
> > raster operations (ROPs).
> >
> > A feature bit for RZ/G2L SoC is introduced to support RZ/G2L with the
> > rest of the SoC supported by this driver.
>
> The RZ/G2L DU seems to be a completely different IP core than the DU in R-
> Car and other RZ SoCs. I think it should be supported by a separate
> driver, or at least with a different (and modularized) CRTC
> implementation. This patch has too many RCAR_DU_FEATURE_RZG2L checks.
Agreed, May be will create separate RZ/G2L specific DU driver based on RCar-DU
driver removing the plane, Group, CMM, LVDS, HDMI and CRTC adaptation for
RZ/G2L
and will send next version for feedback.
Regards,
Biju
>
> > Signed-off-by: Biju Das <biju.das...@bp.renesas.com>
> > ---
> > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 148 ++++++++++++++++++------
> > drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 +
> > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 23 ++++
> > drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 +
> > drivers/gpu/drm/rcar-du/rcar_du_group.c | 5 +
> > drivers/gpu/drm/rcar-du/rcar_du_regs.h | 52 +++++++++
> > 6 files changed, 195 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > index 521446890d3d..aea9178f3e7d 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > @@ -10,6 +10,7 @@
> > #include <linux/clk.h>
> > #include <linux/mutex.h>
> > #include <linux/platform_device.h>
> > +#include <linux/reset.h>
> > #include <linux/sys_soc.h>
> >
> > #include <drm/drm_atomic.h>
> > @@ -219,6 +220,42 @@ static void rcar_du_crtc_set_display_timing(struct
> rcar_du_crtc *rcrtc)
> > u32 dsmr;
> > u32 escr;
> >
> > + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + u32 ditr0, ditr1, ditr2, ditr3, ditr4, ditr5, pbcr0;
> > +
> > + clk_set_rate(rcrtc->extclock, mode_clock);
> > +
> > + ditr0 = (DU_DITR0_DEMD_HIGH
> > + | ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DU_DITR0_VSPOL : 0)
> > + | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DU_DITR0_HSPOL :
> 0));
> > +
> > + ditr1 = DU_DITR1_VSA(mode->vsync_end - mode->vsync_start)
> > + | DU_DITR1_VACTIVE(mode->vdisplay);
> > +
> > + ditr2 = DU_DITR2_VBP(mode->vtotal - mode->vsync_end)
> > + | DU_DITR2_VFP(mode->vsync_start - mode->vdisplay);
> > +
> > + ditr3 = DU_DITR3_HSA(mode->hsync_end - mode->hsync_start)
> > + | DU_DITR3_HACTIVE(mode->hdisplay);
> > +
> > + ditr4 = DU_DITR4_HBP(mode->htotal - mode->hsync_end)
> > + | DU_DITR4_HFP(mode->hsync_start - mode->hdisplay);
> > +
> > + ditr5 = DU_DITR5_VSFT(0) | DU_DITR5_HSFT(0);
> > +
> > + pbcr0 = DU_PBCR0_PB_DEP(0x1F);
> > +
> > + rcar_du_write(rcdu, DU_DITR0, ditr0);
> > + rcar_du_write(rcdu, DU_DITR1, ditr1);
> > + rcar_du_write(rcdu, DU_DITR2, ditr2);
> > + rcar_du_write(rcdu, DU_DITR3, ditr3);
> > + rcar_du_write(rcdu, DU_DITR4, ditr4);
> > + rcar_du_write(rcdu, DU_DITR5, ditr5);
> > + rcar_du_write(rcdu, DU_PBCR0, pbcr0);
> > +
> > + return;
> > + }
> > +
> > if (rcdu->info->dpll_mask & (1 << rcrtc->index)) {
> > unsigned long target = mode_clock;
> > struct dpll_info dpll = { 0 };
> > @@ -531,16 +568,23 @@ static void rcar_du_cmm_setup(struct drm_crtc
> > *crtc)
> >
> > static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc) {
> > - /* Set display off and background to black */
> > - rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0));
> > - rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0));
> > + struct rcar_du_device *rcdu = rcrtc->dev;
> >
> > - /* Configure display timings and output routing */
> > - rcar_du_crtc_set_display_timing(rcrtc);
> > - rcar_du_group_set_routing(rcrtc->group);
> > + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + /* Set display off and background to black */
> > + rcar_du_crtc_write(rcrtc, DOOR, DOOR_RGB(0, 0, 0));
> > + rcar_du_crtc_write(rcrtc, BPOR, BPOR_RGB(0, 0, 0));
> >
> > - /* Start with all planes disabled. */
> > - rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR,
> 0);
> > + /* Configure display timings and output routing */
> > + rcar_du_crtc_set_display_timing(rcrtc);
> > + rcar_du_group_set_routing(rcrtc->group);
> > +
> > + /* Start with all planes disabled. */
> > + rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR :
> DS1PR, 0);
> > + } else {
> > + /* Configure display timings and output routing */
> > + rcar_du_crtc_set_display_timing(rcrtc);
> > + }
> >
> > /* Enable the VSP compositor. */
> > if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) @@ -561,6
> > +605,12 @@ static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc)
> > if (rcrtc->initialized)
> > return 0;
> >
> > + if (rcrtc->rstc) {
> > + ret = reset_control_deassert(rcrtc->rstc);
> > + if (ret < 0)
> > + goto error_reset;
> > + }
> > +
> > ret = clk_prepare_enable(rcrtc->clock);
> > if (ret < 0)
> > return ret;
> > @@ -582,6 +632,9 @@ static int rcar_du_crtc_get(struct rcar_du_crtc
> *rcrtc)
> > clk_disable_unprepare(rcrtc->extclock);
> > error_clock:
> > clk_disable_unprepare(rcrtc->clock);
> > +error_reset:
> > + if (rcrtc->rstc)
> > + reset_control_assert(rcrtc->rstc);
> > return ret;
> > }
> >
> > @@ -591,23 +644,28 @@ static void rcar_du_crtc_put(struct rcar_du_crtc
> > *rcrtc)
> >
> > clk_disable_unprepare(rcrtc->extclock);
> > clk_disable_unprepare(rcrtc->clock);
> > + if (rcrtc->rstc)
> > + reset_control_assert(rcrtc->rstc);
> >
> > rcrtc->initialized = false;
> > }
> >
> > static void rcar_du_crtc_start(struct rcar_du_crtc *rcrtc) {
> > + struct rcar_du_device *rcdu = rcrtc->dev;
> > bool interlaced;
> >
> > - /*
> > - * Select master sync mode. This enables display operation in master
> > - * sync mode (with the HSYNC and VSYNC signals configured as outputs
> and
> > - * actively driven).
> > - */
> > - interlaced = rcrtc->crtc.mode.flags & DRM_MODE_FLAG_INTERLACE;
> > - rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK | DSYSR_SCM_MASK,
> > - (interlaced ? DSYSR_SCM_INT_VIDEO : 0) |
> > - DSYSR_TVM_MASTER);
> > + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + /*
> > + * Select master sync mode. This enables display operation in
> master
> > + * sync mode (with the HSYNC and VSYNC signals configured as
> outputs and
> > + * actively driven).
> > + */
> > + interlaced = rcrtc->crtc.mode.flags & DRM_MODE_FLAG_INTERLACE;
> > + rcar_du_crtc_dsysr_clr_set(rcrtc, DSYSR_TVM_MASK |
> DSYSR_SCM_MASK,
> > + (interlaced ? DSYSR_SCM_INT_VIDEO :
> > 0) |
> > + DSYSR_TVM_MASTER);
> > + }
> >
> > rcar_du_group_start_stop(rcrtc->group, true); } @@ -1229,6 +1287,14
> > @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, unsigned int
> swindex,
> > name = NULL;
> > }
> >
> > + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + rcrtc->rstc = devm_reset_control_get_shared(rcdu->dev, NULL);
> > + if (IS_ERR(rcrtc->rstc)) {
> > + dev_err(rcdu->dev, "can't get cpg reset\n");
> > + return PTR_ERR(rcrtc->rstc);
> > + }
> > + }
> > +
> > rcrtc->clock = devm_clk_get(rcdu->dev, name);
> > if (IS_ERR(rcrtc->clock)) {
> > dev_err(rcdu->dev, "no clock for DU channel %u\n", hwindex);
> @@
> > -1251,6 +1317,14 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int swindex,
> > return ret;
> > }
> >
> > + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + clk = devm_clk_get(rcdu->dev, "vclk");
> > + if (!IS_ERR(clk))
> > + rcrtc->extclock = clk;
> > + else if (PTR_ERR(clk) == -EPROBE_DEFER)
> > + return -EPROBE_DEFER;
> > + }
> > +
> > init_waitqueue_head(&rcrtc->flip_wait);
> > init_waitqueue_head(&rcrtc->vblank_wait);
> > spin_lock_init(&rcrtc->vblank_lock);
> > @@ -1287,27 +1361,29 @@ int rcar_du_crtc_create(struct rcar_du_group
> > *rgrp, unsigned int swindex,
> >
> > drm_crtc_helper_add(crtc, &crtc_helper_funcs);
> >
> > - /* Register the interrupt handler. */
> > - if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ)) {
> > - /* The IRQ's are associated with the CRTC (sw)index. */
> > - irq = platform_get_irq(pdev, swindex);
> > - irqflags = 0;
> > - } else {
> > - irq = platform_get_irq(pdev, 0);
> > - irqflags = IRQF_SHARED;
> > - }
> > + if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_RZG2L)) {
> > + /* Register the interrupt handler. */
> > + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ)) {
> > + /* The IRQ's are associated with the CRTC (sw)index. */
> > + irq = platform_get_irq(pdev, swindex);
> > + irqflags = 0;
> > + } else {
> > + irq = platform_get_irq(pdev, 0);
> > + irqflags = IRQF_SHARED;
> > + }
> >
> > - if (irq < 0) {
> > - dev_err(rcdu->dev, "no IRQ for CRTC %u\n", swindex);
> > - return irq;
> > - }
> > + if (irq < 0) {
> > + dev_err(rcdu->dev, "no IRQ for CRTC %u\n", swindex);
> > + return irq;
> > + }
> >
> > - ret = devm_request_irq(rcdu->dev, irq, rcar_du_crtc_irq, irqflags,
> > - dev_name(rcdu->dev), rcrtc);
> > - if (ret < 0) {
> > - dev_err(rcdu->dev,
> > - "failed to register IRQ for CRTC %u\n", swindex);
> > - return ret;
> > + ret = devm_request_irq(rcdu->dev, irq, rcar_du_crtc_irq,
> irqflags,
> > + dev_name(rcdu->dev), rcrtc);
> > + if (ret < 0) {
> > + dev_err(rcdu->dev,
> > + "failed to register IRQ for CRTC %u\n",
> > swindex);
> > + return ret;
> > + }
> > }
> >
> > rcar_du_crtc_crc_init(rcrtc);
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> > index 66e8839db708..4ec2db46b131 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> > @@ -28,6 +28,7 @@ struct rcar_du_vsp;
> > * @dev: the DU device
> > * @clock: the CRTC functional clock
> > * @extclock: external pixel dot clock (optional)
> > + * @rstc: reset controller (optional)
> > * @mmio_offset: offset of the CRTC registers in the DU MMIO block
> > * @index: CRTC hardware index
> > * @initialized: whether the CRTC has been initialized and clocks
> > enabled @@ -50,6 +51,7 @@ struct rcar_du_crtc {
> > struct rcar_du_device *dev;
> > struct clk *clock;
> > struct clk *extclock;
> > + struct reset_control *rstc;
> > unsigned int mmio_offset;
> > unsigned int index;
> > bool initialized;
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > index f92636001f10..57edc3b3154f 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > @@ -596,6 +596,28 @@ static const struct rcar_du_device_info
> rcar_du_r8a779a0_info = {
> > .dsi_clk_mask = BIT(1) | BIT(0),
> > };
> >
> > +static const struct rcar_du_device_info rcar_du_r9a07g044l_info = {
> > + .gen = 3,
> > + .features = RCAR_DU_FEATURE_CRTC_IRQ
> > + | RCAR_DU_FEATURE_CRTC_CLOCK
> > + | RCAR_DU_FEATURE_RZG2L
> > + | RCAR_DU_FEATURE_VSP1_SOURCE,
> > + .channels_mask = BIT(0),
> > + .routes = {
> > + [RCAR_DU_OUTPUT_DPAD0] = {
> > + .possible_crtcs = BIT(0),
> > + .port = 0,
> > + },
> > + [RCAR_DU_OUTPUT_DSI0] = {
> > + .possible_crtcs = BIT(0),
> > + .port = 1,
> > + },
> > + },
> > + .num_rpf = 2,
> > + .max_width = 1920,
> > + .max_height = 1080,
> > +};
> > +
> > static const struct of_device_id rcar_du_of_table[] = {
> > { .compatible = "renesas,du-r8a7742", .data = &rcar_du_r8a7790_info
> },
> > { .compatible = "renesas,du-r8a7743", .data = &rzg1_du_r8a7743_info
> > }, @@ -621,6 +643,7 @@ static const struct of_device_id
> rcar_du_of_table[] = {
> > { .compatible = "renesas,du-r8a77990", .data =
> &rcar_du_r8a7799x_info },
> > { .compatible = "renesas,du-r8a77995", .data =
> &rcar_du_r8a7799x_info },
> > { .compatible = "renesas,du-r8a779a0", .data =
> > &rcar_du_r8a779a0_info },
> > + { .compatible = "renesas,du-r9a07g044l", .data =
> > +&rcar_du_r9a07g044l_info },
> > { }
> > };
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> > b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> > index e28c2df66f8e..47da9da71bca 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> > @@ -33,6 +33,7 @@ struct rcar_du_device;
> > #define RCAR_DU_FEATURE_TVM_SYNC BIT(4) /* Has TV switch/sync
> modes */
> > #define RCAR_DU_FEATURE_PLANE BIT(5) /* HW supports DU
> planes */
> > #define RCAR_DU_FEATURE_GROUP BIT(6) /* HW supports DU
> groups */
> > +#define RCAR_DU_FEATURE_RZG2L BIT(7) /* Use RZ/G2L
> registers */
> >
> > #define RCAR_DU_QUIRK_ALIGN_128B BIT(0) /* Align pitches to 128
> bytes */
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > index 3612bc9eab1b..632271c2881d 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > @@ -224,6 +224,11 @@ static void __rcar_du_group_start_stop(struct
> > rcar_du_group *rgrp, bool start) {
> > struct rcar_du_device *rcdu = rgrp->dev;
> >
> > + if (!rcar_du_has(rgrp->dev, RCAR_DU_FEATURE_GROUP)) {
> > + rcar_du_write(rgrp->dev, DU_MCR0, start ? DU_MCR0_DI_EN : 0);
> > + return;
> > + }
> > +
> > /*
> > * Group start/stop is controlled by the DRES and DEN bits of DSYSR0
> > * for the first group and DSYSR2 for the second group. On most DU
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> > b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> > index 1cdaa51eb9ac..9e5c8d286bfc 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> > @@ -15,6 +15,58 @@
> > #define DU2_REG_OFFSET 0x40000
> > #define DU3_REG_OFFSET 0x70000
> >
> > +/*
> > +---------------------------------------------------------------------
> > +--------
> > + * RZ/G2L Display Registers
> > + */
> > +
> > +#define DU_MCR0 0x00
> > +#define DU_MCR0_DPI_OE BIT(0)
> > +#define DU_MCR0_DI_EN BIT(8)
> > +#define DU_MCR0_PB_CLR BIT(16)
> > +
> > +#define DU_MSR0 0x04
> > +#define DU_MSR0_ST_DI_BSY BIT(8)
> > +#define DU_MSR0_ST_PB_WFULL BIT(16)
> > +#define DU_MSR0_ST_PB_WINIT BIT(18)
> > +#define DU_MSR0_ST_PB_REMPTY BIT(20)
> > +#define DU_MSR0_ST_PB_RUF BIT(21)
> > +#define DU_MSR0_ST_PB_RINIT BIT(22)
> > +
> > +#define DU_MSR1 0x08
> > +
> > +#define DU_IMR0 0x0C
> > +#define DU_MSR0_IM_PB_RUF BIT(0)
> > +
> > +#define DU_DITR0 0x10
> > +#define DU_DITR0_DPI_CLKMD BIT(0)
> > +#define DU_DITR0_DEMD_LOW 0x0
> > +#define DU_DITR0_DEMD_HIGH (BIT(8) | BIT(9))
> > +#define DU_DITR0_VSPOL BIT(16)
> > +#define DU_DITR0_HSPOL BIT(17)
> > +
> > +#define DU_DITR1 0x14
> > +#define DU_DITR1_VSA(x) ((x) << 0)
> > +#define DU_DITR1_VACTIVE(x) ((x) << 16)
> > +
> > +#define DU_DITR2 0x18
> > +#define DU_DITR2_VBP(x) ((x) << 0)
> > +#define DU_DITR2_VFP(x) ((x) << 16)
> > +
> > +#define DU_DITR3 0x1C
> > +#define DU_DITR3_HSA(x) ((x) << 0)
> > +#define DU_DITR3_HACTIVE(x) ((x) << 16)
> > +
> > +#define DU_DITR4 0x20
> > +#define DU_DITR4_HBP(x) ((x) << 0)
> > +#define DU_DITR4_HFP(x) ((x) << 16)
> > +
> > +#define DU_DITR5 0x24
> > +#define DU_DITR5_VSFT(x) ((x) << 0)
> > +#define DU_DITR5_HSFT(x) ((x) << 16)
> > +
> > +#define DU_PBCR0 0x4C
> > +#define DU_PBCR0_PB_DEP(x) ((x) << 0)
> > +
> > /* --------------------------------------------------------------------
> ---------
> > * Display Control Registers
> > */
>
> --
> Regards,
>
> Laurent Pinchart
