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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/6/19 7:24 PM, Lyude Paul wrote:
Nice! All I've got is a couple of typos I noticed and one question, this looks
great :)

Thanks! I'll clean it up. The response to the question is below.

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

Because we call encoder atomic check previously to allocate VCPI slots - the port should have VCPI allocation before enabling DSC even if it wasn't enabled previously. Therefore, I was thinking, that if encoder atomic check fails to allocate VCPI slots for the port - we shouldn't enable DSC on it and probably should fail atomic check if that is even requested.

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,

--
Thanks,
Mikita Lipski
Software Engineer, AMD
mikita.lipski@xxxxxxx
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux