Re: [PATCH] drm/brdige: analogix_dp: Grab runtime PM reference for DP-AUX

[Brian Norris]

On Fri, Oct 1, 2021 at 1:37 PM Sean Paul <sean@xxxxxxxxxx> wrote:
> On Wed, Sep 29, 2021 at 02:41:03PM -0700, Brian Norris wrote:
> > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> > @@ -1632,8 +1632,23 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux,
> >                                      struct drm_dp_aux_msg *msg)
> >  {
> >       struct analogix_dp_device *dp = to_dp(aux);
> > +     int ret, ret2;
> >
> > -     return analogix_dp_transfer(dp, msg);
> > +     ret = analogix_dp_prepare_panel(dp, true, false);
> > +     if (ret) {
> > +             DRM_DEV_ERROR(dp->dev, "Failed to prepare panel (%d)\n", ret);
>
> s/DRM_DEV_ERROR/drm_err/

Sure. Now that I'm looking a second time, I see the header recommends this.

> > +             return ret;
> > +     }
> > +
> > +     pm_runtime_get_sync(dp->dev);
> > +     ret = analogix_dp_transfer(dp, msg);
> > +     pm_runtime_put(dp->dev);
> > +
> > +     ret2 = analogix_dp_prepare_panel(dp, false, false);
> > +     if (ret2)
> > +             DRM_DEV_ERROR(dp->dev, "Failed to unprepare panel (%d)\n", ret2);
>
> What's the reasoning for not propagating unprepare failures? I feel like that
> should be fair game.

I suppose the underlying reason is laziness, sorry. But a related
reason is the we probably should prefer propagating the
analogix_dp_transfer() error, if it's non-zero, rather than the
unprepare error. That's not too hard to do though, even if it's
slightly more awkward.

> > +
> > +     return ret;
> >  }
> >
> >  struct analogix_dp_device *

v2 coming.

Regards,
Brian