Re: [Intel-gfx] [PATCH 02/19] drm/i915/dp: Store DSC DPCD capabilities in the connector

Lisovskiy, Stanislav
2023-10-09T11:17:16Z

On Fri, Oct 06, 2023 at 04:37:10PM +0300, Imre Deak wrote:
> In an MST topology the DSC capabilities are specific to each connector,
> retrieved either from the sink if it decompresses the stream, or from a
> branch device between the source and the sink in case this branch device
> does the decompression. Accordingly each connector needs to cache its
> own DSC DPCD and FEC capabilities, along with the AUX device through
> which the decompression can be enabled. This patch prepares for that by
> storing the capabilities and the DSC AUX device in the connector, for
> now these just matching the version stored in intel_dp. The follow-up
> patches will convert all users to look up these in the connector instead
> of intel_dp, after which the intel_dp copies are removed.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
> .../drm/i915/display/intel_display_types.h | 6 +++
> drivers/gpu/drm/i915/display/intel_dp.c | 53 +++++++++++++------
> 2 files changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 8d8b2f8d37a99..d6600079bcf74 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -620,6 +620,12 @@ struct intel_connector {
>
> struct intel_dp *mst_port;
>
> + struct {
> + struct drm_dp_aux *dsc_decompression_aux;
> + u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE];
> + u8 fec_capability;
> + } dp;
> +
> /* Work struct to schedule a uevent on link train failure */
> struct work_struct modeset_retry_work;
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1bd11f9e308c1..c7dd65a27a1b0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3467,7 +3467,8 @@ bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
> return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
> }
>
> -static void intel_dp_get_dsc_sink_cap(u8 dpcd_rev, struct intel_dp *intel_dp)
> +static void intel_dp_get_dsc_sink_cap(u8 dpcd_rev, struct intel_dp *intel_dp,
> + struct intel_connector *connector)
> {
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>
> @@ -3475,35 +3476,46 @@ static void intel_dp_get_dsc_sink_cap(u8 dpcd_rev, struct intel_dp *intel_dp)
> * Clear the cached register set to avoid using stale values
> * for the sinks that do not support DSC.
> */
> - memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> + memset(connector->dp.dsc_dpcd, 0, sizeof(connector->dp.dsc_dpcd));
>
> /* Clear fec_capable to avoid using stale values */
> - intel_dp->fec_capable = 0;
> + connector->dp.fec_capability = 0;
>
> /* Cache the DSC DPCD if eDP or DP rev >= 1.4 */
> if ((intel_dp_is_edp(intel_dp) && dpcd_rev >= DP_EDP_14) ||
> (!intel_dp_is_edp(intel_dp) && dpcd_rev >= 0x14)) {
> - if (drm_dp_dpcd_read(&intel_dp->aux, DP_DSC_SUPPORT,
> - intel_dp->dsc_dpcd,
> - sizeof(intel_dp->dsc_dpcd)) < 0)
> + if (drm_dp_dpcd_read(connector->dp.dsc_decompression_aux,
> + DP_DSC_SUPPORT,
> + connector->dp.dsc_dpcd,
> + sizeof(connector->dp.dsc_dpcd)) < 0)
> drm_err(&i915->drm,
> "Failed to read DPCD register 0x%x\n",
> DP_DSC_SUPPORT);
>
> drm_dbg_kms(&i915->drm, "DSC DPCD: %*ph\n",
> - (int)sizeof(intel_dp->dsc_dpcd),
> - intel_dp->dsc_dpcd);
> + (int)sizeof(connector->dp.dsc_dpcd),
> + connector->dp.dsc_dpcd);
>
> /* FEC is supported only on DP 1.4 */
> if (!intel_dp_is_edp(intel_dp) &&
> - drm_dp_dpcd_readb(&intel_dp->aux, DP_FEC_CAPABILITY,
> - &intel_dp->fec_capable) < 0)
> + drm_dp_dpcd_readb(connector->dp.dsc_decompression_aux,
> + DP_FEC_CAPABILITY,
> + &connector->dp.fec_capability) < 0)
> drm_err(&i915->drm,
> "Failed to read FEC DPCD register\n");
>
> drm_dbg_kms(&i915->drm, "FEC CAPABILITY: %x\n",
> - intel_dp->fec_capable);
> + connector->dp.fec_capability);
> }
> +
> + /*
> + * TODO: remove the following intel_dp copies once all users
> + * are converted to look up DSC DPCD/FEC capability via the
> + * connector.
> + */
> + memcpy(intel_dp->dsc_dpcd, connector->dp.dsc_dpcd,
> + sizeof(intel_dp->dsc_dpcd));
> + intel_dp->fec_capable = connector->dp.fec_capability;
> }
>
> static void intel_edp_mso_mode_fixup(struct intel_connector *connector,
> @@ -3595,7 +3607,7 @@ static void intel_edp_mso_init(struct intel_dp *intel_dp)
> }
>
> static bool
> -intel_edp_init_dpcd(struct intel_dp *intel_dp)
> +intel_edp_init_dpcd(struct intel_dp *intel_dp, struct intel_connector *connector)
> {
> struct drm_i915_private *dev_priv =
> to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> @@ -3675,7 +3687,8 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
> /* Read the eDP DSC DPCD registers */
> if (HAS_DSC(dev_priv))
> intel_dp_get_dsc_sink_cap(intel_dp->edp_dpcd[0],
> - intel_dp);
> + intel_dp,
> + connector);
>
> /*
> * If needed, program our source OUI so we can make various Intel-specific AUX services
> @@ -5345,7 +5358,9 @@ intel_dp_detect(struct drm_connector *connector,
> bool force)
> {
> struct drm_i915_private *dev_priv = to_i915(connector->dev);
> - struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
> + struct intel_connector *intel_connector =
> + to_intel_connector(connector);
> + struct intel_dp *intel_dp = intel_attached_dp(intel_connector);
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct intel_encoder *encoder = &dig_port->base;
> enum drm_connector_status status;
> @@ -5368,7 +5383,12 @@ intel_dp_detect(struct drm_connector *connector,
>
> if (status == connector_status_disconnected) {
> memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
> + /*
> + * TODO: Remove clearing the DPCD in intel_dp, once all
> + * user are converted to using the DPCD in connector.
> + */
> memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> + memset(intel_connector->dp.dsc_dpcd, 0, sizeof(intel_connector->dp.dsc_dpcd));
>
> if (intel_dp->is_mst) {
> drm_dbg_kms(&dev_priv->drm,
> @@ -5386,7 +5406,7 @@ intel_dp_detect(struct drm_connector *connector,
> /* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> if (HAS_DSC(dev_priv))
> intel_dp_get_dsc_sink_cap(intel_dp->dpcd[DP_DPCD_REV],
> - intel_dp);
> + intel_dp, intel_connector);
>
> intel_dp_configure_mst(intel_dp);
>
> @@ -5971,7 +5991,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> intel_hpd_enable_detection(encoder);
>
> /* Cache DPCD and EDID for edp. */
> - has_dpcd = intel_edp_init_dpcd(intel_dp);
> + has_dpcd = intel_edp_init_dpcd(intel_dp, intel_connector);
>
> if (!has_dpcd) {
> /* if this fails, presume the device is a ghost */
> @@ -6145,6 +6165,7 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
> intel_dp->pps.active_pipe = vlv_active_pipe(intel_dp);
>
> intel_dp_aux_init(intel_dp);
> + intel_connector->dp.dsc_decompression_aux = &intel_dp->aux;
>
> drm_dbg_kms(&dev_priv->drm,
> "Adding %s connector on [ENCODER:%d:%s]\n",
> --
> 2.39.2
>