On Tue, Mar 17, 2015 at 11:39:58AM +0200, Imre Deak wrote:
> From: "A.Sunil Kamath" <sunil.kamath@xxxxxxxxx>
>
> v2: Modified as per review comments from Imre
> - Mention enabling instead of allowing in the debug trace and
> remove unnecessary comments.
>
> v3:
> - Rebase to latest.
> - Move DC9-related functions from intel_display.c to intel_runtime_pm.c.
>
> v4: (imre)
> - remove DC5 disabling, it's a nop at this point
> - squashed in Suketu's "Assert the requirements to enter or exit DC9"
> patch
> - remove check for RUNTIME_PM from assert_can_enable_dc9, it's not a
> dependency
>
> Signed-off-by: A.Sunil Kamath <sunil.kamath@xxxxxxxxx> (v3)
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 5 +++
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> drivers/gpu/drm/i915/intel_runtime_pm.c | 66 +++++++++++++++++++++++++++++++++
> 3 files changed, 73 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 95532b4..4c781cb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6942,6 +6942,11 @@ enum bxt_phy {
> #define BXT_DE_PLL_PLL_ENABLE (1 << 31)
> #define BXT_DE_PLL_LOCK (1 << 30)
>
> +/* GEN9 DC */
> +#define DC_STATE_EN 0x45504
> +#define DC_STATE_EN_UPTO_DC5 (1<<0)
> +#define DC_STATE_EN_DC9 (1<<3)
> +
> /* Please see hsw_read_dcomp() and hsw_write_dcomp() before using this register,
> * since on HSW we can't write to it using I915_WRITE. */
> #define D_COMP_HSW (MCHBAR_MIRROR_BASE_SNB + 0x5F0C)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4bc2041..262314b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1024,6 +1024,8 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv);
> void bxt_init_cdclk(struct drm_device *dev);
> void bxt_uninit_cdclk(struct drm_device *dev);
> void bxt_ddi_phy_init(struct drm_device *dev);
> +void bxt_enable_dc9(struct drm_i915_private *dev_priv);
> +void bxt_disable_dc9(struct drm_i915_private *dev_priv);
> void intel_dp_get_m_n(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> void intel_dp_set_m_n(struct intel_crtc *crtc, enum link_m_n_set m_n);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index ff5cce3..8fe2fde 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -351,6 +351,72 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS)) | \
> BIT(POWER_DOMAIN_INIT))
>
> +static void assert_can_enable_dc9(struct drm_i915_private *dev_priv)
> +{
> + struct drm_device *dev = dev_priv->dev;
> +
> + WARN(!IS_BROXTON(dev), "Platform doesn't support DC9.\n");
> + WARN((I915_READ(DC_STATE_EN) & DC_STATE_EN_DC9),
> + "DC9 already programmed to be enabled.\n");
> + WARN(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5,
> + "DC5 still not disabled to enable DC9.\n");
> + WARN(I915_READ(HSW_PWR_WELL_DRIVER), "Power well on.\n");
> + WARN(intel_irqs_enabled(dev_priv), "Interrupts not disabled yet.\n");
> +
> + /*
> + * TODO: check for the following to verify the conditions to enter DC9
> + * state are satisfied:
> + * 1] Check relevant display engine registers to verify if mode set
> + * disable sequence was followed.
> + * 2] Check if display uninitialize sequence is initialized.
> + */
> +}
> +
> +static void assert_can_disable_dc9(struct drm_i915_private *dev_priv)
> +{
> + WARN(intel_irqs_enabled(dev_priv), "Interrupts not disabled yet.\n");
> + WARN(!(I915_READ(DC_STATE_EN) & DC_STATE_EN_DC9),
> + "DC9 already programmed to be disabled.\n");
> + WARN(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5,
> + "DC5 still not disabled.\n");
> +
> + /*
> + * TODO: check for the following to verify DC9 state was indeed
> + * entered before programming to disable it:
> + * 1] Check relevant display engine registers to verify if mode
> + * set disable sequence was followed.
> + * 2] Check if display uninitialize sequence is initialized.
> + */
> +}
> +
> +void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> +{
> + uint32_t val;
> +
> + assert_can_enable_dc9(dev_priv);
> +
> + DRM_DEBUG_KMS("Enabling DC9\n");
> +
> + val = I915_READ(DC_STATE_EN);
> + val |= DC_STATE_EN_DC9;
> + I915_WRITE(DC_STATE_EN, val);
> + POSTING_READ(DC_STATE_EN);
> +}
> +
> +void bxt_disable_dc9(struct drm_i915_private *dev_priv)
> +{
> + uint32_t val;
> +
> + assert_can_disable_dc9(dev_priv);
> +
> + DRM_DEBUG_KMS("Disabling DC9\n");
> +
> + val = I915_READ(DC_STATE_EN);
> + val &= ~DC_STATE_EN_DC9;
> + I915_WRITE(DC_STATE_EN, val);
> + POSTING_READ(DC_STATE_EN);
> +}
Standard comment about patch splitting: Please don't add functions or
structures (or new member fields and other variables) without using them
in the same patch: That way the patch can't be understood fully when just
linearly reading through patches and hence somewhat defeats the purpose of
patch splitting.
Instead I recommend to first wire up dummy functions to sketch the larger
picture and then in the next patch fill in details.
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
