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

[Sean Paul <seanpaul@chromium.org>]

On Wed, Jun 13, 2018 at 12:01:21PM -0700, Jeykumar Sankaran wrote:
> On 2018-06-13 09:44, Jordan Crouse wrote:
> > 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@codeaurora.org>
> > > ---
> > >  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(-)

/snip

> > cstate->num_mixers);
> > 
> > Nit - this could be worded a bit better - "too many mixers" would be
> > better, but
> > I have to ask - under what circumstances would the number of mixers be
> > larger
> > than CRTC_DUAL_MIXERS and/or why don't we support a dynamic number of
> > mixers?
> > 
> Comes from downstream driver implementation where CRTC iterates through
> RM free pool to identify mixers tagged with the current crtc id. If the
> previous clean up was screwed up this may have more than 2 mixers. With
> the new state based RM, its very unlikely we hit this condition.
> 

In this case, add a comment with "/* This should never happen */"

I'm just kidding, that would virtually guarantee that it does happen and we
certainly don't need that bad juju around!

Sean

> We do support dynamic mixer counts. Based on the connector (panel)
> resolution,
> CRTC allocates 1 or 2 (panel_width > hw mixer width) mixer block(s) on the
> first
> atomic check. DPU limits max no. of hw mixers that can be ganged up for a
> display to 2.
> 
> > >  		return;
> > >  	}
> > 

/snip

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