Re: [PATCH 01/13] drm/amd/display: Add MST atomic routines
On 31.10.2019 9:16, Kazlauskas, Nicholas wrote:
> On 2019-10-30 3:24 p.m., mikita.lip...@amd.com wrote:
>> From: Mikita Lipski
>>
>> - Adding encoder atomic check to find vcpi slots for a connector
>> - Using DRM helper functions to calculate PBN
>> - Adding connector atomic check to release vcpi slots if connector
>> loses CRTC
>> - Calculate PBN and VCPI slots only once during atomic
>> check and store them on crtc_state to eliminate
>> redundant calculation
>> - Call drm_dp_mst_atomic_check to verify validity of MST topology
>> during state atomic check
>>
>> v2: squashed previous 3 separate patches, removed DSC PBN calculation,
>> and added PBN and VCPI slots properties to amdgpu connector
>>
>> v3:
>> - moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state
>> - updates stream's vcpi_slots and pbn on commit
>> - separated patch from the DSC MST series
>>
>> v4:
>> - set vcpi_slots and pbn properties to dm_connector_state
>> - copy porperties from connector state on to crtc state
>>
>> v5:
>> - keep the pbn and vcpi values only on connnector state
>> - added a void pointer to the stream state instead on two ints,
>> because dc_stream_state is OS agnostic. Pointer points to the
>> current dm_connector_state.
>>
>> v6:
>> - Remove new param from stream
>>
>> v7:
>> - Fix error with using max capable bpc
>>
>> Cc: Jun Lei
>> Cc: Jerry Zuo
>> Cc: Harry Wentland
>> Cc: Nicholas Kazlauskas
>> Cc: Lyude Paul
>> Signed-off-by: Mikita Lipski
> Reviewed-by: Nicholas Kazlauskas
>
> You might want to verify that this still works as you expect when
> changing "max bpc" on an MST display.
>
> My understanding is that it'd still force a new modeset before encoder
> atomic check is called so you *should* have the correct bpc value during
> MST calculations.
Thanks.
It does still works with MST even if you change the mode to the lower
resolution.
The encoder atomic check is called during drm_atomic_helper_check_modeset
so new modeset is already forced then.
>> ---
>>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 65 ++-
>>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
>>.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 ---
>>.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32 +
>>4 files changed, 109 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 48f5b43e2698..d75726013436 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct
>> drm_connector *connector)
>> state->underscan_hborder = 0;
>> state->underscan_vborder = 0;
>> state->base.max_requested_bpc = 8;
>> -
>> +state->vcpi_slots = 0;
>> +state->pbn = 0;
>> if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
>> state->abm_level = amdgpu_dm_abm_level;
>>
>> @@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct
>> drm_connector *connector)
>> new_state->underscan_enable = state->underscan_enable;
>> new_state->underscan_hborder = state->underscan_hborder;
>> new_state->underscan_vborder = state->underscan_vborder;
>> -
>> +new_state->vcpi_slots = state->vcpi_slots;
>> +new_state->pbn = state->pbn;
>> return &new_state->base;
>>}
>>
>> @@ -4606,10 +4608,64 @@ static void dm_encoder_helper_disable(struct
>> drm_encoder *encoder)
>>
>>}
>>
>> +static int convert_dc_color_depth_into_bpc (enum dc_color_depth
>> display_color_depth)
>> +{
>> +switch (display_color_depth) {
>> +case COLOR_DEPTH_666:
>> +return 6;
>> +case COLOR_DEPTH_888:
>> +return 8;
>> +case COLOR_DEPTH_101010:
>> +return 10;
>> +case COLOR_DEPTH_121212:
>> +return 12;
>> +case COLOR_DEPTH_141414:
>> +return 14;
>> +case COLOR_DEPTH_161616:
>> +return 16;
>> +default:
>> +break;
>> +}
>> +return 0;
>> +}
>> +
>>static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
>>struct drm_crtc_state *crtc_state,
>>struct drm_connector_state
>> *conn_state)
>>{
>> +struct drm_atomic_state *state = crtc_state->state;
>> +struct drm_connector *connector = conn_state->connector;
>> +struct amdgpu_dm_connector *aconnector =
>> to_amdgpu_dm_connector(connector);
>> +struct dm_connector_state *dm_new_connector_state =
>> to_dm_connector_state(conn_state);
>> +const struct drm_display_mode *adjusted_mode =
>> &crtc_state->adjusted_mode;
>>
Re: [PATCH 01/13] drm/amd/display: Add MST atomic routines
On 2019-10-30 3:24 p.m., mikita.lip...@amd.com wrote:
> From: Mikita Lipski
>
> - Adding encoder atomic check to find vcpi slots for a connector
> - Using DRM helper functions to calculate PBN
> - Adding connector atomic check to release vcpi slots if connector
> loses CRTC
> - Calculate PBN and VCPI slots only once during atomic
> check and store them on crtc_state to eliminate
> redundant calculation
> - Call drm_dp_mst_atomic_check to verify validity of MST topology
> during state atomic check
>
> v2: squashed previous 3 separate patches, removed DSC PBN calculation,
> and added PBN and VCPI slots properties to amdgpu connector
>
> v3:
> - moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state
> - updates stream's vcpi_slots and pbn on commit
> - separated patch from the DSC MST series
>
> v4:
> - set vcpi_slots and pbn properties to dm_connector_state
> - copy porperties from connector state on to crtc state
>
> v5:
> - keep the pbn and vcpi values only on connnector state
> - added a void pointer to the stream state instead on two ints,
> because dc_stream_state is OS agnostic. Pointer points to the
> current dm_connector_state.
>
> v6:
> - Remove new param from stream
>
> v7:
> - Fix error with using max capable bpc
>
> Cc: Jun Lei
> Cc: Jerry Zuo
> Cc: Harry Wentland
> Cc: Nicholas Kazlauskas
> Cc: Lyude Paul
> Signed-off-by: Mikita Lipski
Reviewed-by: Nicholas Kazlauskas
You might want to verify that this still works as you expect when
changing "max bpc" on an MST display.
My understanding is that it'd still force a new modeset before encoder
atomic check is called so you *should* have the correct bpc value during
MST calculations.
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 65 ++-
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 ---
> .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32 +
> 4 files changed, 109 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 48f5b43e2698..d75726013436 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct
> drm_connector *connector)
> state->underscan_hborder = 0;
> state->underscan_vborder = 0;
> state->base.max_requested_bpc = 8;
> -
> + state->vcpi_slots = 0;
> + state->pbn = 0;
> if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
> state->abm_level = amdgpu_dm_abm_level;
>
> @@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct
> drm_connector *connector)
> new_state->underscan_enable = state->underscan_enable;
> new_state->underscan_hborder = state->underscan_hborder;
> new_state->underscan_vborder = state->underscan_vborder;
> -
> + new_state->vcpi_slots = state->vcpi_slots;
> + new_state->pbn = state->pbn;
> return &new_state->base;
> }
>
> @@ -4606,10 +4608,64 @@ static void dm_encoder_helper_disable(struct
> drm_encoder *encoder)
>
> }
>
> +static int convert_dc_color_depth_into_bpc (enum dc_color_depth
> display_color_depth)
> +{
> + switch (display_color_depth) {
> + case COLOR_DEPTH_666:
> + return 6;
> + case COLOR_DEPTH_888:
> + return 8;
> + case COLOR_DEPTH_101010:
> + return 10;
> + case COLOR_DEPTH_121212:
> + return 12;
> + case COLOR_DEPTH_141414:
> + return 14;
> + case COLOR_DEPTH_161616:
> + return 16;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
> static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
> struct drm_crtc_state *crtc_state,
> struct drm_connector_state
> *conn_state)
> {
> + struct drm_atomic_state *state = crtc_state->state;
> + struct drm_connector *connector = conn_state->connector;
> + struct amdgpu_dm_connector *aconnector =
> to_amdgpu_dm_connector(connector);
> + struct dm_connector_state *dm_new_connector_state =
> to_dm_connector_state(conn_state);
> + const struct drm_display_mode *adjusted_mode =
> &crtc_state->adjusted_mode;
> + struct drm_dp_mst_topology_mgr *mst_mgr;
> + struct drm_dp_mst_port *mst_port;
> + enum dc_color_depth color_depth;
> + int clock, bpp = 0;
> +
> + if (!aconnector->port || !aconnector->dc_sink)
> + return 0;
> +
> + mst_port = aconnector->port;
> + mst_mgr = &aconnector->mst_port->mst_mgr;
> +
> + if (!c
[PATCH 01/13] drm/amd/display: Add MST atomic routines
From: Mikita Lipski
- Adding encoder atomic check to find vcpi slots for a connector
- Using DRM helper functions to calculate PBN
- Adding connector atomic check to release vcpi slots if connector
loses CRTC
- Calculate PBN and VCPI slots only once during atomic
check and store them on crtc_state to eliminate
redundant calculation
- Call drm_dp_mst_atomic_check to verify validity of MST topology
during state atomic check
v2: squashed previous 3 separate patches, removed DSC PBN calculation,
and added PBN and VCPI slots properties to amdgpu connector
v3:
- moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state
- updates stream's vcpi_slots and pbn on commit
- separated patch from the DSC MST series
v4:
- set vcpi_slots and pbn properties to dm_connector_state
- copy porperties from connector state on to crtc state
v5:
- keep the pbn and vcpi values only on connnector state
- added a void pointer to the stream state instead on two ints,
because dc_stream_state is OS agnostic. Pointer points to the
current dm_connector_state.
v6:
- Remove new param from stream
v7:
- Fix error with using max capable bpc
Cc: Jun Lei
Cc: Jerry Zuo
Cc: Harry Wentland
Cc: Nicholas Kazlauskas
Cc: Lyude Paul
Signed-off-by: Mikita Lipski
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 65 ++-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 ---
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32 +
4 files changed, 109 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 48f5b43e2698..d75726013436 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector
*connector)
state->underscan_hborder = 0;
state->underscan_vborder = 0;
state->base.max_requested_bpc = 8;
-
+ state->vcpi_slots = 0;
+ state->pbn = 0;
if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
state->abm_level = amdgpu_dm_abm_level;
@@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct
drm_connector *connector)
new_state->underscan_enable = state->underscan_enable;
new_state->underscan_hborder = state->underscan_hborder;
new_state->underscan_vborder = state->underscan_vborder;
-
+ new_state->vcpi_slots = state->vcpi_slots;
+ new_state->pbn = state->pbn;
return &new_state->base;
}
@@ -4606,10 +4608,64 @@ static void dm_encoder_helper_disable(struct
drm_encoder *encoder)
}
+static int convert_dc_color_depth_into_bpc (enum dc_color_depth
display_color_depth)
+{
+ switch (display_color_depth) {
+ case COLOR_DEPTH_666:
+ return 6;
+ case COLOR_DEPTH_888:
+ return 8;
+ case COLOR_DEPTH_101010:
+ return 10;
+ case COLOR_DEPTH_121212:
+ return 12;
+ case COLOR_DEPTH_141414:
+ return 14;
+ case COLOR_DEPTH_161616:
+ return 16;
+ default:
+ break;
+ }
+ return 0;
+}
+
static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state
*conn_state)
{
+ struct drm_atomic_state *state = crtc_state->state;
+ struct drm_connector *connector = conn_state->connector;
+ struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
+ struct dm_connector_state *dm_new_connector_state =
to_dm_connector_state(conn_state);
+ const struct drm_display_mode *adjusted_mode =
&crtc_state->adjusted_mode;
+ struct drm_dp_mst_topology_mgr *mst_mgr;
+ struct drm_dp_mst_port *mst_port;
+ enum dc_color_depth color_depth;
+ int clock, bpp = 0;
+
+ if (!aconnector->port || !aconnector->dc_sink)
+ return 0;
+
+ mst_port = aconnector->port;
+ mst_mgr = &aconnector->mst_port->mst_mgr;
+
+ if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
+ return 0;
+
+ if (!state->duplicated) {
+ color_depth = convert_color_depth_from_display_info(connector,
conn_state);
+ bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
+ clock = adjusted_mode->clock;
+ dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp);
+ }
+ dm_new_connector_state->vcpi_slots =
drm_dp_atomic_find_vcpi_slots(state,
+
[PATCH 01/13] drm/amd/display: Add MST atomic routines
From: Mikita Lipski
- Adding encoder atomic check to find vcpi slots for a connector
- Using DRM helper functions to calculate PBN
- Adding connector atomic check to release vcpi slots if connector
loses CRTC
- Calculate PBN and VCPI slots only once during atomic
check and store them on crtc_state to eliminate
redundant calculation
- Call drm_dp_mst_atomic_check to verify validity of MST topology
during state atomic check
v2: squashed previous 3 separate patches, removed DSC PBN calculation,
and added PBN and VCPI slots properties to amdgpu connector
v3:
- moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state
- updates stream's vcpi_slots and pbn on commit
- separated patch from the DSC MST series
v4:
- set vcpi_slots and pbn properties to dm_connector_state
- copy porperties from connector state on to crtc state
v5:
- keep the pbn and vcpi values only on connnector state
- added a void pointer to the stream state instead on two ints,
because dc_stream_state is OS agnostic. Pointer points to the
current dm_connector_state.
v6:
- Remove new param from stream
Cc: Jun Lei
Cc: Jerry Zuo
Cc: Harry Wentland
Cc: Nicholas Kazlauskas
Cc: Lyude Paul
Signed-off-by: Mikita Lipski
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 +--
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 32
4 files changed, 86 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 48f5b43e2698..28f6b93ab371 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector
*connector)
state->underscan_hborder = 0;
state->underscan_vborder = 0;
state->base.max_requested_bpc = 8;
-
+ state->vcpi_slots = 0;
+ state->pbn = 0;
if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
state->abm_level = amdgpu_dm_abm_level;
@@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct
drm_connector *connector)
new_state->underscan_enable = state->underscan_enable;
new_state->underscan_hborder = state->underscan_hborder;
new_state->underscan_vborder = state->underscan_vborder;
-
+ new_state->vcpi_slots = state->vcpi_slots;
+ new_state->pbn = state->pbn;
return &new_state->base;
}
@@ -4610,6 +4612,37 @@ static int dm_encoder_helper_atomic_check(struct
drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state
*conn_state)
{
+ struct drm_atomic_state *state = crtc_state->state;
+ struct drm_connector *connector = conn_state->connector;
+ struct amdgpu_dm_connector *aconnector =
to_amdgpu_dm_connector(connector);
+ struct dm_connector_state *dm_new_connector_state =
to_dm_connector_state(conn_state);
+ const struct drm_display_mode *adjusted_mode =
&crtc_state->adjusted_mode;
+ struct drm_dp_mst_topology_mgr *mst_mgr;
+ struct drm_dp_mst_port *mst_port;
+ int clock, bpp = 0;
+
+ if (!aconnector->port || !aconnector->dc_sink)
+ return 0;
+
+ mst_port = aconnector->port;
+ mst_mgr = &aconnector->mst_port->mst_mgr;
+
+ if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
+ return 0;
+
+ if (!state->duplicated) {
+ bpp = (uint8_t)connector->display_info.bpc * 3;
+ clock = adjusted_mode->clock;
+ dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp);
+ }
+ dm_new_connector_state->vcpi_slots =
drm_dp_atomic_find_vcpi_slots(state,
+ mst_mgr,
+ mst_port,
+
dm_new_connector_state->pbn);
+ if (dm_new_connector_state->vcpi_slots < 0) {
+ DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n",
dm_new_connector_state->vcpi_slots);
+ return dm_new_connector_state->vcpi_slots;
+ }
return 0;
}
@@ -7651,6 +7684,11 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
+ /* Perform validation of MST topology in the state*/
+ ret = drm_dp_mst_atomic_check(state);
+ if (ret)
+ goto fail;
+
if (state->legacy_cursor_update) {
/*
* This is a fast cursor update coming from the plane update
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgp
