Re: [PATCH v2 14/24] drm/msm: dpu: Grab the modeset locks in frame_event

Sean Paul
2018-11-16T21:18:45Z

On Fri, Nov 16, 2018 at 12:02:54PM -0800, Jeykumar Sankaran wrote:
> On 2018-11-16 10:42, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> >
> > This patch wraps dpu_core_perf_crtc_release_bw() with modeset locks
> > since it digs into the state objects.
> >
> > Changes in v2:
> > - None
> >
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>
>
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index 80de5289ada3..156f4c77ca44 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -335,7 +335,9 @@ static void dpu_crtc_frame_event_work(struct
> > kthread_work *work)
> > /* release bandwidth and other resources */
> > trace_dpu_crtc_frame_event_done(DRMID(crtc),
> > fevent->event);
> > + drm_modeset_lock_all(crtc->dev);
> > dpu_core_perf_crtc_release_bw(crtc);
> > + drm_modeset_unlock_all(crtc->dev);
> We might need to revisit this locking when we measure for performance as it
> could block the incoming frame locking.
>

Definitely something to keep an eye on.

That said, we really do want it to block the incoming frame since we're reducing
bw. It would be really unfortunate if this happened concurrently with a commit.
If this _does_ cause a performance problem, we should really investigate the
criteria for reducing bw.

Sean

> > } else {
> >
> > trace_dpu_crtc_frame_event_more_pending(DRMID(crtc),
> >
> > fevent->event);
>
> --
> Jeykumar S