| Message ID | 20181116184238.170034-15-sean@poorly.run (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/msm: Various dpu locking and legacy cleanups | expand |
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. > } else { > > trace_dpu_crtc_frame_event_more_pending(DRMID(crtc), > > fevent->event);
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
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); } else { trace_dpu_crtc_frame_event_more_pending(DRMID(crtc), fevent->event);