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

2019-11-26 Thread Mikita Lipski



On 26/11/2019 10:24, Leo wrote:



On 2019-11-16 5:01 p.m., mikita.lip...@amd.com wrote:

From: Mikita Lipski 

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 
Cc: Lyude Paul 
Signed-off-by: Mikita Lipski 
---
  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/


Thanks,
will update the comment.


+ * @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?


Not if a new compression rate is set.
The DSC could still be enabled, but the new device is introduced - 
higher compression will be required, so PBN will change and we will have 
to reassign the slots to the port.




- 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F

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

2019-11-26 Thread Leo
Just realized Lyude wasn't CC'd originally, and I forgot to CC everyone as well 
in the reply :)

Leo

On 2019-11-26 10:24 a.m., Leo wrote:
> 
> 
> On 2019-11-16 5:01 p.m., mikita.lip...@amd.com wrote:
>> From: Mikita Lipski 
>>
>> 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 
>> Cc: Lyude Paul 
>> Signed-off-by: Mikita Lipski 
>> ---
>>  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

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

2019-11-26 Thread Leo


On 2019-11-16 5:01 p.m., mikita.lip...@amd.com wrote:
> From: Mikita Lipski 
> 
> 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 
> Cc: Lyude Paul 
> Signed-off-by: Mikita Lipski 
> ---
>  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

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

2019-11-16 Thread mikita.lipski
From: Mikita Lipski 

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 
Cc: Lyude Paul 
Signed-off-by: Mikita Lipski 
---
 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
+ * @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;
+   }
+
+   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 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,
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx