On Thu, Mar 14, 2019 at 2:11 PM Souza, Jose <jose.souza@xxxxxxxxx> wrote:
>
> Maybe rename the commit message to "drm/i915/ehl: Inherit ICELAKE
> conditional code"?
>
> Also there a few important places that this patch and this series is
> missing:
>
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index 0d743907e7bc..52ff142628d7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -190,7 +190,7 @@ intel_pch_type(const struct drm_i915_private
> *dev_priv, unsigned short id)
> return PCH_CNP;
> case INTEL_PCH_ICP_DEVICE_ID_TYPE:
> DRM_DEBUG_KMS("Found Ice Lake PCH\n");
> - WARN_ON(!IS_ICELAKE(dev_priv));
> + WARN_ON(!IS_GEN(dev_priv, 11));
EHL doesn't use ICP. Support for its PCH will come soon.
> return PCH_ICP;
> default:
> return PCH_NONE;
> @@ -219,7 +219,7 @@ intel_virt_detect_pch(const struct drm_i915_private
> *dev_priv)
> * make an educated guess as to which PCH is really there.
> */
>
> - if (IS_ICELAKE(dev_priv))
> + if (IS_GEN(dev_priv, 11))
ditto
> id = INTEL_PCH_ICP_DEVICE_ID_TYPE;
> else if (IS_CANNONLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
> id = INTEL_PCH_CNP_DEVICE_ID_TYPE;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> b/drivers/gpu/drm/i915/intel_opregion.c
> index 5437effcb396..94511fc8b10f 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -973,9 +973,10 @@ int intel_opregion_setup(struct drm_i915_private
> *dev_priv)
> * opregion 2.1+: rvda is unsigned, relative offset
> from
> * opregion base, and should never point within
> opregion.
> */
> + /* Just not sure about this one */
> if (opregion->header->over.major > 2 ||
> opregion->header->over.minor >= 1 ||
> - IS_ICELAKE(dev_priv)) {
> + IS_GEN(dev_priv, 11)) {
> WARN_ON(rvda < OPREGION_SIZE);
doesn't match code
>
> rvda += asls;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 61c581e33fe3..f6fe44705b4e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4473,10 +4473,10 @@ skl_allocate_pipe_ddb(struct intel_crtc_state
> *cstate,
> memset(&wm->wm[level], 0, sizeof(wm-
> >wm[level]));
>
> /*
> - * Wa_1408961008:icl
> + * Wa_1408961008:icl,ehl
> * Underruns with WM1+ disabled
> */
> - if (IS_ICELAKE(dev_priv) &&
> + if (IS_GEN(dev_priv, 11) &&
yes, this is missing. spec does say this also applied here
> level == 1 && wm->wm[0].plane_en) {
> wm->wm[level].plane_res_b = wm-
> >wm[0].plane_res_b;
> wm->wm[level].plane_res_l = wm-
> >wm[0].plane_res_l;
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 1f063c761f35..fdabb15a9672 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -703,8 +703,8 @@ icl_combo_phy_aux_power_well_enable(struct
> drm_i915_private *dev_priv,
>
> hsw_wait_for_power_well_enable(dev_priv, power_well);
>
> - /* Display WA #1178: icl */
> - if (IS_ICELAKE(dev_priv) &&
> + /* Display WA #1178: icl,ehl */
> + if (IS_GEN(dev_priv, 11) &&
I'm not sure this applies to EHL.
> pw_idx >= ICL_PW_CTL_IDX_AUX_A && pw_idx <=
> ICL_PW_CTL_IDX_AUX_B &&
> !intel_bios_is_port_edp(dev_priv, port)) {
> val = I915_READ(ICL_AUX_ANAOVRD1(pw_idx));
> @@ -4055,7 +4055,7 @@ void intel_power_domains_init_hw(struct
> drm_i915_private *i915, bool resume)
>
> power_domains->initializing = true;
>
> - if (IS_ICELAKE(i915)) {
> + if (IS_GEN(i915, 11)) {
this should be INTEL_GEN(i915) >= 11, to match the current trend of
assuming it's the same for next platform.
/me removes his previous r-b.
Lucas De Marchi
> icl_display_core_init(i915, resume);
> } else if (IS_CANNONLAKE(i915)) {
> cnl_display_core_init(i915, resume);
>
>
>
>
>
> On Wed, 2019-03-13 at 14:11 -0700, Rodrigo Vivi wrote:
> > From: Bob Paauwe <bob.j.paauwe@xxxxxxxxx>
> >
> > Most of the conditional code for ICELAKE also applies to ELKHARTLAKE
> > so use IS_GEN(dev_priv, 11) even for PM and Workarounds for now.
> >
> > Signed-off-by: Bob Paauwe <bob.j.paauwe@xxxxxxxxx>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 2 +-
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++--
> > drivers/gpu/drm/i915/intel_workarounds.c | 8 ++++----
> > 3 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index d73b13ca57a0..61c581e33fe3 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -9468,7 +9468,7 @@ static void nop_init_clock_gating(struct
> > drm_i915_private *dev_priv)
> > */
> > void intel_init_clock_gating_hooks(struct drm_i915_private
> > *dev_priv)
> > {
> > - if (IS_ICELAKE(dev_priv))
> > + if (IS_GEN(dev_priv, 11))
> > dev_priv->display.init_clock_gating =
> > icl_init_clock_gating;
> > else if (IS_CANNONLAKE(dev_priv))
> > dev_priv->display.init_clock_gating =
> > cnl_init_clock_gating;
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 676a89bb8194..1f063c761f35 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3442,7 +3442,7 @@ int intel_power_domains_init(struct
> > drm_i915_private *dev_priv)
> > * The enabling order will be from lower to higher indexed
> > wells,
> > * the disabling order is reversed.
> > */
> > - if (IS_ICELAKE(dev_priv)) {
> > + if (IS_GEN(dev_priv, 11)) {
> > err = set_power_wells(power_domains, icl_power_wells);
> > } else if (IS_CANNONLAKE(dev_priv)) {
> > err = set_power_wells(power_domains, cnl_power_wells);
> > @@ -4203,7 +4203,7 @@ void intel_power_domains_suspend(struct
> > drm_i915_private *i915,
> > intel_power_domains_verify_state(i915);
> > }
> >
> > - if (IS_ICELAKE(i915))
> > + if (IS_GEN(i915, 11))
> > icl_display_core_uninit(i915);
> > else if (IS_CANNONLAKE(i915))
> > cnl_display_core_uninit(i915);
> > diff --git a/drivers/gpu/drm/i915/intel_workarounds.c
> > b/drivers/gpu/drm/i915/intel_workarounds.c
> > index 283e9a4ef3ca..2128cb6cf8c8 100644
> > --- a/drivers/gpu/drm/i915/intel_workarounds.c
> > +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> > @@ -569,7 +569,7 @@ void intel_engine_init_ctx_wa(struct
> > intel_engine_cs *engine)
> >
> > wa_init_start(wal, "context");
> >
> > - if (IS_ICELAKE(i915))
> > + if (IS_GEN(i915, 11))
> > icl_ctx_workarounds_init(engine);
> > else if (IS_CANNONLAKE(i915))
> > cnl_ctx_workarounds_init(engine);
> > @@ -867,7 +867,7 @@ icl_gt_workarounds_init(struct drm_i915_private
> > *i915, struct i915_wa_list *wal)
> > static void
> > gt_init_workarounds(struct drm_i915_private *i915, struct
> > i915_wa_list *wal)
> > {
> > - if (IS_ICELAKE(i915))
> > + if (IS_GEN(i915, 11))
> > icl_gt_workarounds_init(i915, wal);
> > else if (IS_CANNONLAKE(i915))
> > cnl_gt_workarounds_init(i915, wal);
> > @@ -1064,7 +1064,7 @@ void intel_engine_init_whitelist(struct
> > intel_engine_cs *engine)
> >
> > wa_init_start(w, "whitelist");
> >
> > - if (IS_ICELAKE(i915))
> > + if (IS_GEN(i915, 11))
> > icl_whitelist_build(w);
> > else if (IS_CANNONLAKE(i915))
> > cnl_whitelist_build(w);
> > @@ -1112,7 +1112,7 @@ rcs_engine_wa_init(struct intel_engine_cs
> > *engine, struct i915_wa_list *wal)
> > {
> > struct drm_i915_private *i915 = engine->i915;
> >
> > - if (IS_ICELAKE(i915)) {
> > + if (IS_GEN(i915, 11)) {
> > /* This is not an Wa. Enable for better image quality
> > */
> > wa_masked_en(wal,
> > _3D_CHICKEN3,
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
