Message ID | 20200401004120.408586-1-jose.souza@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/6] drm/i915/display: Move out code to return the digital_port of the aux ch | expand |
Hi "José, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip next-20200331] [cannot apply to v5.6] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Jos-Roberto-de-Souza/drm-i915-display-Move-out-code-to-return-the-digital_port-of-the-aux-ch/20200401-094021 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-defconfig (attached as .config) compiler: gcc-7 (Debian 7.4.0-6) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All error/warnings (new ones prefixed by >>): drivers/gpu/drm/i915/display/intel_display_power.c: In function 'icl_tc_phy_aux_power_well_enable': >> drivers/gpu/drm/i915/display/intel_display_power.c:561:40: error: implicit declaration of function 'aux_ch_to_digital_port'; did you mean 'enc_to_dig_port'? [-Werror=implicit-function-declaration] struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); ^~~~~~~~~~~~~~~~~~~~~~ enc_to_dig_port >> drivers/gpu/drm/i915/display/intel_display_power.c:561:40: warning: initialization makes pointer from integer without a cast [-Wint-conversion] >> drivers/gpu/drm/i915/display/intel_display_power.c:564:2: error: too many arguments to function 'icl_tc_port_assert_ref_held' icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_display_power.c:547:13: note: declared here static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_display_power.c: In function 'icl_tc_phy_aux_power_well_disable': drivers/gpu/drm/i915/display/intel_display_power.c:593:40: warning: initialization makes pointer from integer without a cast [-Wint-conversion] struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); ^~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_display_power.c:595:2: error: too many arguments to function 'icl_tc_port_assert_ref_held' icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_display_power.c:547:13: note: declared here static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, ^~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors vim +561 drivers/gpu/drm/i915/display/intel_display_power.c 555 556 static void 557 icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, 558 struct i915_power_well *power_well) 559 { 560 enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > 561 struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); 562 u32 val; 563 > 564 icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); 565 566 val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch)); 567 val &= ~DP_AUX_CH_CTL_TBT_IO; 568 if (power_well->desc->hsw.is_tc_tbt) 569 val |= DP_AUX_CH_CTL_TBT_IO; 570 intel_de_write(dev_priv, DP_AUX_CH_CTL(aux_ch), val); 571 572 hsw_power_well_enable(dev_priv, power_well); 573 574 if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) { 575 enum tc_port tc_port; 576 577 tc_port = TGL_AUX_PW_TO_TC_PORT(power_well->desc->hsw.idx); 578 intel_de_write(dev_priv, HIP_INDEX_REG(tc_port), 579 HIP_INDEX_VAL(tc_port, 0x2)); 580 581 if (intel_de_wait_for_set(dev_priv, DKL_CMN_UC_DW_27(tc_port), 582 DKL_CMN_UC_DW27_UC_HEALTH, 1)) 583 drm_warn(&dev_priv->drm, 584 "Timeout waiting TC uC health\n"); 585 } 586 } 587 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi "José, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on drm-tip/drm-tip next-20200331] [cannot apply to v5.6] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Jos-Roberto-de-Souza/drm-i915-display-Move-out-code-to-return-the-digital_port-of-the-aux-ch/20200401-094021 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-s0-20200401 (attached as .config) compiler: gcc-6 (Debian 6.3.0-18+deb9u1) 6.3.0 20170516 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/gpu/drm/i915/display/intel_display_power.c: In function 'icl_tc_phy_aux_power_well_enable': >> drivers/gpu/drm/i915/display/intel_display_power.c:561:40: error: implicit declaration of function 'aux_ch_to_digital_port' [-Werror=implicit-function-declaration] struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); ^~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_display_power.c:561:40: warning: initialization makes pointer from integer without a cast [-Wint-conversion] drivers/gpu/drm/i915/display/intel_display_power.c:564:2: error: too many arguments to function 'icl_tc_port_assert_ref_held' icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_display_power.c:547:13: note: declared here static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_display_power.c: In function 'icl_tc_phy_aux_power_well_disable': drivers/gpu/drm/i915/display/intel_display_power.c:593:40: warning: initialization makes pointer from integer without a cast [-Wint-conversion] struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); ^~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_display_power.c:595:2: error: too many arguments to function 'icl_tc_port_assert_ref_held' icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/i915/display/intel_display_power.c:547:13: note: declared here static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, ^~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: some warnings being treated as errors vim +/aux_ch_to_digital_port +561 drivers/gpu/drm/i915/display/intel_display_power.c 555 556 static void 557 icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, 558 struct i915_power_well *power_well) 559 { 560 enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > 561 struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); 562 u32 val; 563 564 icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); 565 566 val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch)); 567 val &= ~DP_AUX_CH_CTL_TBT_IO; 568 if (power_well->desc->hsw.is_tc_tbt) 569 val |= DP_AUX_CH_CTL_TBT_IO; 570 intel_de_write(dev_priv, DP_AUX_CH_CTL(aux_ch), val); 571 572 hsw_power_well_enable(dev_priv, power_well); 573 574 if (INTEL_GEN(dev_priv) >= 12 && !power_well->desc->hsw.is_tc_tbt) { 575 enum tc_port tc_port; 576 577 tc_port = TGL_AUX_PW_TO_TC_PORT(power_well->desc->hsw.idx); 578 intel_de_write(dev_priv, HIP_INDEX_REG(tc_port), 579 HIP_INDEX_VAL(tc_port, 0x2)); 580 581 if (intel_de_wait_for_set(dev_priv, DKL_CMN_UC_DW_27(tc_port), 582 DKL_CMN_UC_DW27_UC_HEALTH, 1)) 583 drm_warn(&dev_priv->drm, 584 "Timeout waiting TC uC health\n"); 585 } 586 } 587 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 2020-04-01 08:41, José Roberto de Souza wrote: > Moving the code to return the digital port of the aux channel also > removing the intel_phy_is_tc() to make it generic. > digital_port will be needed in icl_tc_phy_aux_power_well_enable() > so adding it as a parameter to icl_tc_port_assert_ref_held(). > > While at at removing the duplicated call to icl_tc_phy_aux_ch() in > icl_tc_port_assert_ref_held(). > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > .../drm/i915/display/intel_display_power.c | 38 ++++++++++--------- > 1 file changed, 21 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 433e5a81dd4d..02a07aa710e4 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -500,26 +500,14 @@ static int power_well_async_ref_count(struct drm_i915_private *dev_priv, > return refs; > } > > -static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, > - struct i915_power_well *power_well) > +static struct intel_digital_port * > +aux_ch_to_digital_port(struct drm_i915_private *dev_priv, > + enum aux_ch aux_ch) This fails the build because icl_tc_port_assert_ref_held was originally guarded by CONFIG_DRM_I915_DEBUG_RUNTIME_PM, but now aux_ch_to_digital_port maybe used outside the scope. > { > - enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > struct intel_digital_port *dig_port = NULL; > struct intel_encoder *encoder; > > - /* Bypass the check if all references are released asynchronously */ > - if (power_well_async_ref_count(dev_priv, power_well) == > - power_well->count) > - return; > - > - aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > - > for_each_intel_encoder(&dev_priv->drm, encoder) { > - enum phy phy = intel_port_to_phy(dev_priv, encoder->port); > - > - if (!intel_phy_is_tc(dev_priv, phy)) > - continue; > - > /* We'll check the MST primary port */ > if (encoder->type == INTEL_OUTPUT_DP_MST) > continue; > @@ -536,6 +524,18 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, > break; > } > > + return dig_port; > +} > + > +static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, > + struct i915_power_well *power_well, > + struct intel_digital_port *dig_port) > +{ > + /* Bypass the check if all references are released asynchronously */ > + if (power_well_async_ref_count(dev_priv, power_well) == > + power_well->count) > + return; > + > if (drm_WARN_ON(&dev_priv->drm, !dig_port)) > return; > > @@ -558,9 +558,10 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, > struct i915_power_well *power_well) > { > enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > + struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); E.g. here. > u32 val; > > - icl_tc_port_assert_ref_held(dev_priv, power_well); > + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); > > val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch)); > val &= ~DP_AUX_CH_CTL_TBT_IO; > @@ -588,7 +589,10 @@ static void > icl_tc_phy_aux_power_well_disable(struct drm_i915_private *dev_priv, > struct i915_power_well *power_well) > { > - icl_tc_port_assert_ref_held(dev_priv, power_well); > + enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > + struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); > + > + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); > > hsw_power_well_disable(dev_priv, power_well); > } > You-Sheng Yang
On Wed, 2020-04-01 at 15:18 +0800, You-Sheng Yang wrote: > On 2020-04-01 08:41, José Roberto de Souza wrote: > > Moving the code to return the digital port of the aux channel also > > removing the intel_phy_is_tc() to make it generic. > > digital_port will be needed in icl_tc_phy_aux_power_well_enable() > > so adding it as a parameter to icl_tc_port_assert_ref_held(). > > > > While at at removing the duplicated call to icl_tc_phy_aux_ch() in > > icl_tc_port_assert_ref_held(). > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > .../drm/i915/display/intel_display_power.c | 38 ++++++++++----- > > ---- > > 1 file changed, 21 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > index 433e5a81dd4d..02a07aa710e4 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > @@ -500,26 +500,14 @@ static int power_well_async_ref_count(struct > > drm_i915_private *dev_priv, > > return refs; > > } > > > > -static void icl_tc_port_assert_ref_held(struct drm_i915_private > > *dev_priv, > > - struct i915_power_well > > *power_well) > > +static struct intel_digital_port * > > +aux_ch_to_digital_port(struct drm_i915_private *dev_priv, > > + enum aux_ch aux_ch) > > This fails the build because icl_tc_port_assert_ref_held was > originally > guarded by CONFIG_DRM_I915_DEBUG_RUNTIME_PM, but now > aux_ch_to_digital_port maybe used outside the scope. Thanks, fixed. > > > { > > - enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > > struct intel_digital_port *dig_port = NULL; > > struct intel_encoder *encoder; > > > > - /* Bypass the check if all references are released > > asynchronously */ > > - if (power_well_async_ref_count(dev_priv, power_well) == > > - power_well->count) > > - return; > > - > > - aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > > - > > for_each_intel_encoder(&dev_priv->drm, encoder) { > > - enum phy phy = intel_port_to_phy(dev_priv, encoder- > > >port); > > - > > - if (!intel_phy_is_tc(dev_priv, phy)) > > - continue; > > - > > /* We'll check the MST primary port */ > > if (encoder->type == INTEL_OUTPUT_DP_MST) > > continue; > > @@ -536,6 +524,18 @@ static void icl_tc_port_assert_ref_held(struct > > drm_i915_private *dev_priv, > > break; > > } > > > > + return dig_port; > > +} > > + > > +static void icl_tc_port_assert_ref_held(struct drm_i915_private > > *dev_priv, > > + struct i915_power_well > > *power_well, > > + struct intel_digital_port > > *dig_port) > > +{ > > + /* Bypass the check if all references are released > > asynchronously */ > > + if (power_well_async_ref_count(dev_priv, power_well) == > > + power_well->count) > > + return; > > + > > if (drm_WARN_ON(&dev_priv->drm, !dig_port)) > > return; > > > > @@ -558,9 +558,10 @@ icl_tc_phy_aux_power_well_enable(struct > > drm_i915_private *dev_priv, > > struct i915_power_well *power_well) > > { > > enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > > + struct intel_digital_port *dig_port = > > aux_ch_to_digital_port(dev_priv, aux_ch); > > E.g. here. > > > u32 val; > > > > - icl_tc_port_assert_ref_held(dev_priv, power_well); > > + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); > > > > val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch)); > > val &= ~DP_AUX_CH_CTL_TBT_IO; > > @@ -588,7 +589,10 @@ static void > > icl_tc_phy_aux_power_well_disable(struct drm_i915_private > > *dev_priv, > > struct i915_power_well *power_well) > > { > > - icl_tc_port_assert_ref_held(dev_priv, power_well); > > + enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); > > + struct intel_digital_port *dig_port = > > aux_ch_to_digital_port(dev_priv, aux_ch); > > + > > + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); > > > > hsw_power_well_disable(dev_priv, power_well); > > } > > > > You-Sheng Yang >
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 433e5a81dd4d..02a07aa710e4 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -500,26 +500,14 @@ static int power_well_async_ref_count(struct drm_i915_private *dev_priv, return refs; } -static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, - struct i915_power_well *power_well) +static struct intel_digital_port * +aux_ch_to_digital_port(struct drm_i915_private *dev_priv, + enum aux_ch aux_ch) { - enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); struct intel_digital_port *dig_port = NULL; struct intel_encoder *encoder; - /* Bypass the check if all references are released asynchronously */ - if (power_well_async_ref_count(dev_priv, power_well) == - power_well->count) - return; - - aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); - for_each_intel_encoder(&dev_priv->drm, encoder) { - enum phy phy = intel_port_to_phy(dev_priv, encoder->port); - - if (!intel_phy_is_tc(dev_priv, phy)) - continue; - /* We'll check the MST primary port */ if (encoder->type == INTEL_OUTPUT_DP_MST) continue; @@ -536,6 +524,18 @@ static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, break; } + return dig_port; +} + +static void icl_tc_port_assert_ref_held(struct drm_i915_private *dev_priv, + struct i915_power_well *power_well, + struct intel_digital_port *dig_port) +{ + /* Bypass the check if all references are released asynchronously */ + if (power_well_async_ref_count(dev_priv, power_well) == + power_well->count) + return; + if (drm_WARN_ON(&dev_priv->drm, !dig_port)) return; @@ -558,9 +558,10 @@ icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); + struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); u32 val; - icl_tc_port_assert_ref_held(dev_priv, power_well); + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); val = intel_de_read(dev_priv, DP_AUX_CH_CTL(aux_ch)); val &= ~DP_AUX_CH_CTL_TBT_IO; @@ -588,7 +589,10 @@ static void icl_tc_phy_aux_power_well_disable(struct drm_i915_private *dev_priv, struct i915_power_well *power_well) { - icl_tc_port_assert_ref_held(dev_priv, power_well); + enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well); + struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch); + + icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port); hsw_power_well_disable(dev_priv, power_well); }
Moving the code to return the digital port of the aux channel also removing the intel_phy_is_tc() to make it generic. digital_port will be needed in icl_tc_phy_aux_power_well_enable() so adding it as a parameter to icl_tc_port_assert_ref_held(). While at at removing the duplicated call to icl_tc_phy_aux_ch() in icl_tc_port_assert_ref_held(). Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- .../drm/i915/display/intel_display_power.c | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-)