[Intel-gfx] [PATCH] drm/i915: Yet another if/else sort of newer to older platforms.
Lucas De Marchi
lucas.demarchi at intel.com
Thu Feb 28 23:19:39 UTC 2019
On Mon, Feb 25, 2019 at 11:11:30AM -0800, Rodrigo Vivi wrote:
>No functional change. Just a reorg to match the preferred
>behavior.
>
>When rebasing internal branch on top of latest sort I noticed
>few more cases that needs to get reordered.
>
>Let's do in a bundle this time and hoping there's no other
>missing places.
>
>v2: Check for HSW/BDW ULT before generic IS_HASWELL or
> IS_BROADWELL or it doesn't work as pointed by Ville.
> But also ULT came afterwards anyway.
>
>Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>Cc: Chris Wilson <chris at chris-wilson.co.uk>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>---
> drivers/gpu/drm/i915/i915_drv.c | 20 ++++----
> drivers/gpu/drm/i915/i915_perf.c | 50 +++++++++---------
> drivers/gpu/drm/i915/intel_cdclk.c | 38 +++++++-------
> drivers/gpu/drm/i915/intel_workarounds.c | 64 ++++++++++++------------
> 4 files changed, 86 insertions(+), 86 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>index c6354f6cdbdb..ed48aac1487d 100644
>--- a/drivers/gpu/drm/i915/i915_drv.c
>+++ b/drivers/gpu/drm/i915/i915_drv.c
>@@ -219,20 +219,20 @@ intel_virt_detect_pch(const struct drm_i915_private *dev_priv)
> * make an educated guess as to which PCH is really there.
> */
>
>- if (IS_GEN(dev_priv, 5))
>- id = INTEL_PCH_IBX_DEVICE_ID_TYPE;
>- else if (IS_GEN(dev_priv, 6) || IS_IVYBRIDGE(dev_priv))
>- id = INTEL_PCH_CPT_DEVICE_ID_TYPE;
>+ if (IS_ICELAKE(dev_priv))
>+ id = INTEL_PCH_ICP_DEVICE_ID_TYPE;
>+ else if (IS_COFFEELAKE(dev_priv) || IS_CANNONLAKE(dev_priv))
if you want to be strict about the order, then this should be:
else if (IS_CANNONLAKE(dev_priv) || IS_COFFELAKE(dev_priv))
>+ id = INTEL_PCH_CNP_DEVICE_ID_TYPE;
>+ else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
ditto
>+ id = INTEL_PCH_SPT_DEVICE_ID_TYPE;
> else if (IS_HSW_ULT(dev_priv) || IS_BDW_ULT(dev_priv))
> id = INTEL_PCH_LPT_LP_DEVICE_ID_TYPE;
> else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> id = INTEL_PCH_LPT_DEVICE_ID_TYPE;
>- else if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
>- id = INTEL_PCH_SPT_DEVICE_ID_TYPE;
>- else if (IS_COFFEELAKE(dev_priv) || IS_CANNONLAKE(dev_priv))
>- id = INTEL_PCH_CNP_DEVICE_ID_TYPE;
>- else if (IS_ICELAKE(dev_priv))
>- id = INTEL_PCH_ICP_DEVICE_ID_TYPE;
>+ else if (IS_GEN(dev_priv, 6) || IS_IVYBRIDGE(dev_priv))
>+ id = INTEL_PCH_CPT_DEVICE_ID_TYPE;
>+ else if (IS_GEN(dev_priv, 5))
>+ id = INTEL_PCH_IBX_DEVICE_ID_TYPE;
>
> if (id)
> DRM_DEBUG_KMS("Assuming PCH ID %04x\n", id);
>diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>index 9ebf99f3d8d3..72a9a35b40e2 100644
>--- a/drivers/gpu/drm/i915/i915_perf.c
>+++ b/drivers/gpu/drm/i915/i915_perf.c
>@@ -2881,12 +2881,24 @@ void i915_perf_register(struct drm_i915_private *dev_priv)
>
> sysfs_attr_init(&dev_priv->perf.oa.test_config.sysfs_metric_id.attr);
>
>- if (IS_HASWELL(dev_priv)) {
>- i915_perf_load_test_config_hsw(dev_priv);
>- } else if (IS_BROADWELL(dev_priv)) {
>- i915_perf_load_test_config_bdw(dev_priv);
>- } else if (IS_CHERRYVIEW(dev_priv)) {
>- i915_perf_load_test_config_chv(dev_priv);
>+ if (IS_ICELAKE(dev_priv)) {
>+ i915_perf_load_test_config_icl(dev_priv);
>+ } else if (IS_CANNONLAKE(dev_priv)) {
>+ i915_perf_load_test_config_cnl(dev_priv);
>+ } else if (IS_COFFEELAKE(dev_priv)) {
>+ if (IS_CFL_GT2(dev_priv))
>+ i915_perf_load_test_config_cflgt2(dev_priv);
>+ if (IS_CFL_GT3(dev_priv))
>+ i915_perf_load_test_config_cflgt3(dev_priv);
>+ } else if (IS_GEMINILAKE(dev_priv)) {
>+ i915_perf_load_test_config_glk(dev_priv);
>+ } else if (IS_KABYLAKE(dev_priv)) {
>+ if (IS_KBL_GT2(dev_priv))
>+ i915_perf_load_test_config_kblgt2(dev_priv);
>+ else if (IS_KBL_GT3(dev_priv))
>+ i915_perf_load_test_config_kblgt3(dev_priv);
>+ } else if (IS_BROXTON(dev_priv)) {
>+ i915_perf_load_test_config_bxt(dev_priv);
> } else if (IS_SKYLAKE(dev_priv)) {
> if (IS_SKL_GT2(dev_priv))
> i915_perf_load_test_config_sklgt2(dev_priv);
>@@ -2894,25 +2906,13 @@ void i915_perf_register(struct drm_i915_private *dev_priv)
> i915_perf_load_test_config_sklgt3(dev_priv);
> else if (IS_SKL_GT4(dev_priv))
> i915_perf_load_test_config_sklgt4(dev_priv);
>- } else if (IS_BROXTON(dev_priv)) {
>- i915_perf_load_test_config_bxt(dev_priv);
>- } else if (IS_KABYLAKE(dev_priv)) {
>- if (IS_KBL_GT2(dev_priv))
>- i915_perf_load_test_config_kblgt2(dev_priv);
>- else if (IS_KBL_GT3(dev_priv))
>- i915_perf_load_test_config_kblgt3(dev_priv);
>- } else if (IS_GEMINILAKE(dev_priv)) {
>- i915_perf_load_test_config_glk(dev_priv);
>- } else if (IS_COFFEELAKE(dev_priv)) {
>- if (IS_CFL_GT2(dev_priv))
>- i915_perf_load_test_config_cflgt2(dev_priv);
>- if (IS_CFL_GT3(dev_priv))
>- i915_perf_load_test_config_cflgt3(dev_priv);
>- } else if (IS_CANNONLAKE(dev_priv)) {
>- i915_perf_load_test_config_cnl(dev_priv);
>- } else if (IS_ICELAKE(dev_priv)) {
>- i915_perf_load_test_config_icl(dev_priv);
>- }
>+ } else if (IS_CHERRYVIEW(dev_priv)) {
>+ i915_perf_load_test_config_chv(dev_priv);
>+ } else if (IS_BROADWELL(dev_priv)) {
>+ i915_perf_load_test_config_bdw(dev_priv);
>+ } else if (IS_HASWELL(dev_priv)) {
>+ i915_perf_load_test_config_hsw(dev_priv);
>+}
>
> if (dev_priv->perf.oa.test_config.id == 0)
> goto sysfs_error;
>diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
>index 26e01a8465af..5d266538036d 100644
>--- a/drivers/gpu/drm/i915/intel_cdclk.c
>+++ b/drivers/gpu/drm/i915/intel_cdclk.c
>@@ -2744,18 +2744,13 @@ void intel_update_rawclk(struct drm_i915_private *dev_priv)
> */
> void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> {
>- if (IS_CHERRYVIEW(dev_priv)) {
>- dev_priv->display.set_cdclk = chv_set_cdclk;
>- dev_priv->display.modeset_calc_cdclk =
>- vlv_modeset_calc_cdclk;
>- } else if (IS_VALLEYVIEW(dev_priv)) {
>- dev_priv->display.set_cdclk = vlv_set_cdclk;
>- dev_priv->display.modeset_calc_cdclk =
>- vlv_modeset_calc_cdclk;
>- } else if (IS_BROADWELL(dev_priv)) {
>- dev_priv->display.set_cdclk = bdw_set_cdclk;
>+ if (IS_ICELAKE(dev_priv)) {
>+ dev_priv->display.set_cdclk = icl_set_cdclk;
>+ dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
>+ } else if (IS_CANNONLAKE(dev_priv)) {
>+ dev_priv->display.set_cdclk = cnl_set_cdclk;
> dev_priv->display.modeset_calc_cdclk =
>- bdw_modeset_calc_cdclk;
>+ cnl_modeset_calc_cdclk;
> } else if (IS_GEN9_LP(dev_priv)) {
> dev_priv->display.set_cdclk = bxt_set_cdclk;
> dev_priv->display.modeset_calc_cdclk =
>@@ -2764,23 +2759,28 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> dev_priv->display.set_cdclk = skl_set_cdclk;
> dev_priv->display.modeset_calc_cdclk =
> skl_modeset_calc_cdclk;
>- } else if (IS_CANNONLAKE(dev_priv)) {
>- dev_priv->display.set_cdclk = cnl_set_cdclk;
>+ } else if (IS_BROADWELL(dev_priv)) {
>+ dev_priv->display.set_cdclk = bdw_set_cdclk;
> dev_priv->display.modeset_calc_cdclk =
>- cnl_modeset_calc_cdclk;
>- } else if (IS_ICELAKE(dev_priv)) {
>- dev_priv->display.set_cdclk = icl_set_cdclk;
>- dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
>+ bdw_modeset_calc_cdclk;
>+ } else if (IS_CHERRYVIEW(dev_priv)) {
>+ dev_priv->display.set_cdclk = chv_set_cdclk;
>+ dev_priv->display.modeset_calc_cdclk =
>+ vlv_modeset_calc_cdclk;
>+ } else if (IS_VALLEYVIEW(dev_priv)) {
>+ dev_priv->display.set_cdclk = vlv_set_cdclk;
>+ dev_priv->display.modeset_calc_cdclk =
>+ vlv_modeset_calc_cdclk;
> }
>
> if (IS_ICELAKE(dev_priv))
> dev_priv->display.get_cdclk = icl_get_cdclk;
> else if (IS_CANNONLAKE(dev_priv))
> dev_priv->display.get_cdclk = cnl_get_cdclk;
>- else if (IS_GEN9_BC(dev_priv))
>- dev_priv->display.get_cdclk = skl_get_cdclk;
> else if (IS_GEN9_LP(dev_priv))
> dev_priv->display.get_cdclk = bxt_get_cdclk;
>+ else if (IS_GEN9_BC(dev_priv))
>+ dev_priv->display.get_cdclk = skl_get_cdclk;
no reason to change these...
> else if (IS_BROADWELL(dev_priv))
> dev_priv->display.get_cdclk = bdw_get_cdclk;
> else if (IS_HASWELL(dev_priv))
>diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
>index 743cf5b00155..3e8138b66191 100644
>--- a/drivers/gpu/drm/i915/intel_workarounds.c
>+++ b/drivers/gpu/drm/i915/intel_workarounds.c
>@@ -862,26 +862,26 @@ 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 (INTEL_GEN(i915) < 8)
>+ if (IS_ICELAKE(i915))
>+ icl_gt_workarounds_init(i915, wal);
>+ else if (IS_CANNONLAKE(i915))
>+ cnl_gt_workarounds_init(i915, wal);
>+ else if (IS_COFFEELAKE(i915))
>+ cfl_gt_workarounds_init(i915, wal);
>+ else if (IS_GEMINILAKE(i915))
>+ glk_gt_workarounds_init(i915, wal);
>+ else if (IS_KABYLAKE(i915))
>+ kbl_gt_workarounds_init(i915, wal);
>+ else if (IS_BROXTON(i915))
>+ bxt_gt_workarounds_init(i915, wal);
>+ else if (IS_SKYLAKE(i915))
>+ skl_gt_workarounds_init(i915, wal);
>+ else if (IS_CHERRYVIEW(i915))
> return;
> else if (IS_BROADWELL(i915))
> return;
>- else if (IS_CHERRYVIEW(i915))
>+ else if (INTEL_GEN(i915) < 8)
> return;
>- else if (IS_SKYLAKE(i915))
>- skl_gt_workarounds_init(i915, wal);
>- else if (IS_BROXTON(i915))
>- bxt_gt_workarounds_init(i915, wal);
>- else if (IS_KABYLAKE(i915))
>- kbl_gt_workarounds_init(i915, wal);
>- else if (IS_GEMINILAKE(i915))
>- glk_gt_workarounds_init(i915, wal);
>- else if (IS_COFFEELAKE(i915))
>- cfl_gt_workarounds_init(i915, wal);
>- else if (IS_CANNONLAKE(i915))
>- cnl_gt_workarounds_init(i915, wal);
>- else if (IS_ICELAKE(i915))
>- icl_gt_workarounds_init(i915, wal);
> else
> MISSING_CASE(INTEL_GEN(i915));
> }
>@@ -1063,26 +1063,26 @@ void intel_engine_init_whitelist(struct intel_engine_cs *engine)
>
> wa_init_start(w, "whitelist");
>
>- if (INTEL_GEN(i915) < 8)
>+ if (IS_ICELAKE(i915))
>+ icl_whitelist_build(w);
>+ else if (IS_CANNONLAKE(i915))
>+ cnl_whitelist_build(w);
>+ else if (IS_COFFEELAKE(i915))
>+ cfl_whitelist_build(w);
>+ else if (IS_GEMINILAKE(i915))
>+ glk_whitelist_build(w);
>+ else if (IS_KABYLAKE(i915))
>+ kbl_whitelist_build(w);
>+ else if (IS_BROXTON(i915))
>+ bxt_whitelist_build(w);
>+ else if (IS_SKYLAKE(i915))
>+ skl_whitelist_build(w);
>+ else if (IS_CHERRYVIEW(i915))
> return;
> else if (IS_BROADWELL(i915))
> return;
>- else if (IS_CHERRYVIEW(i915))
>+ else if (INTEL_GEN(i915) < 8)
make this INTEL_GEN(i915) <= 8) and remove IS_CHERRYVIEW() above?
The meaning would be that "BROADWELL is the only gen8 that has this, the
others and below => do nothing."
I went through each of the if/else chains and did my best to manually
verify them: I didn't find any bug. Comments above are basically
nitpicks. Your call if you are actually going to change them.
Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
Lucas De Marchi
> return;
>- else if (IS_SKYLAKE(i915))
>- skl_whitelist_build(w);
>- else if (IS_BROXTON(i915))
>- bxt_whitelist_build(w);
>- else if (IS_KABYLAKE(i915))
>- kbl_whitelist_build(w);
>- else if (IS_GEMINILAKE(i915))
>- glk_whitelist_build(w);
>- else if (IS_COFFEELAKE(i915))
>- cfl_whitelist_build(w);
>- else if (IS_CANNONLAKE(i915))
>- cnl_whitelist_build(w);
>- else if (IS_ICELAKE(i915))
>- icl_whitelist_build(w);
> else
> MISSING_CASE(INTEL_GEN(i915));
>
>--
>2.20.1
>
More information about the Intel-gfx
mailing list