On Thu, Jul 18, 2013 at 6:54 AM, Daniel Vetter <dan...@ffwll.ch> wrote:
> On Thu, Jul 11, 2013 at 06:45:05PM -0300, Rodrigo Vivi wrote:
>> PSR must be enabled after transcoder and port are running.
>> And it is only available for HSW.
>>
>> v2: move enable/disable to intel_ddi
>> v3: The spec suggests PSR should be disabled even before backlight (by
>> pzanoni)
>> v4: also disabling and enabling whenever panel is disabled/enabled.
>> v5: make it last patch to avoid breaking whenever bisecting. So calling for
>> update and force exit came to this patch along with enable/disable calls.
>> v6: Remove unused and unecessary psr_enable/disable calls, as notice by
>> Paulo.
>>
>> CC: Paulo Zanoni <paulo.r.zan...@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.v...@gmail.com>
>
> Ok, I've slurped this series in with the few bikesheds from Paulo applied
> and two patches not merged:
> - The userspace interface part, since I don't want to commit yet to an
> interface as long as things are unclear.
> - And the invalidation part since imo that parts isn't properly thought
> through yet. See my other mail to Chris' RFC for ideas.
Thank you very much.
>
> Now on the topic fbc, psr and frontbuffer rendering:
>
> Now that I've appeased our dear PDT it's time to stop building castles on
> sand imo.
What about psr at Baytrail? :/
> Items to pay back a bit of the technical dept:
>
> - Untangle the "is fbc/psr" allowed mess. Imo we should add more static
> (i.e. only changes at modesets) information to the pipe config and track
> dynamic reasons for disabling fbc/psr somewhere in the crtc. Also this
> way update_fbc/psr would only need to check dynamic state and more
> important would never need to frob the basic setup (like reallocating
> the compressed buffer and similar things).
>
> - Implement precise frontbuffer rendering tracking and disabling of
> fbc/psr for legacy userspace and rip out the hw tracking. If we have to
> add tons of workarounds (like for fbc), have performance this or just
> can't use it in a bunch of cases (sprites for psr) we might as well just
> track everything in the kernel.
>
> - Testcases. With pipe CRC checksums we should be able to make sure that
> the frontbuffer invalidation actually happens. And if we do it all in
> the kernel the difference should be all-or-nothing, so much easier to
> test than with hw tracking where sometimes something seems to slip
> through.
>
> - low-level improvements. I've only really reviewed the fbc code in-depth
> and discussed a few things with Art, but there's definitely a few things
> we need to do. One of the important things imo is to enable fbc
> everywhere we can to have as wide test coverage as possible for this
> feature.
>
> I'll block future hw enabling in this area on the above list (i.e. vlv psr
> or patches for -internal). I'll reconsider my stance if e.g. vlv psr is
> used as a demonstration vehicle for the refactored psr tracking. But since
> fbc can be used on many more machines (even desktops and apparently even
> when other pipes are enabled) I think we should push that forward first
> and use fbc to create solid tests and interfaces for userspace&kernel to
> cooperate on frontbuffer rendering.
I fully agree with you. I'm going to start playing with fbc and psr at
pipe_config, etc and check that discussion with art and try other
changes in fbc.
I'm just afraid the pressure for psr-vlv will be big as well.
Anyway, I give up on those 2 patches you didn't accepted because I
think this other things have more priority compared to xdm/kde bug.
Since it is disabled by default it won't cause troubles for kde users
and it is working well on gnome and unity that by any chance has a hsw
with edp that supports psr and use i915.enable_psr=1 parameter :/
>
> Cheers, Daniel
>
>> ---
>> drivers/gpu/drm/i915/i915_gem.c | 2 ++
>> drivers/gpu/drm/i915/intel_ddi.c | 2 ++
>> drivers/gpu/drm/i915/intel_display.c | 1 +
>> 3 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> b/drivers/gpu/drm/i915/i915_gem.c
>> index 46bf7e3..703bc69 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -3758,6 +3758,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>> goto unlock;
>> }
>>
>> + intel_edp_psr_force_exit(dev);
>> +
>> /* Count all active objects as busy, even if they are currently not
>> used
>> * by the gpu. Users of this interface expect objects to eventually
>> * become non-busy without any further actions, therefore emit any
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index 324211a..4211925 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -1117,6 +1117,7 @@ static void intel_enable_ddi(struct intel_encoder
>> *intel_encoder)
>> intel_dp_stop_link_train(intel_dp);
>>
>> ironlake_edp_backlight_on(intel_dp);
>> + intel_edp_psr_enable(intel_dp);
>> }
>>
>> if (intel_crtc->eld_vld && type != INTEL_OUTPUT_EDP) {
>> @@ -1147,6 +1148,7 @@ static void intel_disable_ddi(struct intel_encoder
>> *intel_encoder)
>> if (type == INTEL_OUTPUT_EDP) {
>> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>>
>> + intel_edp_psr_disable(intel_dp);
>> ironlake_edp_backlight_off(intel_dp);
>> }
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 10a3629..eb4e49b 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2272,6 +2272,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int
>> y,
>> }
>>
>> intel_update_fbc(dev);
>> + intel_edp_psr_update(dev);
>> mutex_unlock(&dev->struct_mutex);
>>
>> intel_crtc_update_sarea_pos(crtc, x, y);
>> --
>> 1.7.11.7
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
