From: sonika <sonika.jindal@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org, rodrigo.vivi@intel.com
Subject: Re: [PATCH] drm/i915/skl: Enabling PSR on Skylake
Date: Thu, 29 Jan 2015 09:27:14 +0530 [thread overview]
Message-ID: <54C9AF9A.4060404@intel.com> (raw)
In-Reply-To: <20150128160227.GY4764@phenom.ffwll.local>
On Wednesday 28 January 2015 09:32 PM, Daniel Vetter wrote:
> On Thu, Jan 22, 2015 at 02:30:54PM +0530, Sonika Jindal wrote:
>> Mainly taking care of some register offsets, otherwise things are similar to
>> hsw. Also, programming ddi aux to use hardcoded values for psr data select.
>>
>> v2: introduce EDP_PSR_AUX_BASE macro (Chris)
>> v3: Moving to HW tracking for SKL+ platforms, so activating source psr during
>> psr_enabling and then avoiding psr entries and exits for each frontbuffer
>> updates.
>> v4: Using SKL DDI AUX regs instead of changing PSR_AUX regs definition (Rodrigo)
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 3 ++-
>> drivers/gpu/drm/i915/i915_reg.h | 5 +++++
>> drivers/gpu/drm/i915/intel_frontbuffer.c | 7 +++++--
>> drivers/gpu/drm/i915/intel_psr.c | 26 ++++++++++++++++++++++++--
>> 4 files changed, 36 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 66f0c60..3d24872 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2371,7 +2371,8 @@ struct drm_i915_cmd_table {
>> #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) || \
>> - IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
>> + IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
>> + IS_SKYLAKE(dev))
>> #define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
>> IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
>> #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6)
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index a39bb03..a6f06fa 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3748,6 +3748,11 @@ enum punit_power_well {
>> #define DP_AUX_CH_CTL_PRECHARGE_TEST (1 << 11)
>> #define DP_AUX_CH_CTL_BIT_CLOCK_2X_MASK (0x7ff)
>> #define DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT 0
>> +#define DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL (1 << 14)
>> +#define DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL (1 << 13)
>> +#define DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL (1 << 12)
>> +#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (1f << 5)
>> +#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
>> #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1)
>>
>> /*
>> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
>> index 79f6d72..010d550 100644
>> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
>> @@ -156,7 +156,9 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>>
>> intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
>>
>> - intel_psr_invalidate(dev, obj->frontbuffer_bits);
>> +
>> + if (INTEL_INFO(dev)->gen < 9)
>> + intel_psr_invalidate(dev, obj->frontbuffer_bits);
>> }
>>
>> /**
>> @@ -182,7 +184,8 @@ void intel_frontbuffer_flush(struct drm_device *dev,
>>
>> intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
>>
>> - intel_psr_flush(dev, frontbuffer_bits);
>> + if (INTEL_INFO(dev)->gen < 9)
>> + intel_psr_flush(dev, frontbuffer_bits);
> Again no, not going to take wholesale filtering of the sw invalidate
> paths. This needs to be properly tested and pushed down into the psr
> specific invalidate/flush functions on a per-function basis.
>
> I've dropped these two hunks and merged the patch.
> -Daniel
Hi Daniel,
Even SW tracking doesn't work in many cases, like I reported earlier in
ubuntu login screen where we don't get frontbuffer flushes and we don't
enter PSR at all with SW tracking.
I see similar behavior even in fbcon mode. So, I am not sure how you can
say that SW tracking is the only right way.
If there are cases where HW tracking fails (and I know a few), we need
to fix them. I can move this gen check to the intel_psr_* function if
that is the major concern.
-Sonika
>>
>> /*
>> * FIXME: Unconditional fbc flushing here is a rather gross hack and
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index dd0e6e0..4867d5a 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -142,6 +142,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>> struct drm_device *dev = dig_port->base.base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> uint32_t aux_clock_divider;
>> + uint32_t aux_data_reg, aux_ctl_reg;
>> int precharge = 0x3;
>> bool only_standby = dev_priv->vbt.psr.full_link;
>> static const uint8_t aux_msg[] = {
>> @@ -168,16 +169,34 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>> drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>> DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>>
>> + aux_data_reg = (INTEL_INFO(dev)->gen >= 9) ?
>> + DPA_AUX_CH_DATA1 : EDP_PSR_AUX_DATA1(dev);
>> + aux_ctl_reg = (INTEL_INFO(dev)->gen >= 9) ?
>> + DPA_AUX_CH_CTL : EDP_PSR_AUX_CTL(dev);
>> +
>> /* Setup AUX registers */
>> for (i = 0; i < sizeof(aux_msg); i += 4)
>> - I915_WRITE(EDP_PSR_AUX_DATA1(dev) + i,
>> + I915_WRITE(aux_data_reg + i,
>> intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i));
>>
>> - I915_WRITE(EDP_PSR_AUX_CTL(dev),
>> + if (INTEL_INFO(dev)->gen >= 9) {
>> + uint32_t val;
>> +
>> + val = I915_READ(aux_ctl_reg);
>> + val &= ~DP_AUX_CH_CTL_TIME_OUT_MASK;
>> + val |= DP_AUX_CH_CTL_TIME_OUT_1600us;
>> + val &= ~DP_AUX_CH_CTL_MESSAGE_SIZE_MASK;
>> + val |= (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT);
>> + /* Use hardcoded data values for PSR */
>> + val &= ~DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL;
>> + I915_WRITE(aux_ctl_reg, val);
>> + } else {
>> + I915_WRITE(aux_ctl_reg,
>> DP_AUX_CH_CTL_TIME_OUT_400us |
>> (sizeof(aux_msg) << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
>> (precharge << DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
>> (aux_clock_divider << DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT));
>> + }
>> }
>>
>> static void vlv_psr_enable_source(struct intel_dp *intel_dp)
>> @@ -355,6 +374,9 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>>
>> /* Enable PSR on the panel */
>> hsw_psr_enable_sink(intel_dp);
>> +
>> + if (INTEL_INFO(dev)->gen >= 9)
>> + intel_psr_activate(intel_dp);
>> } else {
>> vlv_psr_setup_vsc(intel_dp);
>>
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-01-29 4:04 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-22 9:00 [PATCH] drm/i915/skl: Enabling PSR on Skylake Sonika Jindal
2015-01-22 17:48 ` shuang.he
2015-01-28 16:02 ` Daniel Vetter
2015-01-29 3:57 ` sonika [this message]
2015-01-29 16:11 ` Daniel Vetter
-- strict thread matches above, loose matches on Subject: below --
2015-01-16 8:37 Sonika Jindal
2015-01-16 15:06 ` shuang.he
2015-01-17 4:24 ` Daniel Vetter
2015-01-19 11:40 ` sonika
2015-01-20 9:50 ` Daniel Vetter
2015-01-20 11:49 ` Jindal, Sonika
2015-01-20 22:01 ` Rodrigo Vivi
2015-01-21 4:53 ` sonika
2015-01-21 8:37 ` Daniel Vetter
2015-01-21 8:55 ` sonika
2015-01-21 9:42 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54C9AF9A.4060404@intel.com \
--to=sonika.jindal@intel.com \
--cc=daniel@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
--cc=rodrigo.vivi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.