[Intel-gfx] [PATCH] drm/i915: Hook PSR functionality

[Rodrigo Vivi]

I tried to enable psr using fbc calls and all I got was a frozen machine...
I prefer to stay with the way I'm sure it works

On Mon, Jul 8, 2013 at 4:46 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Mon, Jul 08, 2013 at 10:13:30AM -0300, Paulo Zanoni wrote:
>> 2013/7/1 Rodrigo Vivi <rodrigo.vivi at gmail.com>:
>> > 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 at intel.com>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
>> > ---
>> >  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 c68b90f..f2e135b 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem.c
>> > @@ -3786,6 +3786,8 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>> >                 goto unlock;
>> >         }
>> >
>> > +       intel_edp_psr_force_exit(dev);
>>
>> Shouldn't we enter/leave PSR in exactly the same cases as we
>> enter/leave FBC? This function doesn't seem to touch FBC. I also see,
>> for example, intel_unpin_work_fn calling intel_update_fbc, but at that
>> point we don't update PSR. Also intel_enable_primary and
>> intel_disable_primary also call intel_update_fbc. If we conclude that
>> PSR and FBC need to be disabled/enabled at exactly the same places,
>> can we do some rework to unify this?
>
> I think there's some room for fbc and psr handling code to differ, since
> fbc only works on the primary plane, whereas psr works on the entire
> display (so also needs to update when cursor/sprites change or gamma table
> changes or any other disaplay attribute). But I agree with you that we
> should share calls to psr/fbc placed in common gem/modeset code as much as
> possible.
> -Daniel
>
>>
>>
>> > +
>> >         /* 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 6fafa43..5bbfed0 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -2236,6 +2236,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.8.1.4
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Paulo Zanoni
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at 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