| 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(-)