drm/amd/display: Always allocate initial connector state state

Submitted by Kazlauskas, Nicholas on June 4, 2019, 7:21 p.m.

Details

Message ID 20190604192114.20995-1-nicholas.kazlauskas@amd.com
State Accepted
Commit f04bee34d6e35df26cbb2d65e801adfd0d8fe20d
Headers show
Series "drm/amd/display: Always allocate initial connector state state" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Kazlauskas, Nicholas June 4, 2019, 7:21 p.m.
[Why]
Unlike our regular connectors, MST connectors don't start off with
an initial connector state. This causes a NULL pointer dereference to
occur when attaching the bpc property since it tries to modify the
connector state.

We need an initial connector state on the connector to avoid the crash.

[How]
Use our reset helper to allocate an initial state and reset the values
to their defaults. We were already doing this before, just not for
MST connectors.

Cc: Leo Li <sunpeng.li@amd.com>
Cc: Roman Li <roman.li@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

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 a698c8f272ed..4523ab100bc3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4743,6 +4743,13 @@  void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 {
 	struct amdgpu_device *adev = dm->ddev->dev_private;
 
+	/*
+	 * Some of the properties below require access to state, like bpc.
+	 * Allocate some default initial connector state with our reset helper.
+	 */
+	if (aconnector->base.funcs->reset)
+		aconnector->base.funcs->reset(&aconnector->base);
+
 	aconnector->connector_id = link_index;
 	aconnector->dc_link = link;
 	aconnector->base.interlace_allowed = false;
@@ -4932,9 +4939,6 @@  static int amdgpu_dm_connector_init(struct amdgpu_display_manager *dm,
 			&aconnector->base,
 			&amdgpu_dm_connector_helper_funcs);
 
-	if (aconnector->base.funcs->reset)
-		aconnector->base.funcs->reset(&aconnector->base);
-
 	amdgpu_dm_connector_init_helper(
 		dm,
 		aconnector,

Comments

On 2019-06-04 3:21 p.m., Nicholas Kazlauskas wrote:
> [Why]

> Unlike our regular connectors, MST connectors don't start off with

> an initial connector state. This causes a NULL pointer dereference to

> occur when attaching the bpc property since it tries to modify the

> connector state.

> 

> We need an initial connector state on the connector to avoid the crash.

> 

> [How]

> Use our reset helper to allocate an initial state and reset the values

> to their defaults. We were already doing this before, just not for

> MST connectors.

> 

> Cc: Leo Li <sunpeng.li@amd.com>

> Cc: Roman Li <roman.li@amd.com>

> Cc: Harry Wentland <harry.wentland@amd.com>

> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>


LGTM,

Reviewed-by: Leo Li <sunpeng.li@amd.com>


> ---

>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +++++++---

>   1 file changed, 7 insertions(+), 3 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 a698c8f272ed..4523ab100bc3 100644

> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

> @@ -4743,6 +4743,13 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,

>   {

>   	struct amdgpu_device *adev = dm->ddev->dev_private;

>   

> +	/*

> +	 * Some of the properties below require access to state, like bpc.

> +	 * Allocate some default initial connector state with our reset helper.

> +	 */

> +	if (aconnector->base.funcs->reset)

> +		aconnector->base.funcs->reset(&aconnector->base);

> +

>   	aconnector->connector_id = link_index;

>   	aconnector->dc_link = link;

>   	aconnector->base.interlace_allowed = false;

> @@ -4932,9 +4939,6 @@ static int amdgpu_dm_connector_init(struct amdgpu_display_manager *dm,

>   			&aconnector->base,

>   			&amdgpu_dm_connector_helper_funcs);

>   

> -	if (aconnector->base.funcs->reset)

> -		aconnector->base.funcs->reset(&aconnector->base);

> -

>   	amdgpu_dm_connector_init_helper(

>   		dm,

>   		aconnector,

>