Re: [PATCH v3 1/6] drm/vc4: select PM (openrisc)
On Wed, Sep 22, 2021 at 10:41:56AM +0200, Maxime Ripard wrote:
> Hi Randy,
>
> On Sun, Sep 19, 2021 at 09:40:44AM -0700, Randy Dunlap wrote:
> > On 8/19/21 6:59 AM, Maxime Ripard wrote:
> > > We already depend on runtime PM to get the power domains and clocks for
> > > most of the devices supported by the vc4 driver, so let's just select it
> > > to make sure it's there, and remove the ifdef.
> > >
> > > Signed-off-by: Maxime Ripard
> > > ---
> > > drivers/gpu/drm/vc4/Kconfig| 1 +
> > > drivers/gpu/drm/vc4/vc4_hdmi.c | 2 --
> > > 2 files changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
> > > index 118e8a426b1a..f774ab340863 100644
> > > --- a/drivers/gpu/drm/vc4/Kconfig
> > > +++ b/drivers/gpu/drm/vc4/Kconfig
> > > @@ -9,6 +9,7 @@ config DRM_VC4
> > > select DRM_KMS_CMA_HELPER
> > > select DRM_GEM_CMA_HELPER
> > > select DRM_PANEL_BRIDGE
> > > + select PM
> > > select SND_PCM
> > > select SND_PCM_ELD
> > > select SND_SOC_GENERIC_DMAENGINE_PCM
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > index c2876731ee2d..602203b2d8e1 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > @@ -2107,7 +2107,6 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi
> > > *vc4_hdmi)
> > > return 0;
> > > }
> > > -#ifdef CONFIG_PM
> > > static int vc4_hdmi_runtime_suspend(struct device *dev)
> > > {
> > > struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
> > > @@ -2128,7 +2127,6 @@ static int vc4_hdmi_runtime_resume(struct device
> > > *dev)
> > > return 0;
> > > }
> > > -#endif
> > > static int vc4_hdmi_bind(struct device *dev, struct device *master,
> > > void *data)
> > > {
> > >
> >
> > Hi,
> >
> > FYI.
> >
> > This still causes a build error on arch/openrisc/ since it does not support
> > CONFIG_PM (it does not source "kernel/power/Kconfig" like some other arches
> > do):
> >
> > ./arch/riscv/Kconfig:source "kernel/power/Kconfig"
> > ./arch/x86/Kconfig:source "kernel/power/Kconfig"
> > ./arch/nds32/Kconfig:source "kernel/power/Kconfig"
> > ./arch/sh/Kconfig:source "kernel/power/Kconfig"
> > ./arch/arc/Kconfig:source "kernel/power/Kconfig"
> > ./arch/arm64/Kconfig:source "kernel/power/Kconfig"
> > ./arch/xtensa/Kconfig:source "kernel/power/Kconfig"
> > ./arch/sparc/Kconfig:source "kernel/power/Kconfig"
> > ./arch/arm/Kconfig:source "kernel/power/Kconfig"
> > ./arch/mips/Kconfig:source "kernel/power/Kconfig"
> > ./arch/powerpc/Kconfig:source "kernel/power/Kconfig"
> > ./arch/um/Kconfig:source "kernel/power/Kconfig"
> > ./arch/ia64/Kconfig:source "kernel/power/Kconfig"
> >
> > so with
> > CONFIG_DRM_VC4=y
> > # CONFIG_DRM_VC4_HDMI_CEC is not set
> >
> > I still see
> > ../drivers/gpu/drm/vc4/vc4_hdmi.c:2139:12: warning:
> > 'vc4_hdmi_runtime_suspend' defined but not used [-Wunused-function]
> > 2139 | static int vc4_hdmi_runtime_suspend(struct device *dev)
> > |^~~~
>
> With what version did you get that build error? -rc2 shouldn't have it
> anymore since the runtime_pm hooks introduction got reverted.
-next still contains these patches as Stephen effectively reverted the
changes in Linus' tree when merging in the drm-misc-fixes tree:
https://lore.kernel.org/r/20210920090729.19458...@canb.auug.org.au/
Cheers,
Nathan
Re: [Intel-gfx] [PATCH v3 03/13] drm/dp: add LTTPR DP 2.0 DPCD addresses
On Thu, Sep 09, 2021 at 03:51:55PM +0300, Jani Nikula wrote:
> DP 2.0 brings some new DPCD addresses for PHY repeaters.
>
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Manasi Navare
> Signed-off-by: Jani Nikula
> ---
> include/drm/drm_dp_helper.h | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 1d5b3dbb6e56..f3a61341011d 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -1319,6 +1319,10 @@ struct drm_panel;
> #define DP_MAX_LANE_COUNT_PHY_REPEATER 0xf0004 /*
> 1.4a */
> #define DP_Repeater_FEC_CAPABILITY 0xf0004 /* 1.4 */
> #define DP_PHY_REPEATER_EXTENDED_WAIT_TIMEOUT0xf0005 /*
> 1.4a */
> +#define DP_MAIN_LINK_CHANNEL_CODING_PHY_REPEATER 0xf0006 /* 2.0 */
> +# define DP_PHY_REPEATER_128B132B_SUPPORTED (1 << 0)
> +/* See DP_128B132B_SUPPORTED_LINK_RATES for values */
> +#define DP_PHY_REPEATER_128B132B_RATES 0xf0007 /*
> 2.0 */
>
> enum drm_dp_phy {
> DP_PHY_DPRX,
> --
> 2.30.2
>
>
This patch causes a build failure in -next when combined with the AMD
tree:
In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c:33:
In file included from ./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:70:
In file included from ./drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu_mode.h:36:
./include/drm/drm_dp_helper.h:1322:9: error:
'DP_MAIN_LINK_CHANNEL_CODING_PHY_REPEATER' macro redefined
[-Werror,-Wmacro-redefined]
#define DP_MAIN_LINK_CHANNEL_CODING_PHY_REPEATER0xf0006 /* 2.0 */
^
./drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dp_types.h:881:9: note: previous
definition is here
#define DP_MAIN_LINK_CHANNEL_CODING_PHY_REPEATER0xF0006
^
1 error generated.
Perhaps something like this should be applied during the merge of the
second tree or maybe this patch should be in a branch that could be
shared between the Intel and AMD trees so that this diff could be
applied to the AMD tree directly? Not sure what the standard procedure
for this is.
Cheers,
Nathan
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 234dfbea926a..279863b5c650 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -4590,7 +4590,7 @@ bool dp_retrieve_lttpr_cap(struct dc_link *link)
DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV];
link->dpcd_caps.lttpr_caps.supported_128b_132b_rates.raw =
- lttpr_dpcd_data[DP_PHY_REPEATER_128b_132b_RATES
-
+ lttpr_dpcd_data[DP_PHY_REPEATER_128B132B_RATES -
DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV];
#endif
diff --git a/drivers/gpu/drm/amd/display/dc/dc_dp_types.h
b/drivers/gpu/drm/amd/display/dc/dc_dp_types.h
index a5e798b5da79..8caf9af5ffa2 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_dp_types.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_dp_types.h
@@ -878,8 +878,6 @@ struct psr_caps {
# define DP_DSC_DECODER_COUNT_MASK (0b111 << 5)
# define DP_DSC_DECODER_COUNT_SHIFT5
#define DP_MAIN_LINK_CHANNEL_CODING_SET0x108
-#define DP_MAIN_LINK_CHANNEL_CODING_PHY_REPEATER 0xF0006
-#define DP_PHY_REPEATER_128b_132b_RATES0xF0007
#define DP_128b_132b_TRAINING_AUX_RD_INTERVAL_PHY_REPEATER10xF0022
#define DP_INTRA_HOP_AUX_REPLY_INDICATION (1 << 3)
/* TODO - Use DRM header to replace above once available */
Re: [PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized
On Tue, Sep 14, 2021 at 08:10:14PM +0300, Jani Nikula wrote:
> On Mon, 13 Sep 2021, Nathan Chancellor wrote:
> > On Tue, Aug 24, 2021 at 03:54:24PM -0700, Nathan Chancellor wrote:
> >> Commit 46e2068081e9 ("drm/i915: Disable some extra clang warnings")
> >> disabled -Wsometimes-uninitialized as noisy but there have been a few
> >> fixes to clang that make the false positive rate fairly low so it should
> >> be enabled to help catch obvious mistakes. The first two patches fix
> >> revent instances of this warning then enables it for i915 like the rest
> >> of the tree.
> >>
> >> Cheers,
> >> Nathan
> >>
> >> Nathan Chancellor (3):
> >> drm/i915/selftests: Do not use import_obj uninitialized
> >> drm/i915/selftests: Always initialize err in
> >> igt_dmabuf_import_same_driver_lmem()
> >> drm/i915: Enable -Wsometimes-uninitialized
> >>
> >> drivers/gpu/drm/i915/Makefile| 1 -
> >> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 7 ---
> >> 2 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >>
> >> base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
> >> --
> >> 2.33.0
> >>
> >>
> >
> > Ping, could this be picked up for an -rc as these are very clearly bugs?
>
> Thanks for the patches and review. Pushed to drm-intel-gt-next and
> cherry-picked to drm-intel-fixes, header to -rc2.
Thanks a lot!
Cheers,
Nathan
[PATCH v2] drm/i915: Clean up disabled warnings
i915 enables a wider set of warnings with '-Wall -Wextra' then disables
several with cc-disable-warning. If an unknown flag gets added to
KBUILD_CFLAGS when building with clang, all subsequent calls to
cc-{disable-warning,option} will fail, meaning that all of these
warnings do not get disabled [1].
A separate series will address the root cause of the issue by not adding
these flags when building with clang [2]; however, the symptom of these
extra warnings appearing can be addressed separately by just removing
the calls to cc-disable-warning, which makes the build ever so slightly
faster because the compiler does not need to be called as much before
building.
The following warnings are supported by GCC 4.9 and clang 10.0.1, which
are the minimum supported versions of these compilers so the call to
cc-disable-warning is not necessary. Masahiro cleaned this up for the
reset of the kernel in commit 4c8dd95a723d ("kbuild: add some extra
warning flags unconditionally").
* -Wmissing-field-initializers
* -Wsign-compare
* -Wtype-limits
* -Wunused-parameter
-Wunused-but-set-variable was implemented in clang 13.0.0 and
-Wframe-address was implemented in clang 12.0.0 so the
cc-disable-warning calls are kept for these two warnings.
Lastly, -Winitializer-overrides is clang's version of -Woverride-init,
which is disabled for the specific files that are problematic. clang
added a compatibility alias in clang 8.0.0 so -Winitializer-overrides
can be removed.
[1]: https://lore.kernel.org/r/202108210311.cbtcgoul-...@intel.com/
[2]: https://lore.kernel.org/r/20210824022640.2170859-1-nat...@kernel.org/
Reviewed-by: Nick Desaulniers
Signed-off-by: Nathan Chancellor
---
v1 -> v2: https://lore.kernel.org/r/20210824232237.2085342-1-nat...@kernel.org/
* Rebase on drm-intel-gt-next now that the prerequisite patch series has
been merged: https://lore.kernel.org/r/87wnnj13t5@intel.com/
* Add Nick's reviewed-by tag.
drivers/gpu/drm/i915/Makefile | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index c584188aa15a..fd99374583d5 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -13,13 +13,11 @@
# will most likely get a sudden build breakage... Hopefully we will fix
# new warnings before CI updates!
subdir-ccflags-y := -Wall -Wextra
-subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
-subdir-ccflags-y += $(call cc-disable-warning, type-limits)
-subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
+subdir-ccflags-y += -Wno-unused-parameter
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-sign-compare
subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
-# clang warnings
-subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
-subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
subdir-ccflags-y += $(call cc-disable-warning, frame-address)
subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
base-commit: 43192617f7816bb74584c1df06f57363afd15337
--
2.33.0
Re: [PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized
On Tue, Aug 24, 2021 at 03:54:24PM -0700, Nathan Chancellor wrote:
> Commit 46e2068081e9 ("drm/i915: Disable some extra clang warnings")
> disabled -Wsometimes-uninitialized as noisy but there have been a few
> fixes to clang that make the false positive rate fairly low so it should
> be enabled to help catch obvious mistakes. The first two patches fix
> revent instances of this warning then enables it for i915 like the rest
> of the tree.
>
> Cheers,
> Nathan
>
> Nathan Chancellor (3):
> drm/i915/selftests: Do not use import_obj uninitialized
> drm/i915/selftests: Always initialize err in
> igt_dmabuf_import_same_driver_lmem()
> drm/i915: Enable -Wsometimes-uninitialized
>
> drivers/gpu/drm/i915/Makefile| 1 -
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 7 ---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
>
> base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
> --
> 2.33.0
>
>
Ping, could this be picked up for an -rc as these are very clearly bugs?
Cheers,
Nathan
Re: [PATCH] drm/i915: Clean up disabled warnings
On 8/25/2021 4:03 PM, Nick Desaulniers wrote:
On Tue, Aug 24, 2021 at 4:23 PM Nathan Chancellor wrote:
i915 enables a wider set of warnings with '-Wall -Wextra' then disables
several with cc-disable-warning. If an unknown flag gets added to
KBUILD_CFLAGS when building with clang, all subsequent calls to
cc-{disable-warning,option} will fail, meaning that all of these
warnings do not get disabled [1].
A separate series will address the root cause of the issue by not adding
these flags when building with clang [2]; however, the symptom of these
extra warnings appearing can be addressed separately by just removing
the calls to cc-disable-warning, which makes the build ever so slightly
faster because the compiler does not need to be called as much before
building.
The following warnings are supported by GCC 4.9 and clang 10.0.1, which
are the minimum supported versions of these compilers so the call to
cc-disable-warning is not necessary. Masahiro cleaned this up for the
reset of the kernel in commit 4c8dd95a723d ("kbuild: add some extra
warning flags unconditionally").
* -Wmissing-field-initializers
* -Wsign-compare
* -Wtype-limits
* -Wunused-parameter
-Wunused-but-set-variable was implemented in clang 13.0.0 and
-Wframe-address was implemented in clang 12.0.0 so the
cc-disable-warning calls are kept for these two warnings.
Lastly, -Winitializer-overrides is clang's version of -Woverride-init,
which is disabled for the specific files that are problematic. clang
added a compatibility alias in clang 8.0.0 so -Winitializer-overrides
can be removed.
[1]: https://lore.kernel.org/r/202108210311.cbtcgoul-...@intel.com/
[2]: https://lore.kernel.org/r/20210824022640.2170859-1-nat...@kernel.org/
Signed-off-by: Nathan Chancellor
Thanks for the patch! Do you need to re-ping, rebase, or resend that
other series?
Reviewed-by: Nick Desaulniers
I assume you mean the series below rather than above? I sent this patch
right after that series and it has one set of reviews so I am hoping the
i915 maintainers will pick them up soon so this one can be applied
afterwards or resent.
Thank you for the review!
Cheers,
Nathan
---
NOTE: This is based on my series to enable -Wsometimes-initialized here:
https://lore.kernel.org/r/20210824225427.2065517-1-nat...@kernel.org/
I sent it separately as this can go into whatever release but I would
like for that series to go into 5.15.
drivers/gpu/drm/i915/Makefile | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 335ba9f43d8f..6b38547543b1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -13,13 +13,11 @@
# will most likely get a sudden build breakage... Hopefully we will fix
# new warnings before CI updates!
subdir-ccflags-y := -Wall -Wextra
-subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
-subdir-ccflags-y += $(call cc-disable-warning, type-limits)
-subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
+subdir-ccflags-y += -Wno-unused-parameter
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-sign-compare
subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
-# clang warnings
-subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
-subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
subdir-ccflags-y += $(call cc-disable-warning, frame-address)
subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
prerequisite-patch-id: 31c28450ed7e8785dce967a16db6d52eff3d7d6d
prerequisite-patch-id: 372dfa0e07249f207acc1942ab0e39b13ff229b2
prerequisite-patch-id: 1a585fa6cda50c32ad1e3ac8235d3cff1b599978
--
2.33.0
[PATCH] drm/i915: Clean up disabled warnings
i915 enables a wider set of warnings with '-Wall -Wextra' then disables
several with cc-disable-warning. If an unknown flag gets added to
KBUILD_CFLAGS when building with clang, all subsequent calls to
cc-{disable-warning,option} will fail, meaning that all of these
warnings do not get disabled [1].
A separate series will address the root cause of the issue by not adding
these flags when building with clang [2]; however, the symptom of these
extra warnings appearing can be addressed separately by just removing
the calls to cc-disable-warning, which makes the build ever so slightly
faster because the compiler does not need to be called as much before
building.
The following warnings are supported by GCC 4.9 and clang 10.0.1, which
are the minimum supported versions of these compilers so the call to
cc-disable-warning is not necessary. Masahiro cleaned this up for the
reset of the kernel in commit 4c8dd95a723d ("kbuild: add some extra
warning flags unconditionally").
* -Wmissing-field-initializers
* -Wsign-compare
* -Wtype-limits
* -Wunused-parameter
-Wunused-but-set-variable was implemented in clang 13.0.0 and
-Wframe-address was implemented in clang 12.0.0 so the
cc-disable-warning calls are kept for these two warnings.
Lastly, -Winitializer-overrides is clang's version of -Woverride-init,
which is disabled for the specific files that are problematic. clang
added a compatibility alias in clang 8.0.0 so -Winitializer-overrides
can be removed.
[1]: https://lore.kernel.org/r/202108210311.cbtcgoul-...@intel.com/
[2]: https://lore.kernel.org/r/20210824022640.2170859-1-nat...@kernel.org/
Signed-off-by: Nathan Chancellor
---
NOTE: This is based on my series to enable -Wsometimes-initialized here:
https://lore.kernel.org/r/20210824225427.2065517-1-nat...@kernel.org/
I sent it separately as this can go into whatever release but I would
like for that series to go into 5.15.
drivers/gpu/drm/i915/Makefile | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 335ba9f43d8f..6b38547543b1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -13,13 +13,11 @@
# will most likely get a sudden build breakage... Hopefully we will fix
# new warnings before CI updates!
subdir-ccflags-y := -Wall -Wextra
-subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
-subdir-ccflags-y += $(call cc-disable-warning, type-limits)
-subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
+subdir-ccflags-y += -Wno-unused-parameter
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-sign-compare
subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
-# clang warnings
-subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
-subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
subdir-ccflags-y += $(call cc-disable-warning, frame-address)
subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
prerequisite-patch-id: 31c28450ed7e8785dce967a16db6d52eff3d7d6d
prerequisite-patch-id: 372dfa0e07249f207acc1942ab0e39b13ff229b2
prerequisite-patch-id: 1a585fa6cda50c32ad1e3ac8235d3cff1b599978
--
2.33.0
[PATCH 3/3] drm/i915: Enable -Wsometimes-uninitialized
This warning helps catch uninitialized variables. It should have been
enabled at the same time as commit b2423184ac33 ("drm/i915: Enable
-Wuninitialized") but I did not realize they were disabled separately.
Enable it now that i915 is clean so that it stays that way.
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 642a5b5a1b81..335ba9f43d8f 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -19,7 +19,6 @@ subdir-ccflags-y += $(call cc-disable-warning,
missing-field-initializers)
subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
# clang warnings
subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
-subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
subdir-ccflags-y += $(call cc-disable-warning, frame-address)
subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
--
2.33.0
[PATCH 2/3] drm/i915/selftests: Always initialize err in igt_dmabuf_import_same_driver_lmem()
Clang warns:
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:127:13: warning:
variable 'err' is used uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]
} else if (PTR_ERR(import) != -EOPNOTSUPP) {
^~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:138:9: note:
uninitialized use occurs here
return err;
^~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:127:9: note: remove
the 'if' if its condition is always true
} else if (PTR_ERR(import) != -EOPNOTSUPP) {
^~~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:95:9: note:
initialize the variable 'err' to silence this warning
int err;
^
= 0
The test is expected to pass if i915_gem_prime_import() returns
-EOPNOTSUPP so initialize err to zero in this case.
Fixes: cdb35d1ed6d2 ("drm/i915/gem: Migrate to system at dma-buf attach time
(v7)")
Reported-by: Dan Carpenter
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index 532c7955b300..4a6bb64c3a35 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -128,6 +128,8 @@ static int igt_dmabuf_import_same_driver_lmem(void *arg)
pr_err("i915_gem_prime_import failed with the wrong err=%ld\n",
PTR_ERR(import));
err = PTR_ERR(import);
+ } else {
+ err = 0;
}
dma_buf_put(dmabuf);
--
2.33.0
[PATCH 1/3] drm/i915/selftests: Do not use import_obj uninitialized
Clang warns a couple of times:
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:63:6: warning:
variable 'import_obj' is used uninitialized whenever 'if' condition is
true [-Wsometimes-uninitialized]
if (import != >base) {
^~~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:80:22: note:
uninitialized use occurs here
i915_gem_object_put(import_obj);
^~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:63:2: note: remove
the 'if' if its condition is always false
if (import != >base) {
^~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:38:46: note:
initialize the variable 'import_obj' to silence this warning
struct drm_i915_gem_object *obj, *import_obj;
^
= NULL
Shuffle the import_obj initialization above these if statements so that
it is not used uninitialized.
Fixes: d7b2cb380b3a ("drm/i915/gem: Correct the locking and pin pattern for
dma-buf (v8)")
Reported-by: Dan Carpenter
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index ffae7df5e4d7..532c7955b300 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -59,13 +59,13 @@ static int igt_dmabuf_import_self(void *arg)
err = PTR_ERR(import);
goto out_dmabuf;
}
+ import_obj = to_intel_bo(import);
if (import != >base) {
pr_err("i915_gem_prime_import created a new object!\n");
err = -EINVAL;
goto out_import;
}
- import_obj = to_intel_bo(import);
i915_gem_object_lock(import_obj, NULL);
err = __i915_gem_object_get_pages(import_obj);
@@ -176,6 +176,7 @@ static int igt_dmabuf_import_same_driver(struct
drm_i915_private *i915,
err = PTR_ERR(import);
goto out_dmabuf;
}
+ import_obj = to_intel_bo(import);
if (import == >base) {
pr_err("i915_gem_prime_import reused gem object!\n");
@@ -183,8 +184,6 @@ static int igt_dmabuf_import_same_driver(struct
drm_i915_private *i915,
goto out_import;
}
- import_obj = to_intel_bo(import);
-
i915_gem_object_lock(import_obj, NULL);
err = __i915_gem_object_get_pages(import_obj);
if (err) {
--
2.33.0
[PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized
Commit 46e2068081e9 ("drm/i915: Disable some extra clang warnings")
disabled -Wsometimes-uninitialized as noisy but there have been a few
fixes to clang that make the false positive rate fairly low so it should
be enabled to help catch obvious mistakes. The first two patches fix
revent instances of this warning then enables it for i915 like the rest
of the tree.
Cheers,
Nathan
Nathan Chancellor (3):
drm/i915/selftests: Do not use import_obj uninitialized
drm/i915/selftests: Always initialize err in
igt_dmabuf_import_same_driver_lmem()
drm/i915: Enable -Wsometimes-uninitialized
drivers/gpu/drm/i915/Makefile| 1 -
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 7 ---
2 files changed, 4 insertions(+), 4 deletions(-)
base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
--
2.33.0
Re: [Intel-gfx] [PATCH] drm/i915/selftest: Fix use of err in igt_reset_{fail, nop}_engine()
Ping? This is a pretty clear bug and it is not fixed in -next or
drm-intel at this point.
On Fri, Aug 13, 2021 at 10:11:58AM -0700, Nathan Chancellor wrote:
> Clang warns:
>
> In file included from drivers/gpu/drm/i915/gt/intel_reset.c:1514:
> drivers/gpu/drm/i915/gt/selftest_hangcheck.c:465:62: warning: variable
> 'err' is uninitialized when used here [-Wuninitialized]
> pr_err("[%s] Create context failed: %d!\n", engine->name, err);
> ^~~
> ...
> drivers/gpu/drm/i915/gt/selftest_hangcheck.c:580:62: warning: variable
> 'err' is uninitialized when used here [-Wuninitialized]
> pr_err("[%s] Create context failed: %d!\n", engine->name, err);
> ^~~
> ...
> 2 warnings generated.
>
> This appears to be a copy and paste issue. Use ce directly using the %pe
> specifier to pretty print the error code so that err is not used
> uninitialized in these functions.
>
> Fixes: 3a7b72665ea5 ("drm/i915/selftest: Bump selftest timeouts for
> hangcheck")
> Reported-by: Dan Carpenter
> Signed-off-by: Nathan Chancellor
> ---
> drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> index 08f011f893b2..2c1ed32ca5ac 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> @@ -462,7 +462,7 @@ static int igt_reset_nop_engine(void *arg)
>
> ce = intel_context_create(engine);
> if (IS_ERR(ce)) {
> - pr_err("[%s] Create context failed: %d!\n",
> engine->name, err);
> + pr_err("[%s] Create context failed: %pe!\n",
> engine->name, ce);
> return PTR_ERR(ce);
> }
>
> @@ -577,7 +577,7 @@ static int igt_reset_fail_engine(void *arg)
>
> ce = intel_context_create(engine);
> if (IS_ERR(ce)) {
> - pr_err("[%s] Create context failed: %d!\n",
> engine->name, err);
> + pr_err("[%s] Create context failed: %pe!\n",
> engine->name, ce);
> return PTR_ERR(ce);
> }
>
>
> base-commit: 927dfdd09d8c03ba100ed0c8c3915f8e1d1f5556
> --
> 2.33.0.rc2
[PATCH] drm/radeon: Add break to switch statement in radeonfb_create_pinned_object()
Clang + -Wimplicit-fallthrough warns: drivers/gpu/drm/radeon/radeon_fb.c:170:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] default: ^ drivers/gpu/drm/radeon/radeon_fb.c:170:2: note: insert 'break;' to avoid fall-through default: ^ break; 1 warning generated. Clang's version of this warning is a little bit more pedantic than GCC's. Add the missing break to satisfy it to match what has been done all over the kernel tree. Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/radeon/radeon_fb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index 0b206b052972..c8b545181497 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -167,6 +167,7 @@ static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev, break; case 2: tiling_flags |= RADEON_TILING_SWAP_16BIT; + break; default: break; } base-commit: ba31f97d43be41ca99ab72a6131d7c226306865f -- 2.33.0.rc2
[PATCH] drm/i915/selftest: Fix use of err in igt_reset_{fail, nop}_engine()
Clang warns:
In file included from drivers/gpu/drm/i915/gt/intel_reset.c:1514:
drivers/gpu/drm/i915/gt/selftest_hangcheck.c:465:62: warning: variable
'err' is uninitialized when used here [-Wuninitialized]
pr_err("[%s] Create context failed: %d!\n", engine->name, err);
^~~
...
drivers/gpu/drm/i915/gt/selftest_hangcheck.c:580:62: warning: variable
'err' is uninitialized when used here [-Wuninitialized]
pr_err("[%s] Create context failed: %d!\n", engine->name, err);
^~~
...
2 warnings generated.
This appears to be a copy and paste issue. Use ce directly using the %pe
specifier to pretty print the error code so that err is not used
uninitialized in these functions.
Fixes: 3a7b72665ea5 ("drm/i915/selftest: Bump selftest timeouts for hangcheck")
Reported-by: Dan Carpenter
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 08f011f893b2..2c1ed32ca5ac 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -462,7 +462,7 @@ static int igt_reset_nop_engine(void *arg)
ce = intel_context_create(engine);
if (IS_ERR(ce)) {
- pr_err("[%s] Create context failed: %d!\n",
engine->name, err);
+ pr_err("[%s] Create context failed: %pe!\n",
engine->name, ce);
return PTR_ERR(ce);
}
@@ -577,7 +577,7 @@ static int igt_reset_fail_engine(void *arg)
ce = intel_context_create(engine);
if (IS_ERR(ce)) {
- pr_err("[%s] Create context failed: %d!\n",
engine->name, err);
+ pr_err("[%s] Create context failed: %pe!\n",
engine->name, ce);
return PTR_ERR(ce);
}
base-commit: 927dfdd09d8c03ba100ed0c8c3915f8e1d1f5556
--
2.33.0.rc2
[PATCH] drm/exynos: Always initialize mapping in exynos_drm_register_dma()
In certain randconfigs, clang warns: drivers/gpu/drm/exynos/exynos_drm_dma.c:121:19: warning: variable 'mapping' is uninitialized when used here [-Wuninitialized] priv->mapping = mapping; ^~~ drivers/gpu/drm/exynos/exynos_drm_dma.c:111:16: note: initialize the variable 'mapping' to silence this warning void *mapping; ^ = NULL 1 warning generated. This occurs when CONFIG_EXYNOS_IOMMU is enabled and both CONFIG_ARM_DMA_USE_IOMMU and CONFIG_IOMMU_DMA are disabled, which makes the code look like void *mapping; if (0) mapping = arm_iommu_create_mapping() else if (0) mapping = iommu_get_domain_for_dev() ... priv->mapping = mapping; Add an else branch that initializes mapping to the -ENODEV error pointer so that there is no more warning and the driver does not change during runtime. Reported-by: kernel test robot Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/exynos/exynos_drm_dma.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c index 0644936afee2..bf33c3084cb4 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c @@ -115,6 +115,8 @@ int exynos_drm_register_dma(struct drm_device *drm, struct device *dev, EXYNOS_DEV_ADDR_START, EXYNOS_DEV_ADDR_SIZE); else if (IS_ENABLED(CONFIG_IOMMU_DMA)) mapping = iommu_get_domain_for_dev(priv->dma_dev); + else + mapping = ERR_PTR(-ENODEV); if (IS_ERR(mapping)) return PTR_ERR(mapping); base-commit: 7d549995d4e0d99b68e8a7793a0d23da6fc40fe8 -- 2.32.0.264.g75ae10bc75
Re: [PATCH 31/64] fortify: Explicitly disable Clang support
On 7/27/2021 1:58 PM, Kees Cook wrote: Clang has never correctly compiled the FORTIFY_SOURCE defenses due to a couple bugs: Eliding inlines with matching __builtin_* names https://bugs.llvm.org/show_bug.cgi?id=50322 Incorrect __builtin_constant_p() of some globals https://bugs.llvm.org/show_bug.cgi?id=41459 In the process of making improvements to the FORTIFY_SOURCE defenses, the first (silent) bug (coincidentally) becomes worked around, but exposes the latter which breaks the build. As such, Clang must not be used with CONFIG_FORTIFY_SOURCE until at least latter bug is fixed (in Clang 13), and the fortify routines have been rearranged. Update the Kconfig to reflect the reality of the current situation. Signed-off-by: Kees Cook --- security/Kconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/security/Kconfig b/security/Kconfig index 0ced7fd33e4d..8f0e675e70a4 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -191,6 +191,9 @@ config HARDENED_USERCOPY_PAGESPAN config FORTIFY_SOURCE bool "Harden common str/mem functions against buffer overflows" depends on ARCH_HAS_FORTIFY_SOURCE + # https://bugs.llvm.org/show_bug.cgi?id=50322 + # https://bugs.llvm.org/show_bug.cgi?id=41459 + depends on !CONFIG_CC_IS_CLANG Should be !CC_IS_CLANG, Kconfig is hard :) help Detect overflows of buffers in common string and memory functions where the compiler can determine and validate the buffer sizes. Cheers, Nathan
Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
Hi Will and Robin, On 7/6/2021 10:06 AM, Will Deacon wrote: On Tue, Jul 06, 2021 at 04:39:11PM +0100, Robin Murphy wrote: On 2021-07-06 15:05, Christoph Hellwig wrote: On Tue, Jul 06, 2021 at 03:01:04PM +0100, Robin Murphy wrote: FWIW I was pondering the question of whether to do something along those lines or just scrap the default assignment entirely, so since I hadn't got round to saying that I've gone ahead and hacked up the alternative (similarly untested) for comparison :) TBH I'm still not sure which one I prefer... Claire did implement something like your suggestion originally, but I don't really like it as it doesn't scale for adding multiple global pools, e.g. for the 64-bit addressable one for the various encrypted secure guest schemes. Ah yes, that had slipped my mind, and it's a fair point indeed. Since we're not concerned with a minimal fix for backports anyway I'm more than happy to focus on Will's approach. Another thing is that that looks to take us a quiet step closer to the possibility of dynamically resizing a SWIOTLB pool, which is something that some of the hypervisor protection schemes looking to build on top of this series may want to explore at some point. Ok, I'll split that nasty diff I posted up into a reviewable series and we can take it from there. For what it's worth, I attempted to boot Will's diff on top of Konrad's devel/for-linus-5.14 and it did not work; in fact, I got no output on my monitor period, even with earlyprintk=, and I do not think this machine has a serial console. Robin's fix does work, it survived ten reboots with no issues getting to X and I do not see the KASAN and slub debug messages anymore but I understand that this is not the preferred solution it seems (although Konrad did want to know if it works). I am happy to test any further patches or follow ups as needed, just keep me on CC. Cheers, Nathan
Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
Hi Will and Robin, On Fri, Jul 02, 2021 at 04:13:50PM +0100, Robin Murphy wrote: > On 2021-07-02 14:58, Will Deacon wrote: > > Hi Nathan, > > > > On Thu, Jul 01, 2021 at 12:52:20AM -0700, Nathan Chancellor wrote: > > > On 7/1/2021 12:40 AM, Will Deacon wrote: > > > > On Wed, Jun 30, 2021 at 08:56:51AM -0700, Nathan Chancellor wrote: > > > > > On Wed, Jun 30, 2021 at 12:43:48PM +0100, Will Deacon wrote: > > > > > > On Wed, Jun 30, 2021 at 05:17:27PM +0800, Claire Chang wrote: > > > > > > > `BUG: unable to handle page fault for address: 003a8290` > > > > > > > and > > > > > > > the fact it crashed at `_raw_spin_lock_irqsave` look like the > > > > > > > memory > > > > > > > (maybe dev->dma_io_tlb_mem) was corrupted? > > > > > > > The dev->dma_io_tlb_mem should be set here > > > > > > > (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/probe.c#n2528) > > > > > > > through device_initialize. > > > > > > > > > > > > I'm less sure about this. 'dma_io_tlb_mem' should be pointing at > > > > > > 'io_tlb_default_mem', which is a page-aligned allocation from > > > > > > memblock. > > > > > > The spinlock is at offset 0x24 in that structure, and looking at the > > > > > > register dump from the crash: > > > > > > > > > > > > Jun 29 18:28:42 hp-4300G kernel: RSP: 0018:adb4013db9e8 EFLAGS: > > > > > > 00010006 > > > > > > Jun 29 18:28:42 hp-4300G kernel: RAX: 003a8290 RBX: > > > > > > RCX: 8900572ad580 > > > > > > Jun 29 18:28:42 hp-4300G kernel: RDX: 89005653f024 RSI: > > > > > > 000c RDI: 1d17 > > > > > > Jun 29 18:28:42 hp-4300G kernel: RBP: 0a20d000 R08: > > > > > > 000c R09: > > > > > > Jun 29 18:28:42 hp-4300G kernel: R10: 0a20d000 R11: > > > > > > 89005653f000 R12: 0212 > > > > > > Jun 29 18:28:42 hp-4300G kernel: R13: 1000 R14: > > > > > > 0002 R15: 0020 > > > > > > Jun 29 18:28:42 hp-4300G kernel: FS: 7f1f8898ea40() > > > > > > GS:89005728() knlGS: > > > > > > Jun 29 18:28:42 hp-4300G kernel: CS: 0010 DS: ES: CR0: > > > > > > 80050033 > > > > > > Jun 29 18:28:42 hp-4300G kernel: CR2: 003a8290 CR3: > > > > > > 0001020d CR4: 00350ee0 > > > > > > Jun 29 18:28:42 hp-4300G kernel: Call Trace: > > > > > > Jun 29 18:28:42 hp-4300G kernel: _raw_spin_lock_irqsave+0x39/0x50 > > > > > > Jun 29 18:28:42 hp-4300G kernel: swiotlb_tbl_map_single+0x12b/0x4c0 > > > > > > > > > > > > Then that correlates with R11 holding the 'dma_io_tlb_mem' pointer > > > > > > and > > > > > > RDX pointing at the spinlock. Yet RAX is holding junk :/ > > > > > > > > > > > > I agree that enabling KASAN would be a good idea, but I also think > > > > > > we > > > > > > probably need to get some more information out of > > > > > > swiotlb_tbl_map_single() > > > > > > to see see what exactly is going wrong in there. > > > > > > > > > > I can certainly enable KASAN and if there is any debug print I can add > > > > > or dump anything, let me know! > > > > > > > > I bit the bullet and took v5.13 with swiotlb/for-linus-5.14 merged in, > > > > built > > > > x86 defconfig and ran it on my laptop. However, it seems to work fine! > > > > > > > > Please can you share your .config? > > > > > > Sure thing, it is attached. It is just Arch Linux's config run through > > > olddefconfig. The original is below in case you need to diff it. > > > > > > https://raw.githubusercontent.com/archlinux/svntogit-packages/9045405dc835527164f3034b3ceb9a67c7a53cd4/trunk/config > > > > > > If there is anything more that I can provide, please let me know. > > > > I eventually got this booting (for some reason it was causing LD to SEGV > > trying to link it for a
Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
On Fri, Jul 02, 2021 at 03:16:46PM +0200, Maxime Ripard wrote:
> Hi Nathan,
>
> On Thu, Jul 01, 2021 at 08:29:34PM -0700, Nathan Chancellor wrote:
> > On Mon, May 24, 2021 at 03:18:52PM +0200, Maxime Ripard wrote:
> > > The new gpiod interface takes care of parsing the GPIO flags and to
> > > return the logical value when accessing an active-low GPIO, so switching
> > > to it simplifies a lot the driver.
> > >
> > > Signed-off-by: Maxime Ripard
> > > ---
> > > drivers/gpu/drm/vc4/vc4_hdmi.c | 24 +++-
> > > drivers/gpu/drm/vc4/vc4_hdmi.h | 3 +--
> > > 2 files changed, 8 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > index ccc6c8079dc6..34622c59f6a7 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > > @@ -159,10 +159,9 @@ vc4_hdmi_connector_detect(struct drm_connector
> > > *connector, bool force)
> > > struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> > > bool connected = false;
> > >
> > > - if (vc4_hdmi->hpd_gpio) {
> > > - if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
> > > - vc4_hdmi->hpd_active_low)
> > > - connected = true;
> > > + if (vc4_hdmi->hpd_gpio &&
> > > + gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
> > > + connected = true;
> > > } else if (drm_probe_ddc(vc4_hdmi->ddc)) {
> > > connected = true;
> > > } else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
> > > @@ -1993,7 +1992,6 @@ static int vc4_hdmi_bind(struct device *dev, struct
> > > device *master, void *data)
> > > struct vc4_hdmi *vc4_hdmi;
> > > struct drm_encoder *encoder;
> > > struct device_node *ddc_node;
> > > - u32 value;
> > > int ret;
> > >
> > > vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL);
> > > @@ -2031,18 +2029,10 @@ static int vc4_hdmi_bind(struct device *dev,
> > > struct device *master, void *data)
> > > /* Only use the GPIO HPD pin if present in the DT, otherwise
> > >* we'll use the HDMI core's register.
> > >*/
> > > - if (of_find_property(dev->of_node, "hpd-gpios", )) {
> > > - enum of_gpio_flags hpd_gpio_flags;
> > > -
> > > - vc4_hdmi->hpd_gpio = of_get_named_gpio_flags(dev->of_node,
> > > - "hpd-gpios", 0,
> > > - _gpio_flags);
> > > - if (vc4_hdmi->hpd_gpio < 0) {
> > > - ret = vc4_hdmi->hpd_gpio;
> > > - goto err_put_ddc;
> > > - }
> > > -
> > > - vc4_hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
> > > + vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN);
> > > + if (IS_ERR(vc4_hdmi->hpd_gpio)) {
> > > + ret = PTR_ERR(vc4_hdmi->hpd_gpio);
> > > + goto err_put_ddc;
> > > }
> > >
> > > vc4_hdmi->disable_wifi_frequencies =
> > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h
> > > b/drivers/gpu/drm/vc4/vc4_hdmi.h
> > > index 060bcaefbeb5..2688a55461d6 100644
> > > --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> > > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> > > @@ -146,8 +146,7 @@ struct vc4_hdmi {
> > > /* VC5 Only */
> > > void __iomem *rm_regs;
> > >
> > > - int hpd_gpio;
> > > - bool hpd_active_low;
> > > + struct gpio_desc *hpd_gpio;
> > >
> > > /*
> > >* On some systems (like the RPi4), some modes are in the same
> > > --
> > > 2.31.1
> >
> > This patch as commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
> > causes my Raspberry Pi 3 to lock up shortly after boot in combination
> > with commit 411efa18e4b0 ("drm/vc4: hdmi: Move the HSM clock enable to
> > runtime_pm"). The serial console and ssh are completely unresponsive and
> > I do not see any messages in dmesg with "debug ignore_loglevel". The
> > device is running with a 32-bit kernel (multi_v7_defconfig) with 32-bit
> > userspace. If there is any further information that I can provide,
> > please let me know.
>
Re: [PATCH 2/2] drm/vc4: hdmi: Convert to gpiod
On Mon, May 24, 2021 at 03:18:52PM +0200, Maxime Ripard wrote:
> The new gpiod interface takes care of parsing the GPIO flags and to
> return the logical value when accessing an active-low GPIO, so switching
> to it simplifies a lot the driver.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/vc4/vc4_hdmi.c | 24 +++-
> drivers/gpu/drm/vc4/vc4_hdmi.h | 3 +--
> 2 files changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index ccc6c8079dc6..34622c59f6a7 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -159,10 +159,9 @@ vc4_hdmi_connector_detect(struct drm_connector
> *connector, bool force)
> struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
> bool connected = false;
>
> - if (vc4_hdmi->hpd_gpio) {
> - if (gpio_get_value_cansleep(vc4_hdmi->hpd_gpio) ^
> - vc4_hdmi->hpd_active_low)
> - connected = true;
> + if (vc4_hdmi->hpd_gpio &&
> + gpiod_get_value_cansleep(vc4_hdmi->hpd_gpio)) {
> + connected = true;
> } else if (drm_probe_ddc(vc4_hdmi->ddc)) {
> connected = true;
> } else if (HDMI_READ(HDMI_HOTPLUG) & VC4_HDMI_HOTPLUG_CONNECTED) {
> @@ -1993,7 +1992,6 @@ static int vc4_hdmi_bind(struct device *dev, struct
> device *master, void *data)
> struct vc4_hdmi *vc4_hdmi;
> struct drm_encoder *encoder;
> struct device_node *ddc_node;
> - u32 value;
> int ret;
>
> vc4_hdmi = devm_kzalloc(dev, sizeof(*vc4_hdmi), GFP_KERNEL);
> @@ -2031,18 +2029,10 @@ static int vc4_hdmi_bind(struct device *dev, struct
> device *master, void *data)
> /* Only use the GPIO HPD pin if present in the DT, otherwise
>* we'll use the HDMI core's register.
>*/
> - if (of_find_property(dev->of_node, "hpd-gpios", )) {
> - enum of_gpio_flags hpd_gpio_flags;
> -
> - vc4_hdmi->hpd_gpio = of_get_named_gpio_flags(dev->of_node,
> - "hpd-gpios", 0,
> - _gpio_flags);
> - if (vc4_hdmi->hpd_gpio < 0) {
> - ret = vc4_hdmi->hpd_gpio;
> - goto err_put_ddc;
> - }
> -
> - vc4_hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
> + vc4_hdmi->hpd_gpio = devm_gpiod_get_optional(dev, "hpd", GPIOD_IN);
> + if (IS_ERR(vc4_hdmi->hpd_gpio)) {
> + ret = PTR_ERR(vc4_hdmi->hpd_gpio);
> + goto err_put_ddc;
> }
>
> vc4_hdmi->disable_wifi_frequencies =
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
> index 060bcaefbeb5..2688a55461d6 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.h
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
> @@ -146,8 +146,7 @@ struct vc4_hdmi {
> /* VC5 Only */
> void __iomem *rm_regs;
>
> - int hpd_gpio;
> - bool hpd_active_low;
> + struct gpio_desc *hpd_gpio;
>
> /*
>* On some systems (like the RPi4), some modes are in the same
> --
> 2.31.1
Hi Maxime,
This patch as commit 6800234ceee0 ("drm/vc4: hdmi: Convert to gpiod")
causes my Raspberry Pi 3 to lock up shortly after boot in combination
with commit 411efa18e4b0 ("drm/vc4: hdmi: Move the HSM clock enable to
runtime_pm"). The serial console and ssh are completely unresponsive and
I do not see any messages in dmesg with "debug ignore_loglevel". The
device is running with a 32-bit kernel (multi_v7_defconfig) with 32-bit
userspace. If there is any further information that I can provide,
please let me know.
Cheers,
Nathan
Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
Hi Will and Claire,
On Wed, Jun 30, 2021 at 12:43:48PM +0100, Will Deacon wrote:
> On Wed, Jun 30, 2021 at 05:17:27PM +0800, Claire Chang wrote:
> > On Wed, Jun 30, 2021 at 9:43 AM Nathan Chancellor wrote:
> > >
> > > On Thu, Jun 24, 2021 at 11:55:20PM +0800, Claire Chang wrote:
> > > > Propagate the swiotlb_force into io_tlb_default_mem->force_bounce and
> > > > use it to determine whether to bounce the data or not. This will be
> > > > useful later to allow for different pools.
> > > >
> > > > Signed-off-by: Claire Chang
> > > > Reviewed-by: Christoph Hellwig
> > > > Tested-by: Stefano Stabellini
> > > > Tested-by: Will Deacon
> > > > Acked-by: Stefano Stabellini
> > >
> > > This patch as commit af452ec1b1a3 ("swiotlb: Use is_swiotlb_force_bounce
> > > for swiotlb data bouncing") causes my Ryzen 3 4300G system to fail to
> > > get to an X session consistently (although not every single time),
> > > presumably due to a crash in the AMDGPU driver that I see in dmesg.
> > >
> > > I have attached logs at af452ec1b1a3 and f127c9556a8e and I am happy
> > > to provide any further information, debug, or test patches as necessary.
> >
> > Are you using swiotlb=force? or the swiotlb_map is called because of
> > !dma_capable?
> > (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/kernel/dma/direct.h#n93)
>
> The command line is in the dmesg:
>
> | Kernel command line: initrd=\amd-ucode.img
> initrd=\initramfs-linux-next-llvm.img
> root=PARTUUID=8680aa0c-cf09-4a69-8cf3-970478040ee7 rw intel_pstate=no_hwp
> irqpoll
>
> but I worry that this looks _very_ similar to the issue reported by Qian
> Cai which we thought we had fixed. Nathan -- is the failure deterministic?
Yes, for the most part. It does not happen every single boot so when I
was bisecting, I did a series of seven boots and only considered the
revision good when all seven of them made it to LightDM's greeter. My
results that I notated show most bad revisions failed anywhere from four
to six times.
> > `BUG: unable to handle page fault for address: 003a8290` and
> > the fact it crashed at `_raw_spin_lock_irqsave` look like the memory
> > (maybe dev->dma_io_tlb_mem) was corrupted?
> > The dev->dma_io_tlb_mem should be set here
> > (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/probe.c#n2528)
> > through device_initialize.
>
> I'm less sure about this. 'dma_io_tlb_mem' should be pointing at
> 'io_tlb_default_mem', which is a page-aligned allocation from memblock.
> The spinlock is at offset 0x24 in that structure, and looking at the
> register dump from the crash:
>
> Jun 29 18:28:42 hp-4300G kernel: RSP: 0018:adb4013db9e8 EFLAGS: 00010006
> Jun 29 18:28:42 hp-4300G kernel: RAX: 003a8290 RBX:
> RCX: 8900572ad580
> Jun 29 18:28:42 hp-4300G kernel: RDX: 89005653f024 RSI: 000c
> RDI: 1d17
> Jun 29 18:28:42 hp-4300G kernel: RBP: 0a20d000 R08: 000c
> R09:
> Jun 29 18:28:42 hp-4300G kernel: R10: 0a20d000 R11: 89005653f000
> R12: 0212
> Jun 29 18:28:42 hp-4300G kernel: R13: 1000 R14: 0002
> R15: 0020
> Jun 29 18:28:42 hp-4300G kernel: FS: 7f1f8898ea40()
> GS:89005728() knlGS:
> Jun 29 18:28:42 hp-4300G kernel: CS: 0010 DS: ES: CR0:
> 80050033
> Jun 29 18:28:42 hp-4300G kernel: CR2: 003a8290 CR3: 0001020d
> CR4: 00350ee0
> Jun 29 18:28:42 hp-4300G kernel: Call Trace:
> Jun 29 18:28:42 hp-4300G kernel: _raw_spin_lock_irqsave+0x39/0x50
> Jun 29 18:28:42 hp-4300G kernel: swiotlb_tbl_map_single+0x12b/0x4c0
>
> Then that correlates with R11 holding the 'dma_io_tlb_mem' pointer and
> RDX pointing at the spinlock. Yet RAX is holding junk :/
>
> I agree that enabling KASAN would be a good idea, but I also think we
> probably need to get some more information out of swiotlb_tbl_map_single()
> to see see what exactly is going wrong in there.
I can certainly enable KASAN and if there is any debug print I can add
or dump anything, let me know!
Cheers,
Nathan
Re: [PATCH v2] fb_defio: Remove custom address_space_operations
On 5/30/2021 2:14 PM, Matthew Wilcox wrote:
On Sun, May 30, 2021 at 12:13:05PM -0700, Nathan Chancellor wrote:
Hi Matthew,
On Wed, Mar 10, 2021 at 06:55:30PM +, Matthew Wilcox (Oracle) wrote:
There's no need to give the page an address_space. Leaving the
page->mapping as NULL will cause the VM to handle set_page_dirty()
the same way that it's handled now, and that was the only reason to
set the address_space in the first place.
Signed-off-by: Matthew Wilcox (Oracle)
Reviewed-by: Christoph Hellwig
Reviewed-by: William Kucharski
This patch in mainline as commit ccf953d8f3d6 ("fb_defio: Remove custom
address_space_operations") causes my Hyper-V based VM to no longer make
it to a graphical environment.
Hi Nathan,
Thanks for the report. I sent Daniel a revert patch with a full
explanation last week, which I assume he'll queue up for a pull soon.
You can just git revert ccf953d8f3d6 for yourself until that shows up.
Sorry for the inconvenience.
Thank you for the quick response! I will keep an eye out for the patch
while reverting it locally in the meantime.
Cheers,
Nathan
Re: [PATCH] drm/msm/dsi: fix 32-bit clang warning
On 5/14/2021 2:30 PM, Arnd Bergmann wrote:
From: Arnd Bergmann
clang is a little overzealous with warning about a constant conversion
in an untaken branch of a ternary expression:
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:975:48: error: implicit conversion
from 'unsigned long long' to 'unsigned long' changes value from 50 to
705032704 [-Werror,-Wconstant-conversion]
.max_pll_rate = (50ULL < ULONG_MAX) ? 50UL : ULONG_MAX,
^~~~
Rewrite this to use a preprocessor conditional instead to avoid the
warning.
Fixes: 076437c9e360 ("drm/msm/dsi: move min/max PLL rate to phy config")
Signed-off-by: Arnd Bergmann
Reviewed-by: Nathan Chancellor
---
As found with another patch, using __builtin_choose_expr() would
likely also work here, but doesn't seem any more readable.
---
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index e76ce40a12ab..accd6b4eb7c2 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -972,7 +972,11 @@ const struct msm_dsi_phy_cfg dsi_phy_7nm_cfgs = {
.restore_pll_state = dsi_7nm_pll_restore_state,
},
.min_pll_rate = 6UL,
- .max_pll_rate = (50ULL < ULONG_MAX) ? 50ULL : ULONG_MAX,
+#ifdef CONFIG_64BIT
+ .max_pll_rate = 50UL,
+#else
+ .max_pll_rate = ULONG_MAX,
+#endif
.io_start = { 0xae94400, 0xae96400 },
.num_dsi_phy = 2,
.quirks = DSI_PHY_7NM_QUIRK_V4_1,
[PATCH] fbmem: Correct position of '__maybe_unused' in proc_fb_seq_ops
Clang warns:
drivers/video/fbdev/core/fbmem.c:736:21: warning: attribute
declaration must precede definition [-Wignored-attributes]
static const struct __maybe_unused seq_operations proc_fb_seq_ops = {
^
./include/linux/compiler_attributes.h:273:56: note: expanded from macro
'__maybe_unused'
#define __maybe_unused __attribute__((__unused__))
^
./include/linux/seq_file.h:31:8: note: previous definition is here
struct seq_operations {
^
1 warning generated.
The attribute should not split the type 'struct seq_operations'. Move it
before the struct keyword so that it works properly and there is no more
warning.
Fixes: b9d79e4ca4ff ("fbmem: Mark proc_fb_seq_ops as __maybe_unused")
Link: https://github.com/ClangBuiltLinux/linux/issues/1371
Reported-by: kernel test robot
Signed-off-by: Nathan Chancellor
---
drivers/video/fbdev/core/fbmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 52c606c0f8a2..84c484f37b4a 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -733,7 +733,7 @@ static int fb_seq_show(struct seq_file *m, void *v)
return 0;
}
-static const struct __maybe_unused seq_operations proc_fb_seq_ops = {
+static const __maybe_unused struct seq_operations proc_fb_seq_ops = {
.start = fb_seq_start,
.next = fb_seq_next,
.stop = fb_seq_stop,
base-commit: b9d79e4ca4ff23543d6b33c736ba07c1f0a9dcb1
--
2.31.1.362.g311531c9de
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/tegra: Fix shift overflow in tegra_shared_plane_atomic_update
Clang warns:
drivers/gpu/drm/tegra/hub.c:513:11: warning: shift count >= width of
type [-Wshift-count-overflow]
base |= BIT(39);
^~~
BIT is unsigned long, which is 32-bit on ARCH=arm, hence the overflow
warning. Switch to BIT_ULL, which is 64-bit and will not overflow.
Fixes: 7b6f846785f4 ("drm/tegra: Support sector layout on Tegra194")
Link: https://github.com/ClangBuiltLinux/linux/issues/1351
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/tegra/hub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index 79bff8b48271..bfae8a02f55b 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -510,7 +510,7 @@ static void tegra_shared_plane_atomic_update(struct
drm_plane *plane,
* dGPU sector layout.
*/
if (tegra_plane_state->tiling.sector_layout ==
TEGRA_BO_SECTOR_LAYOUT_GPU)
- base |= BIT(39);
+ base |= BIT_ULL(39);
#endif
tegra_plane_writel(p, tegra_plane_state->format, DC_WIN_COLOR_DEPTH);
base-commit: 0265531f0897f890da3f9c2958707af099c7d974
--
2.31.1.272.g89b43f80a5
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu/display: initialize the variable 'i'
On Mon, Feb 22, 2021 at 11:50:06PM +, Simon Ser wrote:
> On Tuesday, February 23rd, 2021 at 12:44 AM, Nathan Chancellor
> wrote:
>
> > On Mon, Feb 22, 2021 at 11:05:17PM +, Simon Ser wrote:
> > > On Monday, February 22nd, 2021 at 8:25 PM, Souptick Joarder
> > > wrote:
> > >
> > > > >> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:9804:38:
> > > > >> warning: variable 'i' is uninitialized when used here
> > > > >> [-Wuninitialized]
> > > >timing = >detailed_timings[i];
> > > > ^
> > > >drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:9714:7:
> > > > note: initialize the variable 'i' to silence this warning
> > > >int i;
> > > > ^
> > > > = 0
> > > >1 warning generated.
> > > >
> > > > Initialize the variable 'i'.
> > >
> > > Hm, I see this variable already initialized in the loop:
> > >
> > > for (i = 0; i < 4; i++) {
> > >
> > > This is the branch agd5f/drm-next.
> >
> > That is in the
> >
> > if (amdgpu_dm_connector->dc_sink->sink_signal ==
> > SIGNAL_TYPE_DISPLAY_PORT
> > || amdgpu_dm_connector->dc_sink->sink_signal ==
> > SIGNAL_TYPE_EDP) {
> >
> > branch not the
> >
> > } else if (edid && amdgpu_dm_connector->dc_sink->sink_signal ==
> > SIGNAL_TYPE_HDMI_TYPE_A) {
> >
> > branch, where i is indeed used uninitialized like clang complains about.
> >
> > I am not at all familiar with the code so I cannot say if this fix is
> > the proper one but it is definitely a legitimate issue.
>
> I think you have an outdated branch. In my checkout, i is not used in the
> first
> branch, and is initialized in the second one.
>
> https://cgit.freedesktop.org/~agd5f/linux/tree/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c?h=drm-next#n9700
That branch is the outdated one:
https://git.kernel.org/linus/a897913a819191550ab2fa2784d3c3ada3a096d3
Please see:
https://gitlab.freedesktop.org/agd5f/linux/-/blob/amd-staging-drm-next/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c#L9854
It was introduced by commit f9b4f20c4777 ("drm/amd/display: Add Freesync
HDMI support to DM").
Cheers,
Nathan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu/display: initialize the variable 'i'
On Mon, Feb 22, 2021 at 11:05:17PM +, Simon Ser wrote:
> On Monday, February 22nd, 2021 at 8:25 PM, Souptick Joarder
> wrote:
>
> > >> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:9804:38:
> > >> warning: variable 'i' is uninitialized when used here
> > >> [-Wuninitialized]
> >timing = >detailed_timings[i];
> > ^
> >drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:9714:7:
> > note: initialize the variable 'i' to silence this warning
> >int i;
> > ^
> > = 0
> >1 warning generated.
> >
> > Initialize the variable 'i'.
>
> Hm, I see this variable already initialized in the loop:
>
> for (i = 0; i < 4; i++) {
>
> This is the branch agd5f/drm-next.
That is in the
if (amdgpu_dm_connector->dc_sink->sink_signal ==
SIGNAL_TYPE_DISPLAY_PORT
|| amdgpu_dm_connector->dc_sink->sink_signal ==
SIGNAL_TYPE_EDP) {
branch not the
} else if (edid && amdgpu_dm_connector->dc_sink->sink_signal ==
SIGNAL_TYPE_HDMI_TYPE_A) {
branch, where i is indeed used uninitialized like clang complains about.
I am not at all familiar with the code so I cannot say if this fix is
the proper one but it is definitely a legitimate issue.
Cheers,
Nathan
> > Reported-by: kernel test robot
> > Signed-off-by: Souptick Joarder
> > ---
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index a22a53d..e96d3d9 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -9717,7 +9717,7 @@ static bool parse_hdmi_amd_vsdb(struct
> > amdgpu_dm_connector *aconnector,
> > void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
> > struct edid *edid)
> > {
> > - int i;
> > + int i = 0;
> > struct detailed_timing *timing;
> > struct detailed_non_pixel *data;
> > struct detailed_data_monitor_range *range;
> > --
> > 1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for 5.12-rc1
On Sun, Feb 21, 2021 at 03:07:17PM -0800, Linus Torvalds wrote:
> On Thu, Feb 18, 2021 at 10:06 PM Dave Airlie wrote:
> >
> > Let me know if there are any issues,
>
> gcc was happy, and I obviously already pushed out my merge, but then
> when I did my clang build afterwards, it reports:
>
> drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu_cmn.c:764:2: warning:
> variable 'structure_size' is used uninitialized whenever switch
> default is taken [-Wsometimes-uninitialized]
> default:
> ^~~
> drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu_cmn.c:770:23: note:
> uninitialized use occurs here
> memset(header, 0xFF, structure_size);
>^~
>
> and clang is very very right. That "default" case is completely
> broken, and will generate a randomly sized memset. Not good.
>
> Presumably that default case never happens, but if so it shouldn't exist.
>
> Perhaps better yet, make the "default" case just do a "return" instead
> of a break. Breaking out of the switch statement to code that cannot
> possibly work is all kinds of mindless.
>
> Kevin/Alex? This was introduced by commit de4b7cd8cb87
> ("drm/amd/pm/swsmu: unify the init soft gpu metrics function")
>
> Linus
I sent https://lore.kernel.org/r/20210218224849.5591-1-nat...@kernel.org/
a few days ago and Kevin reviewed it, just seems like Alex needs to pick
it up.
Cheers,
Nathan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/pm/swsmu: Avoid using structure_size uninitialized in smu_cmn_init_soft_gpu_metrics
Clang warns:
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu_cmn.c:764:2: warning:
variable 'structure_size' is used uninitialized whenever switch default
is taken [-Wsometimes-uninitialized]
default:
^~~
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu_cmn.c:770:23: note:
uninitialized use occurs here
memset(header, 0xFF, structure_size);
^~
drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu_cmn.c:753:25: note:
initialize the variable 'structure_size' to silence this warning
uint16_t structure_size;
^
= 0
1 warning generated.
Return in the default case, as the size of the header will not be known.
Fixes: de4b7cd8cb87 ("drm/amd/pm/swsmu: unify the init soft gpu metrics
function")
Link: https://github.com/ClangBuiltLinux/linux/issues/1304
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
index bb620fdd4cd2..bcedd4d92e35 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
@@ -762,7 +762,7 @@ void smu_cmn_init_soft_gpu_metrics(void *table, uint8_t
frev, uint8_t crev)
structure_size = sizeof(struct gpu_metrics_v2_0);
break;
default:
- break;
+ return;
}
#undef METRICS_VERSION
--
2.30.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: Enable -Wuninitialized
-Wunintialized was disabled in commit c5627461490e ("drm/i915: Disable
-Wuninitialized") because there were two warnings that were false
positives. The first was due to DECLARE_WAIT_QUEUE_HEAD_ONSTACK, which
was fixed in LLVM 9.0.0. The second was in busywait_stop, which was
fixed in LLVM 10.0.0 (issue 415). The kernel's minimum version for LLVM
is 10.0.1 so this warning can be safely enabled, where it has already
caught a couple bugs.
Link: https://github.com/ClangBuiltLinux/linux/issues/220
Link: https://github.com/ClangBuiltLinux/linux/issues/415
Link: https://github.com/ClangBuiltLinux/linux/issues/499
Link:
https://github.com/llvm/llvm-project/commit/2e040398f8d691cc378c1abb098824ff49f3f28f
Link:
https://github.com/llvm/llvm-project/commit/c667cdc850c2aa821ffeedbc08c24bc985c59edd
Fixes: c5627461490e ("drm/i915: Disable -Wuninitialized")
References: 2ea4a7ba9bf6 ("drm/i915/gt: Avoid uninitialized use of rpcurupei in
frequency_show")
References: 2034c2129bc4 ("drm/i915/display: Ensure that ret is always
initialized in icl_combo_phy_verify_state")
Reported-by: Arnd Bergmann
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 6d9e81ea67f4..60b60204004f 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -21,7 +21,6 @@ subdir-ccflags-y += $(call cc-disable-warning,
unused-but-set-variable)
subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
-subdir-ccflags-y += $(call cc-disable-warning, uninitialized)
subdir-ccflags-y += $(call cc-disable-warning, frame-address)
subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
base-commit: f40ddce88593482919761f74910f42f4b84c004b
--
2.30.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 80/80] ARM: dts: bcm2711: Enable the display pipeline
On Thu, Oct 01, 2020 at 11:22:03AM +0200, Nicolas Saenz Julienne wrote: > On Wed, 2020-09-30 at 09:38 -0700, Nathan Chancellor wrote: > > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: > > > Hi Nathan, > > > > > > On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > > > > On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > > > > > Now that all the drivers have been adjusted for it, let's bring in the > > > > > necessary device tree changes. > > > > > > > > > > The VEC and PV3 are left out for now, since it will require a more > > > > > specific > > > > > clock setup. > > > > > > > > > > Reviewed-by: Dave Stevenson > > > > > Tested-by: Chanwoo Choi > > > > > Tested-by: Hoegeun Kwon > > > > > Tested-by: Stefan Wahren > > > > > Signed-off-by: Maxime Ripard > > > > > > > > Apologies if this has already been reported or have a solution but this > > > > patch (and presumably series) breaks output to the serial console after > > > > a certain point during init. On Raspbian, I see systemd startup messages > > > > then the output just turns into complete garbage. It looks like this > > > > patch is merged first in linux-next, which is why my bisect fell on the > > > > DRM merge. I am happy to provide whatever information could be helpful > > > > for debugging this. I am on the latest version of the firmware > > > > (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). > > > > > > Unfortunately, the miniUART is in the same clock tree than the core > > > clock and will thus have those kind of issues when the core clock is > > > changed (which is also something that one should expect when using the > > > DRM or other drivers). > > > > > > The only real workaround there would be to switch to one of the PL011 > > > UARTs. I guess we can also somehow make the UART react to the core clock > > > frequency changes, but that's going to require some effort > > > > > > Maxime > > > > Ack, thank you for the reply! There does not really seem to be a whole > > ton of documentation around using one of the other PL011 UARTs so for > > now, I will just revert this commit locally. > > Nathan, a less intrusive solution would be to add 'core_freq_min=500' into > your > config.txt. > > Funnily enough core_freq=500 doesn't seem to work in that regard. It might be > related with what Maxime is commenting. > > Regards, > Nicolas > Excellent, thank you for the tip, that worked well! Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 80/80] ARM: dts: bcm2711: Enable the display pipeline
On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: > Hi Nathan, > > On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: > > On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > > > Now that all the drivers have been adjusted for it, let's bring in the > > > necessary device tree changes. > > > > > > The VEC and PV3 are left out for now, since it will require a more > > > specific > > > clock setup. > > > > > > Reviewed-by: Dave Stevenson > > > Tested-by: Chanwoo Choi > > > Tested-by: Hoegeun Kwon > > > Tested-by: Stefan Wahren > > > Signed-off-by: Maxime Ripard > > > > Apologies if this has already been reported or have a solution but this > > patch (and presumably series) breaks output to the serial console after > > a certain point during init. On Raspbian, I see systemd startup messages > > then the output just turns into complete garbage. It looks like this > > patch is merged first in linux-next, which is why my bisect fell on the > > DRM merge. I am happy to provide whatever information could be helpful > > for debugging this. I am on the latest version of the firmware > > (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). > > Unfortunately, the miniUART is in the same clock tree than the core > clock and will thus have those kind of issues when the core clock is > changed (which is also something that one should expect when using the > DRM or other drivers). > > The only real workaround there would be to switch to one of the PL011 > UARTs. I guess we can also somehow make the UART react to the core clock > frequency changes, but that's going to require some effort > > Maxime Ack, thank you for the reply! There does not really seem to be a whole ton of documentation around using one of the other PL011 UARTs so for now, I will just revert this commit locally. Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 80/80] ARM: dts: bcm2711: Enable the display pipeline
On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > Now that all the drivers have been adjusted for it, let's bring in the > necessary device tree changes. > > The VEC and PV3 are left out for now, since it will require a more specific > clock setup. > > Reviewed-by: Dave Stevenson > Tested-by: Chanwoo Choi > Tested-by: Hoegeun Kwon > Tested-by: Stefan Wahren > Signed-off-by: Maxime Ripard Apologies if this has already been reported or have a solution but this patch (and presumably series) breaks output to the serial console after a certain point during init. On Raspbian, I see systemd startup messages then the output just turns into complete garbage. It looks like this patch is merged first in linux-next, which is why my bisect fell on the DRM merge. I am happy to provide whatever information could be helpful for debugging this. I am on the latest version of the firmware (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). $ git bisect log # bad: [49e7e3e905e437a02782019570f70997e2da9101] Add linux-next specific files for 20200929 # good: [fb0155a09b0224a7147cb07a4ce6034c8d29667f] Merge tag 'nfs-for-5.9-3' of git://git.linux-nfs.org/projects/trondmy/linux-nfs git bisect start '49e7e3e905e437a02782019570f70997e2da9101' 'fb0155a09b0224a7147cb07a4ce6034c8d29667f' # good: [a07bf9042465c0e4ab28947daf70517f99ef021f] Merge remote-tracking branch 'bluetooth/master' into master git bisect good a07bf9042465c0e4ab28947daf70517f99ef021f # bad: [546d06883722dfc5823d53042b924f4b4f76a459] Merge remote-tracking branch 'spi/for-next' into master git bisect bad 546d06883722dfc5823d53042b924f4b4f76a459 # good: [06c14f5c2d311100a447caf60ecbcf558e4e60fe] Merge tag 'mediatek-drm-next-5.10' of https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux into drm-next git bisect good 06c14f5c2d311100a447caf60ecbcf558e4e60fe # bad: [606865c11f2ed6429c7eddcbc59d3295771d41a4] Merge remote-tracking branch 'sound-asoc/for-next' into master git bisect bad 606865c11f2ed6429c7eddcbc59d3295771d41a4 # bad: [7492f2f52acc72a1d08ad1f1d722237ba66189b9] Merge remote-tracking branch 'regmap/for-next' into master git bisect bad 7492f2f52acc72a1d08ad1f1d722237ba66189b9 # good: [0b7e44d39c8aa7536352b57af2265e92fc253e4f] integrity: Asymmetric digsig supports SM2-with-SM3 algorithm git bisect good 0b7e44d39c8aa7536352b57af2265e92fc253e4f # good: [2ce595ba1cd7a2bc053fcc937b7bbbf743c21818] Merge remote-tracking branch 'nand/nand/next' into master git bisect good 2ce595ba1cd7a2bc053fcc937b7bbbf743c21818 # good: [10e07ca312548f90d5e0fc1d862209285c9a858c] gpu/drm/radeon: fix spelling typo in comments git bisect good 10e07ca312548f90d5e0fc1d862209285c9a858c # bad: [be877462417f05af69729a9eeda1332b15e81de8] Merge remote-tracking branch 'imx-drm/imx-drm/next' into master git bisect bad be877462417f05af69729a9eeda1332b15e81de8 # bad: [7a80fa2ada067aa633a91bce67f6c3e39aad7504] Merge remote-tracking branch 'drm-intel/for-linux-next' into master git bisect bad 7a80fa2ada067aa633a91bce67f6c3e39aad7504 # good: [f4a336053725b689a65021edca26f8d058c0d6b4] drm/amdgpu/display: fix CFLAGS setup for DCN30 git bisect good f4a336053725b689a65021edca26f8d058c0d6b4 # bad: [64e05a0ebd7e6047c9f67c685fe8d4ec79a397ba] Merge remote-tracking branch 'drm/drm-next' into master git bisect bad 64e05a0ebd7e6047c9f67c685fe8d4ec79a397ba # good: [a4b0c1f80de8ec3f473b02918556650b683f044d] Merge remote-tracking branch 'crypto/master' into master git bisect good a4b0c1f80de8ec3f473b02918556650b683f044d # first bad commit: [64e05a0ebd7e6047c9f67c685fe8d4ec79a397ba] Merge remote-tracking branch 'drm/drm-next' into master Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: Simplify condition in try_disable_dsc
Clang warns:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:637:8:
warning: logical not is only applied to the left hand side of this
comparison [-Wlogical-not-parentheses]
&& !params[i].clock_force_enable ==
DSC_CLK_FORCE_DEFAULT) {
^ ~~
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:637:8:
note: add parentheses after the '!' to evaluate the comparison first
&& !params[i].clock_force_enable ==
DSC_CLK_FORCE_DEFAULT) {
^
(
)
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:637:8:
note: add parentheses around left hand side expression to silence this
warning
&& !params[i].clock_force_enable ==
DSC_CLK_FORCE_DEFAULT) {
^
()
1 warning generated.
The expression "!a == 0" can be more simply written as "a", which makes
it easier to reason about the logic and prevents the warning.
Fixes: 0749ddeb7d6c ("drm/amd/display: Add DSC force disable to dsc_clock_en
debugfs entry")
Link: https://github.com/ClangBuiltLinux/linux/issues/1158
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 9d7333a36fac..0852a24ee392 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -634,7 +634,7 @@ static void try_disable_dsc(struct drm_atomic_state *state,
for (i = 0; i < count; i++) {
if (vars[i].dsc_enabled
&& vars[i].bpp_x16 ==
params[i].bw_range.max_target_bpp_x16
- && !params[i].clock_force_enable ==
DSC_CLK_FORCE_DEFAULT) {
+ && params[i].clock_force_enable) {
kbps_increase[i] = params[i].bw_range.stream_kbps -
params[i].bw_range.max_kbps;
tried[i] = false;
remaining_to_try += 1;
base-commit: 6651cdf3bfeeaeb499db11668313666bf756579a
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vc4: Fix bitwise OR versus ternary operator in vc4_plane_mode_set
Clang warns:
drivers/gpu/drm/vc4/vc4_plane.c:901:27: warning: operator '?:' has lower
precedence than '|'; '|' will be evaluated first
[-Wbitwise-conditional-parentheses]
fb->format->has_alpha ?
~ ^
drivers/gpu/drm/vc4/vc4_plane.c:901:27: note: place parentheses around
the '|' expression to silence this warning
fb->format->has_alpha ?
~ ^
drivers/gpu/drm/vc4/vc4_plane.c:901:27: note: place parentheses around
the '?:' expression to evaluate it first
fb->format->has_alpha ?
~~^
1 warning generated.
Add the parentheses as that was clearly intended, otherwise
SCALER5_CTL2_ALPHA_PREMULT won't be added to the list.
Fixes: c54619b0bfb3 ("drm/vc4: Add support for the BCM2711 HVS5")
Link: https://github.com/ClangBuiltLinux/linux/issues/1150
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/vc4/vc4_plane.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 24d7e6db6fdd..89543fa8ca4d 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -898,8 +898,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane,
vc4_dlist_write(vc4_state,
VC4_SET_FIELD(state->alpha >> 4,
SCALER5_CTL2_ALPHA) |
- fb->format->has_alpha ?
- SCALER5_CTL2_ALPHA_PREMULT : 0 |
+ (fb->format->has_alpha ?
+ SCALER5_CTL2_ALPHA_PREMULT : 0) |
(mix_plane_alpha ?
SCALER5_CTL2_ALPHA_MIX : 0) |
VC4_SET_FIELD(fb->format->has_alpha ?
base-commit: 8c3c818c23a5bbce6ff180dd2ee04415241df77c
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vc4: Update type of reg parameter in vc4_hdmi_{read, write}
Clang warns 100+ times in the vc4 driver along the lines of:
drivers/gpu/drm/vc4/vc4_hdmi_phy.c:518:13: warning: implicit conversion
from enumeration type 'enum vc4_hdmi_field' to different enumeration
type 'enum vc4_hdmi_regs' [-Wenum-conversion]
HDMI_WRITE(HDMI_TX_PHY_POWERDOWN_CTL,
~~~^~
The HDMI_READ and HDMI_WRITE macros pass in enumerators of type
vc4_hdmi_field but vc4_hdmi_write and vc4_hdmi_read expect a enumerator
of type vc4_hdmi_regs, causing a warning for every instance of this.
Update the parameter type so there is no more mismatch.
Fixes: 311e305fdb4e ("drm/vc4: hdmi: Implement a register layout abstraction")
Link: https://github.com/ClangBuiltLinux/linux/issues/1149
Signed-off-by: Nathan Chancellor
---
Note, the variable names in these functions do not really make much
sense after this patch but attempting to flip the variable names made
everything feel even weirder. Feel free to rewrite this in whatever way
you prefer, I just don't want my builds to be chalk full of warnings :)
drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
index 47364bd3960d..7c6b4818f245 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi_regs.h
@@ -392,7 +392,7 @@ void __iomem *__vc4_hdmi_get_field_base(struct vc4_hdmi
*hdmi,
}
static inline u32 vc4_hdmi_read(struct vc4_hdmi *hdmi,
- enum vc4_hdmi_regs reg)
+ enum vc4_hdmi_field reg)
{
const struct vc4_hdmi_register *field;
const struct vc4_hdmi_variant *variant = hdmi->variant;
@@ -417,7 +417,7 @@ static inline u32 vc4_hdmi_read(struct vc4_hdmi *hdmi,
#define HDMI_READ(reg) vc4_hdmi_read(vc4_hdmi, reg)
static inline void vc4_hdmi_write(struct vc4_hdmi *hdmi,
- enum vc4_hdmi_regs reg,
+ enum vc4_hdmi_field reg,
u32 value)
{
const struct vc4_hdmi_register *field;
base-commit: 8c3c818c23a5bbce6ff180dd2ee04415241df77c
--
2.28.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Tue, Sep 08, 2020 at 11:43:45AM +0200, Christoph Hellwig wrote: > And because I like replying to myself so much, here is a link to the > version with the arm cleanup patch applied. Unlike the previous two > attempts this has at least survived very basic sanity testing: > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-ranges.2 Tested-by: Nathan Chancellor Thank you for fixing this up quickly! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 02, 2020 at 06:11:08PM -0400, Jim Quinlan wrote: > On Wed, Sep 2, 2020 at 5:53 PM Nathan Chancellor > wrote: > > > > On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote: > > > The new field 'dma_range_map' in struct device is used to facilitate the > > > use of single or multiple offsets between mapping regions of cpu addrs and > > > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only > > > capable of holding a single uniform offset and had no region bounds > > > checking. > > > > > > The function of_dma_get_range() has been modified so that it takes a > > > single > > > argument -- the device node -- and returns a map, NULL, or an error code. > > > The map is an array that holds the information regarding the DMA regions. > > > Each range entry contains the address offset, the cpu_start address, the > > > dma_start address, and the size of the region. > > > > > > of_dma_configure() is the typical manner to set range offsets but there > > > are > > > a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel > > > driver code. These cases now invoke the function > > > dma_attach_offset_range(dev, cpu_addr, dma_addr, size). > > > > > > Signed-off-by: Jim Quinlan > > > --- > > > arch/arm/include/asm/dma-mapping.h| 10 +-- > > > arch/arm/mach-keystone/keystone.c | 17 +++-- > > > arch/sh/drivers/pci/pcie-sh7786.c | 9 +-- > > > arch/x86/pci/sta2x11-fixup.c | 7 +- > > > drivers/acpi/arm64/iort.c | 5 +- > > > drivers/base/core.c | 2 + > > > drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +- > > > drivers/iommu/io-pgtable-arm.c| 2 +- > > > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +- > > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +- > > > drivers/of/address.c | 72 +-- > > > drivers/of/device.c | 43 ++- > > > drivers/of/of_private.h | 10 +-- > > > drivers/of/unittest.c | 34 ++--- > > > drivers/remoteproc/remoteproc_core.c | 8 ++- > > > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +- > > > drivers/usb/core/message.c| 9 ++- > > > drivers/usb/core/usb.c| 7 +- > > > include/linux/device.h| 4 +- > > > include/linux/dma-direct.h| 8 +-- > > > include/linux/dma-mapping.h | 36 ++ > > > kernel/dma/coherent.c | 10 +-- > > > kernel/dma/mapping.c | 66 + > > > 23 files changed, 265 insertions(+), 115 deletions(-) > > > > Apologies if this has already been reported or is known but this commit > > is now in next-20200902 and it causes my Raspberry Pi 4 to no longer > > make it to userspace, instead spewing mmc errors: > > > > That commit causes my Raspberry Pi 4 to no longer make it to userspace, > > instead spewing mmc errors: > > > > [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] > > [0.00] Linux version 5.9.0-rc3-4-geef520b232c6-dirty > > (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 > > (https://github.com/llvm/llvm-project.git > > b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 > > (https://github.com/llvm/llvm-project.git > > b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 > > 13:48:49 MST 2020 > > [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 > > ... > > [1.459752] raspberrypi-firmware soc:firmware: Attached to firmware from > > 2020-08-24T18:50:56 > > [1.57] dwc2 fe98.usb: supply vusb_d not found, using dummy > > regulator > > [1.507454] dwc2 fe98.usb: supply vusb_a not found, using dummy > > regulator > > [1.615547] dwc2 fe98.usb: EPs: 8, dedicated fifos, 4080 entries in > > SPRAM > > [1.627537] sdhci-iproc fe30.sdhci: allocated mmc-pwrseq > > [1.665497] mmc0: SDHCI controller on fe30.sdhci [fe30.sdhci] > > using PIO > > [1.690601] mmc0: queuing unknown CIS tuple 0x80 (2 bytes) > > [1.697892] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) > > [1.705173] mmc0: queuing unknown CIS tuple 0
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote: > > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote: > [snip] > > > Hello Nathan, > > > > > > Can you tell me how much memory your RPI has and if all of it is > > > > This is the 4GB version. > > > > > accessible by the PCIe device? Could you also please include the DTS > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the > > > PCIe DT before Linux boots -- could you describe what is going on > > > there? > > > > Unfortunately, I am not familiar with how to get this information. If > > you could provide some instructions for how to do so, I am more than > > happy to. I am not very knowleagable about the inner working of the Pi, > > I mainly use it as a test platform for making sure that LLVM does not > > cause problems on real devices. > > Can you bring the dtc application to your Pi root filesystem, and if so, can > you run the following: > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb Sure, the result is attached. > or cat /sys/firmware/fdt > device.dtb > > and attach the resulting file? > > > > > > Finally, can you attach the text of the full boot log? > > > > I have attached a working and broken boot log. Thank you for the quick > > response! > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by any > chance? Of course. A new log is attached with the debug output from that config. > I have a suspicion that this part of the DTS for the bcm2711.dtsi platform > is at fault: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/bcm2711.dtsi#n264 > > and the resulting dma-ranges parsing is just not working for reasons to be > determined. > -- > Florian Let me know if you need anything else out of me. Cheers, Nathan device.dtb Description: Binary data [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] [0.00] Linux version 5.9.0-rc3-next-20200902-dirty (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 17:41:42 MST 2020 [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 [0.00] efi: UEFI not found. [0.00] Reserved memory: created CMA memory pool at 0x3740, size 64 MiB [0.00] OF: reserved mem: initialized node linux,cma, compatible id shared-dma-pool [0.00] NUMA: No NUMA configuration found [0.00] NUMA: Faking a node at [mem 0x-0xfbff] [0.00] NUMA: NODE_DATA [mem 0xfb81f100-0xfb820fff] [0.00] Zone ranges: [0.00] DMA [mem 0x-0x3fff] [0.00] DMA32[mem 0x4000-0xfbff] [0.00] Normal empty [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0x3b3f] [0.00] node 0: [mem 0x4000-0xfbff] [0.00] Initmem setup node 0 [mem 0x-0xfbff] [0.00] percpu: Embedded 23 pages/cpu s54168 r8192 d31848 u94208 [0.00] Detected PIPT I-cache on CPU0 [0.00] CPU features: detected: EL2 vector hardening [0.00] CPU features: kernel page table isolation forced ON by KASLR [0.00] CPU features: detected: Kernel page table isolation (KPTI) [0.00] ARM_SMCCC_ARCH_WORKAROUND_1 missing from firmware [0.00] CPU features: detected: ARM errata 1165522, 1319367, or 1530923 [0.00] Built 1 zonelists, mobility grouping on. Total pages: 996912 [0.00] Policy zone: DMA32 [0.00] Kernel command line: dma.dmachans=0x71f5 bcm2709.boardrev=0xc03112 bcm2709.serial=0xb78d398 bcm2709.uart_clock=4800 bcm2709.disk_led_gpio=42 bcm2709.disk_led_active_low=0 smsc95xx.macaddr=DC:A6:32:60:6C:87 vc_mem.mem_base=0x3ec0 vc_mem.mem_size=0x4000 console=ttyS1,115200 console=tty1 root=PARTUUID=45a8dd8a-02 rootfstype=ext4 elevator=deadline fsck.repair=yes rootwait plymouth.ignore-serial-consoles [0.00] Kernel parameter elevator= does not have any effect anymore. [0.00] Please use sysfs to set IO scheduler for individual devices. [0.00] Dentry cache hash table entries: 524288 (order: 10, 4194304 bytes, linear) [0.00] Inode-cache hash table entries: 262144 (order: 9, 2097152 bytes, linear) [0.00] mem auto-init: stack:off, heap alloc:off, heap free:off [0.00] software IO TLB: mapped [mem 0x3340-0x3740] (64
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Mon, Aug 24, 2020 at 03:30:20PM -0400, Jim Quinlan wrote: > The new field 'dma_range_map' in struct device is used to facilitate the > use of single or multiple offsets between mapping regions of cpu addrs and > dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only > capable of holding a single uniform offset and had no region bounds > checking. > > The function of_dma_get_range() has been modified so that it takes a single > argument -- the device node -- and returns a map, NULL, or an error code. > The map is an array that holds the information regarding the DMA regions. > Each range entry contains the address offset, the cpu_start address, the > dma_start address, and the size of the region. > > of_dma_configure() is the typical manner to set range offsets but there are > a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel > driver code. These cases now invoke the function > dma_attach_offset_range(dev, cpu_addr, dma_addr, size). > > Signed-off-by: Jim Quinlan > --- > arch/arm/include/asm/dma-mapping.h| 10 +-- > arch/arm/mach-keystone/keystone.c | 17 +++-- > arch/sh/drivers/pci/pcie-sh7786.c | 9 +-- > arch/x86/pci/sta2x11-fixup.c | 7 +- > drivers/acpi/arm64/iort.c | 5 +- > drivers/base/core.c | 2 + > drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +- > drivers/iommu/io-pgtable-arm.c| 2 +- > .../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +- > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +- > drivers/of/address.c | 72 +-- > drivers/of/device.c | 43 ++- > drivers/of/of_private.h | 10 +-- > drivers/of/unittest.c | 34 ++--- > drivers/remoteproc/remoteproc_core.c | 8 ++- > .../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +- > drivers/usb/core/message.c| 9 ++- > drivers/usb/core/usb.c| 7 +- > include/linux/device.h| 4 +- > include/linux/dma-direct.h| 8 +-- > include/linux/dma-mapping.h | 36 ++ > kernel/dma/coherent.c | 10 +-- > kernel/dma/mapping.c | 66 + > 23 files changed, 265 insertions(+), 115 deletions(-) Apologies if this has already been reported or is known but this commit is now in next-20200902 and it causes my Raspberry Pi 4 to no longer make it to userspace, instead spewing mmc errors: That commit causes my Raspberry Pi 4 to no longer make it to userspace, instead spewing mmc errors: [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] [0.00] Linux version 5.9.0-rc3-4-geef520b232c6-dirty (nathan@ubuntu-n2-xlarge-x86) (ClangBuiltLinux clang version 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb), LLD 12.0.0 (https://github.com/llvm/llvm-project.git b21ddded8f04fee925bbf9e6458347104b5b99eb)) #1 SMP PREEMPT Wed Sep 2 13:48:49 MST 2020 [0.00] Machine model: Raspberry Pi 4 Model B Rev 1.2 ... [1.459752] raspberrypi-firmware soc:firmware: Attached to firmware from 2020-08-24T18:50:56 [1.57] dwc2 fe98.usb: supply vusb_d not found, using dummy regulator [1.507454] dwc2 fe98.usb: supply vusb_a not found, using dummy regulator [1.615547] dwc2 fe98.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM [1.627537] sdhci-iproc fe30.sdhci: allocated mmc-pwrseq [1.665497] mmc0: SDHCI controller on fe30.sdhci [fe30.sdhci] using PIO [1.690601] mmc0: queuing unknown CIS tuple 0x80 (2 bytes) [1.697892] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) [1.705173] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) [1.713788] mmc0: queuing unknown CIS tuple 0x80 (7 bytes) [1.721228] mmc0: queuing unknown CIS tuple 0x80 (3 bytes) [1.732062] mmc1: SDHCI controller on fe34.emmc2 [fe34.emmc2] using ADMA [1.741828] ALSA device list: [1.744885] No soundcards found. [1.748540] Waiting for root device PARTUUID=45a8dd8a-02... [1.788865] random: fast init done [1.793489] mmc1: unrecognised SCR structure version 4 [1.798814] mmc1: error -22 whilst initialising SD card [1.813969] mmc0: new high speed SDIO card at address 0001 [1.883178] mmc1: unrecognised SCR structure version 2 [1.888423] mmc1: error -22 whilst initialising SD card [1.964069] mmc1: unrecognised SCR structure version 4 [1.969314] mmc1: error -22 whilst initialising SD card [2.061225] mmc1: unrecognised SCR structure version 4 [2.066470] mmc1: error -22 whilst initialising SD card [3.160476] mmc1: unrecognised SCR structure version 4 [3.165718] mmc1: error -22 whilst initialising SD card This is what it looks like before that
[PATCH] drm/i915/display: Ensure that ret is always initialized in icl_combo_phy_verify_state
Clang warns:
drivers/gpu/drm/i915/display/intel_combo_phy.c:268:3: warning: variable
'ret' is uninitialized when used here [-Wuninitialized]
ret &= check_phy_reg(dev_priv, phy, ICL_PORT_TX_DW8_LN0(phy),
^~~
drivers/gpu/drm/i915/display/intel_combo_phy.c:261:10: note: initialize
the variable 'ret' to silence this warning
bool ret;
^
= 0
1 warning generated.
In practice, the bug this warning appears to be concerned with would not
actually matter because ret gets initialized to the return value of
cnl_verify_procmon_ref_values. However, that does appear to be a bug
since it means the first hunk of the patch this fixes won't actually do
anything (since the values of check_phy_reg won't factor into the final
ret value). Initialize ret to true then make all of the assignments a
bitwise AND with itself so that the function always does what it should
do.
Fixes: 239bef676d8e ("drm/i915/display: Implement new combo phy initialization
step")
Link: https://github.com/ClangBuiltLinux/linux/issues/1094
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/display/intel_combo_phy.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_combo_phy.c
b/drivers/gpu/drm/i915/display/intel_combo_phy.c
index eccaa79cb4a9..a4b8aa6d0a9e 100644
--- a/drivers/gpu/drm/i915/display/intel_combo_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_combo_phy.c
@@ -258,7 +258,7 @@ static bool phy_is_master(struct drm_i915_private
*dev_priv, enum phy phy)
static bool icl_combo_phy_verify_state(struct drm_i915_private *dev_priv,
enum phy phy)
{
- bool ret;
+ bool ret = true;
u32 expected_val = 0;
if (!icl_combo_phy_enabled(dev_priv, phy))
@@ -276,7 +276,7 @@ static bool icl_combo_phy_verify_state(struct
drm_i915_private *dev_priv,
DCC_MODE_SELECT_CONTINUOSLY);
}
- ret = cnl_verify_procmon_ref_values(dev_priv, phy);
+ ret &= cnl_verify_procmon_ref_values(dev_priv, phy);
if (phy_is_master(dev_priv, phy)) {
ret &= check_phy_reg(dev_priv, phy, ICL_PORT_COMP_DW8(phy),
base-commit: ca0e494af5edb59002665bf12871e94b4163a257
--
2.28.0.rc0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/omap: Remove aggregate initialization of new_mode in omap_connector_mode_valid
After commit 42acb06b01b1 ("drm: pahole struct drm_display_mode"), clang
warns:
drivers/gpu/drm/omapdrm/omap_connector.c:92:39: warning: braces around
scalar initializer [-Wbraced-scalar-init]
struct drm_display_mode new_mode = { { 0 } };
^~
1 warning generated.
After the struct was shuffled, the second set of braces is no longer
needed because we are not initializing a structure (struct list_head)
but a regular integer (int clock).
However, looking into it further, this initialization is pointless
because new_mode is used as the destination of drm_mode_copy, where the
members of new_mode will just be completely overwritten with the members
of mode. Just remove the initialization of new_mode so that there is no
more warning and we don't need to worry about updating the
initialization if the structure ever get shuffled again.
Link: https://github.com/ClangBuiltLinux/linux/issues/1059
Suggested-by: Nick Desaulniers
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/omapdrm/omap_connector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c
b/drivers/gpu/drm/omapdrm/omap_connector.c
index 528764566b17..ce4da1511920 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -89,7 +89,7 @@ static enum drm_mode_status omap_connector_mode_valid(struct
drm_connector *conn
struct drm_display_mode *mode)
{
struct omap_connector *omap_connector = to_omap_connector(connector);
- struct drm_display_mode new_mode = { { 0 } };
+ struct drm_display_mode new_mode;
enum drm_mode_status status;
status = omap_connector_mode_fixup(omap_connector->output, mode,
base-commit: 27f11fea33608cbd321a97cbecfa2ef97dcc1821
--
2.27.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: Mark check_shadow_context_ppgtt as maybe unused
When CONFIG_DRM_I915_DEBUG_GEM is not set, clang warns:
drivers/gpu/drm/i915/gvt/scheduler.c:884:1: warning: function
'check_shadow_context_ppgtt' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
check_shadow_context_ppgtt(struct execlist_ring_context *c, struct
intel_vgpu_mm *m)
^
1 warning generated.
This warning is similar to -Wunused-function but rather than warning
that the function is completely unused, it warns that it is used in some
expression within the file but that expression will be evaluated to a
constant or be optimized away in the final assembly, essentially making
it appeared used but really isn't. Usually, this happens when a function
or variable is only used in sizeof, where it will appear to be used but
will be evaluated at compile time and not be required to be emitted.
In this case, the function is only used in GEM_BUG_ON, which is defined
as BUILD_BUG_ON_INVALID, which intentionally follows this pattern. To
fix this warning, add __maybe_unused to make it clear that this is
intentional depending on the configuration.
Fixes: bec3df930fbd ("drm/i915/gvt: Support PPGTT table load command")
Link: https://github.com/ClangBuiltLinux/linux/issues/1027
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/gvt/scheduler.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index f776c92de8d7..0fb1df71c637 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -880,7 +880,7 @@ static void update_guest_pdps(struct intel_vgpu *vgpu,
gpa + i * 8, [7 - i], 4);
}
-static bool
+static __maybe_unused bool
check_shadow_context_ppgtt(struct execlist_ring_context *c, struct
intel_vgpu_mm *m)
{
if (m->ppgtt_mm.root_entry_type == GTT_TYPE_PPGTT_ROOT_L4_ENTRY) {
base-commit: bdecf38f228bcca73b31ada98b5b7ba1215eb9c9
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: Remove duplicate inline specifier on write_pte
When building with clang:
drivers/gpu/drm/i915/gt/gen8_ppgtt.c:392:24: warning: duplicate
'inline' declaration specifier [-Wduplicate-decl-specifier]
declaration specifier [-Wduplicate-decl-specifier]
static __always_inline inline void
^
include/linux/compiler_types.h:138:16: note: expanded from macro
'inline'
#define inline inline __gnu_inline __inline_maybe_unused notrace
^
1 warning generated.
__always_inline is defined as 'inline __attribute__((__always_inline))'
so we do not need to specify it twice.
Fixes: 84eac0c65940 ("drm/i915/gt: Force pte cacheline to main memory")
Link: https://github.com/ClangBuiltLinux/linux/issues/1024
Reported-by: kbuild test robot
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
index 2dc88e76ebec..699125928272 100644
--- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
@@ -389,7 +389,7 @@ static int gen8_ppgtt_alloc(struct i915_address_space *vm,
return err;
}
-static __always_inline inline void
+static __always_inline void
write_pte(gen8_pte_t *pte, const gen8_pte_t val)
{
/* Magic delays? Or can we refine these to flush all in one pass? */
base-commit: e098d7762d602be640c53565ceca342f81e55ad2
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: Avoid integer overflow in amdgpu_device_suspend_display_audio
When building with Clang:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:4160:53: warning: overflow in
expression; result is -294967296 with type 'long' [-Winteger-overflow]
expires = ktime_get_mono_fast_ns() + NSEC_PER_SEC * 4L;
^
1 warning generated.
Multiplication happens first due to order of operations and both
NSEC_PER_SEC and 4 are long literals so the expression overflows. To
avoid this, make 4 an unsigned long long literal, which matches the
type of expires (u64).
Fixes: 3f12acc8d6d4 ("drm/amdgpu: put the audio codec into suspend state before
gpu reset V3")
Link: https://github.com/ClangBuiltLinux/linux/issues/1017
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6f93af972b0a..caa38e7d502e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4157,7 +4157,7 @@ static int amdgpu_device_suspend_display_audio(struct
amdgpu_device *adev)
* the audio controller default autosuspend delay setting.
* 4S used here is guaranteed to cover that.
*/
- expires = ktime_get_mono_fast_ns() + NSEC_PER_SEC * 4L;
+ expires = ktime_get_mono_fast_ns() + NSEC_PER_SEC * 4ULL;
while (!pm_runtime_status_suspended(&(p->dev))) {
if (!pm_runtime_suspend(&(p->dev)))
base-commit: fb9d670f57e3f6478602328bbbf71138be06ca4f
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915/gt: Avoid uninitialized use of rpcurupei in frequency_show
When building with clang + -Wuninitialized:
drivers/gpu/drm/i915/gt/debugfs_gt_pm.c:407:7: warning: variable
'rpcurupei' is uninitialized when used here [-Wuninitialized]
rpcurupei,
^
drivers/gpu/drm/i915/gt/debugfs_gt_pm.c:304:16: note: initialize the
variable 'rpcurupei' to silence this warning
u32 rpcurupei, rpcurup, rpprevup;
^
= 0
1 warning generated.
rpupei is assigned twice; based on the second argument to
intel_uncore_read, it seems this one should have been assigned to
rpcurupei.
Fixes: 9c878557b1eb ("drm/i915/gt: Use the RPM config register to determine clk
frequencies")
Link: https://github.com/ClangBuiltLinux/linux/issues/1016
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/i915/gt/debugfs_gt_pm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c
b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c
index 3d3ef62ed89f..f6ba66206273 100644
--- a/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/debugfs_gt_pm.c
@@ -336,7 +336,7 @@ static int frequency_show(struct seq_file *m, void *unused)
rpdeclimit = intel_uncore_read(uncore, GEN6_RP_DOWN_THRESHOLD);
rpstat = intel_uncore_read(uncore, GEN6_RPSTAT1);
- rpupei = intel_uncore_read(uncore, GEN6_RP_CUR_UP_EI) &
GEN6_CURICONT_MASK;
+ rpcurupei = intel_uncore_read(uncore, GEN6_RP_CUR_UP_EI) &
GEN6_CURICONT_MASK;
rpcurup = intel_uncore_read(uncore, GEN6_RP_CUR_UP) &
GEN6_CURBSYTAVG_MASK;
rpprevup = intel_uncore_read(uncore, GEN6_RP_PREV_UP) &
GEN6_CURBSYTAVG_MASK;
rpcurdownei = intel_uncore_read(uncore, GEN6_RP_CUR_DOWN_EI) &
GEN6_CURIAVG_MASK;
base-commit: 0fd02a5d3eb7020a7e1801f8d7f01891071c85e4
--
2.26.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: re-disable -Wframe-address
On Sun, Apr 26, 2020 at 02:42:15PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > The top level Makefile disables this warning. When building an > i386_defconfig with Clang, this warning is triggered a whole bunch via > includes of headers from perf. > > Link: https://github.com/ClangBuiltLinux/continuous-integration/pull/182 > Signed-off-by: Nick Desaulniers This is not technically a clang specific warning but I assume it is only visible with clang so this location is probably fine. Reviewed-by: Nathan Chancellor ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/legacy: Fix type for drm_local_map.offset
On Thu, Apr 02, 2020 at 10:59:26PM +0100, Chris Wilson wrote:
> drm_local_map.offset is not only used for resource_size_t but also
> dma_addr_t which may be of different sizes.
>
> Reported-by: Nathan Chancellor
> Fixes: 8e4ff9b56957 ("drm: Remove the dma_alloc_coherent wrapper for internal
> usage")
> Signed-off-by: Chris Wilson
> Cc: Dave Airlie
> Cc: Nathan Chancellor
> Cc: Linus Torvalds
> ---
> include/drm/drm_legacy.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h
> index dcef3598f49e..aed382c17b26 100644
> --- a/include/drm/drm_legacy.h
> +++ b/include/drm/drm_legacy.h
> @@ -136,7 +136,7 @@ struct drm_sg_mem {
> * Kernel side of a mapping
> */
> struct drm_local_map {
> - resource_size_t offset; /**< Requested physical address (0 for SAREA)*/
> + dma_addr_t offset; /**< Requested physical address (0 for SAREA)*/
> unsigned long size; /**< Requested physical size (bytes) */
> enum drm_map_type type; /**< Type of memory to map */
> enum drm_map_flags flags;/**< Flags */
> --
> 2.20.1
>
Thanks for the quick fix!
I ran it through my set of build tests and nothing else seems to have
broken (at least not any more than it already is...).
Tested-by: Nathan Chancellor # build
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for 5.7-rc1
On Wed, Apr 01, 2020 at 03:50:42PM +1000, Dave Airlie wrote:
> Hey Linus,
>
> This is the main drm pull request for 5.7-rc1.
>
> Writing the changelog seemed a bit quieter, but it looks about average.
>
> I've got a separate merge for some VM interactions for TTM huge pages, but
> I'll
> send that once this is landed.
>
> It didn't seem to have many major conflicts, but my git trees have a bad habit
> of finding the shared rerere cache and lulling me into thinking it merged
> fine.
>
> Highlights:
> i915 enables Tigerlake by default
> i915 and amdgpu have initial OLED backlight support
> vmwgfx add support to enable OpenGL 4 userspace
> zero length arrays are mostly removed.
>
> Regards,
> Dave.
> drm: Remove the dma_alloc_coherent wrapper for internal usage
This patch causes a build regression on arm32 in certain configurations
(I found it with Debian's armmp config).
$ printf 'CONFIG_XEN=y\nCONFIG_DRM_LEGACY=y\n' >>
arch/arm/configs/multi_v7_defconfig
$ make -j$(nproc) -s ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- defconfig
drivers/gpu/drm/drm_bufs.o
drivers/gpu/drm/drm_bufs.c: In function 'drm_addmap_core':
drivers/gpu/drm/drm_bufs.c:328:8: error: passing argument 3 of
'dma_alloc_coherent' from incompatible pointer type
[-Werror=incompatible-pointer-types]
328 |>offset,
|^~~~
||
|resource_size_t * {aka unsigned int *}
In file included from ./include/linux/pci-dma-compat.h:8,
from ./include/linux/pci.h:2392,
from ./include/drm/drm_pci.h:35,
from drivers/gpu/drm/drm_bufs.c:46:
./include/linux/dma-mapping.h:642:15: note: expected 'dma_addr_t *' {aka 'long
long unsigned int *'} but argument is of type 'resource_size_t *' {aka
'unsigned int *'}
642 | dma_addr_t *dma_handle, gfp_t gfp)
| ^~
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:268: drivers/gpu/drm/drm_bufs.o] Error 1
make[3]: *** [scripts/Makefile.build:505: drivers/gpu/drm] Error 2
make[2]: *** [scripts/Makefile.build:505: drivers/gpu] Error 2
make[1]: *** [Makefile:1703: drivers] Error 2
make: *** [Makefile:328: __build_one_by_one] Error 2
This fixes it but I am not sure if it is proper or not (could be
problematic if CONFIG_PHYS_ADDR_T_64BIT is set but
CONFIG_ARCH_DMA_ADDR_T_64BIT is not, not sure if that is possible) so I
figured I'd report it and let you guys deal with it.
Cheers,
Nathan
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index dcabf5698333..9282fd075424 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -325,7 +325,7 @@ static int drm_addmap_core(struct drm_device *dev,
resource_size_t offset,
* need to point to a 64bit variable first. */
map->handle = dma_alloc_coherent(>pdev->dev,
map->size,
->offset,
+(dma_addr_t *)>offset,
GFP_KERNEL);
if (!map->handle) {
kfree(map);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: Fix 64-bit division error on 32-bit platforms in mod_freesync_build_vrr_params
When building arm32 allyesconfig,
ld.lld: error: undefined symbol: __aeabi_uldivmod
>>> referenced by freesync.c
>>>
>>> gpu/drm/amd/display/modules/freesync/freesync.o:(mod_freesync_build_vrr_params)
>>> in archive drivers/built-in.a
>>> did you mean: __aeabi_uidivmod
>>> defined in: arch/arm/lib/lib.a(lib1funcs.o)
Use div_u64 in the two locations that do 64-bit divisior, which both
have a u64 dividend and u32 divisor.
Fixes: 349a370781de ("drm/amd/display: LFC not working on 2.0x range monitors")
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index 8911f01671aa..c33454a9e0b4 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -761,10 +761,10 @@ void mod_freesync_build_vrr_params(struct mod_freesync
*mod_freesync,
// If a monitor reports exactly max refresh of 2x of min, enforce it on
nominal
rounded_nominal_in_uhz =
- ((nominal_field_rate_in_uhz + 5) / 10) * 10;
+ div_u64(nominal_field_rate_in_uhz + 5, 10) *
10;
if (in_config->max_refresh_in_uhz == (2 *
in_config->min_refresh_in_uhz) &&
in_config->max_refresh_in_uhz == rounded_nominal_in_uhz)
- min_refresh_in_uhz = nominal_field_rate_in_uhz / 2;
+ min_refresh_in_uhz = div_u64(nominal_field_rate_in_uhz, 2);
if (!vrr_settings_require_update(core_freesync,
in_config, (unsigned int)min_refresh_in_uhz, (unsigned
int)max_refresh_in_uhz,
--
2.26.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Cast remain to unsigned long in eb_relocate_vma
On Mon, Mar 16, 2020 at 02:41:23PM -0700, Nick Desaulniers wrote: > On Fri, Feb 14, 2020 at 7:36 AM Michel Dänzer wrote: > > > > On 2020-02-14 12:49 p.m., Jani Nikula wrote: > > > On Fri, 14 Feb 2020, Chris Wilson wrote: > > >> Quoting Jani Nikula (2020-02-14 06:36:15) > > >>> On Thu, 13 Feb 2020, Nathan Chancellor wrote: > > >>>> A recent commit in clang added -Wtautological-compare to -Wall, which > > >>>> is > > >>>> enabled for i915 after -Wtautological-compare is disabled for the rest > > >>>> of the kernel so we see the following warning on x86_64: > > >>>> > > >>>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1433:22: warning: > > >>>> result of comparison of constant 576460752303423487 with expression of > > >>>> type 'unsigned int' is always false > > >>>> [-Wtautological-constant-out-of-range-compare] > > >>>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > > >>>> ^ > > >>>> ../include/linux/compiler.h:78:42: note: expanded from macro > > >>>> 'unlikely' > > >>>> # define unlikely(x)__builtin_expect(!!(x), 0) > > >>>> ^ > > >>>> 1 warning generated. > > >>>> > > >>>> It is not wrong in the case where ULONG_MAX > UINT_MAX but it does not > > >>>> account for the case where this file is built for 32-bit x86, where > > >>>> ULONG_MAX == UINT_MAX and this check is still relevant. > > >>>> > > >>>> Cast remain to unsigned long, which keeps the generated code the same > > >>>> (verified with clang-11 on x86_64 and GCC 9.2.0 on x86 and x86_64) and > > >>>> the warning is silenced so we can catch more potential issues in the > > >>>> future. > > >>>> > > >>>> Link: https://github.com/ClangBuiltLinux/linux/issues/778 > > >>>> Suggested-by: Michel Dänzer > > >>>> Signed-off-by: Nathan Chancellor > > >>> > > >>> Works for me as a workaround, > > >> > > >> But the whole point was that the compiler could see that it was > > >> impossible and not emit the code. Doesn't this break that? > > > > > > It seems that goal and the warning are fundamentally incompatible. > > > > Not really: > > > > if (sizeof(remain) >= sizeof(unsigned long) && > > unlikely(remain > N_RELOC(ULONG_MAX))) > > return -EINVAL; > > > > In contrast to the cast, this doesn't generate any machine code on 64-bit: > > > > https://godbolt.org/z/GmUE4S > > > > but still generates the same code on 32-bit: > > > > https://godbolt.org/z/hAoz8L > > Exactly. > > This check is only a tautology when `sizeof(long) == sizeof(int)` (ie. > ILP32 platforms, like 32b x86), notice how BOTH GCC AND Clang generate > exactly the same code: https://godbolt.org/z/6ShrDM > > Both compilers eliminate the check when `-m32` is not set, and > generate the exact same check otherwise. How about: > ``` > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index d3f4f28e9468..25b9d3f3ad57 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -1415,8 +1415,10 @@ static int eb_relocate_vma(struct > i915_execbuffer *eb, struct eb_vma *ev) > > urelocs = u64_to_user_ptr(entry->relocs_ptr); > remain = entry->relocation_count; > +#ifndef CONFIG_64BIT > if (unlikely(remain > N_RELOC(ULONG_MAX))) > return -EINVAL; > +#endif > > /* > * We must check that the entire relocation array is safe > ``` > > We now have 4 proposed solutions: > 1. > https://lore.kernel.org/lkml/20191123195321.41305-1-natechancel...@gmail.com/ > 2. > https://lore.kernel.org/lkml/20200211050808.29463-1-natechancel...@gmail.com/ > 3. > https://lore.kernel.org/lkml/20200214054706.33870-1-natechancel...@gmail.com/ > 4. my diff above > Let's please come to a resolution on this. This is the only warning on an x86_64 defconfig build. Apologies if we are being too persistent or nagging but we need guidance from the i915 maintainers on which solution they would prefer so it can be picked up. I understand you all are busy and I appreciate the work you all do but I do not want this to fall between the cracks because it is annoying to constantly see this warning. Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/amdgpu: Remove unnecessary variable shadow in gfx_v9_0_rlcg_wreg
clang warns:
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:6: warning: variable 'shadow'
is used uninitialized whenever 'if' condition is
false [-Wsometimes-uninitialized]
if (offset == grbm_cntl || offset == grbm_idx)
^
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:757:6: note: uninitialized use
occurs here
if (shadow) {
^~
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:2: note: remove the 'if' if
its condition is always true
if (offset == grbm_cntl || offset == grbm_idx)
^~
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:738:13: note: initialize the
variable 'shadow' to silence this warning
bool shadow;
^
= 0
1 warning generated.
shadow is only assigned in one condition and used as the condition for
another if statement; combine the two if statements and remove shadow
to make the code cleaner and resolve this warning.
Fixes: 2e0cc4d48b91 ("drm/amdgpu: revise RLCG access path")
Link: https://github.com/ClangBuiltLinux/linux/issues/936
Suggested-by: Joe Perches
Reviewed-by: Nick Desaulniers
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Remove shadow altogether, as suggested by Joe Perches.
* Add Nick's Reviewed-by, as I assume it still stands.
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 7bc2486167e7..496b9edca3c3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -735,7 +735,6 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32
offset, u32 v)
static void *spare_int;
static uint32_t grbm_cntl;
static uint32_t grbm_idx;
- bool shadow;
scratch_reg0 = adev->rmmio +
(adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG0_BASE_IDX] + mmSCRATCH_REG0)*4;
scratch_reg1 = adev->rmmio +
(adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG1_BASE_IDX] + mmSCRATCH_REG1)*4;
@@ -751,10 +750,7 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32
offset, u32 v)
return;
}
- if (offset == grbm_cntl || offset == grbm_idx)
- shadow = true;
-
- if (shadow) {
+ if (offset == grbm_cntl || offset == grbm_idx) {
if (offset == grbm_cntl)
writel(v, scratch_reg2);
else if (offset == grbm_idx)
--
2.26.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: Initialize shadow to false in gfx_v9_0_rlcg_wreg
clang warns:
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:6: warning: variable 'shadow'
is used uninitialized whenever 'if' condition is
false [-Wsometimes-uninitialized]
if (offset == grbm_cntl || offset == grbm_idx)
^
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:757:6: note: uninitialized use
occurs here
if (shadow) {
^~
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:2: note: remove the 'if' if
its condition is always true
if (offset == grbm_cntl || offset == grbm_idx)
^~
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:738:13: note: initialize the
variable 'shadow' to silence this warning
bool shadow;
^
= 0
1 warning generated.
It is not wrong so initialize shadow to false to ensure shadow is always
used initialized.
Fixes: 2e0cc4d48b91 ("drm/amdgpu: revise RLCG access path")
Link: https://github.com/ClangBuiltLinux/linux/issues/936
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 7bc2486167e7..affbff76758c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -735,7 +735,7 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32
offset, u32 v)
static void *spare_int;
static uint32_t grbm_cntl;
static uint32_t grbm_idx;
- bool shadow;
+ bool shadow = false;
scratch_reg0 = adev->rmmio +
(adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG0_BASE_IDX] + mmSCRATCH_REG0)*4;
scratch_reg1 = adev->rmmio +
(adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG1_BASE_IDX] + mmSCRATCH_REG1)*4;
--
2.26.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: Remove pointless NULL checks in dmub_psr_copy_settings
Clang warns:
drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dmub_psr.c:147:31: warning:
address of 'pipe_ctx->plane_res' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (!pipe_ctx || !_ctx->plane_res || !_ctx->stream_res)
~ ~~^
drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dmub_psr.c:147:56: warning:
address of 'pipe_ctx->stream_res' will always evaluate to 'true'
[-Wpointer-bool-conversion]
if (!pipe_ctx || !_ctx->plane_res || !_ctx->stream_res)
~ ~~^~
2 warnings generated.
As long as pipe_ctx is not NULL, the address of members in this struct
cannot be NULL, which means these checks will always evaluate to false.
Fixes: 4c1a1335dfe0 ("drm/amd/display: Driverside changes to support PSR in
DMCUB")
Link: https://github.com/ClangBuiltLinux/linux/issues/915
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
index 2c932c29f1f9..a9e1c01e9d9b 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dmub_psr.c
@@ -144,7 +144,7 @@ static bool dmub_psr_copy_settings(struct dmub_psr *dmub,
}
}
- if (!pipe_ctx || !_ctx->plane_res || !_ctx->stream_res)
+ if (!pipe_ctx)
return false;
// First, set the psr version
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Cast remain to unsigned long in eb_relocate_vma
On Fri, Feb 14, 2020 at 08:32:19AM +, Chris Wilson wrote: > Quoting Jani Nikula (2020-02-14 06:36:15) > > On Thu, 13 Feb 2020, Nathan Chancellor wrote: > > > A recent commit in clang added -Wtautological-compare to -Wall, which is > > > enabled for i915 after -Wtautological-compare is disabled for the rest > > > of the kernel so we see the following warning on x86_64: > > > > > > ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1433:22: warning: > > > result of comparison of constant 576460752303423487 with expression of > > > type 'unsigned int' is always false > > > [-Wtautological-constant-out-of-range-compare] > > > if (unlikely(remain > N_RELOC(ULONG_MAX))) > > > ^ > > > ../include/linux/compiler.h:78:42: note: expanded from macro 'unlikely' > > > # define unlikely(x)__builtin_expect(!!(x), 0) > > > ^ > > > 1 warning generated. > > > > > > It is not wrong in the case where ULONG_MAX > UINT_MAX but it does not > > > account for the case where this file is built for 32-bit x86, where > > > ULONG_MAX == UINT_MAX and this check is still relevant. > > > > > > Cast remain to unsigned long, which keeps the generated code the same > > > (verified with clang-11 on x86_64 and GCC 9.2.0 on x86 and x86_64) and > > > the warning is silenced so we can catch more potential issues in the > > > future. > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/778 > > > Suggested-by: Michel Dänzer > > > Signed-off-by: Nathan Chancellor > > > > Works for me as a workaround, > > But the whole point was that the compiler could see that it was > impossible and not emit the code. Doesn't this break that? > -Chris As noted in the commit message, I ran diff <(objdump -Dr) <(objdump -Dr) on objects files compiled with and without the patch with clang and gcc for x86_64 and gcc for i386 (i386 does not build with clang) and there was zero difference aside from the file names. At the end of the day, I do not really care how the warning get fixed, just that it does since it is the only one on x86_64 defconfig. Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
On Thu, Feb 13, 2020 at 04:37:15PM +0200, Jani Nikula wrote: > On Wed, 12 Feb 2020, Michel Dänzer wrote: > > On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: > >> On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: > >>> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: > >>>> On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > >>>>> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > >>>>>> A recent commit in clang added -Wtautological-compare to -Wall, which > >>>>>> is > >>>>>> enabled for i915 so we see the following warning: > >>>>>> > >>>>>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > >>>>>> result of comparison of constant 576460752303423487 with expression of > >>>>>> type 'unsigned int' is always false > >>>>>> [-Wtautological-constant-out-of-range-compare] > >>>>>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > >>>>>> ^ > >>>>>> > >>>>>> This warning only happens on x86_64 but that check is relevant for > >>>>>> 32-bit x86 so we cannot remove it. > >>>>> > >>>>> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > >>>>> in both cases, and remain is a 32-bit value in both cases. How can it be > >>>>> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > >>>>> > >>>> > >>>> Hi Michel, > >>>> > >>>> Can't this condition be true when UINT_MAX == ULONG_MAX? > >>> > >>> Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. > >>> > >>> > >>> Anyway, this suggests a possible better solution: > >>> > >>> #if UINT_MAX == ULONG_MAX > >>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > >>> return -EINVAL; > >>> #endif > >>> > >>> > >>> Or if that can't be used for some reason, something like > >>> > >>> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) > >>> return -EINVAL; > >>> > >>> should silence the warning. > >> > >> I do like this one better than the former. > > > > FWIW, one downside of this one compared to all alternatives (presumably) > > is that it might end up generating actual code even on 64-bit, which > > always ends up skipping the return. > > I like this better than the UINT_MAX == ULONG_MAX comparison because > that creates a dependency on the type of remain. > > Then again, a sufficiently clever compiler could see through the cast, > and flag the warning anyway... Would you prefer a patch that adds that cast rather than silencing the warning outright? It does appear to work for clang. Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: Cast remain to unsigned long in eb_relocate_vma
A recent commit in clang added -Wtautological-compare to -Wall, which is enabled for i915 after -Wtautological-compare is disabled for the rest of the kernel so we see the following warning on x86_64: ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1433:22: warning: result of comparison of constant 576460752303423487 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare] if (unlikely(remain > N_RELOC(ULONG_MAX))) ^ ../include/linux/compiler.h:78:42: note: expanded from macro 'unlikely' # define unlikely(x)__builtin_expect(!!(x), 0) ^ 1 warning generated. It is not wrong in the case where ULONG_MAX > UINT_MAX but it does not account for the case where this file is built for 32-bit x86, where ULONG_MAX == UINT_MAX and this check is still relevant. Cast remain to unsigned long, which keeps the generated code the same (verified with clang-11 on x86_64 and GCC 9.2.0 on x86 and x86_64) and the warning is silenced so we can catch more potential issues in the future. Link: https://github.com/ClangBuiltLinux/linux/issues/778 Suggested-by: Michel Dänzer Signed-off-by: Nathan Chancellor --- Round 3 :) Previous threads/patches: https://lore.kernel.org/lkml/20191123195321.41305-1-natechancel...@gmail.com/ https://lore.kernel.org/lkml/20200211050808.29463-1-natechancel...@gmail.com/ drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 60c984e10c4a..47f4d8ab281e 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -1430,7 +1430,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma) urelocs = u64_to_user_ptr(entry->relocs_ptr); remain = entry->relocation_count; - if (unlikely(remain > N_RELOC(ULONG_MAX))) + if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) return -EINVAL; /* -- 2.25.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
On Thu, Feb 13, 2020 at 02:43:21PM -0800, Nick Desaulniers wrote: > On Wed, Feb 12, 2020 at 9:17 AM Michel Dänzer wrote: > > > > On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: > > > On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: > > >> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: > > >>> On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > > >>>> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > > >>>>> A recent commit in clang added -Wtautological-compare to -Wall, which > > >>>>> is > > >>>>> enabled for i915 so we see the following warning: > > >>>>> > > >>>>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > > >>>>> result of comparison of constant 576460752303423487 with expression of > > >>>>> type 'unsigned int' is always false > > >>>>> [-Wtautological-constant-out-of-range-compare] > > >>>>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > > >>>>> ^ > > >>>>> > > >>>>> This warning only happens on x86_64 but that check is relevant for > > >>>>> 32-bit x86 so we cannot remove it. > > >>>> > > >>>> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > > >>>> in both cases, and remain is a 32-bit value in both cases. How can it > > >>>> be > > >>>> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > > >>>> > > >>> > > >>> Hi Michel, > > >>> > > >>> Can't this condition be true when UINT_MAX == ULONG_MAX? > > >> > > >> Oh, right, I think I was wrongly thinking long had 64 bits even on > > >> 32-bit. > > >> > > >> > > >> Anyway, this suggests a possible better solution: > > >> > > >> #if UINT_MAX == ULONG_MAX > > >> if (unlikely(remain > N_RELOC(ULONG_MAX))) > > >> return -EINVAL; > > >> #endif > > >> > > >> > > >> Or if that can't be used for some reason, something like > > >> > > >> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) > > >> return -EINVAL; > > >> > > >> should silence the warning. > > > > > > I do like this one better than the former. > > > > FWIW, one downside of this one compared to all alternatives (presumably) > > is that it might end up generating actual code even on 64-bit, which > > always ends up skipping the return. > > The warning is pointing out that the conditional is always false, > which is correct on 64b. The check is only active for 32b. > https://godbolt.org/z/oQrgT_ > The cast silences the warning for 64b. (Note that GCC and Clang also > generate precisely the same instruction sequences in my example, just > GCC doesn't warn on such tautologies). Thanks for confirming! I'll send a patch to add the cast later tonight. Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: Don't take the address of skip_scdc_overwrite in dc_link_detect_helper
Clang warns:
../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:980:36:
warning: address of 'sink->edid_caps.panel_patch.skip_scdc_overwrite'
will always evaluate to 'true' [-Wpointer-bool-conversion]
if (>edid_caps.panel_patch.skip_scdc_overwrite)
~~ ^~~
1 warning generated.
This is probably not what was intended so remove the address of
operator, which matches how skip_scdc_overwrite is handled in the rest
of the driver.
While we're here, drop an extra newline after this if block.
Fixes: a760fc1bff03 ("drm/amd/display: add monitor patch to disable SCDC
read/write")
Link: https://github.com/ClangBuiltLinux/linux/issues/879
Signed-off-by: Nathan Chancellor
---
As an aside, I don't see skip_scdc_overwrite assigned a value anywhere,
is this working as intended?
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 24d99849be5e..a3bfa05c545e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -977,10 +977,9 @@ static bool dc_link_detect_helper(struct dc_link *link,
if ((prev_sink != NULL) && ((edid_status == EDID_THE_SAME) ||
(edid_status == EDID_OK)))
same_edid = is_same_edid(_sink->dc_edid,
>dc_edid);
- if (>edid_caps.panel_patch.skip_scdc_overwrite)
+ if (sink->edid_caps.panel_patch.skip_scdc_overwrite)
link->ctx->dc->debug.hdmi20_disable = true;
-
if (link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT &&
sink_caps.transaction_type ==
DDC_TRANSACTION_TYPE_I2C_OVER_AUX) {
/*
--
2.25.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: > On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: > > On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > >> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > >>> A recent commit in clang added -Wtautological-compare to -Wall, which is > >>> enabled for i915 so we see the following warning: > >>> > >>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > >>> result of comparison of constant 576460752303423487 with expression of > >>> type 'unsigned int' is always false > >>> [-Wtautological-constant-out-of-range-compare] > >>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > >>> ^ > >>> > >>> This warning only happens on x86_64 but that check is relevant for > >>> 32-bit x86 so we cannot remove it. > >> > >> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > >> in both cases, and remain is a 32-bit value in both cases. How can it be > >> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > >> > > > > Hi Michel, > > > > Can't this condition be true when UINT_MAX == ULONG_MAX? > > Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. > > > Anyway, this suggests a possible better solution: > > #if UINT_MAX == ULONG_MAX > if (unlikely(remain > N_RELOC(ULONG_MAX))) > return -EINVAL; > #endif > > > Or if that can't be used for some reason, something like > > if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) > return -EINVAL; > > should silence the warning. I do like this one better than the former. > > > Either of these should be better than completely disabling the warning > for the whole file. Normally, I would agree but I am currently planning to leave -Wtautological-constant-out-of-range-compare disabled when I turn on -Wtautological-compare for the whole kernel because there are plenty of locations in the kernel where these kind of checks depend on various kernel configuration options and the general attitude of kernel developers is that this particular warning is not really helpful for that reason. I'll see if there is a general consensus before moving further since I know i915 turns on a bunch of extra warnings from the rest of the kernel (hence why we are in this situation). Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > > A recent commit in clang added -Wtautological-compare to -Wall, which is > > enabled for i915 so we see the following warning: > > > > ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > > result of comparison of constant 576460752303423487 with expression of > > type 'unsigned int' is always false > > [-Wtautological-constant-out-of-range-compare] > > if (unlikely(remain > N_RELOC(ULONG_MAX))) > > ^ > > > > This warning only happens on x86_64 but that check is relevant for > > 32-bit x86 so we cannot remove it. > > That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > in both cases, and remain is a 32-bit value in both cases. How can it be > larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > Hi Michel, Can't this condition be true when UINT_MAX == ULONG_MAX? clang does not warn on a 32-bit x86 build from what I remember. Honestly, my understanding of overflow is pretty shoddy, this is mostly based on what I have heard from others. I sent a patch trying to remove that check but had it rejected: https://lore.kernel.org/lkml/20191123195321.41305-1-natechancel...@gmail.com/ Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: Disable -Wtautological-constant-out-of-range-compare
A recent commit in clang added -Wtautological-compare to -Wall, which is enabled for i915 so we see the following warning: ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: result of comparison of constant 576460752303423487 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare] if (unlikely(remain > N_RELOC(ULONG_MAX))) ^ This warning only happens on x86_64 but that check is relevant for 32-bit x86 so we cannot remove it. -Wtautological-compare on a whole has good warnings but this one is not really relevant for the kernel because of all of the different configurations that are used to build the kernel. When -Wtautological-compare is enabled for the kernel, this option will remain disabled so do that for i915 now. Link: https://github.com/ClangBuiltLinux/linux/issues/778 Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/i915/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 38df01c23176..55dbcca179c7 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -21,6 +21,7 @@ subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable) subdir-ccflags-y += $(call cc-disable-warning, sign-compare) subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized) subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides) +subdir-ccflags-y += $(call cc-disable-warning, tautological-constant-out-of-range-compare) subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror # Fine grained warnings disable -- 2.25.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
A recent commit in clang added -Wtautological-compare to -Wall, which is enabled for i915 so we see the following warning: ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: result of comparison of constant 576460752303423487 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare] if (unlikely(remain > N_RELOC(ULONG_MAX))) ^ This warning only happens on x86_64 but that check is relevant for 32-bit x86 so we cannot remove it. -Wtautological-compare on a whole has good warnings but this one is not really relevant for the kernel because of all of the different configurations that are used to build the kernel. When -Wtautological-compare is enabled for the kernel, this option will remain disabled so do that for i915 now. Link: https://github.com/ClangBuiltLinux/linux/issues/778 Signed-off-by: Nathan Chancellor --- v1 -> v2: https://lore.kernel.org/lkml/20200211050808.29463-1-natechancel...@gmail.com/ * Fix patch application due to basing on a local tree that had -Wuninitialized turned on. Can confirm that patch applies on latest -next now. drivers/gpu/drm/i915/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index b8c5f8934dbd..159355eb43a9 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -22,6 +22,7 @@ subdir-ccflags-y += $(call cc-disable-warning, sign-compare) subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized) subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides) subdir-ccflags-y += $(call cc-disable-warning, uninitialized) +subdir-ccflags-y += $(call cc-disable-warning, tautological-constant-out-of-range-compare) subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror # Fine grained warnings disable -- 2.25.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: Fix implicit enum conversion in gfx_v9_4_ras_error_inject
Clang warns:
../drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c:967:35: warning: implicit
conversion from enumeration type 'enum amdgpu_ras_block' to different
enumeration type 'enum ta_ras_block' [-Wenum-conversion]
block_info.block_id = info->head.block;
~ ~~~^
1 warning generated.
Use the function added in commit 828cfa29093f ("drm/amdgpu: Fix amdgpu
ras to ta enums conversion") that handles this conversion explicitly.
Fixes: 4c461d89db4f ("drm/amdgpu: add RAS support for the gfx block of
Arcturus")
Link: https://github.com/ClangBuiltLinux/linux/issues/849
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
index e19d275f3f7d..f099f13d7f1e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_4.c
@@ -964,7 +964,7 @@ int gfx_v9_4_ras_error_inject(struct amdgpu_device *adev,
void *inject_if)
if (!amdgpu_ras_is_supported(adev, AMDGPU_RAS_BLOCK__GFX))
return -EINVAL;
- block_info.block_id = info->head.block;
+ block_info.block_id = amdgpu_ras_block_to_ta(info->head.block);
block_info.sub_block_index = info->head.sub_block_index;
block_info.inject_error_type = amdgpu_ras_error_to_ta(info->head.type);
block_info.address = info->address;
--
2.25.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: Remove tautological compare in eb_relocate_vma
On Tue, Dec 03, 2019 at 10:45:22AM -0800, Nick Desaulniers wrote: > On Tue, Dec 3, 2019 at 5:42 AM Chris Wilson wrote: > > > > Quoting Nick Desaulniers (2019-12-02 19:18:20) > > > On Sat, Nov 23, 2019 at 12:05 PM Chris Wilson > > > wrote: > > > > > > > > Quoting Nathan Chancellor (2019-11-23 19:53:22) > > > > > -Wtautological-compare was recently added to -Wall in LLVM, which > > > > > exposed an if statement in i915 that is always false: > > > > > > > > > > ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > > > > > result of comparison of constant 576460752303423487 with expression of > > > > > type 'unsigned int' is always false > > > > > [-Wtautological-constant-out-of-range-compare] > > > > > if (unlikely(remain > N_RELOC(ULONG_MAX))) > > > > > ^ > > > > > > > > > > Since remain is an unsigned int, it can never be larger than UINT_MAX, > > > > > which is less than ULONG_MAX / sizeof(struct > > > > > drm_i915_gem_relocation_entry). > > > > > Remove this statement to fix the warning. > > > > > > > > The check should remain as we do want to document the overflow > > > > calculation, and it should represent the types used -- it's much easier > > > > > > What do you mean "represent the types used?" Are you concerned that > > > the type of drm_i915_gem_exec_object2->relocation_count might change > > > in the future? > > > > We may want to change the restriction, yes. > > > > > > to review a stub than trying to find a missing overflow check. If the > > > > overflow cannot happen as the types are wide enough, no problem, the > > > > compiler can remove the known false branch. > > > > > > What overflow are you trying to protect against here? > > > > These values are under user control, our validation steps should be > > clear and easy to check. If we have the types wrong, if the checks are > > wrong, we need to fix them. If the code is removed because it can be > > evaluated by the compiler to be redundant, it is much harder for us to > > verify that we have tried to validate user input. > > > > > > Tautology here has a purpose for conveying information to the reader. > > > > > > Well leaving a warning unaddressed is also not a solution. Either > > > replace it with a comment or turn off the warning for your subdir. > > > > My personal preference would be to use a bunch of central macros for the > > various type/kmalloc overflows, and have the warnings suppressed there > > since they are very much about documenting user input validation. > > -Chris > > Is kmalloc_array what you're looking for? Looks like it has the > `check_mul_overflow` call in it. I don't think kmalloc_array is right because we are not validating an allocation. I am not sure that any of these overflow macros are correct, we would probably need something new but I am not sure. Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] fbcon: Adjust indentation in set_con2fb_map
Clang warns:
../drivers/video/fbdev/core/fbcon.c:915:3: warning: misleading
indentation; statement is not part of the previous 'if'
[-Wmisleading-indentation]
return err;
^
../drivers/video/fbdev/core/fbcon.c:912:2: note: previous statement is
here
if (!search_fb_in_map(info_idx))
^
1 warning generated.
This warning occurs because there is a space before the tab on this
line. This happens on several lines in this function; normalize them
so that the indentation is consistent with the Linux kernel coding
style and clang no longer warns.
This warning was introduced before the beginning of git history so no
fixes tab.
Link: https://github.com/ClangBuiltLinux/linux/issues/824
Signed-off-by: Nathan Chancellor
---
drivers/video/fbdev/core/fbcon.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index c9235a2f42f8..9d2c43e345a4 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -866,7 +866,7 @@ static int set_con2fb_map(int unit, int newidx, int user)
int oldidx = con2fb_map[unit];
struct fb_info *info = registered_fb[newidx];
struct fb_info *oldinfo = NULL;
- int found, err = 0;
+ int found, err = 0;
WARN_CONSOLE_UNLOCKED();
@@ -888,31 +888,30 @@ static int set_con2fb_map(int unit, int newidx, int user)
con2fb_map[unit] = newidx;
if (!err && !found)
- err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
-
+ err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
/*
* If old fb is not mapped to any of the consoles,
* fbcon should release it.
*/
- if (!err && oldinfo && !search_fb_in_map(oldidx))
- err = con2fb_release_oldinfo(vc, oldinfo, info, unit, oldidx,
-found);
+ if (!err && oldinfo && !search_fb_in_map(oldidx))
+ err = con2fb_release_oldinfo(vc, oldinfo, info, unit, oldidx,
+found);
- if (!err) {
- int show_logo = (fg_console == 0 && !user &&
-logo_shown != FBCON_LOGO_DONTSHOW);
+ if (!err) {
+ int show_logo = (fg_console == 0 && !user &&
+logo_shown != FBCON_LOGO_DONTSHOW);
- if (!found)
- fbcon_add_cursor_timer(info);
- con2fb_map_boot[unit] = newidx;
- con2fb_init_display(vc, info, unit, show_logo);
+ if (!found)
+ fbcon_add_cursor_timer(info);
+ con2fb_map_boot[unit] = newidx;
+ con2fb_init_display(vc, info, unit, show_logo);
}
if (!search_fb_in_map(info_idx))
info_idx = newidx;
- return err;
+ return err;
}
/*
--
2.24.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] fbmem: Adjust indentation in fb_prepare_logo and fb_blank
Clang warns:
../drivers/video/fbdev/core/fbmem.c:665:3: warning: misleading
indentation; statement is not part of the previous 'else'
[-Wmisleading-indentation]
if (fb_logo.depth > 4 && depth > 4) {
^
../drivers/video/fbdev/core/fbmem.c:661:2: note: previous statement is
here
else
^
../drivers/video/fbdev/core/fbmem.c:1075:3: warning: misleading
indentation; statement is not part of the previous 'if'
[-Wmisleading-indentation]
return ret;
^
../drivers/video/fbdev/core/fbmem.c:1072:2: note: previous statement is
here
if (!ret)
^
2 warnings generated.
This warning occurs because there are spaces before the tabs on these
lines. Normalize the indentation in these functions so that it is
consistent with the Linux kernel coding style and clang no longer warns.
Fixes: 1692b37c99d5 ("fbdev: Fix logo if logo depth is less than framebuffer
depth")
Link: https://github.com/ClangBuiltLinux/linux/issues/825
Signed-off-by: Nathan Chancellor
---
drivers/video/fbdev/core/fbmem.c | 36
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 0662b61fdb50..bf63cc0e6b65 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -662,20 +662,20 @@ int fb_prepare_logo(struct fb_info *info, int rotate)
fb_logo.depth = 1;
- if (fb_logo.depth > 4 && depth > 4) {
- switch (info->fix.visual) {
- case FB_VISUAL_TRUECOLOR:
- fb_logo.needs_truepalette = 1;
- break;
- case FB_VISUAL_DIRECTCOLOR:
- fb_logo.needs_directpalette = 1;
- fb_logo.needs_cmapreset = 1;
- break;
- case FB_VISUAL_PSEUDOCOLOR:
- fb_logo.needs_cmapreset = 1;
- break;
- }
- }
+ if (fb_logo.depth > 4 && depth > 4) {
+ switch (info->fix.visual) {
+ case FB_VISUAL_TRUECOLOR:
+ fb_logo.needs_truepalette = 1;
+ break;
+ case FB_VISUAL_DIRECTCOLOR:
+ fb_logo.needs_directpalette = 1;
+ fb_logo.needs_cmapreset = 1;
+ break;
+ case FB_VISUAL_PSEUDOCOLOR:
+ fb_logo.needs_cmapreset = 1;
+ break;
+ }
+ }
height = fb_logo.logo->height;
if (fb_center_logo)
@@ -1060,19 +1060,19 @@ fb_blank(struct fb_info *info, int blank)
struct fb_event event;
int ret = -EINVAL;
- if (blank > FB_BLANK_POWERDOWN)
- blank = FB_BLANK_POWERDOWN;
+ if (blank > FB_BLANK_POWERDOWN)
+ blank = FB_BLANK_POWERDOWN;
event.info = info;
event.data =
if (info->fbops->fb_blank)
- ret = info->fbops->fb_blank(blank, info);
+ ret = info->fbops->fb_blank(blank, info);
if (!ret)
fb_notifier_call_chain(FB_EVENT_BLANK, );
- return ret;
+ return ret;
}
EXPORT_SYMBOL(fb_blank);
--
2.24.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: msm: mdp4: Adjust indentation in mdp4_dsi_encoder_enable
Clang warns:
../drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c:124:3: warning:
misleading indentation; statement is not part of the previous 'if'
[-Wmisleading-indentation]
mdp4_crtc_set_config(encoder->crtc,
^
../drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c:121:2: note:
previous statement is here
if (mdp4_dsi_encoder->enabled)
^
This warning occurs because there is a space after the tab on this line.
Remove it so that the indentation is consistent with the Linux kernel
coding style and clang no longer warns.
Fixes: 776638e73a19 ("drm/msm/dsi: Add a mdp4 encoder for DSI")
Link: https://github.com/ClangBuiltLinux/linux/issues/792
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
index 772f0753ed38..aaf2f26f8505 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_dsi_encoder.c
@@ -121,7 +121,7 @@ static void mdp4_dsi_encoder_enable(struct drm_encoder
*encoder)
if (mdp4_dsi_encoder->enabled)
return;
-mdp4_crtc_set_config(encoder->crtc,
+ mdp4_crtc_set_config(encoder->crtc,
MDP4_DMA_CONFIG_PACK_ALIGN_MSB |
MDP4_DMA_CONFIG_DEFLKR_EN |
MDP4_DMA_CONFIG_DITHER_EN |
--
2.24.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amdgpu: Ensure ret is always initialized when using SOC15_WAIT_ON_RREG
Commit b0f3cd3191cd ("drm/amdgpu: remove unnecessary JPEG2.0 code from
VCN2.0") introduced a new clang warning in the vcn_v2_0_stop function:
../drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c:1082:2: warning: variable 'r'
is used uninitialized whenever 'while' loop exits because its condition
is false [-Wsometimes-uninitialized]
SOC15_WAIT_ON_RREG(VCN, 0, mmUVD_STATUS, UVD_STATUS__IDLE, 0x7, r);
^~
../drivers/gpu/drm/amd/amdgpu/../amdgpu/soc15_common.h:55:10: note:
expanded from macro 'SOC15_WAIT_ON_RREG'
while ((tmp_ & (mask)) != (expected_value)) { \
^~~
../drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c:1083:6: note: uninitialized use
occurs here
if (r)
^
../drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c:1082:2: note: remove the
condition if it is always true
SOC15_WAIT_ON_RREG(VCN, 0, mmUVD_STATUS, UVD_STATUS__IDLE, 0x7, r);
^
../drivers/gpu/drm/amd/amdgpu/../amdgpu/soc15_common.h:55:10: note:
expanded from macro 'SOC15_WAIT_ON_RREG'
while ((tmp_ & (mask)) != (expected_value)) { \
^
../drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c:1072:7: note: initialize the
variable 'r' to silence this warning
int r;
^
= 0
1 warning generated.
To prevent warnings like this from happening in the future, make the
SOC15_WAIT_ON_RREG macro initialize its ret variable before the while
loop that can time out. This macro's return value is always checked so
it should set ret in both the success and fail path.
Link: https://github.com/ClangBuiltLinux/linux/issues/776
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/amdgpu/soc15_common.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/soc15_common.h
b/drivers/gpu/drm/amd/amdgpu/soc15_common.h
index 839f186e1182..19e870c79896 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc15_common.h
+++ b/drivers/gpu/drm/amd/amdgpu/soc15_common.h
@@ -52,6 +52,7 @@
uint32_t old_ = 0; \
uint32_t tmp_ =
RREG32(adev->reg_offset[ip##_HWIP][inst][reg##_BASE_IDX] + reg); \
uint32_t loop = adev->usec_timeout; \
+ ret = 0;\
while ((tmp_ & (mask)) != (expected_value)) { \
if (old_ != tmp_) { \
loop = adev->usec_timeout; \
--
2.24.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: Use NULL for pointer assignment in copy_stream_update_to_stream
Clang warns:
../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1965:26: warning:
expression which evaluates to zero treated as a null pointer constant of
type 'struct dc_dsc_config *' [-Wnon-literal-null-conversion]
update->dsc_config = false;
^
../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:1971:25: warning:
expression which evaluates to zero treated as a null pointer constant of
type 'struct dc_dsc_config *' [-Wnon-literal-null-conversion]
update->dsc_config = false;
^
2 warnings generated.
Fixes: f6fe4053b91f ("drm/amd/display: Use a temporary copy of the current
state when updating DSC config")
Link: https://github.com/ClangBuiltLinux/linux/issues/777
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/display/dc/core/dc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index c7db4f4810c6..2645d20e8c4c 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1962,13 +1962,13 @@ static void copy_stream_update_to_stream(struct dc *dc,
if (!dc->res_pool->funcs->validate_bandwidth(dc,
dsc_validate_context, true)) {
stream->timing.dsc_cfg = old_dsc_cfg;
stream->timing.flags.DSC = old_dsc_enabled;
- update->dsc_config = false;
+ update->dsc_config = NULL;
}
dc_release_state(dsc_validate_context);
} else {
DC_ERROR("Failed to allocate new validate context for
DSC change\n");
- update->dsc_config = false;
+ update->dsc_config = NULL;
}
}
}
--
2.24.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: Remove tautological compare in eb_relocate_vma
-Wtautological-compare was recently added to -Wall in LLVM, which
exposed an if statement in i915 that is always false:
../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning:
result of comparison of constant 576460752303423487 with expression of
type 'unsigned int' is always false
[-Wtautological-constant-out-of-range-compare]
if (unlikely(remain > N_RELOC(ULONG_MAX)))
^
Since remain is an unsigned int, it can never be larger than UINT_MAX,
which is less than ULONG_MAX / sizeof(struct drm_i915_gem_relocation_entry).
Remove this statement to fix the warning.
Fixes: 2889caa92321 ("drm/i915: Eliminate lots of iterations over the
execobjects array")
Link: https://github.com/ClangBuiltLinux/linux/issues/778
Link:
https://github.com/llvm/llvm-project/commit/9740f9f0b6e5d7d5104027aee7edc9c5202dd052
Signed-off-by: Nathan Chancellor
---
NOTE: Another possible fix for this is to change ULONG_MAX to UINT_MAX
but I am not sure that is what was intended by this check. I'm
happy to respin if that is the case.
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index f0998f1225af..9ed4379b4bc8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1482,8 +1482,6 @@ static int eb_relocate_vma(struct i915_execbuffer *eb,
struct i915_vma *vma)
urelocs = u64_to_user_ptr(entry->relocs_ptr);
remain = entry->relocation_count;
- if (unlikely(remain > N_RELOC(ULONG_MAX)))
- return -EINVAL;
/*
* We must check that the entire relocation array is safe
--
2.24.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/vmwgfx: Use dma-coherent memory for high-bandwidth port messaging
On Wed, Nov 13, 2019 at 10:51:42AM +0100, Thomas Hellström (VMware) wrote:
> From: Thomas Hellstrom
>
> With AMD-SEV high-bandwidth port messaging runs into trouble since the
> message content is encrypted using the vm-specific key, and the
> hypervisor is unable to read it.
>
> So use unencrypted dma-coherent bounce buffers for temporary message
> storage space. Allocating that memory is expensive so a future
> optimization might include a static unencrypted memory area for messages.
>
> Signed-off-by: Thomas Hellstrom
> Reviewed-by: Brian Paul
Hi Thomas,
The 0day team has been doing clang builds for us and sending the results
to our mailing list for triage; this patch causes the following warning.
Seems legitimate, mind taking a look at it and resolving it how you see
fit?
Cheers,
Nathan
On Thu, Nov 14, 2019 at 03:36:44AM +0800, kbuild test robot wrote:
> CC: kbuild-...@lists.01.org
> In-Reply-To: <20191113095144.2981-1-thomas...@shipmail.org>
> References: <20191113095144.2981-1-thomas...@shipmail.org>
> TO: "Thomas Hellström (VMware)"
> CC: dri-devel@lists.freedesktop.org, Thomas Hellstrom
> , Brian Paul , Thomas Hellstrom
> , Brian Paul
> CC: Thomas Hellstrom , Brian Paul
>
> Hi "Thomas,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v5.4-rc7 next-20191113]
> [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/Thomas-Hellstr-m-VMware/drm-vmwgfx-Use-dma-coherent-memory-for-high-bandwidth-port-messaging/20191114-020818
> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> 0e3f1ad80fc8cb0c517fd9a9afb22752b741fa76
> config: x86_64-rhel-7.6 (attached as .config)
> compiler: clang version 10.0.0 (git://gitmirror/llvm_project
> 335ac2eb662ce5f1888e2a50310b01fba2d40d68)
> 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
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:441:6: warning: variable
> >> 'reply_handle' is used uninitialized whenever '||' condition is true
> >> [-Wsometimes-uninitialized]
>if (vmw_send_msg(, msg) ||
>^~~
>drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:467:47: note: uninitialized use occurs
> here
>dma_free_coherent(dev, reply_len + 1, reply, reply_handle);
> ^~~~
>drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:441:6: note: remove the '||' if its
> condition is always false
>if (vmw_send_msg(, msg) ||
>^~
>drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:421:37: note: initialize the variable
> 'reply_handle' to silence this warning
>dma_addr_t req_handle, reply_handle;
> ^
>= 0
>1 warning generated.
>
> vim +441 drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
>
> 89da76fde68de1 Sinclair Yeh 2016-04-27 400
> 89da76fde68de1 Sinclair Yeh 2016-04-27 401
> 89da76fde68de1 Sinclair Yeh 2016-04-27 402 /**
> 89da76fde68de1 Sinclair Yeh 2016-04-27 403 * vmw_host_get_guestinfo:
> Gets a GuestInfo parameter
> 89da76fde68de1 Sinclair Yeh 2016-04-27 404 *
> 89da76fde68de1 Sinclair Yeh 2016-04-27 405 * Gets the value of a
> GuestInfo.* parameter. The value returned will be in
> 89da76fde68de1 Sinclair Yeh 2016-04-27 406 * a string, and it is up
> to the caller to post-process.
> 89da76fde68de1 Sinclair Yeh 2016-04-27 407 *
> 6bdb21230a2a01 Thomas Hellstrom 2019-11-13 408 * @dev: Pointer to struct
> device used for coherent memory allocation
> 89da76fde68de1 Sinclair Yeh 2016-04-27 409 * @guest_info_param:
> Parameter to get, e.g. GuestInfo.svga.gl3
> 89da76fde68de1 Sinclair Yeh 2016-04-27 410 * @buffer: if NULL,
> *reply_len will contain reply size.
> 89da76fde68de1 Sinclair Yeh 2016-04-27 411 * @length: size of the
> reply_buf. Set to size of reply upon return
> 89da76fde68de1 Sinclair Yeh 2016-04-27 412 *
> 89da76fde68de1 Sinclair Yeh 2016-04-27 413 * Returns: 0 on success
> 89da76fde68de1 Sinclair Yeh 2016-04-27 414 */
> 6bdb21230a2a01 Thomas Hellstrom 2019-11-13 415 int
> vmw_host_get_guestinfo(struct device *dev, const char *guest_info_param,
> 89da76fde68de1 Sinclair Yeh 2016-04-27 416 char
> *buffer, size_t *length)
> 89da76fde68de1 Sinclair Yeh 2016-04-27 417 {
> 89da76fde68de1 Sinclair Yeh 2016-04-27 418 struct
[PATCH -next] drm/amd/display: Add a conversion function for transmitter and phy_id enums
Clang warns:
../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:2520:42:
error: implicit conversion from enumeration type 'enum transmitter' to
different enumeration type 'enum physical_phy_id'
[-Werror,-Wenum-conversion]
psr_context->smuPhyId = link->link_enc->transmitter;
~ ^~~
1 error generated.
As the comment above this assignment states, this is intentional. To
match previous warnings of this nature, add a conversion function that
explicitly converts between the enums and warns when there is a
mismatch.
See commit 828cfa29093f ("drm/amdgpu: Fix amdgpu ras to ta enums
conversion") and commit d9ec5cfd5a2e ("drm/amd/display: Use switch table
for dc_to_smu_clock_type") for previous examples of this.
Fixes: e0d08a40a63b ("drm/amd/display: Add debugfs entry for reading psr state")
Link: https://github.com/ClangBuiltLinux/linux/issues/758
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 38 ++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 7b18087be585..38dfe460e13b 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -2447,6 +2447,41 @@ bool dc_link_get_psr_state(const struct dc_link *link,
uint32_t *psr_state)
return true;
}
+static inline enum physical_phy_id
+transmitter_to_phy_id(enum transmitter transmitter_value)
+{
+ switch (transmitter_value) {
+ case TRANSMITTER_UNIPHY_A:
+ return PHYLD_0;
+ case TRANSMITTER_UNIPHY_B:
+ return PHYLD_1;
+ case TRANSMITTER_UNIPHY_C:
+ return PHYLD_2;
+ case TRANSMITTER_UNIPHY_D:
+ return PHYLD_3;
+ case TRANSMITTER_UNIPHY_E:
+ return PHYLD_4;
+ case TRANSMITTER_UNIPHY_F:
+ return PHYLD_5;
+ case TRANSMITTER_NUTMEG_CRT:
+ return PHYLD_6;
+ case TRANSMITTER_TRAVIS_CRT:
+ return PHYLD_7;
+ case TRANSMITTER_TRAVIS_LCD:
+ return PHYLD_8;
+ case TRANSMITTER_UNIPHY_G:
+ return PHYLD_9;
+ case TRANSMITTER_COUNT:
+ return PHYLD_COUNT;
+ case TRANSMITTER_UNKNOWN:
+ return PHYLD_UNKNOWN;
+ default:
+ WARN_ONCE(1, "Unknown transmitter value %d\n",
+ transmitter_value);
+ return PHYLD_0;
+ }
+}
+
bool dc_link_setup_psr(struct dc_link *link,
const struct dc_stream_state *stream, struct psr_config
*psr_config,
struct psr_context *psr_context)
@@ -2517,7 +2552,8 @@ bool dc_link_setup_psr(struct dc_link *link,
/* Hardcoded for now. Can be Pcie or Uniphy (or Unknown)*/
psr_context->phyType = PHY_TYPE_UNIPHY;
/*PhyId is associated with the transmitter id*/
- psr_context->smuPhyId = link->link_enc->transmitter;
+ psr_context->smuPhyId =
+ transmitter_to_phy_id(link->link_enc->transmitter);
psr_context->crtcTimingVerticalTotal = stream->timing.v_total;
psr_context->vsyncRateHz = div64_u64(div64_u64((stream->
--
2.24.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/6] [RESEND] drm/amdgpu: work around llvm bug #42576
On Wed, Oct 02, 2019 at 09:51:37AM -0700, 'Nick Desaulniers' via Clang Built Linux wrote: > > Apparently this bug is still present in both the released clang-9 > > and the current development version of clang-10. > > I was hoping we would not need a workaround in clang-9+, but > > it seems that we do. > > I think I'd rather: > 1. mark AMDGPU BROKEN if CC_IS_CLANG. There are numerous other issues building >a working driver here. The only reason I am not thrilled about this is we will lose out on warning coverage while the compiler bug gets fixed. I think the AMDGPU drivers have been the single biggest source of clang warnings. I think something like: depends on CC_IS_GCC || (CC_IS_CLANG && COMPILE_TEST) would end up avoiding the runtime issues and give us warning coverage. The only issue is that we would still need this patch... Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/display: Fix 32-bit divide error in wait_for_alt_mode
When building arm32 allyesconfig:
ld.lld: error: undefined symbol: __aeabi_uldivmod
>>> referenced by dc_link.c
>>> gpu/drm/amd/display/dc/core/dc_link.o:(wait_for_alt_mode) in archive
>>> drivers/built-in.a
>>> referenced by dc_link.c
>>> gpu/drm/amd/display/dc/core/dc_link.o:(wait_for_alt_mode) in archive
>>> drivers/built-in.a
time_taken_in_ns is of type unsigned long long so we need to use div_u64
to avoid this error.
Fixes: b5b1f4554904 ("drm/amd/display: Enable type C hotplug")
Reported-by: Randy Dunlap
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index f2d78d7b089e..8634923b 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -721,7 +721,7 @@ bool wait_for_alt_mode(struct dc_link *link)
time_taken_in_ns = dm_get_elapse_time_in_ns(
link->ctx, finish_timestamp, enter_timestamp);
DC_LOG_WARNING("Alt mode entered finished after %llu
ms\n",
- time_taken_in_ns / 100);
+ div_u64(time_taken_in_ns, 100));
return true;
}
@@ -730,7 +730,7 @@ bool wait_for_alt_mode(struct dc_link *link)
time_taken_in_ns = dm_get_elapse_time_in_ns(link->ctx, finish_timestamp,
enter_timestamp);
DC_LOG_WARNING("Alt mode has timed out after %llu ms\n",
- time_taken_in_ns / 100);
+ div_u64(time_taken_in_ns, 100));
return false;
}
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/powerplay: Zero initialize some variables
Clang warns (only Navi warning shown but Arcturus warns as well):
drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1534:4: warning:
variable 'asic_default_power_limit' is used uninitialized whenever '?:'
condition is false [-Wsometimes-uninitialized]
smu_read_smc_arg(smu, _default_power_limit);
^~~~
drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:588:3: note:
expanded from macro 'smu_read_smc_arg'
((smu)->funcs->read_smc_arg? (smu)->funcs->read_smc_arg((smu), (arg)) :
0)
^~
drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1550:30: note:
uninitialized use occurs here
smu->default_power_limit = asic_default_power_limit;
^~~~
drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1534:4: note:
remove the '?:' if its condition is always true
smu_read_smc_arg(smu, _default_power_limit);
^
drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:588:3: note:
expanded from macro 'smu_read_smc_arg'
((smu)->funcs->read_smc_arg? (smu)->funcs->read_smc_arg((smu), (arg)) :
0)
^
drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:1517:35: note:
initialize the variable 'asic_default_power_limit' to silence this
warning
uint32_t asic_default_power_limit;
^
= 0
1 warning generated.
As the code is currently written, if read_smc_arg were ever NULL, arg
would fail to be initialized but the code would continue executing as
normal because the return value would just be zero.
There are a few different possible solutions to resolve this class
of warnings which have appeared in these drivers before:
1. Assume the function pointer will never be NULL and eliminate the
wrapper macros.
2. Have the wrapper macros initialize arg when the function pointer is
NULL.
3. Have the wrapper macros return an error code instead of 0 when the
function pointer is NULL so that the callsites can properly bail out
before arg can be used.
4. Initialize arg at the top of its function.
Number four is the path of least resistance right now as every other
change will be driver wide so do that here. I only make the comment
now as food for thought.
Fixes: b4af964e75c4 ("drm/amd/powerplay: make power limit retrieval as asic
specific")
Link: https://github.com/ClangBuiltLinux/linux/issues/627
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 2 +-
drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 215f7173fca8..b92eded7374f 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -1326,7 +1326,7 @@ static int arcturus_get_power_limit(struct smu_context
*smu,
bool asic_default)
{
PPTable_t *pptable = smu->smu_table.driver_pptable;
- uint32_t asic_default_power_limit;
+ uint32_t asic_default_power_limit = 0;
int ret = 0;
int power_src;
diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index 106352a4fb82..d844bc8411aa 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -1514,7 +1514,7 @@ static int navi10_get_power_limit(struct smu_context *smu,
bool asic_default)
{
PPTable_t *pptable = smu->smu_table.driver_pptable;
- uint32_t asic_default_power_limit;
+ uint32_t asic_default_power_limit = 0;
int ret = 0;
int power_src;
--
2.23.0.rc1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/7] drm/amd/display: Use proper enum conversion functions
On Wed, Jul 03, 2019 at 10:52:16PM -0700, Nathan Chancellor wrote:
> clang warns:
>
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_pp_smu.c:336:8:
> warning: implicit conversion from enumeration type 'enum smu_clk_type'
> to different enumeration type 'enum amd_pp_clock_type'
> [-Wenum-conversion]
> dc_to_smu_clock_type(clk_type),
> ^~~
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_pp_smu.c:421:14:
> warning: implicit conversion from enumeration type 'enum
> amd_pp_clock_type' to different enumeration type 'enum smu_clk_type'
> [-Wenum-conversion]
> dc_to_pp_clock_type(clk_type),
> ^~
>
> There are functions to properly convert between all of these types, use
> them so there are no longer any warnings.
>
> Fixes: a43913ea50a5 ("drm/amd/powerplay: add function
> get_clock_by_type_with_latency for navi10")
> Fixes: e5e4e22391c2 ("drm/amd/powerplay: add interface to get clock by type
> with latency for display (v2)")
> Link: https://github.com/ClangBuiltLinux/linux/issues/586
> Signed-off-by: Nathan Chancellor
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> index eac09bfe3be2..0f76cfff9d9b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
> @@ -333,7 +333,7 @@ bool dm_pp_get_clock_levels_by_type(
> }
> } else if (adev->smu.funcs && adev->smu.funcs->get_clock_by_type) {
> if (smu_get_clock_by_type(>smu,
> - dc_to_smu_clock_type(clk_type),
> + dc_to_pp_clock_type(clk_type),
> _clks)) {
> get_default_clock_levels(clk_type, dc_clks);
> return true;
> @@ -418,7 +418,7 @@ bool dm_pp_get_clock_levels_by_type_with_latency(
> return false;
> } else if (adev->smu.ppt_funcs &&
> adev->smu.ppt_funcs->get_clock_by_type_with_latency) {
> if (smu_get_clock_by_type_with_latency(>smu,
> -
> dc_to_pp_clock_type(clk_type),
> +
> dc_to_smu_clock_type(clk_type),
> _clks))
> return false;
> }
> --
> 2.22.0
>
Gentle ping for review, this is the last remaining warning that I see
from amdgpu on next-20190718.
Cheers,
Nathan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/7] drm/amd/powerplay: Use proper enums in vega20_print_clk_levels
On Mon, Jul 15, 2019 at 11:25:29AM +0200, Arnd Bergmann wrote:
> On Thu, Jul 4, 2019 at 7:52 AM Nathan Chancellor
> wrote:
> >
> > clang warns:
> >
> > drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:995:39: warning:
> > implicit conversion from enumeration type 'PPCLK_e' to different
> > enumeration type 'enum smu_clk_type' [-Wenum-conversion]
> > ret = smu_get_current_clk_freq(smu, PPCLK_SOCCLK, );
> > ~~^~~
> > drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:1016:39: warning:
> > implicit conversion from enumeration type 'PPCLK_e' to different
> > enumeration type 'enum smu_clk_type' [-Wenum-conversion]
> > ret = smu_get_current_clk_freq(smu, PPCLK_FCLK, );
> > ~~^
> > drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:1031:39: warning:
> > implicit conversion from enumeration type 'PPCLK_e' to different
> > enumeration type 'enum smu_clk_type' [-Wenum-conversion]
> > ret = smu_get_current_clk_freq(smu, PPCLK_DCEFCLK, );
> > ~~^~~~
> >
> > The values are mapped one to one in vega20_get_smu_clk_index so just use
> > the proper enums here.
> >
> > Fixes: 096761014227 ("drm/amd/powerplay: support sysfs to get socclk, fclk,
> > dcefclk")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/587
> > Signed-off-by: Nathan Chancellor
> > ---
>
> Adding Kevin Wang for further review, as he sent a related patch in
> d36893362d22 ("drm/amd/powerplay: fix smu clock type change miss error")
>
> I assume this one is still required as it triggers the same warning.
> Kevin, can you have a look?
>
> Arnd
Indeed, this one and https://github.com/ClangBuiltLinux/linux/issues/586
are still outstanding.
https://patchwork.freedesktop.org/patch/315581/
Cheers,
Nathan
>
> > drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> > b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> > index 0f14fe14ecd8..e62dd6919b24 100644
> > --- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> > +++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
> > @@ -992,7 +992,7 @@ static int vega20_print_clk_levels(struct smu_context
> > *smu,
> > break;
> >
> > case SMU_SOCCLK:
> > - ret = smu_get_current_clk_freq(smu, PPCLK_SOCCLK, );
> > + ret = smu_get_current_clk_freq(smu, SMU_SOCCLK, );
> > if (ret) {
> > pr_err("Attempt to get current socclk Failed!");
> > return ret;
> > @@ -1013,7 +1013,7 @@ static int vega20_print_clk_levels(struct smu_context
> > *smu,
> > break;
> >
> > case SMU_FCLK:
> > - ret = smu_get_current_clk_freq(smu, PPCLK_FCLK, );
> > + ret = smu_get_current_clk_freq(smu, SMU_FCLK, );
> > if (ret) {
> > pr_err("Attempt to get current fclk Failed!");
> > return ret;
> > @@ -1028,7 +1028,7 @@ static int vega20_print_clk_levels(struct smu_context
> > *smu,
> > break;
> >
> > case SMU_DCEFCLK:
> > - ret = smu_get_current_clk_freq(smu, PPCLK_DCEFCLK, );
> > + ret = smu_get_current_clk_freq(smu, SMU_DCEFCLK, );
> > if (ret) {
> > pr_err("Attempt to get current dcefclk Failed!");
> > return ret;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display: return 'NULL' instead of 'false' from dcn20_acquire_idle_pipe_for_layer
On Fri, Jul 12, 2019 at 11:39:52AM +0200, Arnd Bergmann wrote:
> clang complains that 'false' is a not a pointer:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_resource.c:2428:10:
> error: expression which evaluates to zero treated as a null pointer constant
> of type 'struct pipe_ctx *' [-Werror,-Wnon-literal-null-conversion]
> return false;
>
> Changing it to 'NULL' looks like the right thing that will shut up
> the warning and make it easier to read, while not changing behavior.
>
> Fixes: 7ed4e6352c16 ("drm/amd/display: Add DCN2 HW Sequencer and Resource")
> Signed-off-by: Arnd Bergmann
Reviewed-by: Nathan Chancellor
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/7] drm/amd/display: Use proper enum conversion functions
On Tue, Jul 09, 2019 at 08:51:33PM +0200, Arnd Bergmann wrote: > On Thu, Jul 4, 2019 at 7:52 AM Nathan Chancellor > wrote: > > > > clang warns: > > > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_pp_smu.c:336:8: > > warning: implicit conversion from enumeration type 'enum smu_clk_type' > > to different enumeration type 'enum amd_pp_clock_type' > > [-Wenum-conversion] > > dc_to_smu_clock_type(clk_type), > > ^~~ > > drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_pp_smu.c:421:14: > > warning: implicit conversion from enumeration type 'enum > > amd_pp_clock_type' to different enumeration type 'enum smu_clk_type' > > [-Wenum-conversion] > > dc_to_pp_clock_type(clk_type), > > ^~ > > > > There are functions to properly convert between all of these types, use > > them so there are no longer any warnings. > > I had a different solution for this one as well. The difference is that your > patch keeps the types and assumes that the functions do the right thing > (i.e. the warning was correct), while my version assumes that the code > works correctly, but the types are wrong (a false positive warning). > > One of the two patches is correct, the other one is broken, but I have > no idea which one. > > Arnd Indeed. I would personally argue that if using the correct conversion functions (which are here to specifically avoid this type of warning) causes issues, this code should probably not be using enumerated types at all since the entire point is to enforce semantic correctness, not just be a special way to represent small integer values, especially in the case where the CLK values are completely different numerical values in various enumerated types. I think enumerated type casts are ugly and defeat the purpose of them but that's obviously just my opinion :) Hopefully we can get some clarity on this soon. Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/7] amdgpu clang warning fixes on next-20190703
On Mon, Jul 08, 2019 at 11:55:50AM -0400, Alex Deucher wrote: > Applied the series. thanks! > > Alex Thank you :) I don't see the enum conversion ones in your current tree. If they indeed caused issues, could you guys please look into fixing the warnings properly yourselves (maybe something like Arnd's patch?) or let me know how you would like them fixed (explicit casts, changing type to int, etc)? https://patchwork.freedesktop.org/patch/316466/ Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [1/2] drm/amd/powerplay: smu_v11_0: fix uninitialized variable use
On Mon, Jul 08, 2019 at 04:07:58PM +0200, Arnd Bergmann wrote:
> A mistake in the error handling caused an uninitialized
> variable to be used:
>
> drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1102:10: error: variable
> 'freq' is used uninitialized whenever '?:' condition is false
> [-Werror,-Wsometimes-uninitialized]
> ret = smu_get_current_clk_freq_by_table(smu, clk_id, );
>^
> drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:880:3: note:
> expanded from macro 'smu_get_current_clk_freq_by_table'
> ((smu)->ppt_funcs->get_current_clk_freq_by_table ?
> (smu)->ppt_funcs->get_current_clk_freq_by_table((smu), (clk_type), (value)) :
> 0)
> ^~~
> drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1114:2: note:
> uninitialized use occurs here
> freq *= 100;
> ^~~~
> drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1102:10: note: remove the
> '?:' if its condition is always true
> ret = smu_get_current_clk_freq_by_table(smu, clk_id, );
>^
> drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:880:3: note:
> expanded from macro 'smu_get_current_clk_freq_by_table'
> ((smu)->ppt_funcs->get_current_clk_freq_by_table ?
> (smu)->ppt_funcs->get_current_clk_freq_by_table((smu), (clk_type), (value)) :
> 0)
> ^
> drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1095:15: note: initialize
> the variable 'freq' to silence this warning
> uint32_t freq;
> ^
> = 0
>
> Bail out of smu_v11_0_get_current_clk_freq() before we get there.
>
> Fixes: e36182490dec ("drm/amd/powerplay: fix dpm freq unit error (10KHz ->
> Mhz)")
> Signed-off-by: Arnd Bergmann
> ---
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index c3f9714e9047..bd89a13b6679 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1099,9 +1099,11 @@ static int smu_v11_0_get_current_clk_freq(struct
> smu_context *smu,
> return -EINVAL;
>
> /* if don't has GetDpmClockFreq Message, try get current clock by
> SmuMetrics_t */
> - if (smu_msg_get_index(smu, SMU_MSG_GetDpmClockFreq) == 0)
> + if (smu_msg_get_index(smu, SMU_MSG_GetDpmClockFreq) == 0) {
> ret = smu_get_current_clk_freq_by_table(smu, clk_id, );
> - else {
> + if (ret)
> + return ret;
I am kind of surprised that this fixes the warning. If I am interpreting
the warning correctly, it is complaining that the member
get_current_clk_freq_by_table could be NULL and not be called to
initialize freq and that entire statement will become 0. If that is the
case, it seems like this added error handling won't fix that problem,
right?
Cheers,
Nathan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/powerplay: work around enum conversion warnings
Hi Arnd,
On Mon, Jul 08, 2019 at 03:57:06PM +0200, Arnd Bergmann wrote:
> A couple of calls to smu_get_current_clk_freq() and smu_force_clk_levels()
> pass constants of the wrong type, leading to warnings with clang-8:
>
> drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:995:39: error: implicit
> conversion from enumeration type 'PPCLK_e' to different enumeration type
> 'enum smu_clk_type' [-Werror,-Wenum-conversion]
> ret = smu_get_current_clk_freq(smu, PPCLK_SOCCLK, );
> ~~^~~
> drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:775:82: note:
> expanded from macro 'smu_get_current_clk_freq'
> ((smu)->funcs->get_current_clk_freq?
> (smu)->funcs->get_current_clk_freq((smu), (clk_id), (value)) : 0)
>
> I could not figure out what the purpose is of mixing the types
> like this and if it is written like this intentionally.
> Assuming this is all correct, adding an explict case is an
> easy way to shut up the warnings.
>
> Fixes: bc0fcffd36ba ("drm/amd/powerplay: Unify smu handle task function (v2)")
> Fixes: 096761014227 ("drm/amd/powerplay: support sysfs to get socclk, fclk,
> dcefclk")
> Signed-off-by: Arnd Bergmann
I sent a series last week for all of the clang warnings that were added
in this driver recently.
https://lore.kernel.org/lkml/20190704055217.45860-1-natechancel...@gmail.com/
I think it is safe to use the CLK enums from the expected type (from
what I could see from going down the code flow rabbit hole).
https://lore.kernel.org/lkml/20190704055217.45860-4-natechancel...@gmail.com/
https://lore.kernel.org/lkml/20190704055217.45860-7-natechancel...@gmail.com/
Cheers,
Nathan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/7] drm/amd/powerplay: Zero initialize freq in smu_v11_0_get_current_clk_freq
clang warns (trimmed for brevity):
drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1098:10: warning:
variable 'freq' is used uninitialized whenever '?:' condition is false
[-Wsometimes-uninitialized]
ret = smu_get_current_clk_freq_by_table(smu, clk_id, );
^
If get_current_clk_freq_by_table is ever NULL, freq will fail to be
properly initialized. Zero initialize it to avoid using uninitialized
stack values.
smu_get_current_clk_freq_by_table expands to a ternary operator
conditional on smu->funcs->get_current_clk_freq_by_table being not NULL.
When this is false, freq will be uninitialized. Zero initialize freq to
avoid using random stack values if that ever happens.
Fixes: e36182490dec ("drm/amd/powerplay: fix dpm freq unit error (10KHz ->
Mhz)")
Link: https://github.com/ClangBuiltLinux/linux/issues/585
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 632a20587c8b..a6f8cd6df7f1 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -1088,7 +1088,7 @@ static int smu_v11_0_get_current_clk_freq(struct
smu_context *smu,
uint32_t *value)
{
int ret = 0;
- uint32_t freq;
+ uint32_t freq = 0;
if (clk_id >= SMU_CLK_COUNT || !value)
return -EINVAL;
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/7] amdgpu clang warning fixes on next-20190703
Hi all, I don't do threaded patches very often so if I have messed something up, please forgive me :) This series fixes all of the clang warnings that I saw added in next-20190703. The full list is visible in the gist linked below and each full individual warning can be seen in the GitHub link in each patch. https://gist.github.com/5411af08b96c99b14e60c60800e99a47 All of the warnings are fixed in what I believe is the optimal way but the enum conversion warnings were the trickiest; please review carefully as the code paths for some of them have changed (especially in patch 3 and 6). Thank you! Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/7] drm/amd/display: Use proper enum conversion functions
clang warns:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_pp_smu.c:336:8:
warning: implicit conversion from enumeration type 'enum smu_clk_type'
to different enumeration type 'enum amd_pp_clock_type'
[-Wenum-conversion]
dc_to_smu_clock_type(clk_type),
^~~
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_pp_smu.c:421:14:
warning: implicit conversion from enumeration type 'enum
amd_pp_clock_type' to different enumeration type 'enum smu_clk_type'
[-Wenum-conversion]
dc_to_pp_clock_type(clk_type),
^~
There are functions to properly convert between all of these types, use
them so there are no longer any warnings.
Fixes: a43913ea50a5 ("drm/amd/powerplay: add function
get_clock_by_type_with_latency for navi10")
Fixes: e5e4e22391c2 ("drm/amd/powerplay: add interface to get clock by type
with latency for display (v2)")
Link: https://github.com/ClangBuiltLinux/linux/issues/586
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
index eac09bfe3be2..0f76cfff9d9b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c
@@ -333,7 +333,7 @@ bool dm_pp_get_clock_levels_by_type(
}
} else if (adev->smu.funcs && adev->smu.funcs->get_clock_by_type) {
if (smu_get_clock_by_type(>smu,
- dc_to_smu_clock_type(clk_type),
+ dc_to_pp_clock_type(clk_type),
_clks)) {
get_default_clock_levels(clk_type, dc_clks);
return true;
@@ -418,7 +418,7 @@ bool dm_pp_get_clock_levels_by_type_with_latency(
return false;
} else if (adev->smu.ppt_funcs &&
adev->smu.ppt_funcs->get_clock_by_type_with_latency) {
if (smu_get_clock_by_type_with_latency(>smu,
-
dc_to_pp_clock_type(clk_type),
+
dc_to_smu_clock_type(clk_type),
_clks))
return false;
}
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/7] drm/amd/powerplay: Use memset to initialize metrics structs
clang warns:
drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:601:33: warning:
suggest braces around initialization of subobject [-Wmissing-braces]
static SmuMetrics_t metrics = {0};
^
{}
drivers/gpu/drm/amd/amdgpu/../powerplay/navi10_ppt.c:905:26: warning:
suggest braces around initialization of subobject [-Wmissing-braces]
SmuMetrics_t metrics = {0};
^
{}
2 warnings generated.
One way to fix these warnings is to add additional braces like clang
suggests; however, there has been a bit of push back from some
maintainers[1][2], who just prefer memset as it is unambiguous, doesn't
depend on a particular compiler version[3], and properly initializes all
subobjects. Do that here so there are no more warnings.
[1]: https://lore.kernel.org/lkml/022e41c0-8465-dc7a-a45c-64187ecd9...@amd.com/
[2]:
https://lore.kernel.org/lkml/20181128.215241.702406654469517539.da...@davemloft.net/
[3]: https://lore.kernel.org/lkml/20181116150432.2408a...@redhat.com/
Fixes: 98e1a543c7b1 ("drm/amd/powerplay: add function get current clock freq
interface for navi10")
Fixes: ab43c4bf1cc8 ("drm/amd/powerplay: fix fan speed show error (for hwmon
pwm)")
Link: https://github.com/ClangBuiltLinux/linux/issues/583
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
index e00397f84b2f..f5d2ada05bc6 100644
--- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
@@ -598,12 +598,14 @@ static int navi10_get_current_clk_freq_by_table(struct
smu_context *smu,
enum smu_clk_type clk_type,
uint32_t *value)
{
- static SmuMetrics_t metrics = {0};
+ static SmuMetrics_t metrics;
int ret = 0, clk_id = 0;
if (!value)
return -EINVAL;
+ memset(, 0, sizeof(metrics));
+
ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS, (void *),
false);
if (ret)
return ret;
@@ -902,12 +904,14 @@ static bool navi10_is_dpm_running(struct smu_context *smu)
static int navi10_get_fan_speed(struct smu_context *smu, uint16_t *value)
{
- SmuMetrics_t metrics = {0};
+ SmuMetrics_t metrics;
int ret = 0;
if (!value)
return -EINVAL;
+ memset(, 0, sizeof(metrics));
+
ret = smu_update_table(smu, SMU_TABLE_SMU_METRICS,
(void *), false);
if (ret)
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/7] drm/amd/powerplay: Use proper enums in smu_adjust_power_state_dynamic
clang warns:
drivers/gpu/drm/amd/amdgpu/../powerplay/amdgpu_smu.c:1374:30: warning:
implicit conversion from enumeration type 'enum pp_clock_type' to
different enumeration type 'enum smu_clk_type' [-Wenum-conversion]
smu_force_clk_levels(smu, PP_SCLK, 1 << sclk_mask);
~~^~~~
drivers/gpu/drm/amd/amdgpu/../powerplay/amdgpu_smu.c:1375:30: warning:
implicit conversion from enumeration type 'enum pp_clock_type' to
different enumeration type 'enum smu_clk_type' [-Wenum-conversion]
smu_force_clk_levels(smu, PP_MCLK, 1 << mclk_mask);
~~^~~~
This appears to be a copy and paste fail from when this was a call to
vega20_force_clk_levels.
Fixes: bc0fcffd36ba ("drm/amd/powerplay: Unify smu handle task function (v2)")
Link: https://github.com/ClangBuiltLinux/linux/issues/584
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 31152d495f69..e897469f7431 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1371,8 +1371,8 @@ int smu_adjust_power_state_dynamic(struct smu_context
*smu,
_mask);
if (ret)
return ret;
- smu_force_clk_levels(smu, PP_SCLK, 1 << sclk_mask);
- smu_force_clk_levels(smu, PP_MCLK, 1 << mclk_mask);
+ smu_force_clk_levels(smu, SMU_SCLK, 1 << sclk_mask);
+ smu_force_clk_levels(smu, SMU_MCLK, 1 << mclk_mask);
break;
case AMD_DPM_FORCED_LEVEL_MANUAL:
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/7] drm/amdgpu/mes10.1: Fix header guard
clang warns:
In file included from drivers/gpu/drm/amd/amdgpu/nv.c:53:
drivers/gpu/drm/amd/amdgpu/../amdgpu/mes_v10_1.h:24:9: warning:
'__MES_V10_1_H__' is used as a header guard here, followed by #define of
a different macro [-Wheader-guard]
#ifndef __MES_V10_1_H__
^~~
drivers/gpu/drm/amd/amdgpu/../amdgpu/mes_v10_1.h:25:9: note:
'__MES_v10_1_H__' is defined here; did you mean '__MES_V10_1_H__'?
#define __MES_v10_1_H__
^~~
__MES_V10_1_H__
1 warning generated.
Capitalize the V.
Fixes: 886f82aa7a1d ("drm/amdgpu/mes10.1: add ip block mes10.1 (v2)")
Link: https://github.com/ClangBuiltLinux/linux/issues/582
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/amdgpu/mes_v10_1.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.h
b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.h
index 17b9b53fa892..9afd6ddb01e9 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.h
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.h
@@ -22,7 +22,7 @@
*/
#ifndef __MES_V10_1_H__
-#define __MES_v10_1_H__
+#define __MES_V10_1_H__
extern const struct amdgpu_ip_block_version mes_v10_1_ip_block;
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/7] drm/amd/powerplay: Use proper enums in vega20_print_clk_levels
clang warns:
drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:995:39: warning:
implicit conversion from enumeration type 'PPCLK_e' to different
enumeration type 'enum smu_clk_type' [-Wenum-conversion]
ret = smu_get_current_clk_freq(smu, PPCLK_SOCCLK, );
~~^~~
drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:1016:39: warning:
implicit conversion from enumeration type 'PPCLK_e' to different
enumeration type 'enum smu_clk_type' [-Wenum-conversion]
ret = smu_get_current_clk_freq(smu, PPCLK_FCLK, );
~~^
drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:1031:39: warning:
implicit conversion from enumeration type 'PPCLK_e' to different
enumeration type 'enum smu_clk_type' [-Wenum-conversion]
ret = smu_get_current_clk_freq(smu, PPCLK_DCEFCLK, );
~~^~~~
The values are mapped one to one in vega20_get_smu_clk_index so just use
the proper enums here.
Fixes: 096761014227 ("drm/amd/powerplay: support sysfs to get socclk, fclk,
dcefclk")
Link: https://github.com/ClangBuiltLinux/linux/issues/587
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
index 0f14fe14ecd8..e62dd6919b24 100644
--- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
@@ -992,7 +992,7 @@ static int vega20_print_clk_levels(struct smu_context *smu,
break;
case SMU_SOCCLK:
- ret = smu_get_current_clk_freq(smu, PPCLK_SOCCLK, );
+ ret = smu_get_current_clk_freq(smu, SMU_SOCCLK, );
if (ret) {
pr_err("Attempt to get current socclk Failed!");
return ret;
@@ -1013,7 +1013,7 @@ static int vega20_print_clk_levels(struct smu_context
*smu,
break;
case SMU_FCLK:
- ret = smu_get_current_clk_freq(smu, PPCLK_FCLK, );
+ ret = smu_get_current_clk_freq(smu, SMU_FCLK, );
if (ret) {
pr_err("Attempt to get current fclk Failed!");
return ret;
@@ -1028,7 +1028,7 @@ static int vega20_print_clk_levels(struct smu_context
*smu,
break;
case SMU_DCEFCLK:
- ret = smu_get_current_clk_freq(smu, PPCLK_DCEFCLK, );
+ ret = smu_get_current_clk_freq(smu, SMU_DCEFCLK, );
if (ret) {
pr_err("Attempt to get current dcefclk Failed!");
return ret;
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/7] drm/amd/powerplay: Zero initialize current_rpm in vega20_get_fan_speed_percent
clang warns (trimmed for brevity):
drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:3023:8: warning:
variable 'current_rpm' is used uninitialized whenever '?:' condition is
false [-Wsometimes-uninitialized]
ret = smu_get_current_rpm(smu, _rpm);
^~
smu_get_current_rpm expands to a ternary operator conditional on
smu->funcs->get_current_rpm being not NULL. When this is false,
current_rpm will be uninitialized. Zero initialize current_rpm to
avoid using random stack values if that ever happens.
Fixes: ee0db82027ee ("drm/amd/powerplay: move PPTable_t uses into asic level")
Link: https://github.com/ClangBuiltLinux/linux/issues/588
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
index e62dd6919b24..e37b39987587 100644
--- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
@@ -3016,8 +3016,7 @@ static int vega20_get_fan_speed_percent(struct
smu_context *smu,
uint32_t *speed)
{
int ret = 0;
- uint32_t percent = 0;
- uint32_t current_rpm;
+ uint32_t current_rpm = 0, percent = 0;
PPTable_t *pptable = smu->smu_table.driver_pptable;
ret = smu_get_current_rpm(smu, _rpm);
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dsi: Add parentheses to quirks check in dsi_phy_hw_v3_0_lane_settings
Clang warns:
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: warning: logical not is
only applied to the left hand side of this bitwise operator
[-Wlogical-not-parentheses]
if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
^ ~
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: note: add parentheses
after the '!' to evaluate the bitwise operator first
if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
^
( )
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: note: add parentheses
around left hand side expression to silence this warning
if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
^
()
1 warning generated.
Add parentheses around the bitwise AND so it is evaluated first then
negated.
Fixes: 3dbbf8f09e83 ("drm/msm/dsi: Add old timings quirk for 10nm phy")
Link: https://github.com/ClangBuiltLinux/linux/547
Reported-by: kbuild test robot
Reviewed-by: Jeffrey Hugo
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
index eb28937f4b34..47403d4f2d28 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
@@ -77,7 +77,7 @@ static void dsi_phy_hw_v3_0_lane_settings(struct msm_dsi_phy
*phy)
tx_dctrl[i]);
}
- if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
+ if (!(phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK)) {
/* Toggle BIT 0 to release freeze I/0 */
dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(3),
0x05);
dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(3),
0x04);
--
2.22.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/msm/dsi: Add parentheses to quirks check in dsi_phy_hw_v3_0_lane_settings
Clang warns:
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: warning: logical not is
only applied to the left hand side of this bitwise operator
[-Wlogical-not-parentheses]
if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
^ ~
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: note: add parentheses
after the '!' to evaluate the bitwise operator first
if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
^
( )
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: note: add parentheses
around left hand side expression to silence this warning
if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
^
()
1 warning generated.
Add parentheses around the bitwise AND so it is evaluated first then
negated.
Fixes: 3dbbf8f09e83 ("drm/msm/dsi: Add old timings quirk for 10nm phy")
Link: https://github.com/ClangBuiltLinux/linux/issues/547
Reported-by: kbuild test robot
Reviewed-by: Jeffrey Hugo
Reviewed-by: Sean Paul
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Fix broken link (thanks to Sean for pointing it out)
* Add Sean's reviewed-by
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
index eb28937f4b34..47403d4f2d28 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
@@ -77,7 +77,7 @@ static void dsi_phy_hw_v3_0_lane_settings(struct msm_dsi_phy
*phy)
tx_dctrl[i]);
}
- if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
+ if (!(phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK)) {
/* Toggle BIT 0 to release freeze I/0 */
dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(3),
0x05);
dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(3),
0x04);
--
2.22.0
Re: [PATCH] drm/msm/dsi: Add parentheses to quirks check in dsi_phy_hw_v3_0_lane_settings
On Wed, Jun 19, 2019 at 03:13:40PM -0400, Sean Paul wrote:
> On Wed, Jun 19, 2019 at 12:19 PM Nathan Chancellor
> wrote:
> >
> > Clang warns:
> >
> > drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: warning: logical not is
> > only applied to the left hand side of this bitwise operator
> > [-Wlogical-not-parentheses]
> > if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
> > ^ ~
> > drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: note: add parentheses
> > after the '!' to evaluate the bitwise operator first
> > if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
> > ^
> > ( )
> > drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: note: add parentheses
> > around left hand side expression to silence this warning
> > if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
> > ^
> > ()
> > 1 warning generated.
> >
> > Add parentheses around the bitwise AND so it is evaluated first then
> > negated.
> >
> > Fixes: 3dbbf8f09e83 ("drm/msm/dsi: Add old timings quirk for 10nm phy")
> > Link: https://github.com/ClangBuiltLinux/linux/547
>
> This link is broken, could you please fix it up?
Thanks for catching this, v2 on the way.
Cheers,
Nathan
>
> The rest is:
> Reviewed-by: Sean Paul
>
>
>
> > Reported-by: kbuild test robot
> > Reviewed-by: Jeffrey Hugo
> > Signed-off-by: Nathan Chancellor
> > ---
> > drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> > index eb28937f4b34..47403d4f2d28 100644
> > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c
> > @@ -77,7 +77,7 @@ static void dsi_phy_hw_v3_0_lane_settings(struct
> > msm_dsi_phy *phy)
> > tx_dctrl[i]);
> > }
> >
> > - if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) {
> > + if (!(phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK)) {
> > /* Toggle BIT 0 to release freeze I/0 */
> > dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(3),
> > 0x05);
> > dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(3),
> > 0x04);
> > --
> > 2.22.0
> >
arm32 build failure after abe882a39a9c ("drm/amd/display: fix issue with eDP not detected on driver load")
Hi all,
After commit abe882a39a9c ("drm/amd/display: fix issue with eDP not
detected on driver load") in -next, arm32 allyesconfig builds start
failing at link time:
arm-linux-gnueabi-ld: drivers/gpu/drm/amd/display/dc/core/dc_link.o: in
function `dc_link_detect':
dc_link.c:(.text+0x260c): undefined reference to `__bad_udelay'
arm32 only allows a udelay value of up to 2000, see
arch/arm/include/asm/delay.h for more info.
Please look into this when you have a chance!
Nathan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amdgpu: fix return of an uninitialized value in variable ret
On Fri, May 10, 2019 at 11:08:42AM +0100, Colin King wrote:
> From: Colin Ian King
>
> In the case where is_enable is false and lo_base_addr is non-zero the
> variable ret has not been initialized and is being checked for non-zero
> and potentially garbage is being returned. Fix this by not returning
> ret but instead returning -EINVAL on the zero lo_base_addr case.
>
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: a6ac0b44bab9 ("drm/amdgpu: add df perfmon regs and funcs for xgmi")
> Signed-off-by: Colin Ian King
Reviewed-by: Nathan Chancellor
This fixes a clang warning complaining about the same thing.
> ---
> drivers/gpu/drm/amd/amdgpu/df_v3_6.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> index a5c3558869fb..8c09bf994acd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> +++ b/drivers/gpu/drm/amd/amdgpu/df_v3_6.c
> @@ -398,10 +398,7 @@ static int df_v3_6_start_xgmi_link_cntr(struct
> amdgpu_device *adev,
> NULL);
>
> if (lo_base_addr == 0)
> - ret = -EINVAL;
> -
> - if (ret)
> - return ret;
> + return -EINVAL;
>
> lo_val = RREG32_PCIE(lo_base_addr);
>
> --
> 2.20.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/komeda: Use memset to initialize config_id
Clang warns:
drivers/gpu/drm/arm/display/komeda/komeda_dev.c:76:38: warning: suggest
braces around initialization of subobject [-Wmissing-braces]
union komeda_config_id config_id = {0,};
^
{}
1 warning generated.
One way to fix these warnings is to add additional braces like Clang
suggests; however, there has been a bit of push back from some
maintainers, who just prefer memset as it is unambiguous, doesn't
depend on a particular compiler version, and properly initializes all
subobjects [1][2]. Do that here so there are no more warnings.
[1]: https://lore.kernel.org/lkml/022e41c0-8465-dc7a-a45c-64187ecd9...@amd.com/
[2]:
https://lore.kernel.org/lkml/20181128.215241.702406654469517539.da...@davemloft.net/
Fixes: 4cc734cb79a8 ("drm/komeda: Add sysfs attribute: core_id and config_id")
Link: https://github.com/ClangBuiltLinux/linux/issues/457
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index 9d6c31cca875..e605a518f59a 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -73,9 +73,10 @@ config_id_show(struct device *dev, struct device_attribute
*attr, char *buf)
{
struct komeda_dev *mdev = dev_to_mdev(dev);
struct komeda_pipeline *pipe = mdev->pipelines[0];
- union komeda_config_id config_id = {0,};
+ union komeda_config_id config_id;
int i;
+ memset(_id, 0, sizeof(config_id));
config_id.max_line_sz = pipe->layers[0]->hsize_in.end;
config_id.n_pipelines = mdev->n_pipelines;
config_id.n_scalers = pipe->n_scalers;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Clang warning in drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
turn TA_RAS_ERROR__NONE;
+ }
+}
+
/* called in ip_init and ip_fini */
int amdgpu_ras_init(struct amdgpu_device *adev);
void amdgpu_ras_post_init(struct amdgpu_device *adev);
> On Wed, Mar 20, 2019 at 2:37 AM Koenig, Christian
> wrote:
>
> > Am 20.03.19 um 05:34 schrieb Nathan Chancellor:
> > > On Wed, Mar 20, 2019 at 01:31:27AM +, Pan, Xinhui wrote:
> > >> these two enumerated types are same for now. both of them might change
> > in the future.
> > >>
> > >> I have not used clang, but would .block_id = (int)head->block fix
> > your warning? If such change is acceptable, I can make one then.
> > >>
> > >> Thanks
> > >> xinhui
> > >>
> > >>
> > >> -Original Message-
> > >> From: Nathan Chancellor
> > >> Sent: 2019年3月20日 8:54
> > >> To: Deucher, Alexander ; Koenig, Christian <
> > christian.koe...@amd.com>; Zhou, David(ChunMing) ;
> > Pan, Xinhui
> > >> Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > linux-ker...@vger.kernel.org; clang-built-li...@googlegroups.com
> > >> Subject: Clang warning in drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> > >>
> > >> Hi all,
> > >>
> > >> The introduction of this file in commit dbd249c24427 ("drm/amdgpu: add
> > amdgpu_ras.c to support ras (v2)") introduces the following Clang
> > >> warnings:
> > >>
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:544:23: warning: implicit
> > conversion from enumeration type 'enum amdgpu_ras_block' to different
> > enumeration type 'enum ta_ras_block' [-Wenum-conversion]
> > >> .block_id = head->block,
> > >> ~~^
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:545:24: warning: implicit
> > conversion from enumeration type 'enum amdgpu_ras_error_type' to different
> > enumeration type 'enum ta_ras_error_type' [-Wenum-conversion]
> > >> .error_type = head->type,
> > >>~~^~~~
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:549:23: warning: implicit
> > conversion from enumeration type 'enum amdgpu_ras_block' to different
> > enumeration type 'enum ta_ras_block' [-Wenum-conversion]
> > >> .block_id = head->block,
> > >> ~~^
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:550:24: warning: implicit
> > conversion from enumeration type 'enum amdgpu_ras_error_type' to different
> > enumeration type 'enum ta_ras_error_type' [-Wenum-conversion]
> > >> .error_type = head->type,
> > >>~~^~~~
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:650:26: warning: implicit
> > conversion from enumeration type 'enum amdgpu_ras_block' to different
> > enumeration type 'enum ta_ras_block' [-Wenum-conversion]
> > >> .block_id = info->head.block,
> > >> ~~~^
> > >> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:651:35: warning: implicit
> > conversion from enumeration type 'enum amdgpu_ras_error_type' to different
> > enumeration type 'enum ta_ras_error_type' [-Wenum-conversion]
> > >> .inject_error_type = info->head.type,
> > >> ~~~^~~~
> > >> 6 warnings generated.
> > >>
> > >> Normally, I would sent a fix for this myself but I am not entirely sure
> > why these two enumerated types exist when one would do since they have the
> > same values minus the prefix. In fact, the ta_ras_{block,error_type} values
> > are never used aside from being defined. Some clarification would be
> > appreciated.
> > >>
> > >> Thank you,
> > >> Nathan
> > > Hi Xinhui,
> > >
> > > Yes, explicitly casting these six spots to int would resolve this
> > > warning.
> >
> > Another question is if it is such a good idea to just silence the warning?
> >
> > Maybe add a amdgpu_ras_error_to_ta() helper to do this casting?
> >
> > When the enums drift away from each other then we can still add warnings
> > to that helper to make sure we don't accidentally cast invalid values
> > around.
> >
> > Regards,
> > Christian.
> >
> > >
> > > Thank you for the quick response!
> > > Nathan
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Clang Built Linux" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to clang-built-linux+unsubscr...@googlegroups.com.
> > To post to this group, send email to clang-built-li...@googlegroups.com.
> > To view this discussion on the web visit
> > https://groups.google.com/d/msgid/clang-built-linux/63518f1f-b808-77b0-aac6-ee1ece669c4b%40amd.com
> > .
> > For more options, visit https://groups.google.com/d/optout.
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/amd/powerplay: Zero initialize num_of_levels in vega20_set_single_dpm_table
When building with -Wsometimes-uninitialized, Clang warns:
drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:456:2: warning:
variable 'num_of_levels' is used uninitialized whenever '?:' condition
is false [-Wsometimes-uninitialized]
smu_read_smc_arg(smu, _of_levels);
^
drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:608:3: note:
expanded from macro 'smu_read_smc_arg'
((smu)->funcs->read_smc_arg? (smu)->funcs->read_smc_arg((smu), (arg)) :
0)
^~
drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:457:7: note:
uninitialized use occurs here
if (!num_of_levels) {
^
drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:456:2: note: remove
the '?:' if its condition is always true
smu_read_smc_arg(smu, _of_levels);
^
drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:608:3: note:
expanded from macro 'smu_read_smc_arg'
((smu)->funcs->read_smc_arg? (smu)->funcs->read_smc_arg((smu), (arg)) :
0)
^
drivers/gpu/drm/amd/amdgpu/../powerplay/vega20_ppt.c:446:27: note:
initialize the variable 'num_of_levels' to silence this warning
uint32_t i, num_of_levels, clk;
^
= 0
1 warning generated.
The if statement it mentions as potentially problematic is currently
always true because the read_smc_arg callback is assigned at the
bottom of this file but Clang can't tell that. If the callback were
ever to disappear, num_of_levels would never be initialized. Just
zero initialize it to ensure that the intent behind this code
remains the same.
Fixes: 870b996f955f ("drm/amd/powerplay: set defalut dpm table for smu")
Link: https://github.com/ClangBuiltLinux/linux/issues/425
Signed-off-by: Nathan Chancellor
---
drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
index 7e9e8ad9a300..41e6f49c9cb6 100644
--- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c
@@ -443,7 +443,7 @@ vega20_set_single_dpm_table(struct smu_context *smu,
PPCLK_e clk_id)
{
int ret = 0;
- uint32_t i, num_of_levels, clk;
+ uint32_t i, num_of_levels = 0, clk;
ret = smu_send_smc_msg_with_param(smu,
SMU_MSG_GetDpmFreqByIndex,
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Clang warning in drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
Hi all,
The introduction of this file in commit dbd249c24427 ("drm/amdgpu: add
amdgpu_ras.c to support ras (v2)") introduces the following Clang
warnings:
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:544:23: warning: implicit conversion
from enumeration type 'enum amdgpu_ras_block' to different enumeration type
'enum ta_ras_block' [-Wenum-conversion]
.block_id = head->block,
~~^
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:545:24: warning: implicit conversion
from enumeration type 'enum amdgpu_ras_error_type' to different enumeration
type 'enum ta_ras_error_type' [-Wenum-conversion]
.error_type = head->type,
~~^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:549:23: warning: implicit conversion
from enumeration type 'enum amdgpu_ras_block' to different enumeration type
'enum ta_ras_block' [-Wenum-conversion]
.block_id = head->block,
~~^
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:550:24: warning: implicit conversion
from enumeration type 'enum amdgpu_ras_error_type' to different enumeration
type 'enum ta_ras_error_type' [-Wenum-conversion]
.error_type = head->type,
~~^~~~
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:650:26: warning: implicit conversion
from enumeration type 'enum amdgpu_ras_block' to different enumeration type
'enum ta_ras_block' [-Wenum-conversion]
.block_id = info->head.block,
~~~^
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:651:35: warning: implicit conversion
from enumeration type 'enum amdgpu_ras_error_type' to different enumeration
type 'enum ta_ras_error_type' [-Wenum-conversion]
.inject_error_type = info->head.type,
~~~^~~~
6 warnings generated.
Normally, I would sent a fix for this myself but I am not entirely sure
why these two enumerated types exist when one would do since they have
the same values minus the prefix. In fact, the ta_ras_{block,error_type}
values are never used aside from being defined. Some clarification would
be appreciated.
Thank you,
Nathan
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
