Re: [PATCH] drm/i915: Fix IPS related flicker

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 26 Jun 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Jun 25, 2015 at 09:58:22AM -0700, Rodrigo Vivi wrote:
>> We cannot let IPS enabled with no plane on the pipe:
>> 
>> BSpec: "IPS cannot be enabled until after at least one plane has
>> been enabled for at least one vertical blank." and "IPS must be
>> disabled while there is still at least one plane enabled on the
>> same pipe as IPS." This restriction apply to HSW and BDW.
>> 
>> However a shortcut path on update primary plane function
>> to make primary plane invisible by setting DSPCTRL to 0
>> was leting IPS enabled while there was no
>> other plane enabled on the pipe causing flickerings that we were
>> believing that it was caused by that other restriction where
>> ips cannot be used when pixel rate is greater than 95% of cdclok.
>> 
>> v2: Don't mess with Atomic path as pointed out by Ville.
>> 
>> v3: Rebase after a long time and atomic path changes.
>>     Accept Ville suggestion of not check !fb
>> 
>> Reference: https://bugs.freedesktop.org/show_bug.cgi?id=85583
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Tested-by: Kenneth Graunke <kenneth@whitecape.org>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Already conflicts since I pulled in Ville's frontbuffer_bits rework. Can
> you please rebase once more?

Preferably on top of drm-intel-next-queued directly, I think. I already
applied the earlier version to drm-intel-next-fixes.

BR,
Jani.


>
> Thanks, Daniel
>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
>>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>>  2 files changed, 14 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index cc68e41..9b0d10b 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11618,6 +11618,16 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>>  		if (visible)
>>  			intel_crtc->atomic.fb_bits |=
>>  			    INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
>> +		else
>> +			/*
>> +			 * FIXME: Actually if we will still have any other
>> +			 * plane enabled on the pipe we could let IPS enabled
>> +			 * still, but for now lets consider that when we make
>> +			 * primary invisible by setting DSPCNTR to 0 on
>> +			 * update_primary_plane function IPS needs to be
>> +			 * disable.
>> +			 */
>> +			intel_crtc->atomic.disable_ips = true;
>>  
>>  		intel_crtc->atomic.wait_for_flips = true;
>>  		intel_crtc->atomic.pre_disable_primary = turn_off;
>> @@ -13724,6 +13734,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
>>  	if (!needs_modeset(crtc->state))
>>  		intel_pre_plane_update(intel_crtc);
>>  
>> +	if (intel_crtc->atomic.disable_ips)
>> +		hsw_disable_ips(intel_crtc);
>> +
>>  	if (intel_crtc->atomic.update_wm)
>>  		intel_update_watermarks(crtc);
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index e90c743..6ffd245 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -498,6 +498,7 @@ struct intel_crtc_atomic_commit {
>>  	/* Sleepable operations to perform before commit */
>>  	bool wait_for_flips;
>>  	bool disable_fbc;
>> +	bool disable_ips;
>>  	bool pre_disable_primary;
>>  	bool update_wm;
>>  	unsigned disabled_planes;
>> -- 
>> 2.1.0
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx