[Intel-gfx] [PATCH] drm/i915: Add Baytrail PSR Support.
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Jan 29 17:38:10 CET 2014
On Wed, Jan 29, 2014 at 01:47:00PM -0200, Rodrigo Vivi wrote:
> On Wed, Jan 29, 2014 at 12:56 PM, Ville Syrjälä
> <ville.syrjala at linux.intel.com> wrote:
> > On Wed, Jan 29, 2014 at 10:47:54AM -0200, Rodrigo Vivi wrote:
> >> This patch adds PSR Support to Baytrail.
> >>
> >> Baytrail cannot easily detect screen updates and force PSR exit.
> >> So we inactivate it on {busy_ioctl, set_domain, sw_finish and mark_busy
> >> and update to enable it back on next display mark_idle.
> >>
> >> v2: Also inactivate PSR on cursor update.
> >> v3: Inactivate PSR on mark_busy, dset_domain and sw_finish_ioctl, and
> >> early on page flip besides avoid initializing inactive/active flag
> >> more than once.
> >> v4: Fix identation issues.
> >> v5: Rebase and add Baytrail per pipe support although leaving PIPE_B
> >> support disabled by for now since it isn't working properly yet.
> >> v6: Removing forgotten comment and useless clkgating definition.
> >>
> >> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_debugfs.c | 36 ++++++-
> >> drivers/gpu/drm/i915/i915_drv.h | 4 +-
> >> drivers/gpu/drm/i915/i915_gem.c | 9 ++
> >> drivers/gpu/drm/i915/i915_reg.h | 37 +++++++
> >> drivers/gpu/drm/i915/intel_display.c | 15 ++-
> >> drivers/gpu/drm/i915/intel_dp.c | 204 +++++++++++++++++++++++++++++------
> >> drivers/gpu/drm/i915/intel_drv.h | 1 +
> >> 7 files changed, 267 insertions(+), 39 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >> index 4b852c6..c28de65 100644
> >> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >> @@ -1900,6 +1900,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> >> struct drm_device *dev = node->minor->dev;
> >> struct drm_i915_private *dev_priv = dev->dev_private;
> >> u32 psrperf = 0;
> >> + u32 statA = 0;
> >> + u32 statB = 0;
> >> bool enabled = false;
> >>
> >> intel_runtime_pm_get(dev_priv);
> >> @@ -1907,14 +1909,38 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> >> seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support));
> >> seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok));
> >>
> >> - enabled = HAS_PSR(dev) &&
> >> - I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
> >> - seq_printf(m, "Enabled: %s\n", yesno(enabled));
> >> + if (HAS_PSR(dev)) {
> >> + if (IS_VALLEYVIEW(dev)) {
> >> + statA = I915_READ(VLV_EDP_PSR_STATUS_CTL(PIPE_A)) &
> >> + VLV_EDP_PSR_CURR_STATE_MASK;
> >> + statB = I915_READ(VLV_EDP_PSR_STATUS_CTL(PIPE_B)) &
> >> + VLV_EDP_PSR_CURR_STATE_MASK;
> >> + enabled = ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
> >> + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE) ||
> >> + (statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
> >> + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE));
> >> + } else
> >> + enabled = I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
> >> + }
> >> + seq_printf(m, "Enabled: %s", yesno(enabled));
> >>
> >> - if (HAS_PSR(dev))
> >> + if (IS_VALLEYVIEW(dev)) {
> >> + if ((statA == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
> >> + (statA == VLV_EDP_PSR_ACTIVE_SF_UPDATE))
> >> + seq_puts(m, " pipe A");
> >> + if ((statB == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
> >> + (statB == VLV_EDP_PSR_ACTIVE_SF_UPDATE))
> >> + seq_puts(m, " pipe B");
> >> + }
> >> +
> >> + seq_puts(m, "\n");
> >> +
> >> + /* VLV PSR has no kind of performance counter */
> >> + if (HAS_PSR(dev) && !IS_VALLEYVIEW(dev)) {
> >> psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
> >> EDP_PSR_PERF_CNT_MASK;
> >> - seq_printf(m, "Performance_Counter: %u\n", psrperf);
> >> + seq_printf(m, "Performance_Counter: %u\n", psrperf);
> >> + }
> >>
> >> intel_runtime_pm_put(dev_priv);
> >> return 0;
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 7c53d4d..34dee24 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -747,6 +747,7 @@ struct i915_psr {
> >> bool sink_support;
> >> bool source_ok;
> >> bool setup_done;
> >> + bool active;
> >> };
> >>
> >> enum intel_pch {
> >> @@ -1866,7 +1867,8 @@ struct drm_i915_file_private {
> >>
> >> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
> >> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
> >> -#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev))
> >> +#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \
> >> + IS_VALLEYVIEW(dev))
> >> #define HAS_PC8(dev) (IS_HASWELL(dev)) /* XXX HSW:ULX */
> >> #define HAS_RUNTIME_PM(dev) (IS_HASWELL(dev))
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >> index 39770f7..01137fe 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> @@ -1256,6 +1256,9 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
> >> goto unlock;
> >> }
> >>
> >> + if (IS_VALLEYVIEW(dev))
> >> + intel_edp_psr_inactivate(dev);
> >> +
> >> /* Try to flush the object off the GPU without holding the lock.
> >> * We will repeat the flush holding the lock in the normal manner
> >> * to catch cases where we are gazumped.
> >> @@ -1299,6 +1302,9 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
> >> if (ret)
> >> return ret;
> >>
> >> + if (IS_VALLEYVIEW(dev))
> >> + intel_edp_psr_inactivate(dev);
> >> +
> >> obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle));
> >> if (&obj->base == NULL) {
> >> ret = -ENOENT;
> >> @@ -4059,6 +4065,9 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
> >> if (ret)
> >> return ret;
> >>
> >> + if (IS_VALLEYVIEW(dev))
> >> + intel_edp_psr_inactivate(dev);
> >
> > The locking for PSR seems to be as fubar as for FBC. Also the front
> > buffer tracking is missing, but I guess I need to make that work for FBC
> > first, and then we need to figure out how to tie it in w/ PSR.
> >
>
> agree... I'll wait your fbc work.
>
> >> +
> >> obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle));
> >> if (&obj->base == NULL) {
> >> ret = -ENOENT;
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >> index cbbaf26..2039d71 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -1968,6 +1968,43 @@
> >> #define BCLRPAT(pipe) _PIPE(pipe, _BCLRPAT_A, _BCLRPAT_B)
> >> #define VSYNCSHIFT(trans) _TRANSCODER(trans, _VSYNCSHIFT_A, _VSYNCSHIFT_B)
> >>
> >> +/* VLV eDP PSR registers */
> >> +#define _PSRCTLA (VLV_DISPLAY_BASE + 0x60090)
> >> +#define _PSRCTLB (VLV_DISPLAY_BASE + 0x61090)
> >> +#define VLV_EDP_PSR_ENABLE (1<<0)
> >> +#define VLV_EDP_PSR_RESET (1<<1)
> >> +#define VLV_EDP_PSR_MODE_MASK (7<<2)
> >> +#define VLV_EDP_PSR_MODE_HW_TIMER (1<<3)
> >> +#define VLV_EDP_PSR_MODE_SW_TIMER (1<<2)
> >> +#define VLV_EDP_PSR_SINGLE_FRAME_UPDATE (1<<7)
> >> +#define VLV_EDP_PSR_ACTIVE_ENTRY (1<<8)
> >> +#define VLV_EDP_PSR_SRC_TRANSMITTER_STATE (1<<9)
> >> +#define VLV_EDP_PSR_DBL_FRAME (1<<10)
> >> +#define VLV_EDP_PSR_FRAME_COUNT_MASK (0xff<<16)
> >> +#define VLV_EDP_PSR_IDLE_FRAME_SHIFT 16
> >> +#define VLV_EDP_PSR_INT_TRANSITION (1<<24)
> >> +#define VLV_EDP_PSR_CTL(pipe) _PIPE(pipe, _PSRCTLA, _PSRCTLB)
> >
> > Can we just name the PSR registers like in the spec? Eg. just
> > VLV_PSRCTL(). Would make it easier to compare things w/ the spec.
> >
>
> Done.
>
> >> +
> >> +#define _VSCSDPA (VLV_DISPLAY_BASE + 0x600a0)
> >> +#define _VSCSDPB (VLV_DISPLAY_BASE + 0x610a0)
> >> +#define VLV_EDP_PSR_SDP_FREQ_MASK (3<<30)
> >> +#define VLV_EDP_PSR_SDP_FREQ_ONCE (1<<31)
> >> +#define VLV_EDP_PSR_SDP_FREQ_EVFRAME (1<<30)
> >> +#define VLV_EDP_VSC_SDP_REG(pipe) _PIPE(pipe, _VSCSDPA, _VSCSDPB)
> >> +
> >> +#define _PSRSTATA (VLV_DISPLAY_BASE + 0x60094)
> >> +#define _PSRSTATB (VLV_DISPLAY_BASE + 0x61094)
> >> +#define VLV_EDP_PSR_LAST_STATE_MASK (7<<3)
> >> +#define VLV_EDP_PSR_CURR_STATE_MASK 7
> >> +#define VLV_EDP_PSR_DISABLED (0<<0)
> >> +#define VLV_EDP_PSR_INACTIVE (1<<0)
> >> +#define VLV_EDP_PSR_IN_TRANS_TO_ACTIVE (2<<0)
> >> +#define VLV_EDP_PSR_ACTIVE_NORFB_UP (3<<0)
> >> +#define VLV_EDP_PSR_ACTIVE_SF_UPDATE (4<<0)
> >> +#define VLV_EDP_PSR_EXIT (5<<0)
> >> +#define VLV_EDP_PSR_IN_TRANS (1<<7)
> >> +#define VLV_EDP_PSR_STATUS_CTL(pipe) _PIPE(pipe, _PSRSTATA, _PSRSTATB)
> >> +
> >> /* HSW+ eDP PSR registers */
> >> #define EDP_PSR_BASE(dev) (IS_HASWELL(dev) ? 0x64800 : 0x6f800)
> >> #define EDP_PSR_CTL(dev) (EDP_PSR_BASE(dev) + 0)
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 1a9aa19..081c8e2 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -7501,6 +7501,9 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> >> u32 base = 0, pos = 0;
> >> bool visible;
> >>
> >> + if (IS_VALLEYVIEW(dev))
> >> + intel_edp_psr_inactivate(dev);
> >> +
> >> if (on)
> >> base = intel_crtc->cursor_addr;
> >>
> >> @@ -8228,16 +8231,20 @@ void intel_mark_idle(struct drm_device *dev)
> >>
> >> if (dev_priv->info->gen >= 6)
> >> gen6_rps_idle(dev->dev_private);
> >> +
> >> + if (IS_VALLEYVIEW(dev))
> >> + intel_edp_psr_update(dev);
> >> }
> >>
> >> +
> >> void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
> >> struct intel_ring_buffer *ring)
> >> {
> >> struct drm_device *dev = obj->base.dev;
> >> struct drm_crtc *crtc;
> >>
> >> - if (!i915.powersave)
> >> - return;
> >> + if (IS_VALLEYVIEW(dev))
> >> + intel_edp_psr_inactivate(dev);
> >>
> >> list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> >> if (!crtc->fb)
> >> @@ -8688,6 +8695,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> > nav> if (work == NULL)
>
> sorry, I didn't get this..
> what did you mean?
Nothing. Just managed to misplace a few characters here I guess. I blame
vim :)
>
> >> return -ENOMEM;
> >>
> >> + /* Inactivate PSR early in page flip */
> >> + if (IS_VALLEYVIEW(dev))
> >> + intel_edp_psr_inactivate(dev);
> >> +
> >> work->event = event;
> >> work->crtc = crtc;
> >> work->old_fb_obj = to_intel_framebuffer(old_fb)->obj;
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 30d4350..e9a0ace 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -1578,21 +1578,35 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >> }
> >> }
> >>
> >> -static bool is_edp_psr(struct drm_device *dev)
> >> +static bool is_edp_psr(struct intel_dp *intel_dp)
> >> +{
> >> + return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
> >> +}
> >> +
> >> +static bool vlv_edp_is_psr_enabled_on_pipe(struct drm_device *dev, int pipe)
> >> {
> >> struct drm_i915_private *dev_priv = dev->dev_private;
> >> + uint32_t val;
> >>
> >> - return dev_priv->psr.sink_support;
> >> + val = I915_READ(VLV_EDP_PSR_STATUS_CTL(pipe)) &
> >> + VLV_EDP_PSR_CURR_STATE_MASK;
> >> + return (val == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
> >> + (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
> >> }
> >>
> >> static bool intel_edp_is_psr_enabled(struct drm_device *dev)
> >> {
> >> struct drm_i915_private *dev_priv = dev->dev_private;
> >>
> >> - if (!HAS_PSR(dev))
> >> - return false;
> >> + if (HAS_PSR(dev)) {
> >> + if (IS_VALLEYVIEW(dev))
> >> + return vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_A) ||
> >> + vlv_edp_is_psr_enabled_on_pipe(dev, PIPE_B);
> >> + else
> >> + return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
> >> + }
> >>
> >> - return I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
> >> + return false;
> >> }
> >>
> >> static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp,
> >> @@ -1626,28 +1640,49 @@ static void intel_edp_psr_write_vsc(struct intel_dp *intel_dp,
> >>
> >> static void intel_edp_psr_setup(struct intel_dp *intel_dp)
> >> {
> >> - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >> + struct drm_device *dev = intel_dig_port->base.base.dev;
> >> struct drm_i915_private *dev_priv = dev->dev_private;
> >> struct edp_vsc_psr psr_vsc;
> >> + uint32_t val;
> >>
> >> if (dev_priv->psr.setup_done)
> >> return;
> >>
> >> - /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
> >> - memset(&psr_vsc, 0, sizeof(psr_vsc));
> >> - psr_vsc.sdp_header.HB0 = 0;
> >> - psr_vsc.sdp_header.HB1 = 0x7;
> >> - psr_vsc.sdp_header.HB2 = 0x2;
> >> - psr_vsc.sdp_header.HB3 = 0x8;
> >> - intel_edp_psr_write_vsc(intel_dp, &psr_vsc);
> >> + if (IS_VALLEYVIEW(dev)) {
> >> + val = I915_READ(VLV_EDP_VSC_SDP_REG(PIPE_A));
> >> + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK;
> >> + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME;
> >> + I915_WRITE(VLV_EDP_VSC_SDP_REG(PIPE_A), val);
> >> +
> >> + val = I915_READ(VLV_EDP_VSC_SDP_REG(PIPE_B));
> >> + val &= ~VLV_EDP_PSR_SDP_FREQ_MASK;
> >> + val |= VLV_EDP_PSR_SDP_FREQ_EVFRAME;
> >> + I915_WRITE(VLV_EDP_VSC_SDP_REG(PIPE_B), val);
> >> + } else {
> >> + /* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
> >> + memset(&psr_vsc, 0, sizeof(psr_vsc));
> >> + psr_vsc.sdp_header.HB0 = 0;
> >> + psr_vsc.sdp_header.HB1 = 0x7;
> >> + psr_vsc.sdp_header.HB2 = 0x2;
> >> + psr_vsc.sdp_header.HB3 = 0x8;
> >> + intel_edp_psr_write_vsc(intel_dp, &psr_vsc);
> >>
> >> - /* Avoid continuous PSR exit by masking memup and hpd */
> >> - I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
> >> - EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
> >> + /* Avoid continuous PSR exit by masking memup and hpd */
> >> + I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
> >> + EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
> >> + }
> >>
> >> dev_priv->psr.setup_done = true;
> >> }
> >>
> >> +static void vlv_edp_psr_enable_sink(struct intel_dp *intel_dp)
> >> +{
> >> + /* Enable PSR in sink */
> >> + intel_dp_aux_native_write_1(intel_dp, DP_PSR_EN_CFG,
> >> + DP_PSR_ENABLE);
> >
> > Don't we want the same main-link poweroff logic as HSW?
>
>
> I can't remember why I'm not disabling main link here... So I did a
> quickly try now and failure on tests was huge... so let's keep it
> simple for now ;)
I think you are disabling the main link currently. Or at least you're
telling the sink that main link is going to be turned off, and you also
leave the VLV_EDP_PSR_SRC_TRANSMITTER_STATE bit unset, so I think that
should make the main link turn off. Well, since the pipe and port remain
active, I don't know if the link actually turns off.
I would assume that if you don't turn off the link, it would be easier
to keep things happy. A bit weird that you had the opposite result.
BTW now that I look at the HSW code, it seems buggy. When the sink
doesn't require link training, we tell it that we're going to turn the
link off, otherwise we tell it link will be on. But then we actually do
the opposite in intel_edp_psr_enable_source().
>
> > Or maybe we
> > should just keep the main link on always as long as we can't enter
> > the low power states w/ DPLL/pipe/port off.
> >
> > Did you already figure out why that's not happening? Looking at the
> > PSRSTAT registers, my guess is that D0i1 is where we end up currently,
> > and that doesn't actually turn off anything but the planes (to stop
> > memory fetches). D0i2 would turn off everything.
>
> no idea.. :(
Did you check the status bit BTW? Does it say D0i1 or D0i2 when you're
in PSR?
>
> > But I guess we should anyway do this in steps. First getting the
> > current stuff in, then trying to get into D0i2 state, and finally
> > getting the display power well turned off when in PSR.
>
> Agree.
>
> >
> > I think once we get to working on D0i2, we'll need to move the PSR
> > wakeup to happen from a workqueue since it essentially requires a
> > full modeset. Even now in your code it's somewhat questionable
> > since you're doing stuff like aux transfers while holding
> > struct_mutex.
>
> uhm... anything we should try now to improve?
Hmm. We already seem to do some PSR setup while holding struct_mutex,
but we don't hold it always. Although I think w/ HSW we end up
protecting the PSR state w/ the modeset locks since we only fiddle
with it during modesets. So for HSW we should just move the PSR update
call in set_base outside struct_mutex.
But then for VLV, you want to call it from various places that already
hold struct_mutex, so it's getting messy. Just moving it to a workqueue
would seem like the easiest option here, and then you could grab the
modeset locks in the work function.
>
> >
> >> +}
> >> +
> >> static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
> >> {
> >> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >> @@ -1678,6 +1713,28 @@ static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
> >> (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
> >> }
> >>
> >> +static void vlv_edp_psr_enable_source(struct intel_dp *intel_dp)
> >> +{
> >> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >> + struct drm_device *dev = intel_dig_port->base.base.dev;
> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> + struct intel_crtc *intel_crtc =
> >> + to_intel_crtc(intel_dig_port->base.base.crtc);
> >> +
> >> + uint32_t idle_frames = 1;
> >> + uint32_t val;
> >> +
> >> + val = I915_READ(VLV_EDP_PSR_CTL(intel_crtc->pipe));
> >
> > I don't think we want to preserve anything here. We need to make sure
> > everything is initialized correctly rather than trusting some old junk
> > in the register.
>
> Done.
>
> >
> >> + val |= VLV_EDP_PSR_ENABLE;
> >> + val &= ~VLV_EDP_PSR_MODE_MASK;
> >> +
> >> + val |= VLV_EDP_PSR_MODE_HW_TIMER;
> >> + val &= ~VLV_EDP_PSR_FRAME_COUNT_MASK;
> >> + val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
> >> +
> >> + I915_WRITE(VLV_EDP_PSR_CTL(intel_crtc->pipe), val);
> >> +}
> >> +
> >> static void intel_edp_psr_enable_source(struct intel_dp *intel_dp)
> >> {
> >> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >> @@ -1719,8 +1776,8 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
> >> return false;
> >> }
> >>
> >> - if ((intel_encoder->type != INTEL_OUTPUT_EDP) ||
> >> - (dig_port->port != PORT_A)) {
> >> + if (HAS_DDI(dev) && ((intel_encoder->type != INTEL_OUTPUT_EDP) ||
> >> + (dig_port->port != PORT_A))) {
> >> DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
> >> return false;
> >> }
> >> @@ -1765,37 +1822,83 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
> >> return false;
> >> }
> >>
> >> + /* Baytrail supports per-pipe PSR configuration, however PSR on
> >> + * PIPE_B isn't working properly. So let's keep it disabled for now. */
> >> + if (IS_VALLEYVIEW(dev) && intel_crtc->pipe != PIPE_A) {
> >> + DRM_DEBUG_KMS("PSR on BYT isn't enabled on pipe B.\n");
> >> + return false;
> >> + }
> >> +
> >> dev_priv->psr.source_ok = true;
> >> return true;
> >> }
> >>
> >> static void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
> >> {
> >> - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >> + struct drm_device *dev = intel_dig_port->base.base.dev;
> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> + struct intel_crtc *intel_crtc =
> >> + to_intel_crtc(intel_dig_port->base.base.crtc);
> >>
> >> - if (!intel_edp_psr_match_conditions(intel_dp) ||
> >> - intel_edp_is_psr_enabled(dev))
> >> - return;
> >> + if (IS_VALLEYVIEW(dev)) {
> >> + if (vlv_edp_is_psr_enabled_on_pipe(dev, intel_crtc->pipe))
> >> + return;
> >> + } else
> >> + if (intel_edp_is_psr_enabled(dev))
> >> + return;
> >>
> >> /* Setup PSR once */
> >> intel_edp_psr_setup(intel_dp);
> >>
> >> /* Enable PSR on the panel */
> >> - intel_edp_psr_enable_sink(intel_dp);
> >> + if (IS_VALLEYVIEW(dev))
> >> + vlv_edp_psr_enable_sink(intel_dp);
> >> + else
> >> + intel_edp_psr_enable_sink(intel_dp);
> >>
> >> /* Enable PSR on the host */
> >> - intel_edp_psr_enable_source(intel_dp);
> >> + if (IS_VALLEYVIEW(dev))
> >> + vlv_edp_psr_enable_source(intel_dp);
> >> + else
> >> + intel_edp_psr_enable_source(intel_dp);
> >> +
> >> + dev_priv->psr.active = true;
> >> }
> >>
> >> void intel_edp_psr_enable(struct intel_dp *intel_dp)
> >> {
> >> - struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >> + if (!is_edp_psr(intel_dp))
> >> + return;
> >>
> >> - if (intel_edp_psr_match_conditions(intel_dp) &&
> >> - !intel_edp_is_psr_enabled(dev))
> >> + if (intel_edp_psr_match_conditions(intel_dp))
> >> intel_edp_psr_do_enable(intel_dp);
> >> }
> >>
> >> +void vlv_edp_psr_disable(struct intel_dp *intel_dp)
> >> +{
> >> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >> + struct drm_device *dev = intel_dig_port->base.base.dev;
> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> + struct intel_crtc *intel_crtc =
> >> + to_intel_crtc(intel_dig_port->base.base.crtc);
> >> + uint32_t val = I915_READ(VLV_EDP_PSR_STATUS_CTL(intel_crtc->pipe));
> >> +
> >> + if (!dev_priv->psr.setup_done)
> >> + return;
> >> +
> >> + intel_edp_psr_inactivate(dev);
> >> +
> >> + if (val & VLV_EDP_PSR_IN_TRANS)
> >> + udelay(250);
> >
> > Might we want a warning if the bit doesn't go down in expected time?
>
> Done.
>
> >
> >> +
> >> + val = I915_READ(VLV_EDP_PSR_CTL(intel_crtc->pipe));
> >> + val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> >> + val &= ~VLV_EDP_PSR_ENABLE;
> >> + val &= ~VLV_EDP_PSR_MODE_MASK;
> >> + I915_WRITE(VLV_EDP_PSR_CTL(intel_crtc->pipe), val);
> >> +}
> >> +
> >> void intel_edp_psr_disable(struct intel_dp *intel_dp)
> >> {
> >> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >> @@ -1817,28 +1920,66 @@ void intel_edp_psr_update(struct drm_device *dev)
> >> {
> >> struct intel_encoder *encoder;
> >> struct intel_dp *intel_dp = NULL;
> >> + struct intel_crtc *intel_crtc;
> >>
> >> list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head)
> >> if (encoder->type == INTEL_OUTPUT_EDP) {
> >> intel_dp = enc_to_intel_dp(&encoder->base);
> >>
> >> - if (!is_edp_psr(dev))
> >> + if (!is_edp_psr(intel_dp))
> >> return;
> >>
> >> - if (!intel_edp_psr_match_conditions(intel_dp))
> >> - intel_edp_psr_disable(intel_dp);
> >> - else
> >> + intel_crtc = to_intel_crtc(encoder->base.crtc);
> >> +
> >> + if (!intel_edp_psr_match_conditions(intel_dp)) {
> >> + if (IS_VALLEYVIEW(dev))
> >> + vlv_edp_psr_disable(intel_dp);
> >> + else
> >> + intel_edp_psr_disable(intel_dp);
> >> + } else
> >> if (!intel_edp_is_psr_enabled(dev))
> >> intel_edp_psr_do_enable(intel_dp);
> >> }
> >> }
> >>
> >> +void intel_edp_psr_inactivate(struct drm_device *dev)
> >> +{
> >> + struct drm_i915_private *dev_priv = dev->dev_private;
> >> + struct intel_encoder *encoder;
> >> + struct intel_crtc *intel_crtc;
> >> + struct intel_dp *intel_dp = NULL;
> >> +
> >> + if (!dev_priv->psr.setup_done || !dev_priv->psr.active)
> >> + return;
> >> +
> >> + list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head)
> >> + if (encoder->type == INTEL_OUTPUT_EDP) {
> >> + intel_dp = enc_to_intel_dp(&encoder->base);
> >> + intel_crtc = to_intel_crtc(encoder->base.crtc);
> >> +
> >> + if (!vlv_edp_is_psr_enabled_on_pipe(dev,
> >> + intel_crtc->pipe))
> >> + continue;
> >> +
> >> + dev_priv->psr.active = false;
> >> + I915_WRITE(VLV_EDP_PSR_CTL(intel_crtc->pipe),
> >> + VLV_EDP_PSR_RESET);
> >> + /* WaClearPSRReset:vlv */
> >> + I915_WRITE(VLV_EDP_PSR_CTL(intel_crtc->pipe), 0);
> >> +
> >> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> >> + }
> >> +}
> >> +
> >> static void intel_disable_dp(struct intel_encoder *encoder)
> >> {
> >> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> >> enum port port = dp_to_dig_port(intel_dp)->port;
> >> struct drm_device *dev = encoder->base.dev;
> >>
> >> + if (IS_VALLEYVIEW(dev))
> >> + vlv_edp_psr_disable(intel_dp);
> >> +
> >> /* 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_backlight_off(intel_dp);
> >> @@ -1895,6 +2036,7 @@ static void vlv_enable_dp(struct intel_encoder *encoder)
> >> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> >>
> >> intel_edp_backlight_on(intel_dp);
> >> + intel_edp_psr_enable(intel_dp);
> >> }
> >>
> >> 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 71c1371..82026ef 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -748,6 +748,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp);
> >> void intel_edp_psr_enable(struct intel_dp *intel_dp);
> >> void intel_edp_psr_disable(struct intel_dp *intel_dp);
> >> void intel_edp_psr_update(struct drm_device *dev);
> >> +void intel_edp_psr_inactivate(struct drm_device *dev);
> >>
> >>
> >> /* intel_dsi.c */
> >> --
> >> 1.8.1.2
> >
> > --
> > Ville Syrjälä
> > Intel OTC
>
> Thank you very much!
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list