| Message ID | 20181024222840.25683-13-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Display Stream Compression enabling on eDP/DP | expand |
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@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > Reviewed-by: Anusha Srivatsa <anusha.srivatsa@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. Manasi > }; > > struct intel_crtc { > -- > 2.18.0 >
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@intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > Reviewed-by: Anusha Srivatsa <anusha.srivatsa@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. > > Manasi > > > }; > > > > struct intel_crtc { > > -- > > 2.18.0 > >
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@intel.com> > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > Reviewed-by: Anusha Srivatsa <anusha.srivatsa@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? Manasi > > > > Manasi > > > > > }; > > > > > > struct intel_crtc { > > > -- > > > 2.18.0 > > > > > -- > Ville Syrjälä > Intel
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@intel.com> > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > Reviewed-by: Anusha Srivatsa <anusha.srivatsa@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.
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; }; struct intel_crtc {