Nice! All I've got is a couple of typos I noticed and one question, this looks
great :)
On Tue, 2019-12-03 at 09:35 -0500, mikita.lipski@xxxxxxx wrote:
> From: Mikita Lipski <mikita.lipski@xxxxxxx>
>
> 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@xxxxxxx>
> Cc: Lyude Paul <lyude@xxxxxxxxxx>
> Signed-off-by: Mikita Lipski <mikita.lipski@xxxxxxx>
> ---
> 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 f1d883960831..5e549f48ffb8 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4742,6 +4742,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
Typo here
> + * @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
> + *
> + */
> +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;
> + }
Just double checking-does this handle the case where we're enabling DSC on a
port that didn't previously have a VCPI allocation because it wasn't enabled
previously? Or do we not need to handle that here
Assuming you did the right thing here, with the small typo fixes:
Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>
> +
> + 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;
> + }
> +
> + 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 0f813d6346aa..830c94b7f45d 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -502,6 +502,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;
> };
>
> @@ -773,6 +774,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,
--
Cheers,
Lyude Paul
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
