On Wed, 2015-11-18 at 11:12 +0100, Daniel Vetter wrote:
> On Thu, Nov 05, 2015 at 10:50:04AM -0800, Rodrigo Vivi wrote:
> > PSR is still disabled by default, but even passing
> > i915.enable_psr=1
> > at this point we weren't able to get PSR working because with
> > fastboot by default in place we weren't executing the path that
> > enables
> > encoder and consequently PSR.
> >
> > Now with psr_ready in place and PSR using crtc signature we can
> > move
> > its enable/disable sequences from the encoder enable to the post
> > atomic modeset functions.
> >
> > i915.enable_psr parameter is still used to enable/disable psr
> > feature
> > on the next primary plane update. So current test cases that relies
> > on this flow still works.
> >
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_ddi.c | 2 --
> > drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++
> > drivers/gpu/drm/i915/intel_dp.c | 5 -----
> > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > 4 files changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index b8f8dee..36db970 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2404,7 +2404,6 @@ static void intel_enable_ddi(struct
> > intel_encoder *intel_encoder)
> > intel_dp_stop_link_train(intel_dp);
> >
> > intel_edp_backlight_on(intel_dp);
> > - intel_psr_enable(intel_crtc);
> > intel_edp_drrs_enable(intel_dp);
> > }
> >
> > @@ -2432,7 +2431,6 @@ static void intel_disable_ddi(struct
> > intel_encoder *intel_encoder)
> > struct intel_dp *intel_dp =
> > enc_to_intel_dp(encoder);
> >
> > intel_edp_drrs_disable(intel_dp);
> > - intel_psr_disable(intel_crtc);
> > intel_edp_backlight_off(intel_dp);
> > }
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 869929d..f67e2ee 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4687,6 +4687,9 @@ static void intel_post_plane_update(struct
> > intel_crtc *crtc)
> > if (atomic->enable_ips)
> > intel_ips_enable(crtc);
> >
> > + if (atomic->enable_psr)
> > + intel_psr_enable(crtc);
>
> What we need here is a post-modeset fixup hook for
> encoders/connectors.
> That would avoid the layering violation of going through a crtc and
> then
> doing the crtc->encoder lookup you do in the previous patch. And we
> have
> other uses for this, e.g. mipi self refresh, fixing up infoframes and
> all
> those things.
I see your point but I'm not sure if I totally got your idea.
So would it be a generic hook when we detect encoder/connector being
enabled/disabled?
and how to determine that? could you please elaborate your idea a bit
more?
> -Daniel
Thank you very much,
Rodrigo.
>
> > +
> > if (atomic->post_enable_primary)
> > intel_post_enable_primary(&crtc->base);
> >
> > @@ -4705,6 +4708,9 @@ static void intel_pre_plane_update(struct
> > intel_crtc *crtc)
> > if (crtc->atomic.disable_ips)
> > intel_ips_disable_if_alone(crtc);
> >
> > + if (crtc->atomic.disable_psr)
> > + intel_psr_disable(crtc);
> > +
> > if (atomic->pre_disable_primary)
> > intel_pre_disable_primary(&crtc->base);
> >
> > @@ -11560,9 +11566,18 @@ int intel_plane_atomic_calc_changes(struct
> > drm_crtc_state *crtc_state,
> > intel_crtc->atomic.disable_ips = true;
> >
> > intel_crtc->atomic.disable_fbc = true;
> > +
> > + intel_crtc->atomic.disable_psr = true;
> > }
> > if (visible && intel_crtc->config->ips_ready)
> > intel_crtc->atomic.enable_ips = true;
> > +
> > + if (visible && intel_crtc->config->psr_ready) {
> > + if (i915.enable_psr)
> > + intel_crtc->atomic.enable_psr =
> > true;
> > + else
> > + intel_crtc->atomic.disable_psr =
> > true;
> > + }
> > /*
> > * FBC does not work on some platforms for rotated
> > * planes, so disable it when rotation is not 0
> > and
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 92f59cc..f0ee497 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2373,9 +2373,6 @@ static void intel_disable_dp(struct
> > intel_encoder *encoder)
> > if (crtc->config->has_audio)
> > intel_audio_codec_disable(encoder);
> >
> > - if (HAS_PSR(dev) && !HAS_DDI(dev))
> > - intel_psr_disable(crtc);
> > -
> > /* Make sure the panel is off before trying to change the
> > mode. But also
> > * ensure that we have vdd while we switch off the panel.
> > */
> > intel_edp_panel_vdd_on(intel_dp);
> > @@ -2629,10 +2626,8 @@ static void g4x_enable_dp(struct
> > intel_encoder *encoder)
> > static void vlv_enable_dp(struct intel_encoder *encoder)
> > {
> > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder
> > ->base);
> > - struct intel_crtc *crtc = to_intel_crtc(encoder
> > ->base.crtc);
> >
> > intel_edp_backlight_on(intel_dp);
> > - intel_psr_enable(crtc);
> > }
> >
> > static void g4x_pre_enable_dp(struct intel_encoder *encoder)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index cafe4c1..d599d54 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -531,6 +531,7 @@ struct intel_crtc_atomic_commit {
> > /* Sleepable operations to perform before commit */
> > bool disable_fbc;
> > bool disable_ips;
> > + bool disable_psr;
> > bool disable_cxsr;
> > bool pre_disable_primary;
> > bool update_wm_pre, update_wm_post;
> > @@ -540,6 +541,7 @@ struct intel_crtc_atomic_commit {
> > bool wait_vblank;
> > bool update_fbc;
> > bool enable_ips;
> > + bool enable_psr;
> > bool post_enable_primary;
> > unsigned update_sprite_watermarks;
> > };
> > --
> > 2.4.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
