diff mbox

drm/i915/skl: Correctly updating sprite wm parameter

Message ID 1418102955-14357-1-git-send-email-sonika.jindal@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sonika.jindal@intel.com Dec. 9, 2014, 5:29 a.m. UTC
From: Sonika Jindal <sonika.jindal@intel.com>

The pipe wm parameters is not correctly updated with sprite parameters
because it copies them for each plane from plane_list to the sprite
offset in pipe wm parameters. Since plane_list also contains primary and
cursor planes, we end up updating wrong params for sprites.

Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Dec. 9, 2014, 10:27 a.m. UTC | #1
On Tue, Dec 09, 2014 at 10:59:15AM +0530, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@intel.com>
> 
> The pipe wm parameters is not correctly updated with sprite parameters
> because it copies them for each plane from plane_list to the sprite
> offset in pipe wm parameters. Since plane_list also contains primary and
> cursor planes, we end up updating wrong params for sprites.
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8d182a6..4a16f9b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3301,7 +3301,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
>  	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
>  		struct intel_plane *intel_plane = to_intel_plane(plane);
>  
> -		if (intel_plane->pipe == pipe)
> +		if (intel_plane->pipe == pipe &&
> +			plane->type == DRM_PLANE_TYPE_OVERLAY)
>  			p->plane[i++] = intel_plane->wm;
>  	}
>  }

With the great unification of planes both in hw with skl+ and in software
(with Matt's recent work): Can't we just ditch the cursor/primary plane
special cases?
-Daniel
sonika.jindal@intel.com Dec. 9, 2014, 11:22 a.m. UTC | #2
Yes, that can be done but that might involve more changes and testing.
I am not sure who can signup for this, I can check with people though.
But for now to fix the sprite plane wm this check would be required.

Regards,
Sonika

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, December 9, 2014 3:57 PM
To: Jindal, Sonika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/skl: Correctly updating sprite wm parameter

On Tue, Dec 09, 2014 at 10:59:15AM +0530, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@intel.com>
> 
> The pipe wm parameters is not correctly updated with sprite parameters 
> because it copies them for each plane from plane_list to the sprite 
> offset in pipe wm parameters. Since plane_list also contains primary 
> and cursor planes, we end up updating wrong params for sprites.
> 
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c 
> b/drivers/gpu/drm/i915/intel_pm.c index 8d182a6..4a16f9b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3301,7 +3301,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
>  	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
>  		struct intel_plane *intel_plane = to_intel_plane(plane);
>  
> -		if (intel_plane->pipe == pipe)
> +		if (intel_plane->pipe == pipe &&
> +			plane->type == DRM_PLANE_TYPE_OVERLAY)
>  			p->plane[i++] = intel_plane->wm;
>  	}
>  }

With the great unification of planes both in hw with skl+ and in software (with Matt's recent work): Can't we just ditch the cursor/primary plane special cases?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Shuang He Dec. 9, 2014, 12:50 p.m. UTC | #3
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK              +3-4              362/366              361/366
SNB                                  448/450              448/450
IVB                                  497/498              497/498
BYT                                  289/289              289/289
HSW                                  563/564              563/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 ILK  igt_kms_flip_nonblocking-read      DMESG_WARN(1, M26)PASS(3, M26M37)      PASS(1, M26)
 ILK  igt_kms_setmode_invalid-clone-exclusive-crtc      DMESG_WARN(1, M26)PASS(3, M26M37)      PASS(1, M26)
*ILK  igt_kms_flip_busy-flip-interruptible      PASS(2, M26)      DMESG_WARN(1, M26)
*ILK  igt_kms_flip_flip-vs-dpms-interruptible      PASS(2, M26)      DMESG_WARN(1, M26)
*ILK  igt_kms_flip_flip-vs-panning      PASS(2, M26)      DMESG_WARN(1, M26)
 ILK  igt_kms_flip_flip-vs-rmfb-interruptible      DMESG_WARN(1, M26)PASS(3, M26M37)      PASS(1, M26)
*ILK  igt_kms_flip_wf_vblank-ts-check      PASS(2, M26)      NSPT(1, M26)
Note: You need to pay more attention to line start with '*'
Tvrtko Ursulin Dec. 12, 2014, 2:34 p.m. UTC | #4
On 12/09/2014 05:29 AM, sonika.jindal@intel.com wrote:
> From: Sonika Jindal <sonika.jindal@intel.com>
>
> The pipe wm parameters is not correctly updated with sprite parameters
> because it copies them for each plane from plane_list to the sprite
> offset in pipe wm parameters. Since plane_list also contains primary and
> cursor planes, we end up updating wrong params for sprites.
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8d182a6..4a16f9b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3301,7 +3301,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
>   	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
>   		struct intel_plane *intel_plane = to_intel_plane(plane);
>
> -		if (intel_plane->pipe == pipe)
> +		if (intel_plane->pipe == pipe &&
> +			plane->type == DRM_PLANE_TYPE_OVERLAY)
>   			p->plane[i++] = intel_plane->wm;
>   	}
>   }
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Daniel Vetter Dec. 15, 2014, 8:55 a.m. UTC | #5
On Fri, Dec 12, 2014 at 02:34:37PM +0000, Tvrtko Ursulin wrote:
> 
> On 12/09/2014 05:29 AM, sonika.jindal@intel.com wrote:
> >From: Sonika Jindal <sonika.jindal@intel.com>
> >
> >The pipe wm parameters is not correctly updated with sprite parameters
> >because it copies them for each plane from plane_list to the sprite
> >offset in pipe wm parameters. Since plane_list also contains primary and
> >cursor planes, we end up updating wrong params for sprites.
> >
> >Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_pm.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >index 8d182a6..4a16f9b 100644
> >--- a/drivers/gpu/drm/i915/intel_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_pm.c
> >@@ -3301,7 +3301,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
> >  	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> >  		struct intel_plane *intel_plane = to_intel_plane(plane);
> >
> >-		if (intel_plane->pipe == pipe)
> >+		if (intel_plane->pipe == pipe &&
> >+			plane->type == DRM_PLANE_TYPE_OVERLAY)
> >  			p->plane[i++] = intel_plane->wm;
> >  	}
> >  }
> >
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Queued for -next, thanks for the patch.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8d182a6..4a16f9b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3301,7 +3301,8 @@  static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
 		struct intel_plane *intel_plane = to_intel_plane(plane);
 
-		if (intel_plane->pipe == pipe)
+		if (intel_plane->pipe == pipe &&
+			plane->type == DRM_PLANE_TYPE_OVERLAY)
 			p->plane[i++] = intel_plane->wm;
 	}
 }