2013/4/17 Daniel Vetter <daniel at ffwll.ch>:
> On Tue, Apr 16, 2013 at 01:47:44PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> This patch implements "Display Sequences for Package C8", from the
>> "Display Mode Set Sequence" section from the Haswell documentation.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>>
>> Hi
>>
>> This patch was tested and the machines do enter C8+, so it shows our driver is
>> not preventing C8+. There may be additional patches in the future to allow C8+
>> in more cases, but they shouldn't stop this patch from being merged.
>>
>> Please also notice that we need i915.disable_power_well=1 because we can't allow
>> C8+ if the power well is enabled. So even if we merge this patch, the feature
>> won't be enabled by default. This is not really a problem and it allows us to
>> properly test everything without needing to rebase a 600-line patch every time.
>>
>> I wrote this patch on top of drm-intel-next-queued + the 4 patches I already
>> sent to this list:
>> - 2 patches that fix CPT FDI RX polarity bugs (already reviewed by Imre)
>> - 2 patches that enable FIFO underruns and Poison messages (not reviewed yet)
>
> Ok, a bunch of high-level bikesheds and a bunch of questions:
>
> - It looks like we also kill the gt interrupts, or at least the forwarding
> of them. What happens if package c8 is all enabled and someone tries to
> submit a batchbuffer? Depending upon the answer an igt test-case would
> be interesting (i.e. switch all outputs off, wait a bit to reach c8,
> then submit batch).
I don't have a clear answer for this right now and the documentation
doesn't say anything, but we know that we can only enter PC8 if we're
in RC6. Another thing we need to consider is: even if we *allow* PC8+
it doesn't mean we will *reach* PC8+, so it won't be easy to write an
i-g-t test for this. For example, one of the things I tried was to run
glxgears and try to get to PC8+, but then I discovered that while
glxgears was running we couldn't get past PC7. I need to do more
tests.
Another thing we discovered is that after we get back from PC8 we need
to call i915_gem_init_swizzling(). I don't know if there's something
else we need to call, but the current patch appears to be working.
Ideas? Of course, an i-g-t test for this would be good.
>
> - You have a bit a mess between refcounting and plain checks whether
> package c8 is allowed. To keep sane we should use refcounting everywhere
> and convert all the register checks in hsw_can_allow_package_c8 into
> WARN_ONs/assert.
I implemented it as we discussed on IRC a few days ago. Check the new version.
> And imo the two encoder/connector loops at the top
> should be moved into ->global_modeset_resources simply to decide whether
> we need to grab a refcount. Maybe the modeset state needs a
> has_package_c8_refcount bool.
Moving the loops to modeset_global_resources doesn't make sense as the
function needs to be called unconditionally. But this piece of code
changed a little bit, so you might want to take a look at how the new
code works.
>
> Essentially the the crtc/encoder checks and all the register checks
> _should_ agree. If they do not, there's a bug in our state tracking, and
> I want such a bug to result in a loud WARN backtrace if at all possible.
Yeah, now I'm only checking for used crtcs. I'm sure if the checks
don't agree we'll have many other WARNs triggered in other pieces of
our code.
>
> - The irq safe/restore needs to be rethought - atm a lot of the irq
> register setup crucially depends upon correct ordering of register
> writes with irq handler enabling/disabling: A few registers are only
> ever touched by the setup code when the irq handler is guaranteed to not
> run, which allows us to touch them in the irq handler without locking.
> So just grabbing the irq spinlock is not enough.
Why? Please explain more. It would also be good if you could point to
which piece of code you're talking about. We grab the irq-safe
spinlock and then we disable irqs.
>
> But before we go down this rabbit hole and try to fix it: Is setting all
> these irq register values mandated by hw (i.e. we can go into pc8
> without them)? Or is this just a case of bspec recommend a possible
> approach (and trying to make it clear that most interrupts are gone)?
The docs have an explicit requirement to disable all the interrupts
except for the PCH hotplug ones, so I guess this is the only tested
path. It also explicitly says that if the PCH interrupt is not 0 we
don't reach C8+. I don't think it's a good idea to not follow this.
>
> - Slightly related: I've just merged Egberts hpd irq storm mitigation
> work, so unconditionally enabling hpd interrupt is a no-go no.
Yeah, I changed this, now we only enable the interrupts that were
already enabled.
>
> - Still on the topic of interrupts: A bunch of these interrupt register we
> never use. Imo it'd be better to set the right values at irq setup time
> and just put a big WARN into the pc8 code to make sure it's still
> correct. Somewhat on the edge is the FDI_RX register: We only use it
> when modesetting is enabled, so imo it would be better to write the
> desired value into these regs once fdi link training completed (or the
> pipe is shut down, if we ever bother to handle fdi link failures at
> runtime). Similar for fifo underrun reporting.
I wrote some patches to initialize all these registers and I'm going
to send them along with the updated C8+ patch. But I really think that
on the PC8+ code we should treat these interrupts just like we treat
all the other interrupts, because if we ever start using these
interrupts we'll need to properly treat them like we treat the others.
>
> Of course in all cases a check in the pc8 code would be good.
The spec says we need to make sure all pending interrupts are clear,
but I'm not sure what we need to do if we reach the PC8+ code and
still have some pending IIR stuff. For now I just put a TODO comment
and DRM_ERROR in case we find any non-zero IIR register. Suggestions
on how to handle this are welcome.
>
> - And finally some bikesheds on the naming ;-) The
> hsw_package_c8_wakeup/sleep calls look a bit funny in the rather
> platform-agnostic connector detect callbacks. Generally we opt for a
> more generic intel_foo_meh pattern in such cases. For the generic
> interface I also vote for a more descriptive and less technical term,
> since maybe on SoC platforms the same concept is called a bit
> differently. Imo aux_display instead of package_c8 makes it much clearer
> what exactly we need to have powered on to do edid reads. Lastly I think
> we should consider using the established naming convention of the
> runtime pm framework - the refcounting uses
> runtime_forbid/allow.
>
> So all together I vote for the generic interface to be called something
> like
>
> intel_aux_display_runtime_forbid/allow
I changed to intel_aux_display_shutdown_forbid/allow, because we
forbid the shutdown, not the "runtime".
>
> Of course, hsw_package_c8 is perfect for the low-level implementation
> and every place interfacing with hsw platform code (i.e. everything else
> but connector state). So imo the above could be tiny static inline
> wrappers for now.
They are tiny 3-line functions now.
I'll send the updated version of this patch along with the pre-work to
initialize the interrupts.I guess we can continue the discussion as
replies to the new patches if you want.
Thanks for the review,
Paulo
>
> Cheers, Daniel
>>
>> Thanks,
>> Paulo
>>
>> drivers/gpu/drm/i915/i915_dma.c | 1 +
>> drivers/gpu/drm/i915/i915_drv.h | 29 +++
>> drivers/gpu/drm/i915/i915_reg.h | 27 ++
>> drivers/gpu/drm/i915/intel_crt.c | 26 +-
>> drivers/gpu/drm/i915/intel_display.c | 459 ++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_dp.c | 11 +-
>> drivers/gpu/drm/i915/intel_drv.h | 2 +
>> drivers/gpu/drm/i915/intel_hdmi.c | 3 +
>> 8 files changed, 549 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 3b315ba..bc81ab9 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -1630,6 +1630,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>> spin_lock_init(&dev_priv->gpu_error.lock);
>> spin_lock_init(&dev_priv->rps.lock);
>> mutex_init(&dev_priv->dpio_lock);
>> + mutex_init(&dev_priv->c8_lock);
>>
>> mutex_init(&dev_priv->rps.hw_lock);
>> mutex_init(&dev_priv->modeset_restore_lock);
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index b0da4ae..b64d0e3 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -504,6 +504,29 @@ struct intel_gmbus {
>> struct drm_i915_private *dev_priv;
>> };
>>
>> +struct i915_c8_saved_registers {
>> + u32 de_imr;
>> + u32 de_ier;
>> + u32 aud_imr;
>> + u32 aud_ier;
>> + u32 gt_imr;
>> + u32 gt_ier;
>> + u32 pm_imr;
>> + u32 pm_ier;
>> + u32 srd_imr;
>> + u32 hotplug_ctl;
>> + u32 err_int;
>> +
>> + u32 sde_imr;
>> + u32 sde_ier;
>> + u32 fdirx_imr;
>> + u32 gtcpch_imr;
>> + u32 shotplug_ctl;
>> + u32 serr_int;
>> +
>> + u32 lcpll_freq;
>> +};
>> +
>> struct i915_suspend_saved_registers {
>> u8 saveLBB;
>> u32 saveDSPACNTR;
>> @@ -1068,6 +1091,12 @@ typedef struct drm_i915_private {
>>
>> struct i915_suspend_saved_registers regfile;
>>
>> + struct i915_c8_saved_registers c8_regfile;
>> + bool allowing_package_c8;
>> + /* Wake ups should happen when allowing_package_c8 is true. */
>> + int c8_wakeup_refcnt;
>> + struct mutex c8_lock;
>> +
>> /* Old dri1 support infrastructure, beware the dragons ya fools entering
>> * here! */
>> struct i915_dri1_state dri1;
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index d954612..6808ae5 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2013,6 +2013,8 @@
>> #define BLC_PWM_CPU_CTL2 0x48250
>> #define BLC_PWM_CPU_CTL 0x48254
>>
>> +#define HSW_BLC_PWM2_CTL 0x48350
>> +
>> /* PCH CTL1 is totally different, all but the below bits are reserved. CTL2 is
>> * like the normal CTL from gen4 and earlier. Hooray for confusing naming. */
>> #define BLC_PWM_PCH_CTL1 0xc8250
>> @@ -2021,6 +2023,12 @@
>> #define BLM_PCH_POLARITY (1 << 29)
>> #define BLC_PWM_PCH_CTL2 0xc8254
>>
>> +#define UTIL_PIN_CTL 0x48400
>> +#define UTIL_PIN_ENABLE (1 << 31)
>> +
>> +#define PCH_GTC_CTL 0xe7000
>> +#define PCH_GTC_ENABLE (1 << 31)
>> +
>> /* TV port control */
>> #define TV_CTL 0x68000
>> /** Enables the TV encoder */
>> @@ -3535,6 +3543,16 @@
>> #define DEIIR 0x44008
>> #define DEIER 0x4400c
>>
>> +#define AUDIMR 0x44084
>> +#define AUDIIR 0x44088
>> +#define AUDIER 0x4408c
>> +
>> +#define SRDIMR 0x64834
>> +#define SRDIIR 0x64838
>> +
>> +#define PCH_GTCIMR 0xe7054
>> +#define PCH_GTCIIR 0xe7058
>> +
>> /* GT interrupt.
>> * Note that for gen6+ the ring-specific interrupt bits do alias with the
>> * corresponding bits in the per-ring interrupt control registers. */
>> @@ -4709,6 +4727,8 @@
>> #define SBI_SSCAUXDIV_FINALDIV2SEL(x) ((x)<<4)
>> #define SBI_DBUFF0 0x2a00
>> #define SBI_DBUFF0_ENABLE (1<<0)
>> +#define SBI_GEN0 0x1f00
>> +#define SBI_GEN0_ENABLE (1<<0)
>>
>> /* LPT PIXCLK_GATE */
>> #define PIXCLK_GATE 0xC6020
>> @@ -4774,7 +4794,14 @@
>> #define LCPLL_CLK_FREQ_450 (0<<26)
>> #define LCPLL_CD_CLOCK_DISABLE (1<<25)
>> #define LCPLL_CD2X_CLOCK_DISABLE (1<<23)
>> +#define LCPLL_POWER_DOWN_ALLOW (1<<22)
>> #define LCPLL_CD_SOURCE_FCLK (1<<21)
>> +#define LCPLL_CD_SOURCE_FCLK_DONE (1<<19)
>> +
>> +#define D_COMP (MCHBAR_MIRROR_BASE_SNB + 0x5F0C)
>> +#define D_COMP_RCOMP_IN_PROGRESS (1<<9)
>> +#define D_COMP_COMP_FORCE (1<<8)
>> +#define D_COMP_COMP_DISABLE (1<<0)
>>
>> /* Pipe WM_LINETIME - watermark line time */
>> #define PIPE_WM_LINETIME_A 0x45270
>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>> index 1ae2d7f..ee98731 100644
>> --- a/drivers/gpu/drm/i915/intel_crt.c
>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>> @@ -588,10 +588,13 @@ static enum drm_connector_status
>> intel_crt_detect(struct drm_connector *connector, bool force)
>> {
>> struct drm_device *dev = connector->dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> struct intel_crt *crt = intel_attached_crt(connector);
>> enum drm_connector_status status;
>> struct intel_load_detect_pipe tmp;
>>
>> + hsw_package_c8_wakeup(dev_priv);
>> +
>> if (I915_HAS_HOTPLUG(dev)) {
>> /* We can not rely on the HPD pin always being correctly wired
>> * up, for example many KVM do not pass it through, and so
>> @@ -599,23 +602,30 @@ intel_crt_detect(struct drm_connector *connector, bool force)
>> */
>> if (intel_crt_detect_hotplug(connector)) {
>> DRM_DEBUG_KMS("CRT detected via hotplug\n");
>> - return connector_status_connected;
>> + status = connector_status_connected;
>> + goto out;
>> } else
>> DRM_DEBUG_KMS("CRT not detected via hotplug\n");
>> }
>>
>> - if (intel_crt_detect_ddc(connector))
>> - return connector_status_connected;
>> + if (intel_crt_detect_ddc(connector)) {
>> + status = connector_status_connected;
>> + goto out;
>> + }
>>
>> /* Load detection is broken on HPD capable machines. Whoever wants a
>> * broken monitor (without edid) to work behind a broken kvm (that fails
>> * to have the right resistors for HP detection) needs to fix this up.
>> * For now just bail out. */
>> - if (I915_HAS_HOTPLUG(dev))
>> - return connector_status_disconnected;
>> + if (I915_HAS_HOTPLUG(dev)) {
>> + status = connector_status_disconnected;
>> + goto out;
>> + }
>>
>> - if (!force)
>> - return connector->status;
>> + if (!force) {
>> + status = connector->status;
>> + goto out;
>> + }
>>
>> /* for pre-945g platforms use load detect */
>> if (intel_get_load_detect_pipe(connector, NULL, &tmp)) {
>> @@ -627,6 +637,8 @@ intel_crt_detect(struct drm_connector *connector, bool force)
>> } else
>> status = connector_status_unknown;
>>
>> +out:
>> + hsw_package_c8_sleep(dev_priv);
>> return status;
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index cd9fa46..9e6d6e4 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -5091,6 +5091,66 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
>> mutex_unlock(&dev_priv->dpio_lock);
>> }
>>
>> +/* Sequence to enable CLKOUT_DP */
>> +static void lpt_enable_clkout_dp(struct drm_i915_private *dev_priv)
>> +{
>> + uint32_t val;
>> +
>> + mutex_lock(&dev_priv->dpio_lock);
>> +
>> + val = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
>> + val &= ~SBI_SSCCTL_DISABLE;
>> + val |= SBI_SSCCTL_PATHALT;
>> + intel_sbi_write(dev_priv, SBI_SSCCTL, val, SBI_ICLK);
>> +
>> + udelay(24);
>> +
>> + val = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
>> + val &= ~SBI_SSCCTL_PATHALT;
>> + intel_sbi_write(dev_priv, SBI_SSCCTL, val, SBI_ICLK);
>> +
>> + if (IS_ULT(dev_priv->dev)) {
>> + val = intel_sbi_read(dev_priv, SBI_GEN0, SBI_ICLK);
>> + val |= SBI_GEN0_ENABLE;
>> + intel_sbi_write(dev_priv, SBI_GEN0, val, SBI_ICLK);
>> + } else {
>> + val = intel_sbi_read(dev_priv, SBI_DBUFF0, SBI_ICLK);
>> + val |= SBI_DBUFF0_ENABLE;
>> + intel_sbi_write(dev_priv, SBI_DBUFF0, val, SBI_ICLK);
>> + }
>> +
>> + mutex_unlock(&dev_priv->dpio_lock);
>> +}
>> +
>> +/* Sequence to disable CLKOUT_DP */
>> +static void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv)
>> +{
>> + uint32_t val;
>> +
>> + mutex_lock(&dev_priv->dpio_lock);
>> +
>> + if (IS_ULT(dev_priv->dev)) {
>> + val = intel_sbi_read(dev_priv, SBI_GEN0, SBI_ICLK);
>> + val &= ~SBI_GEN0_ENABLE;
>> + intel_sbi_write(dev_priv, SBI_GEN0, val, SBI_ICLK);
>> + } else {
>> + val = intel_sbi_read(dev_priv, SBI_DBUFF0, SBI_ICLK);
>> + val &= ~SBI_DBUFF0_ENABLE;
>> + intel_sbi_write(dev_priv, SBI_DBUFF0, val, SBI_ICLK);
>> + }
>> +
>> + val = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
>> + if (!(val & SBI_SSCCTL_PATHALT)) {
>> + val |= SBI_SSCCTL_PATHALT;
>> + intel_sbi_write(dev_priv, SBI_SSCCTL, val, SBI_ICLK);
>> + udelay(32);
>> + }
>> + val |= SBI_SSCCTL_DISABLE;
>> + intel_sbi_write(dev_priv, SBI_SSCCTL, val, SBI_ICLK);
>> +
>> + mutex_unlock(&dev_priv->dpio_lock);
>> +}
>> +
>> /*
>> * Initialize reference clocks when the driver loads
>> */
>> @@ -5751,6 +5811,403 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>> return true;
>> }
>>
>> +static void hsw_disable_lcpll(struct drm_i915_private *dev_priv)
>> +{
>> + uint32_t val;
>> +
>> + val = I915_READ(LCPLL_CTL);
>> +
>> + dev_priv->c8_regfile.lcpll_freq = val & LCPLL_CLK_FREQ_MASK;
>> +
>> + val |= LCPLL_CD_SOURCE_FCLK;
>> + I915_WRITE(LCPLL_CTL, val);
>> + POSTING_READ(LCPLL_CTL);
>> +
>> + udelay(1);
>> +
>> + val = I915_READ(LCPLL_CTL);
>> + if (!(val & LCPLL_CD_SOURCE_FCLK_DONE))
>> + DRM_ERROR("Switching to FCLK failed\n");
>> +
>> + val |= LCPLL_PLL_DISABLE;
>> + I915_WRITE(LCPLL_CTL, val);
>> + POSTING_READ(LCPLL_CTL);
>> +
>> + if (wait_for((I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK) == 0, 1))
>> + DRM_ERROR("LCPLL still locked\n");
>> +
>> + val = I915_READ(D_COMP);
>> + val |= D_COMP_COMP_DISABLE;
>> + I915_WRITE(D_COMP, val);
>> + POSTING_READ(D_COMP);
>> +
>> + udelay(2);
>> +
>> + val = I915_READ(D_COMP);
>> + if (val & D_COMP_RCOMP_IN_PROGRESS)
>> + DRM_ERROR("D_COMP RCOMP still in progress\n");
>> +
>> + val = I915_READ(LCPLL_CTL);
>> + val |= LCPLL_POWER_DOWN_ALLOW;
>> + I915_WRITE(LCPLL_CTL, val);
>> + POSTING_READ(LCPLL_CTL);
>> +}
>> +
>> +static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
>> +{
>> + uint32_t val;
>> +
>> + val = I915_READ(LCPLL_CTL);
>> +
>> + if ((val & LCPLL_CLK_FREQ_MASK) != dev_priv->c8_regfile.lcpll_freq)
>> + DRM_ERROR("LCPLL frequency changed\n");
>> +
>> + if (val & LCPLL_POWER_DOWN_ALLOW) {
>> + val &= ~LCPLL_POWER_DOWN_ALLOW;
>> + I915_WRITE(LCPLL_CTL, val);
>> + }
>> +
>> + if (val & LCPLL_CD_SOURCE_FCLK) {
>> + val = I915_READ(D_COMP);
>> + val |= D_COMP_COMP_FORCE;
>> + val &= ~D_COMP_COMP_DISABLE;
>> + I915_WRITE(D_COMP, val);
>> +
>> + val = I915_READ(LCPLL_CTL);
>> + val &= ~LCPLL_PLL_DISABLE;
>> + I915_WRITE(LCPLL_CTL, val);
>> + POSTING_READ(LCPLL_CTL);
>> +
>> + if (wait_for(I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK, 5))
>> + DRM_ERROR("LCPLL not locked yet\n");
>> +
>> + val = I915_READ(LCPLL_CTL);
>> + val &= ~LCPLL_CD_SOURCE_FCLK;
>> + I915_WRITE(LCPLL_CTL, val);
>> + POSTING_READ(LCPLL_CTL);
>> +
>> + udelay(1);
>> +
>> + val = I915_READ(LCPLL_CTL);
>> + if (val & LCPLL_CD_SOURCE_FCLK_DONE)
>> + DRM_ERROR("Switching back to LCPLL failed\n");
>> + }
>> +}
>> +
>> +static void hsw_disable_interrupts(struct drm_i915_private *dev_priv)
>> +{
>> + struct i915_c8_saved_registers *c8_regfile = &dev_priv->c8_regfile;
>> + unsigned long irqflags;
>> + uint32_t val, deier, sdeier;
>> +
>> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>> +
>> + /* TODO: clear all pending graphics interrupts. */
>> + val = I915_READ(DEIIR);
>> + if (val)
>> + DRM_ERROR("Pending interrupt: DEIIR 0x%08x\n", val);
>> +
>> + val = I915_READ(AUDIIR);
>> + if (val)
>> + DRM_ERROR("Pending interrupt: AUDIIR 0x%08x\n", val);
>> +
>> + val = I915_READ(GTIIR);
>> + if (val)
>> + DRM_ERROR("Pending interrupt: GTIIR 0x%08x\n", val);
>> +
>> + val = I915_READ(GEN6_PMIIR);
>> + if (val)
>> + DRM_ERROR("Pending interrupt: PMIIR 0x%08x\n", val);
>> +
>> + val = I915_READ(SRDIIR);
>> + if (val)
>> + DRM_ERROR("Pending interrupt: SRDIIR 0x%08x\n", val);
>> +
>> + val = I915_READ(SDEIIR);
>> + if (val)
>> + DRM_ERROR("Pending interrupt: SDEIIR 0x%08x\n", val);
>> +
>> + val = I915_READ(_FDI_RXA_IIR);
>> + if (val)
>> + DRM_ERROR("Pending interrupt: FDIRXAIIR 0x%08x\n", val);
>> +
>> + val = I915_READ(PCH_GTCIIR);
>> + if (val)
>> + DRM_ERROR("Pending interrupt: GTCPCHIIR 0x%08x\n", val);
>> +
>> + c8_regfile->de_imr = I915_READ(DEIMR);
>> + c8_regfile->de_ier = I915_READ(DEIER);
>> + c8_regfile->aud_imr = I915_READ(AUDIMR);
>> + c8_regfile->aud_ier = I915_READ(AUDIER);
>> + c8_regfile->gt_imr = I915_READ(GTIMR);
>> + c8_regfile->gt_ier = I915_READ(GTIER);
>> + c8_regfile->pm_imr = I915_READ(GEN6_PMIMR);
>> + c8_regfile->pm_ier = I915_READ(GEN6_PMIER);
>> + c8_regfile->srd_imr = I915_READ(SRDIMR);
>> + c8_regfile->hotplug_ctl = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
>> + c8_regfile->err_int = I915_READ(GEN7_ERR_INT);
>> +
>> + c8_regfile->sde_imr = I915_READ(SDEIMR);
>> + c8_regfile->sde_ier = I915_READ(SDEIER);
>> + c8_regfile->fdirx_imr = I915_READ(_FDI_RXA_IMR);
>> + c8_regfile->gtcpch_imr = I915_READ(PCH_GTCIMR);
>> + c8_regfile->shotplug_ctl = I915_READ(PCH_PORT_HOTPLUG);
>> + c8_regfile->serr_int = I915_READ(SERR_INT);
>> +
>> + deier = DE_MASTER_IRQ_CONTROL | DE_PCH_EVENT_IVB;
>> + I915_WRITE(DEIMR, ~deier);
>> + I915_WRITE(DEIER, deier);
>> +
>> + I915_WRITE(AUDIMR, 0xFFFFFFFF);
>> + I915_WRITE(AUDIER, 0);
>> +
>> + I915_WRITE(GTIMR, 0xFFFFFFFF);
>> + I915_WRITE(GTIER, 0);
>> +
>> + I915_WRITE(GEN6_PMIMR, 0xFFFFFFFF);
>> + I915_WRITE(GEN6_PMIER, 0);
>> +
>> + I915_WRITE(SRDIMR, 0xFFFFFFFF);
>> +
>> + I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, 0);
>> +
>> + I915_WRITE(GEN7_ERR_INT, 0xFFFFFFFF);
>> +
>> + sdeier = SDE_PORTD_HOTPLUG_CPT | SDE_PORTC_HOTPLUG_CPT |
>> + SDE_PORTB_HOTPLUG_CPT | SDE_CRT_HOTPLUG_CPT;
>> + I915_WRITE(SDEIMR, ~sdeier);
>> + I915_WRITE(SDEIER, sdeier);
>> +
>> + I915_WRITE(_FDI_RXA_IMR, 0xFFFFFFFF);
>> + I915_WRITE(PCH_GTCIMR, 0xFFFFFFFF);
>> +
>> + I915_WRITE(PCH_PORT_HOTPLUG, PORTD_HOTPLUG_ENABLE |
>> + PORTC_HOTPLUG_ENABLE |
>> + PORTB_HOTPLUG_ENABLE);
>> + I915_WRITE(SERR_INT, 0xFFFFFFFF);
>> +
>> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>> +}
>> +
>> +static void hsw_restore_interrupts(struct drm_i915_private *dev_priv)
>> +{
>> + struct i915_c8_saved_registers *c8_regfile = &dev_priv->c8_regfile;
>> + unsigned long irqflags;
>> +
>> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
>> +
>> + I915_WRITE(DEIMR, c8_regfile->de_imr);
>> + I915_WRITE(DEIER, c8_regfile->de_ier);
>> + I915_WRITE(AUDIMR, c8_regfile->aud_imr);
>> + I915_WRITE(AUDIER, c8_regfile->aud_ier);
>> + I915_WRITE(GTIMR, c8_regfile->gt_imr);
>> + I915_WRITE(GTIER, c8_regfile->gt_ier);
>> + I915_WRITE(GEN6_PMIMR, c8_regfile->pm_imr);
>> + I915_WRITE(GEN6_PMIER, c8_regfile->pm_ier);
>> + I915_WRITE(SRDIMR, c8_regfile->srd_imr);
>> + I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, c8_regfile->hotplug_ctl);
>> + I915_WRITE(GEN7_ERR_INT, c8_regfile->err_int);
>> +
>> + I915_WRITE(SDEIMR, c8_regfile->sde_imr);
>> + I915_WRITE(SDEIER, c8_regfile->sde_ier);
>> + I915_WRITE(_FDI_RXA_IMR, c8_regfile->fdirx_imr);
>> + I915_WRITE(PCH_GTCIMR, c8_regfile->gtcpch_imr);
>> + I915_WRITE(PCH_PORT_HOTPLUG, c8_regfile->shotplug_ctl);
>> + I915_WRITE(SERR_INT, c8_regfile->serr_int);
>> +
>> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>> +}
>> +
>> +/*
>> + * This function should only be called by:
>> + * - hsw_set_package_c8
>> + * - hsw_package_c8_sleep
>> + */
>> +static void hsw_allow_package_c8(struct drm_i915_private *dev_priv)
>> +{
>> + uint32_t val;
>> +
>> + WARN_ON(!mutex_is_locked(&dev_priv->c8_lock));
>> +
>> + /* Disable SCLKGATE_DIS workaround.
>> + * TODO: we should keep this always disabled and only enable it when
>> + * needed. */
>> + if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
>> + val = I915_READ(SOUTH_DSPCLK_GATE_D);
>> + val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
>> + I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
>> + }
>> +
>> + lpt_disable_clkout_dp(dev_priv);
>> + hsw_disable_interrupts(dev_priv);
>> + hsw_disable_lcpll(dev_priv);
>> +}
>> +
>> +/*
>> + * This function should only be called by:
>> + * - hsw_set_package_c8
>> + * - hsw_package_c8_wakeup
>> + */
>> +static void hsw_disallow_package_c8(struct drm_i915_private *dev_priv)
>> +{
>> + uint32_t val;
>> +
>> + WARN_ON(!mutex_is_locked(&dev_priv->c8_lock));
>> +
>> + hsw_restore_lcpll(dev_priv);
>> + hsw_restore_interrupts(dev_priv);
>> + lpt_enable_clkout_dp(dev_priv);
>> +
>> + /* Enable SCLKGATE_DIS workaround.
>> + * TODO: we should keep this always disabled and only enable it when
>> + * needed. */
>> + if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
>> + val = I915_READ(SOUTH_DSPCLK_GATE_D);
>> + val |= PCH_LP_PARTITION_LEVEL_DISABLE;
>> + I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
>> + }
>> +}
>> +
>> +static bool hsw_can_allow_package_c8(struct drm_i915_private *dev_priv)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> + struct intel_crtc *crtc;
>> + struct intel_encoder *encoder;
>> + uint32_t val;
>> + int used_crtcs = 0, used_encoders = 0;
>> +
>> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head)
>> + if (crtc->base.enabled)
>> + used_crtcs++;
>> +
>> + list_for_each_entry(encoder, &dev->mode_config.encoder_list,
>> + base.head)
>> + if (encoder->connectors_active)
>> + used_encoders++;
>> +
>> + if (used_crtcs || used_encoders) {
>> + DRM_DEBUG_KMS("Not allowing C8: %d crtcs and %d encoders enabled\n",
>> + used_crtcs, used_encoders);
>> + return false;
>> + }
>> +
>> + val = I915_READ(SPLL_CTL);
>> + if (val & SPLL_PLL_ENABLE) {
>> + DRM_DEBUG_KMS("Not allowing C8: SPLL enabled\n");
>> + return false;
>> + }
>> +
>> + val = I915_READ(WRPLL_CTL1);
>> + if (val & WRPLL_PLL_ENABLE) {
>> + DRM_DEBUG_KMS("Not allowing C8: WRPLL1 enabled\n");
>> + return false;
>> + }
>> +
>> + val = I915_READ(WRPLL_CTL2);
>> + if (val & WRPLL_PLL_ENABLE) {
>> + DRM_DEBUG_KMS("Not allowing C8: WRPLL2 enabled\n");
>> + return false;
>> + }
>> +
>> + val = I915_READ(HSW_PWR_WELL_DRIVER);
>> + if (val != 0) {
>> + DRM_DEBUG_KMS("Not allowing C8: power well on\n");
>> + return false;
>> + }
>> +
>> + val = I915_READ(PCH_PP_STATUS);
>> + if (val & PP_ON) {
>> + DRM_DEBUG_KMS("Not allowing C8: panel power on\n");
>> + return false;
>> + }
>> +
>> + val = I915_READ(BLC_PWM_CPU_CTL2);
>> + if (val & BLM_PWM_ENABLE) {
>> + DRM_DEBUG_KMS("Not allowing C8: CPU PWM1 enabled\n");
>> + return false;
>> + }
>> +
>> + val = I915_READ(HSW_BLC_PWM2_CTL);
>> + if (val & BLM_PWM_ENABLE) {
>> + DRM_DEBUG_KMS("Not allowing C8: CPU PWM2 enabled\n");
>> + return false;
>> + }
>> +
>> + val = I915_READ(BLC_PWM_PCH_CTL1);
>> + if (val & BLM_PCH_PWM_ENABLE) {
>> + DRM_DEBUG_KMS("Not allowing C8: PCH PWM1 enabled\n");
>> + return false;
>> + }
>> +
>> + val = I915_READ(UTIL_PIN_CTL);
>> + if (val & UTIL_PIN_ENABLE) {
>> + DRM_DEBUG_KMS("Not allowing C8: utility pin enabled\n");
>> + return false;
>> + }
>> +
>> + val = I915_READ(PCH_GTC_CTL);
>> + if (val & PCH_GTC_ENABLE) {
>> + DRM_DEBUG_KMS("Not allowing C8: PCH GTC enabled\n");
>> + return false;
>> + }
>> +
>> + DRM_DEBUG_KMS("Allowing package C8+\n");
>> + return true;
>> +}
>> +
>> +void hsw_package_c8_wakeup(struct drm_i915_private *dev_priv)
>> +{
>> + if (!dev_priv->allowing_package_c8)
>> + return;
>> +
>> + mutex_lock(&dev_priv->c8_lock);
>> +
>> + dev_priv->c8_wakeup_refcnt++;
>> +
>> + if (dev_priv->c8_wakeup_refcnt == 1) {
>> + DRM_DEBUG_KMS("Waking up from C8\n");
>> + hsw_disallow_package_c8(dev_priv);
>> + }
>> +
>> + mutex_unlock(&dev_priv->c8_lock);
>> +}
>> +
>> +void hsw_package_c8_sleep(struct drm_i915_private *dev_priv)
>> +{
>> + if (!dev_priv->allowing_package_c8)
>> + return;
>> +
>> + mutex_lock(&dev_priv->c8_lock);
>> +
>> + dev_priv->c8_wakeup_refcnt--;
>> +
>> + if (dev_priv->c8_wakeup_refcnt == 0) {
>> + DRM_DEBUG_KMS("Sleeping back to C8\n");
>> + hsw_allow_package_c8(dev_priv);
>> + }
>> +
>> + mutex_unlock(&dev_priv->c8_lock);
>> +}
>> +
>> +static void hsw_set_package_c8(struct drm_i915_private *dev_priv)
>> +{
>> + bool allow = hsw_can_allow_package_c8(dev_priv);
>> +
>> + mutex_lock(&dev_priv->c8_lock);
>> +
>> + if (allow & !dev_priv->allowing_package_c8) {
>> + hsw_allow_package_c8(dev_priv);
>> + dev_priv->allowing_package_c8 = true;
>> + dev_priv->c8_wakeup_refcnt = 0;
>> + } else if (!allow & dev_priv->allowing_package_c8) {
>> + hsw_disallow_package_c8(dev_priv);
>> + dev_priv->allowing_package_c8 = false;
>> + WARN_ON(dev_priv->c8_wakeup_refcnt);
>> + }
>> +
>> + mutex_unlock(&dev_priv->c8_lock);
>> +}
>> +
>> static void haswell_modeset_global_resources(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -5778,6 +6235,8 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
>> enable = true;
>>
>> intel_set_power_well(dev, enable);
>> +
>> + hsw_set_package_c8(dev_priv);
>> }
>>
>> static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 8845e82..033b45e 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -2318,9 +2318,12 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> struct drm_device *dev = connector->dev;
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> enum drm_connector_status status;
>> struct edid *edid = NULL;
>>
>> + hsw_package_c8_wakeup(dev_priv);
>> +
>> intel_dp->has_audio = false;
>>
>> if (HAS_PCH_SPLIT(dev))
>> @@ -2329,7 +2332,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>> status = g4x_dp_detect(intel_dp);
>>
>> if (status != connector_status_connected)
>> - return status;
>> + goto out;
>>
>> intel_dp_probe_oui(intel_dp);
>>
>> @@ -2345,7 +2348,11 @@ intel_dp_detect(struct drm_connector *connector, bool force)
>>
>> if (intel_encoder->type != INTEL_OUTPUT_EDP)
>> intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
>> - return connector_status_connected;
>> + status = connector_status_connected;
>> +
>> +out:
>> + hsw_package_c8_sleep(dev_priv);
>> + return status;
>> }
>>
>> static int intel_dp_get_modes(struct drm_connector *connector)
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 58ff2bf..2b5a352 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -698,6 +698,8 @@ extern void intel_update_fbc(struct drm_device *dev);
>> extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
>> extern void intel_gpu_ips_teardown(void);
>>
>> +extern void hsw_package_c8_wakeup(struct drm_i915_private *dev_priv);
>> +extern void hsw_package_c8_sleep(struct drm_i915_private *dev_priv);
>> extern bool intel_using_power_well(struct drm_device *dev);
>> extern void intel_init_power_well(struct drm_device *dev);
>> extern void intel_set_power_well(struct drm_device *dev, bool enable);
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index ee4a8da..6531158b 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -819,6 +819,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>> struct edid *edid;
>> enum drm_connector_status status = connector_status_disconnected;
>>
>> + hsw_package_c8_wakeup(dev_priv);
>> +
>> intel_hdmi->has_hdmi_sink = false;
>> intel_hdmi->has_audio = false;
>> intel_hdmi->rgb_quant_range_selectable = false;
>> @@ -846,6 +848,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>> intel_encoder->type = INTEL_OUTPUT_HDMI;
>> }
>>
>> + hsw_package_c8_sleep(dev_priv);
>> return status;
>> }
>>
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> 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
--
Paulo Zanoni
