Hi Philipp,

Thanks for the feedback.

> -----Original Message-----
> From: Philipp Zabel <[email protected]>
> Sent: Monday, April 24, 2023 7:38 PM
> To: Biju Das <[email protected]>
> Cc: David Airlie <[email protected]>; Daniel Vetter <[email protected]>; dri-
> [email protected]; [email protected]; Geert
> Uytterhoeven <[email protected]>; Fabrizio Castro
> <[email protected]>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> [email protected]>
> 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 <[email protected]>
> [...]
> > 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

Reply via email to