diff mbox series

[v6,22/28] drm/i915/dp: Populate DSC PPS SDP and send PPS infoframes

Message ID 20181024222840.25683-23-manasi.d.navare@intel.com (mailing list archive)
State New, archived
Headers show
Series Display Stream Compression enabling on eDP/DP | expand

Commit Message

Navare, Manasi Oct. 24, 2018, 10:28 p.m. UTC 
DSC PPS secondary data packet infoframes are filled with
DSC picure parameter set metadata according to the DSC standard.
These infoframes are sent to the sink device and used during DSC
decoding.

v2:
* Rebase ond drm-tip

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/intel_vdsc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Ville Syrjala Oct. 25, 2018, 2:09 p.m. UTC | #1 
On Wed, Oct 24, 2018 at 03:28:34PM -0700, Manasi Navare wrote:
> DSC PPS secondary data packet infoframes are filled with
> DSC picure parameter set metadata according to the DSC standard.
> These infoframes are sent to the sink device and used during DSC
> decoding.
> 
> v2:
> * Rebase ond drm-tip
> 
> 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/intel_vdsc.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
> index b0fc716bbbfd..4b4b812d68f3 100644
> --- a/drivers/gpu/drm/i915/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> @@ -988,6 +988,25 @@ static void intel_configure_pps_for_dsc_encoder(struct intel_encoder *encoder,
>  	}
>  }
>  
> +static void intel_dp_send_dsc_pps_sdp(struct intel_encoder *encoder,
> +				      struct intel_crtc_state *crtc_state)

const crtc_state

s/send/write/ ?

> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;
> +	struct drm_dsc_pps_infoframe dp_dsc_pps_sdp;
> +
> +	/* Prepare DP SDP PPS header as per DP 1.4 spec, Table 2-123 */
> +	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp);
> +
> +	/* Fill the PPS payload bytes as per DSC spec 1.2 Table 4-1 */
> +	drm_dsc_pps_infoframe_pack(&dp_dsc_pps_sdp, vdsc_cfg);
> +
> +	intel_dig_port->write_infoframe(encoder, crtc_state,
> +					DP_SDP_PPS, &dp_dsc_pps_sdp,
> +					sizeof(dp_dsc_pps_sdp));
> +}
> +
>  void intel_dsc_enable(struct intel_encoder *encoder,
>  		      struct intel_crtc_state *crtc_state)
>  {
> @@ -997,5 +1016,7 @@ void intel_dsc_enable(struct intel_encoder *encoder,
>  
>  	intel_configure_pps_for_dsc_encoder(encoder, crtc_state);
>  
> +	intel_dp_send_dsc_pps_sdp(encoder, crtc_state);
> +
>  	return;
>  }
> -- 
> 2.18.0
Navare, Manasi Oct. 25, 2018, 8:07 p.m. UTC | #2 
On Thu, Oct 25, 2018 at 05:09:42PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 24, 2018 at 03:28:34PM -0700, Manasi Navare wrote:
> > DSC PPS secondary data packet infoframes are filled with
> > DSC picure parameter set metadata according to the DSC standard.
> > These infoframes are sent to the sink device and used during DSC
> > decoding.
> > 
> > v2:
> > * Rebase ond drm-tip
> > 
> > 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/intel_vdsc.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
> > index b0fc716bbbfd..4b4b812d68f3 100644
> > --- a/drivers/gpu/drm/i915/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> > @@ -988,6 +988,25 @@ static void intel_configure_pps_for_dsc_encoder(struct intel_encoder *encoder,
> >  	}
> >  }
> >  
> > +static void intel_dp_send_dsc_pps_sdp(struct intel_encoder *encoder,
> > +				      struct intel_crtc_state *crtc_state)
> 
> const crtc_state

Yes wil make this a const
> 
> s/send/write/ ?

Hmm in terms of VDSC, the SDP packet is the one that gets sent to the sink from source
after we write the infoframe.
So I named it as _send_dsc_pps_sdp, but I am okay changing that to write_dsc_pps_sdp
since all we are doing is writing an infoframe that gets sent out.

Manasi

> 
> > +{
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > +	struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;
> > +	struct drm_dsc_pps_infoframe dp_dsc_pps_sdp;
> > +
> > +	/* Prepare DP SDP PPS header as per DP 1.4 spec, Table 2-123 */
> > +	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp);
> > +
> > +	/* Fill the PPS payload bytes as per DSC spec 1.2 Table 4-1 */
> > +	drm_dsc_pps_infoframe_pack(&dp_dsc_pps_sdp, vdsc_cfg);
> > +
> > +	intel_dig_port->write_infoframe(encoder, crtc_state,
> > +					DP_SDP_PPS, &dp_dsc_pps_sdp,
> > +					sizeof(dp_dsc_pps_sdp));
> > +}
> > +
> >  void intel_dsc_enable(struct intel_encoder *encoder,
> >  		      struct intel_crtc_state *crtc_state)
> >  {
> > @@ -997,5 +1016,7 @@ void intel_dsc_enable(struct intel_encoder *encoder,
> >  
> >  	intel_configure_pps_for_dsc_encoder(encoder, crtc_state);
> >  
> > +	intel_dp_send_dsc_pps_sdp(encoder, crtc_state);
> > +
> >  	return;
> >  }
> > -- 
> > 2.18.0
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Navare, Manasi Oct. 30, 2018, 11:45 p.m. UTC | #3 
On Thu, Oct 25, 2018 at 05:09:42PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 24, 2018 at 03:28:34PM -0700, Manasi Navare wrote:
> > DSC PPS secondary data packet infoframes are filled with
> > DSC picure parameter set metadata according to the DSC standard.
> > These infoframes are sent to the sink device and used during DSC
> > decoding.
> > 
> > v2:
> > * Rebase ond drm-tip
> > 
> > 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/intel_vdsc.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
> > index b0fc716bbbfd..4b4b812d68f3 100644
> > --- a/drivers/gpu/drm/i915/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> > @@ -988,6 +988,25 @@ static void intel_configure_pps_for_dsc_encoder(struct intel_encoder *encoder,
> >  	}
> >  }
> >  
> > +static void intel_dp_send_dsc_pps_sdp(struct intel_encoder *encoder,
> > +				      struct intel_crtc_state *crtc_state)
> 
> const crtc_state

Changing this to const crtc_state started giving me an error when I get the
struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;

So this is making me think that since dp_dsc_cfg is written during compute_config,
and there on we just read it, we dont need to pass *vdsc_cfg , we can
just pass the struct directly right?

Manasi

> 
> s/send/write/ ?
> 
> > +{
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > +	struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;
> > +	struct drm_dsc_pps_infoframe dp_dsc_pps_sdp;
> > +
> > +	/* Prepare DP SDP PPS header as per DP 1.4 spec, Table 2-123 */
> > +	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp);
> > +
> > +	/* Fill the PPS payload bytes as per DSC spec 1.2 Table 4-1 */
> > +	drm_dsc_pps_infoframe_pack(&dp_dsc_pps_sdp, vdsc_cfg);
> > +
> > +	intel_dig_port->write_infoframe(encoder, crtc_state,
> > +					DP_SDP_PPS, &dp_dsc_pps_sdp,
> > +					sizeof(dp_dsc_pps_sdp));
> > +}
> > +
> >  void intel_dsc_enable(struct intel_encoder *encoder,
> >  		      struct intel_crtc_state *crtc_state)
> >  {
> > @@ -997,5 +1016,7 @@ void intel_dsc_enable(struct intel_encoder *encoder,
> >  
> >  	intel_configure_pps_for_dsc_encoder(encoder, crtc_state);
> >  
> > +	intel_dp_send_dsc_pps_sdp(encoder, crtc_state);
> > +
> >  	return;
> >  }
> > -- 
> > 2.18.0
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Ville Syrjala Oct. 31, 2018, 1:09 p.m. UTC | #4 
On Tue, Oct 30, 2018 at 04:45:35PM -0700, Manasi Navare wrote:
> On Thu, Oct 25, 2018 at 05:09:42PM +0300, Ville Syrjälä wrote:
> > On Wed, Oct 24, 2018 at 03:28:34PM -0700, Manasi Navare wrote:
> > > DSC PPS secondary data packet infoframes are filled with
> > > DSC picure parameter set metadata according to the DSC standard.
> > > These infoframes are sent to the sink device and used during DSC
> > > decoding.
> > > 
> > > v2:
> > > * Rebase ond drm-tip
> > > 
> > > 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/intel_vdsc.c | 21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
> > > index b0fc716bbbfd..4b4b812d68f3 100644
> > > --- a/drivers/gpu/drm/i915/intel_vdsc.c
> > > +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> > > @@ -988,6 +988,25 @@ static void intel_configure_pps_for_dsc_encoder(struct intel_encoder *encoder,
> > >  	}
> > >  }
> > >  
> > > +static void intel_dp_send_dsc_pps_sdp(struct intel_encoder *encoder,
> > > +				      struct intel_crtc_state *crtc_state)
> > 
> > const crtc_state
> 
> Changing this to const crtc_state started giving me an error when I get the
> struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;

const struct drm_dsc_config ...

> 
> So this is making me think that since dp_dsc_cfg is written during compute_config,
> and there on we just read it, we dont need to pass *vdsc_cfg , we can
> just pass the struct directly right?

Not sure what you're asking here.

> 
> Manasi
> 
> > 
> > s/send/write/ ?
> > 
> > > +{
> > > +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > +	struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;
> > > +	struct drm_dsc_pps_infoframe dp_dsc_pps_sdp;
> > > +
> > > +	/* Prepare DP SDP PPS header as per DP 1.4 spec, Table 2-123 */
> > > +	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp);
> > > +
> > > +	/* Fill the PPS payload bytes as per DSC spec 1.2 Table 4-1 */
> > > +	drm_dsc_pps_infoframe_pack(&dp_dsc_pps_sdp, vdsc_cfg);
> > > +
> > > +	intel_dig_port->write_infoframe(encoder, crtc_state,
> > > +					DP_SDP_PPS, &dp_dsc_pps_sdp,
> > > +					sizeof(dp_dsc_pps_sdp));
> > > +}
> > > +
> > >  void intel_dsc_enable(struct intel_encoder *encoder,
> > >  		      struct intel_crtc_state *crtc_state)
> > >  {
> > > @@ -997,5 +1016,7 @@ void intel_dsc_enable(struct intel_encoder *encoder,
> > >  
> > >  	intel_configure_pps_for_dsc_encoder(encoder, crtc_state);
> > >  
> > > +	intel_dp_send_dsc_pps_sdp(encoder, crtc_state);
> > > +
> > >  	return;
> > >  }
> > > -- 
> > > 2.18.0
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_vdsc.c b/drivers/gpu/drm/i915/intel_vdsc.c
index b0fc716bbbfd..4b4b812d68f3 100644
--- a/drivers/gpu/drm/i915/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/intel_vdsc.c
@@ -988,6 +988,25 @@  static void intel_configure_pps_for_dsc_encoder(struct intel_encoder *encoder,
 	}
 }
 
+static void intel_dp_send_dsc_pps_sdp(struct intel_encoder *encoder,
+				      struct intel_crtc_state *crtc_state)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;
+	struct drm_dsc_pps_infoframe dp_dsc_pps_sdp;
+
+	/* Prepare DP SDP PPS header as per DP 1.4 spec, Table 2-123 */
+	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp);
+
+	/* Fill the PPS payload bytes as per DSC spec 1.2 Table 4-1 */
+	drm_dsc_pps_infoframe_pack(&dp_dsc_pps_sdp, vdsc_cfg);
+
+	intel_dig_port->write_infoframe(encoder, crtc_state,
+					DP_SDP_PPS, &dp_dsc_pps_sdp,
+					sizeof(dp_dsc_pps_sdp));
+}
+
 void intel_dsc_enable(struct intel_encoder *encoder,
 		      struct intel_crtc_state *crtc_state)
 {
@@ -997,5 +1016,7 @@  void intel_dsc_enable(struct intel_encoder *encoder,
 
 	intel_configure_pps_for_dsc_encoder(encoder, crtc_state);
 
+	intel_dp_send_dsc_pps_sdp(encoder, crtc_state);
+
 	return;
 }