Re: [PATCH] drm/i915: Use enum plane_id for frontbuffer tracking

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

 



On Wed, 2018-01-24 at 17:16 +0200, Ville Syrjälä wrote:
> On Wed, Jan 24, 2018 at 04:16:19AM +0000, Pandiyan, Dhinakaran wrote:
> > On Tue, 2018-01-23 at 20:33 +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > 
> > > Replace the ad-hoc plane indexing scheme used by the frontbuffer
> > > tracking with enum plane_id.
> > > 
> > > The old video overlay not being part of the plane_id namespace
> > > will just be given the high bit.
> > 
> > I'm curious why the bit corresponding to PLANE_SPRITE0 is not re-used
> > for the overlay. From a cursory read seems like platforms with overlays
> > and sprites are mutually exclusive.
> 
> Currently yes, but maybe not in the future. I still have dreams of
> exposing all the planes on these platforms, which would mean having
> at least one sprite plane that's not the video overlay. I haven't
> really figured out how to make it all work out nicely with enum
> plane_id though as that's a per-pipe thing and the planes on the
> old hw are not necessarily tied to a single pipe. Perhaps one
> option would be to assign the plane ids globally, ie. PRIMARY==A,
> PLANE1==B, PLANE2==C, etc. Not sure if that would actually work out
> though.
> 
> > 
> > Related question, how different is this overlay from the sprite planes?
> 
> Very different. I would like to expose it as a drm_plane eventually,
> but there's a bit of actual work involved to convert the overlay
> register updates to use mmio, and to handle the render cache
> reconfiguration (which still involves stuffing MI commands to the
> ring) in a decent way.
> 
> I do have a basic mmio conversion sitting on some branch somewhere,

I'm interested in seeing how this is done, is this in your public repo?
And thanks for the explanation.

-DK

> but I don't think I got it to look as nice as I'd like. Plus it's
> probably bitrotted enough by now that it needs some rework anyway.
> 
> > 
> > 
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h      | 11 +++--------
> > >  drivers/gpu/drm/i915/intel_display.c |  4 ++--
> > >  drivers/gpu/drm/i915/intel_fbc.c     |  2 +-
> > >  drivers/gpu/drm/i915/intel_sprite.c  |  4 +++-
> > >  4 files changed, 9 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 8333692dac5a..bd545b1c9546 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2404,16 +2404,11 @@ enum hdmi_force_audio {
> > >   *
> > >   * We have one bit per pipe and per scanout plane type.
> > >   */
> > > -#define INTEL_MAX_SPRITE_BITS_PER_PIPE 5
> > >  #define INTEL_FRONTBUFFER_BITS_PER_PIPE 8
> > > -#define INTEL_FRONTBUFFER_PRIMARY(pipe) \
> > > -	(1 << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> > > -#define INTEL_FRONTBUFFER_CURSOR(pipe) \
> > > -	(1 << (1 + (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))))
> > > -#define INTEL_FRONTBUFFER_SPRITE(pipe, plane) \
> > > -	(1 << (2 + plane + (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))))
> > > +#define INTEL_FRONTBUFFER(pipe, plane_id) \
> > > +	(1 << ((plane_id) + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> > >  #define INTEL_FRONTBUFFER_OVERLAY(pipe) \
> > > -	(1 << (2 + INTEL_MAX_SPRITE_BITS_PER_PIPE + (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe))))
> > > +	(1 << (INTEL_FRONTBUFFER_BITS_PER_PIPE - 1 + INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> > >  #define INTEL_FRONTBUFFER_ALL_MASK(pipe) \
> > >  	(0xff << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index d585ce4c8732..3cc35add362f 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -13168,7 +13168,7 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
> > >  	else
> > >  		primary->i9xx_plane = (enum i9xx_plane_id) pipe;
> > >  	primary->id = PLANE_PRIMARY;
> > > -	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
> > > +	primary->frontbuffer_bit = INTEL_FRONTBUFFER(pipe, primary->id);
> > >  	primary->check_plane = intel_check_primary_plane;
> > >  
> > >  	if (INTEL_GEN(dev_priv) >= 9) {
> > > @@ -13289,7 +13289,7 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
> > >  	cursor->pipe = pipe;
> > >  	cursor->i9xx_plane = (enum i9xx_plane_id) pipe;
> > >  	cursor->id = PLANE_CURSOR;
> > > -	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
> > > +	cursor->frontbuffer_bit = INTEL_FRONTBUFFER(pipe, cursor->id);
> > >  
> > >  	if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
> > >  		cursor->update_plane = i845_update_cursor;
> > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > > index 9dc2b8b5f2db..a8a8a80497a8 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > @@ -1373,7 +1373,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
> > >  
> > >  	for_each_pipe(dev_priv, pipe) {
> > >  		fbc->possible_framebuffer_bits |=
> > > -				INTEL_FRONTBUFFER_PRIMARY(pipe);
> > > +			INTEL_FRONTBUFFER(pipe, PLANE_PRIMARY);
> > >  
> > >  		if (fbc_on_pipe_a_only(dev_priv))
> > >  			break;
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index a92c748ca8ab..18ef0392362e 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -345,6 +345,8 @@ skl_plane_get_hw_state(struct intel_plane *plane)
> > >  	return ret;
> > >  }
> > >  
> > > +
> > > +
> > >  static void
> > >  chv_update_csc(struct intel_plane *plane, uint32_t format)
> > >  {
> > > @@ -1427,7 +1429,7 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
> > >  	intel_plane->pipe = pipe;
> > >  	intel_plane->i9xx_plane = plane;
> > >  	intel_plane->id = PLANE_SPRITE0 + plane;
> > > -	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
> > > +	intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER(pipe, intel_plane->id);
> > >  	intel_plane->check_plane = intel_check_sprite_plane;
> > >  
> > >  	possible_crtcs = (1 << pipe);
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux