[PATCH] drm/amd/display: remove usage of legacy_cursor_update

[PATCH] drm/amd/display: remove usage of legacy_cursor_update

[S, Shirish]

From: Shirish S <shirish.s@amd.com>

Currently the atomic check code uses legacy_cursor_update to differnetiate if the cursor plane is being requested by the user, which is not required as we shall be updating plane only if modeset is requested/required.

Have tested cursor plane and underlay get updated seamlessly, without any lag or frame drops.

Signed-off-by: Shirish S <shirish.s@amd.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 +++++++----------------
 1 file changed, 12 insertions(+), 29 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 8638f1c..2df2e32 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4752,8 +4752,6 @@ static int dm_update_planes_state(struct dc *dc,  static int amdgpu_dm_atomic_check(struct drm_device *dev,
 				  struct drm_atomic_state *state)
 {
-	int i;
-	int ret;
 	struct amdgpu_device *adev = dev->dev_private;
 	struct dc *dc = adev->dm.dc;
 	struct dm_atomic_state *dm_state = to_dm_atomic_state(state); @@ -4761,6 +4759,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 	struct drm_connector_state *old_con_state, *new_con_state;
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+	int ret, i;
 
 	/*
 	 * This bool will be set for true for any modeset/reset @@ -4772,36 +4771,20 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
-	/*
-	 * legacy_cursor_update should be made false for SoC's having
-	 * a dedicated hardware plane for cursor in amdgpu_dm_atomic_commit(),
-	 * otherwise for software cursor plane,
-	 * we should not add it to list of affected planes.
-	 */
-	if (state->legacy_cursor_update) {
-		for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
-			if (new_crtc_state->color_mgmt_changed) {
-				ret = drm_atomic_add_affected_planes(state, crtc);
-				if (ret)
-					goto fail;
-			}
-		}
-	} else {
-		for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
-			if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
-				continue;
+	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
+		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
+			continue;
 
-			if (!new_crtc_state->enable)
-				continue;
+		if (!new_crtc_state->enable)
+			continue;
 
-			ret = drm_atomic_add_affected_connectors(state, crtc);
-			if (ret)
-				return ret;
+		ret = drm_atomic_add_affected_connectors(state, crtc);
+		if (ret)
+			return ret;
 
-			ret = drm_atomic_add_affected_planes(state, crtc);
-			if (ret)
-				goto fail;
-		}
+		ret = drm_atomic_add_affected_planes(state, crtc);
+		if (ret)
+			goto fail;
 	}
 
 	dm_state->context = dc_create_state();
--
2.7.4

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

Re: [PATCH] drm/amd/display: remove usage of legacy_cursor_update

[Andrey Grodzovsky]

[-- Attachment #1.1: Type: text/plain, Size: 3576 bytes --]



On 2017-11-23 01:16 AM, S, Shirish wrote:
> From: Shirish S <shirish.s-5C7GfCeVMHo@public.gmane.org>
>
> Currently the atomic check code uses legacy_cursor_update to differnetiate if the cursor plane is being requested by the user, which is not required as we shall be updating plane only if modeset is requested/required.
>
> Have tested cursor plane and underlay get updated seamlessly, without any lag or frame drops.
>
> Signed-off-by: Shirish S <shirish.s-5C7GfCeVMHo@public.gmane.org>
> Reviewed-by: Harry Wentland <harry.wentland-5C7GfCeVMHo@public.gmane.org>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 +++++++----------------
>   1 file changed, 12 insertions(+), 29 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 8638f1c..2df2e32 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4752,8 +4752,6 @@ static int dm_update_planes_state(struct dc *dc,  static int amdgpu_dm_atomic_check(struct drm_device *dev,
>   				  struct drm_atomic_state *state)
>   {
> -	int i;
> -	int ret;
>   	struct amdgpu_device *adev = dev->dev_private;
>   	struct dc *dc = adev->dm.dc;
>   	struct dm_atomic_state *dm_state = to_dm_atomic_state(state); @@ -4761,6 +4759,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>   	struct drm_connector_state *old_con_state, *new_con_state;
>   	struct drm_crtc *crtc;
>   	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> +	int ret, i;
>   
>   	/*
>   	 * This bool will be set for true for any modeset/reset @@ -4772,36 +4771,20 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
>   	if (ret)
>   		goto fail;
>   
> -	/*
> -	 * legacy_cursor_update should be made false for SoC's having
> -	 * a dedicated hardware plane for cursor in amdgpu_dm_atomic_commit(),
> -	 * otherwise for software cursor plane,
> -	 * we should not add it to list of affected planes.
> -	 */
> -	if (state->legacy_cursor_update) {
> -		for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
> -			if (new_crtc_state->color_mgmt_changed) {
> -				ret = drm_atomic_add_affected_planes(state, crtc);

I don't even understand this, why even bundle together cursor updates 
and gamma settings...

Change is Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>

Thanks,
Andrey

> -				if (ret)
> -					goto fail;
> -			}
> -		}
> -	} else {
> -		for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> -			if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> -				continue;
> +	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> +		if (!drm_atomic_crtc_needs_modeset(new_crtc_state))
> +			continue;
>   
> -			if (!new_crtc_state->enable)
> -				continue;
> +		if (!new_crtc_state->enable)
> +			continue;
>   
> -			ret = drm_atomic_add_affected_connectors(state, crtc);
> -			if (ret)
> -				return ret;
> +		ret = drm_atomic_add_affected_connectors(state, crtc);
> +		if (ret)
> +			return ret;
>   
> -			ret = drm_atomic_add_affected_planes(state, crtc);
> -			if (ret)
> -				goto fail;
> -		}
> +		ret = drm_atomic_add_affected_planes(state, crtc);
> +		if (ret)
> +			goto fail;
>   	}
>   
>   	dm_state->context = dc_create_state();
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 4748 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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