| Message ID | 20170508114902.18965-6-mahesh1.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Mahesh Kumar schreef op ma 08-05-2017 om 17:18 [+0530]: > Fail the flip if no FB is present but plane_state is set as visible. > Above is not a valid combination so instead of continue fail the > flip. Why is this patch necessary? drm_atomic_plane_check handles this. ~Maarten
Hi, On Monday 08 May 2017 05:18 PM, Lankhorst, Maarten wrote: > Mahesh Kumar schreef op ma 08-05-2017 om 17:18 [+0530]: >> Fail the flip if no FB is present but plane_state is set as visible. >> Above is not a valid combination so instead of continue fail the >> flip. > Why is this patch necessary? drm_atomic_plane_check handles this. Ideally we should never get such combination here. But current WM code checks for this situation and even if it's true it proceeds further. This patch just corrects the WM code flow decision. I also think some of these checks are redundant here. -Mahesh > > ~Maarten
On Mon, May 08, 2017 at 05:31:30PM +0530, Mahesh Kumar wrote: > Hi, > > > On Monday 08 May 2017 05:18 PM, Lankhorst, Maarten wrote: > > Mahesh Kumar schreef op ma 08-05-2017 om 17:18 [+0530]: > > > Fail the flip if no FB is present but plane_state is set as visible. > > > Above is not a valid combination so instead of continue fail the > > > flip. > > Why is this patch necessary? drm_atomic_plane_check handles this. > Ideally we should never get such combination here. But current WM code > checks for this situation and even if it's true it proceeds further. This > patch just corrects the WM code flow decision. > I also think some of these checks are redundant here. Yeah, WARN()'s are basically "I'm sure this could never happen" type assertions. Of course sometimes we restructure parts of the code and forget about assumptions we've made elsewhere (or we just plain screw up and add new bugs to existing code), so we wind up hitting the WARN()'s anyway. If proceeding on here could lead to a panic (which I think it could since we dereference the fb in skl_compute_plane_wm()), then adding a sensible bail-out here seems okay to me; the extra paranoia only costs one extra line of code. Matt > > -Mahesh > > > > > ~Maarten >
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 6414701ef09e..1d7c67d7e539 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3942,7 +3942,8 @@ skl_compute_wm_level(const struct drm_i915_private *dev_priv, if (!intel_pstate) intel_pstate = to_intel_plane_state(plane->state); - WARN_ON(!intel_pstate->base.fb); + if (WARN_ON(!intel_pstate->base.fb)) + return -EINVAL; ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][intel_plane->id]);
Fail the flip if no FB is present but plane_state is set as visible. Above is not a valid combination so instead of continue fail the flip. Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)