[Intel-gfx] [PATCH v6 12/28] drm/i915/dp: Add DSC params and DSC config to intel_crtc_state

[Ville Syrjälä]

On Wed, Oct 31, 2018 at 09:05:05AM -0700, Manasi Navare wrote:
> On Wed, Oct 31, 2018 at 03:10:15PM +0200, Ville Syrjälä wrote:
> > On Tue, Oct 30, 2018 at 04:53:49PM -0700, Manasi Navare wrote:
> > > On Wed, Oct 24, 2018 at 03:28:24PM -0700, Manasi Navare wrote:
> > > > Basic DSC parameters and DSC configuration data needs to be computed
> > > > for each of the requested mode during atomic check. This is
> > > > required since for certain modes, valid DSC parameters and config
> > > > data might not be computed in which case compression cannot be
> > > > enabled for that mode.
> > > > For that reason we need to add these params and config structure
> > > > to the intel_crtc_state so that if valid this state information
> > > > can directly be used while enabling DSC in atomic commit.
> > > > 
> > > > v2:
> > > > * Rebase on drm-tip (Manasi)
> > > > 
> > > > Cc: Gaurav K Singh <gaurav.k.singh at intel.com>
> > > > Cc: Jani Nikula <jani.nikula at linux.intel.com>
> > > > Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> > > > Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > > > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> > > > Reviewed-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.h  | 1 +
> > > >  drivers/gpu/drm/i915/intel_drv.h | 9 +++++++++
> > > >  2 files changed, 10 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 2d7761b8ac07..45fd7894722b 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -53,6 +53,7 @@
> > > >  #include <drm/drm_auth.h>
> > > >  #include <drm/drm_cache.h>
> > > >  #include <drm/drm_util.h>
> > > > +#include <drm/drm_dsc.h>
> > > >  
> > > >  #include "i915_params.h"
> > > >  #include "i915_reg.h"
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > index 62c051098859..27d47950f438 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -931,6 +931,15 @@ struct intel_crtc_state {
> > > >  
> > > >  	/* Output down scaling is done in LSPCON device */
> > > >  	bool lspcon_downsampling;
> > > > +
> > > > +	/* Display Stream compression state */
> > > > +	struct {
> > > > +		bool compression_enable;
> > > > +		bool dsc_split;
> > > > +		u16 compressed_bpp;
> > > > +		u8 slice_count;
> > > > +	} dsc_params;
> > > > +	struct drm_dsc_config dp_dsc_cfg;
> > > 
> > > Ville, Jani should this be defined as a pointer to struct drm_dsc_config?
> > > struct drm_dsc_config *dp_dsc_cfg
> > > since we populate this in intel_dp_compute_config and then on only read during
> > > commit.
> > 
> > Pointer to where exactly? You need the memory for it somewhere.
> >
> 
> struct drm_dsc_config  is defined in drm_dsc.h and gets populated later in the dp_compute_config.
> So I guess wwe can leave it as strcut drm_dsc_config dp_dsc_cfg; in crtc_state?
> Pass a pointer to that while populating in the compute-config() but then later in the commit
> when we have to read from this to write the sdp infoframe, we just pass the struct directly, no 
> need to pass a pointer since its gonna be read and also crtc_state will be a const
> 
> Is that correct?

You don't want to be passing large structures by value, if that's what
you're suggesting.

-- 
Ville Syrjälä
Intel