[PATCH v8 4/5] drm: Add RZ/G2L DU Support

[Biju Das]

Hi Philipp,

Thanks for the feedback.

> -----Original Message-----
> From: Philipp Zabel <p.zabel at pengutronix.de>
> Sent: Monday, April 24, 2023 7:38 PM
> To: Biju Das <biju.das.jz at bp.renesas.com>
> Cc: David Airlie <airlied at gmail.com>; Daniel Vetter <daniel at ffwll.ch>; dri-
> devel at lists.freedesktop.org; linux-renesas-soc at vger.kernel.org; Geert
> Uytterhoeven <geert+renesas at glider.be>; Fabrizio Castro
> <fabrizio.castro.jz at renesas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj at bp.renesas.com>
> Subject: Re: [PATCH v8 4/5] drm: Add RZ/G2L DU Support
> 
> Hi Biju,
> 
> On Mon, Apr 24, 2023 at 05:10:23PM +0100, 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 RPFs to support the blending of two picture layers and
> > raster operations (ROPs).
> >
> > The DU module is connected to VSPD. Add RZ/G2L DU support for RZ/G2L
> > alike SoCs.
> >
> > Signed-off-by: Biju Das <biju.das.jz at bp.renesas.com>
> [...]
> > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > new file mode 100644
> > index 000000000000..af877d0dadc2
> > --- /dev/null
> > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
> > @@ -0,0 +1,716 @@
> [...]
> > +static int rzg2l_du_crtc_get(struct rzg2l_du_crtc *rcrtc) {
> > +	int ret;
> > +
> > +	/*
> > +	 * Guard against double-get, as the function is called from both the
> > +	 * .atomic_enable() and .atomic_begin() handlers.
> > +	 */
> > +	if (rcrtc->initialized)
> > +		return 0;
> > +
> > +	ret = clk_prepare_enable(rcrtc->rzg2l_clocks.aclk);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = clk_prepare_enable(rcrtc->rzg2l_clocks.pclk);
> > +	if (ret < 0)
> > +		goto error_clock;
> > +
> > +	ret = reset_control_deassert(rcrtc->rstc);
> > +	if (ret < 0)
> > +		goto error_reset;
> > +
> > +	rzg2l_du_crtc_setup(rcrtc);
> > +	rcrtc->initialized = true;
> > +
> > +	return 0;
> > +
> > +error_reset:
> > +	reset_control_assert(rcrtc->rstc);
> 
> If deassertion did not succeed, there is no need to assert.
> Worse, for shared reset controls this messes up the deassert_count.
> You can just drop the reset_control_assert() here.

Agreed, will drop reset_control_assert()

Cheers,
Biju