Re: [PATCH 11/11] drm/i915: Hook PSR functionality

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 18, 2013 at 6:54 AM, Daniel Vetter <daniel@xxxxxxxx> 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.zanoni@xxxxxxxxx>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxx
>> 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux