Re: [DPU PATCH 4/4] drm/msm/dpu: use private obj to track hw resources

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 12, 2018 at 06:17:47PM -0700, Jeykumar Sankaran wrote:
> Switch to state based resource management. This patch
> overhauls the resource manager and HW allocation methods by
> maintaining the global resource pool and allocated hw
> blocks in respective drm component states.
> 
> Global resource manager(RM) is tracked in private object.
> Allocation strategy is switched from single point allocation
> of HW resources for the display pipeline to per component
> based allocation, where each drm component allocates HW
> blocks mapped to it's domain and tracks them in their respective
> state objects.
> 
> Fixes resource contention due to race conditions between
> user space and display thread by reserving resources
> only in atomic check.
> 
> Signed-off-by: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c           | 210 +++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h           |  59 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 223 ++----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h        |   4 -
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h   |   9 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |  32 +-
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c   |  86 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |  19 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h            |   8 +-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             | 805 ++++++---------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h             | 149 ++--
>  11 files changed, 534 insertions(+), 1070 deletions(-)

Ok, there's a lot going on here. It's pretty easy to review megapatches where
the diffstat is mostly negative. However, this patch has a lot of code deleted
and moving around, along with the new private obj. It's really hard to review
changes like this.

Could you please split this up into a bunch of simple patches which do one
thing? ie: Moving topology is a patch, using cstate instead of crtc is a patch,
using private obj is a patch, etc, etc.

Basically, cut things down into small enough pieces such that each patch can
be easily explained without using "and" in the commit message :-)

/snip

> +
>  	dpu_crtc = to_dpu_crtc(crtc);
>  	cstate = to_dpu_crtc_state(crtc->state);
>  	mode = &cstate->base.adjusted_mode;
>  	priv = crtc->dev->dev_private;
> +	dpu_kms = to_dpu_kms(priv->kms);
> +
> +	/* accessing after swap state. piv_obj.state is the current state */

s/piv_obj/priv_obj/

> +	dpu_priv_state = to_dpu_private_state(dpu_kms->priv_obj.state);
>  
>  	DPU_DEBUG("crtc%d\n", crtc->base.id);
>  

/snip

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux