Re: [PATCH 24/25] drm/msm/dpu: remove mutex locking for RM interfaces

Sean Paul
2018-10-09T19:57:04Z

On Mon, Oct 08, 2018 at 09:27:41PM -0700, Jeykumar Sankaran wrote:
> Since HW reservations are happening through atomic_check
> and all the display commits are catered by a single commit thread,
> it is not necessary to protect the interfaces by a separate
> mutex.
>
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 24 ------------------------
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 --
> 2 files changed, 26 deletions(-)
>

/snip

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index 8676fa5..9acbeba 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -24,11 +24,9 @@
> * struct dpu_rm - DPU dynamic hardware resource manager
> * @hw_blks: array of lists of hardware resources present in the system, one
> * list per type of hardware block
> - * @rm_lock: resource manager mutex
> */
> struct dpu_rm {
> struct list_head hw_blks[DPU_HW_BLK_MAX];

At this point, there's really not much point to even having the rm. It's just
another level of indirection that IMO complicates the code. If you look
at the usage of hw_blks, the code is always looking at a specific type of
hw_blk, so the array is unnecessary.

dpu_kms could just keep a few arrays/lists of the hw types, and the crtc and encoder
reserve functions can just go in crtc/encoder.

Sean

> - struct mutex rm_lock;
> };
>
> /**
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

--
Sean Paul, Software Engineer, Google / Chromium OS