Re: [PATCH v7 11/17] drm/dp_mst: Add DSC enablement helpers to DRM

From: Leo <sunpeng.li-5C7GfCeVMHo@public.gmane.org>
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH v7 11/17] drm/dp_mst: Add DSC enablement helpers to DRM
Date: Tue, 26 Nov 2019 10:24:30 -0500	[thread overview]
Message-ID: <fe56a601-777d-f8db-c229-f155a162f100@amd.com> (raw)
In-Reply-To: <20191116220128.16598-12-mikita.lipski-5C7GfCeVMHo@public.gmane.org>

On 2019-11-16 5:01 p.m., mikita.lipski@amd.com wrote:
> From: Mikita Lipski <mikita.lipski@amd.com>
> 
> Adding a helper function to be called by
> drivers outside of DRM to enable DSC on
> the MST ports.
> 
> Function is called to recalculate VCPI allocation
> if DSC is enabled and raise the DSC flag to enable.
> In case of disabling DSC the flag is set to false
> and recalculation of VCPI slots is expected to be done
> in encoder's atomic_check.
> 
> v2: squash separate functions into one and call it per
> port
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Mikita Lipski <mikita.lipski@amd.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 61 +++++++++++++++++++++++++++
>  include/drm/drm_dp_mst_helper.h       |  5 +++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 94bb259ab73e..98cc93d5ddd7 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3876,6 +3876,67 @@ drm_dp_mst_atomic_check_topology_state(struct drm_dp_mst_topology_mgr *mgr,
>  	return 0;
>  }
>  
> +/**
> + * drm_dp_mst_atomic_enable_dsc - Set DSC Enable Flag to On/Off
> + * @state: Pointer to the new drm_atomic_state 
> + * @pointer: Pointer to the affected MST Port

s/@pointer/@port/

> + * @pbn: Newly recalculated bw required for link with DSC enabled
> + * @pbn_div: Divider to calculate correct number of pbn per slot
> + * @enable: Boolean flag enabling or disabling DSC on the port
> + *
> + * This function enables DSC on the given Port
> + * by recalculating its vcpi from pbn provided
> + * and sets dsc_enable flag to keep track of which
> + * ports have DSC enabled

Would be good to document the return value as well.

> + *
> + */
> +int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state,
> +				 struct drm_dp_mst_port *port,
> +				 int pbn, int pbn_div,
> +				 bool enable)
> +{
> +	struct drm_dp_mst_topology_state *mst_state;
> +	struct drm_dp_vcpi_allocation *pos;
> +	bool found = false;
> +	int vcpi = 0;
> +
> +	mst_state = drm_atomic_get_mst_topology_state(state, port->mgr);
> +
> +	if (IS_ERR(mst_state))
> +		return PTR_ERR(mst_state);
> +
> +	list_for_each_entry(pos, &mst_state->vcpis, next) {
> +		if (pos->port == port) {
> +			found = true;
> +			break;
> +		}
> +	}
> +
> +	if (!found) {
> +		DRM_DEBUG_ATOMIC("[MST PORT:%p] Couldn't find VCPI allocation in mst state %p\n",
> +				 port, mst_state);
> +		return -EINVAL;
> +	}
> +
> +	if (pos->dsc_enabled == enable) {
> +		DRM_DEBUG_ATOMIC("[MST PORT:%p] DSC flag is already set to %d, returning %d VCPI slots\n",
> +				 port, enable, pos->vcpi);
> +		vcpi = pos->vcpi;

Do we want to early return here?

- Leo

> +	}
> +
> +	if (enable) {
> +		vcpi = drm_dp_atomic_find_vcpi_slots(state, port->mgr, port, pbn, pbn_div);
> +		DRM_DEBUG_ATOMIC("[MST PORT:%p] Enabling DSC flag, reallocating %d VCPI slots on the port\n",
> +				 port, vcpi);
> +		if (vcpi < 0)
> +			return -EINVAL;
> +	}
> +
> +	pos->dsc_enabled = enable;
> +
> +	return vcpi;
> +}
> +EXPORT_SYMBOL(drm_dp_mst_atomic_enable_dsc);
>  /**
>   * drm_dp_mst_atomic_check - Check that the new state of an MST topology in an
>   * atomic update is valid
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index fc19094b06c3..b1b00de3083b 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -431,6 +431,7 @@ struct drm_dp_payload {
>  struct drm_dp_vcpi_allocation {
>  	struct drm_dp_mst_port *port;
>  	int vcpi;
> +	bool dsc_enabled;
>  	struct list_head next;
>  };
>  
> @@ -663,6 +664,10 @@ drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state,
>  			      struct drm_dp_mst_topology_mgr *mgr,
>  			      struct drm_dp_mst_port *port, int pbn,
>  			      int pbn_div);
> +int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state,
> +				 struct drm_dp_mst_port *port,
> +				 int pbn, int pbn_div,
> +				 bool enable);
>  int __must_check
>  drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state,
>  				 struct drm_dp_mst_topology_mgr *mgr,
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx