[PATCH] drm/xe: replace format-less snprintf() with strscpy()
From: Arnd Bergmann
Using snprintf() with a format string from task->comm is a bit
dangerous since the string may be controlled by unprivileged
userspace:
drivers/gpu/drm/xe/xe_devcoredump.c: In function 'devcoredump_snapshot':
drivers/gpu/drm/xe/xe_devcoredump.c:184:9: error: format not a string literal
and no format arguments [-Werror=format-security]
184 | snprintf(ss->process_name, sizeof(ss->process_name),
process_name);
| ^~~~
In this case there is no reason for an snprintf(), so use a simpler
string copy.
Fixes: b10d0c5e9df7 ("drm/xe: Add process name to devcoredump")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/xe/xe_devcoredump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c
b/drivers/gpu/drm/xe/xe_devcoredump.c
index 1643d44f8bc4..1973bfaece40 100644
--- a/drivers/gpu/drm/xe/xe_devcoredump.c
+++ b/drivers/gpu/drm/xe/xe_devcoredump.c
@@ -181,7 +181,7 @@ static void devcoredump_snapshot(struct xe_devcoredump
*coredump,
if (task)
process_name = task->comm;
}
- snprintf(ss->process_name, sizeof(ss->process_name), process_name);
+ strscpy(ss->process_name, process_name);
if (task)
put_task_struct(task);
--
2.39.2
[PATCH] udmabuf: add CONFIG_MMU dependency
From: Arnd Bergmann
There is no !CONFIG_MMU version of vmf_insert_pfn():
arm-linux-gnueabi-ld: drivers/dma-buf/udmabuf.o: in function `udmabuf_vm_fault':
udmabuf.c:(.text+0xaa): undefined reference to `vmf_insert_pfn'
Fixes: f7254e043ff1 ("udmabuf: use vmf_insert_pfn and VM_PFNMAP for handling
mmap")
Signed-off-by: Arnd Bergmann
---
drivers/dma-buf/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
index e4dc53a36428..b46eb8a552d7 100644
--- a/drivers/dma-buf/Kconfig
+++ b/drivers/dma-buf/Kconfig
@@ -35,6 +35,7 @@ config UDMABUF
default n
depends on DMA_SHARED_BUFFER
depends on MEMFD_CREATE || COMPILE_TEST
+ depends on MMU
help
A driver to let userspace turn memfd regions into dma-bufs.
Qemu can use this to create host dmabufs for guest framebuffers.
--
2.39.2
[PATCH 4/4] drm/amd/display: Move 'struct scaler_data' off stack
From: Arnd Bergmann
The scaler_data structure is implicitly copied onto the stack twice by
being returned from a function. This is usually a bad idea, but it
was not flagged by the compiler until a recent addition that pushed
it over the 1024 byte function stack limit:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_translation_helper.c: In
function 'populate_dml_plane_cfg_from_plane_state':
drivers/gpu/drm/amd/amdgpu/../display/dc/dml2/dml2_translation_helper.c:1075:1:
error: the frame size of 1032 bytes is larger than 1024 bytes
[-Werror=frame-larger-than=]
Use an explicit kzalloc() and memcpy() instead here to keep it off the
stack.
Fixes: 00c391102abc ("drm/amd/display: Add misc DC changes for DCN401")
Fixes: 7966f319c66d ("drm/amd/display: Introduce DML2")
Signed-off-by: Arnd Bergmann
---
.../display/dc/dml2/dml2_translation_helper.c | 56 ++-
1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
index 705985d3f407..c04ebf5434c9 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
+++ b/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c
@@ -927,7 +927,7 @@ static void populate_dml_surface_cfg_from_plane_state(enum
dml_project_id dml2_p
}
}
-static struct scaler_data get_scaler_data_for_plane(const struct
dc_plane_state *in, struct dc_state *context)
+static void get_scaler_data_for_plane(const struct dc_plane_state *in, struct
dc_state *context, struct scaler_data *out)
{
int i;
struct pipe_ctx *temp_pipe = >res_ctx.temp_pipe;
@@ -948,7 +948,7 @@ static struct scaler_data get_scaler_data_for_plane(const
struct dc_plane_state
}
ASSERT(i < MAX_PIPES);
- return temp_pipe->plane_res.scl_data;
+ memcpy(out, _pipe->plane_res.scl_data, sizeof(*out));
}
static void populate_dummy_dml_plane_cfg(struct dml_plane_cfg_st *out,
unsigned int location, const struct dc_stream_state *in)
@@ -1007,27 +1007,31 @@ static void populate_dummy_dml_plane_cfg(struct
dml_plane_cfg_st *out, unsigned
static void populate_dml_plane_cfg_from_plane_state(struct dml_plane_cfg_st
*out, unsigned int location, const struct dc_plane_state *in, struct dc_state
*context)
{
- const struct scaler_data scaler_data = get_scaler_data_for_plane(in,
context);
+ struct scaler_data *scaler_data = kzalloc(sizeof(*scaler_data),
GFP_KERNEL);
+ if (!scaler_data)
+ return;
+
+ get_scaler_data_for_plane(in, context, scaler_data);
out->CursorBPP[location] = dml_cur_32bit;
out->CursorWidth[location] = 256;
out->GPUVMMinPageSizeKBytes[location] = 256;
- out->ViewportWidth[location] = scaler_data.viewport.width;
- out->ViewportHeight[location] = scaler_data.viewport.height;
- out->ViewportWidthChroma[location] = scaler_data.viewport_c.width;
- out->ViewportHeightChroma[location] = scaler_data.viewport_c.height;
- out->ViewportXStart[location] = scaler_data.viewport.x;
- out->ViewportYStart[location] = scaler_data.viewport.y;
- out->ViewportXStartC[location] = scaler_data.viewport_c.x;
- out->ViewportYStartC[location] = scaler_data.viewport_c.y;
+ out->ViewportWidth[location] = scaler_data->viewport.width;
+ out->ViewportHeight[location] = scaler_data->viewport.height;
+ out->ViewportWidthChroma[location] = scaler_data->viewport_c.width;
+ out->ViewportHeightChroma[location] = scaler_data->viewport_c.height;
+ out->ViewportXStart[location] = scaler_data->viewport.x;
+ out->ViewportYStart[location] = scaler_data->viewport.y;
+ out->ViewportXStartC[location] = scaler_data->viewport_c.x;
+ out->ViewportYStartC[location] = scaler_data->viewport_c.y;
out->ViewportStationary[location] = false;
- out->ScalerEnabled[location] = scaler_data.ratios.horz.value !=
dc_fixpt_one.value ||
- scaler_data.ratios.horz_c.value !=
dc_fixpt_one.value ||
- scaler_data.ratios.vert.value !=
dc_fixpt_one.value ||
- scaler_data.ratios.vert_c.value !=
dc_fixpt_one.value;
+ out->ScalerEnabled[location] = scaler_data->ratios.horz.value !=
dc_fixpt_one.value ||
+ scaler_data->ratios.horz_c.value !=
dc_fixpt_one.value ||
+ scaler_data->ratios.vert.value !=
dc_fixpt_one.value ||
+ scaler_data->ratios.vert_c.value !=
dc_fixpt_one.value;
/* Current driver code base uses LBBitPerPixel as 57. There is a
discrepancy
* from the HW/DML teams about this value. Initialize LBBitPerPixel
with the
@@ -1043,25 +1
[PATCH 3/4] drm/amd/display: avoid large on-stack structures
From: Arnd Bergmann
Putting excessively large objects on a function stack causes
a warning about possibly overflowing the 8KiB of kernel stack:
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn401/dcn401_resource.c: In
function 'dcn401_update_bw_bounding_box':
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn401/dcn401_resource.c:1599:1:
error: the frame size of 1196 bytes is larger than 1024 bytes
[-Werror=frame-larger-than=]
1599 | }
| ^
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_state.c: In function
'dc_state_create':
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_state.c:221:1: error: the
frame size of 1196 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
221 | }
| ^
Use dynamic allocation instead.
Fixes: e779f4587f61 ("drm/amd/display: Add handling for DC power mode")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/amd/display/dc/core/dc_state.c | 16 +++-
.../display/dc/resource/dcn401/dcn401_resource.c | 16 +++-
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
index 70928223b642..8ea9391c60b7 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_state.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_state.c
@@ -193,7 +193,11 @@ static void init_state(struct dc *dc, struct dc_state
*state)
struct dc_state *dc_state_create(struct dc *dc, struct dc_state_create_params
*params)
{
#ifdef CONFIG_DRM_AMD_DC_FP
- struct dml2_configuration_options dml2_opt = dc->dml2_options;
+ struct dml2_configuration_options *dml2_opt;
+
+ dml2_opt = kmemdup(>dml2_options, sizeof(*dml2_opt), GFP_KERNEL);
+ if (!dml2_opt)
+ return NULL;
#endif
struct dc_state *state = kvzalloc(sizeof(struct dc_state),
GFP_KERNEL);
@@ -207,12 +211,14 @@ struct dc_state *dc_state_create(struct dc *dc, struct
dc_state_create_params *p
#ifdef CONFIG_DRM_AMD_DC_FP
if (dc->debug.using_dml2) {
- dml2_opt.use_clock_dc_limits = false;
- dml2_create(dc, _opt, >bw_ctx.dml2);
+ dml2_opt->use_clock_dc_limits = false;
+ dml2_create(dc, dml2_opt, >bw_ctx.dml2);
- dml2_opt.use_clock_dc_limits = true;
- dml2_create(dc, _opt, >bw_ctx.dml2_dc_power_source);
+ dml2_opt->use_clock_dc_limits = true;
+ dml2_create(dc, dml2_opt, >bw_ctx.dml2_dc_power_source);
}
+
+ kfree(dml2_opt);
#endif
kref_init(>refcount);
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
b/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
index 247bac177d1b..8dfb0a3d21cb 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn401/dcn401_resource.c
@@ -1581,21 +1581,27 @@ static struct dc_cap_funcs cap_funcs = {
static void dcn401_update_bw_bounding_box(struct dc *dc, struct clk_bw_params
*bw_params)
{
- struct dml2_configuration_options dml2_opt = dc->dml2_options;
+ struct dml2_configuration_options *dml2_opt;
+
+ dml2_opt = kmemdup(>dml2_options, sizeof(*dml2_opt), GFP_KERNEL);
+ if (!dml2_opt)
+ return;
DC_FP_START();
dcn401_update_bw_bounding_box_fpu(dc, bw_params);
- dml2_opt.use_clock_dc_limits = false;
+ dml2_opt->use_clock_dc_limits = false;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2)
- dml2_reinit(dc, _opt, >current_state->bw_ctx.dml2);
+ dml2_reinit(dc, dml2_opt, >current_state->bw_ctx.dml2);
- dml2_opt.use_clock_dc_limits = true;
+ dml2_opt->use_clock_dc_limits = true;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2_dc_power_source)
- dml2_reinit(dc, _opt,
>current_state->bw_ctx.dml2_dc_power_source);
+ dml2_reinit(dc, dml2_opt,
>current_state->bw_ctx.dml2_dc_power_source);
DC_FP_END();
+
+ kfree(dml2_opt);
}
enum dc_status dcn401_patch_unknown_plane_state(struct dc_plane_state
*plane_state)
--
2.39.2
[PATCH 2/4] [RESEND] drm/amd/display: fix graphics_object_id size
From: Arnd Bergmann
The graphics_object_id structure is meant to fit into 32 bits, as it's
passed by value in and out of functions. A recent change increased
the size to 128 bits, so it's now always passed by reference, which
is clearly not intended and ends up producing a compile-time warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c: In function
'construct_phy':
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c:743:1: error: the
frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Add back the bitfields to revert to the original size, while keeping
the 'enum' type change.
fec85f995a4b ("drm/amd/display: Fix compiler redefinition warnings for certain
configs")
Signed-off-by: Arnd Bergmann
---
Originally sent as
https://lore.kernel.org/all/20240418083421.3956461-2-a...@kernel.org/
---
drivers/gpu/drm/amd/display/include/grph_object_id.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/include/grph_object_id.h
b/drivers/gpu/drm/amd/display/include/grph_object_id.h
index 08ee0350b31f..54e33062b3c0 100644
--- a/drivers/gpu/drm/amd/display/include/grph_object_id.h
+++ b/drivers/gpu/drm/amd/display/include/grph_object_id.h
@@ -226,8 +226,8 @@ enum dp_alt_mode {
struct graphics_object_id {
uint32_t id:8;
- enum object_enum_id enum_id;
- enum object_type type;
+ enum object_enum_id enum_id :4;
+ enum object_type type :4;
uint32_t reserved:16; /* for padding. total size should be u32 */
};
--
2.39.2
[PATCH 1/4] [RESEND] drm/amd/display: dynamically allocate dml2_configuration_options structures
From: Arnd Bergmann
This structure is too large to fit on a stack, as shown by the
newly introduced warnings from a recent code change:
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn32/dcn32_resource.c: In
function 'dcn32_update_bw_bounding_box':
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn32/dcn32_resource.c:2019:1:
error: the frame size of 1180 bytes is larger than 1024 bytes
[-Werror=frame-larger-than=]
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn321/dcn321_resource.c: In
function 'dcn321_update_bw_bounding_box':
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn321/dcn321_resource.c:1597:1:
error: the frame size of 1180 bytes is larger than 1024 bytes
[-Werror=frame-larger-than=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_state.c: In function
'dc_state_create':
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_state.c:219:1: error: the
frame size of 1184 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Instead of open-coding the assignment of a large structure to a stack
variable, use an explicit kmemdup() in each case to move it off the stack.
Fixes: e779f4587f61 ("drm/amd/display: Add handling for DC power mode")
Signed-off-by: Arnd Bergmann
---
Originally sent as
https://lore.kernel.org/all/20240418083421.3956461-1-a...@kernel.org/
---
.../display/dc/resource/dcn32/dcn32_resource.c | 16 +++-
.../display/dc/resource/dcn321/dcn321_resource.c | 16 +++-
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
index 022d320be1d5..0f11d7c8791c 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
@@ -2007,21 +2007,27 @@ void dcn32_calculate_wm_and_dlg(struct dc *dc, struct
dc_state *context,
static void dcn32_update_bw_bounding_box(struct dc *dc, struct clk_bw_params
*bw_params)
{
- struct dml2_configuration_options dml2_opt = dc->dml2_options;
+ struct dml2_configuration_options *dml2_opt;
+
+ dml2_opt = kmemdup(>dml2_options, sizeof(dc->dml2_options),
GFP_KERNEL);
+ if (!dml2_opt)
+ return;
DC_FP_START();
dcn32_update_bw_bounding_box_fpu(dc, bw_params);
- dml2_opt.use_clock_dc_limits = false;
+ dml2_opt->use_clock_dc_limits = false;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2)
- dml2_reinit(dc, _opt, >current_state->bw_ctx.dml2);
+ dml2_reinit(dc, dml2_opt, >current_state->bw_ctx.dml2);
- dml2_opt.use_clock_dc_limits = true;
+ dml2_opt->use_clock_dc_limits = true;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2_dc_power_source)
- dml2_reinit(dc, _opt,
>current_state->bw_ctx.dml2_dc_power_source);
+ dml2_reinit(dc, dml2_opt,
>current_state->bw_ctx.dml2_dc_power_source);
DC_FP_END();
+
+ kfree(dml2_opt);
}
static struct resource_funcs dcn32_res_pool_funcs = {
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
index e4b360d89b3b..07ca6f58447d 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
@@ -1581,21 +1581,27 @@ static struct dc_cap_funcs cap_funcs = {
static void dcn321_update_bw_bounding_box(struct dc *dc, struct clk_bw_params
*bw_params)
{
- struct dml2_configuration_options dml2_opt = dc->dml2_options;
+ struct dml2_configuration_options *dml2_opt;
+
+ dml2_opt = kmemdup(>dml2_options, sizeof(dc->dml2_options),
GFP_KERNEL);
+ if (!dml2_opt)
+ return;
DC_FP_START();
dcn321_update_bw_bounding_box_fpu(dc, bw_params);
- dml2_opt.use_clock_dc_limits = false;
+ dml2_opt->use_clock_dc_limits = false;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2)
- dml2_reinit(dc, _opt, >current_state->bw_ctx.dml2);
+ dml2_reinit(dc, dml2_opt, >current_state->bw_ctx.dml2);
- dml2_opt.use_clock_dc_limits = true;
+ dml2_opt->use_clock_dc_limits = true;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2_dc_power_source)
- dml2_reinit(dc, _opt,
>current_state->bw_ctx.dml2_dc_power_source);
+ dml2_reinit(dc, dml2_opt,
>current_state->bw_ctx.dml2_dc_power_source);
DC_FP_END();
+
+ kfree(dml2_opt);
}
static struct resource_funcs dcn321_res_pool_funcs = {
--
2.39.2
[PATCH] drm/amdkfd: select CONFIG_CRC16
From: Arnd Bergmann
The amdkfd support fails to link when CONFIG_CRC16 is disabled:
aarch64-linux-ld: drivers/gpu/drm/amd/amdkfd/kfd_topology.o: in function
`kfd_topology_add_device':
kfd_topology.c:(.text+0x3a4c): undefined reference to `crc16'
This is a library module that needs to be selected from every user.
Fixes: 3ed181b8ff43 ("drm/amdkfd: Ensure gpu_id is unique")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/amd/amdkfd/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig
b/drivers/gpu/drm/amd/amdkfd/Kconfig
index d3c3d3ab7225..f82595af34bf 100644
--- a/drivers/gpu/drm/amd/amdkfd/Kconfig
+++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
@@ -6,6 +6,7 @@
config HSA_AMD
bool "HSA kernel driver for AMD GPU devices"
depends on DRM_AMDGPU && (X86_64 || ARM64 || PPC64)
+ select CRC16
select HMM_MIRROR
select MMU_NOTIFIER
select DRM_AMDGPU_USERPTR
--
2.39.2
Re: [v7 3/7] arm64: defconfig: Enable HIMAX_HX83102 panel
On Thu, May 16, 2024, at 14:09, Doug Anderson wrote: > On Thu, May 16, 2024 at 6:43 AM Doug Anderson wrote: >> On Wed, May 15, 2024 at 11:55 PM wrote: >> > On 16/05/2024 08:43, cong yang wrote: >> > >> > Yeah we usually don't mess with arch specific defconfig from drm tree >> >> In general I agree that makes sense. In this case, though, the new >> config symbol was introduced in the previous patch and split off an >> existing symbol. Updating "all" of the configs (AKA just arm64) that >> had the old symbol to also have the new symbol seems like the nice >> thing to do and it feels like it makes sense to land in the same tree >> that did the "split" just to cause the least confusion to anyone >> affected. >> >> In any case, if it's going to land in some other tree then I guess the >> question is whether it needs to wait a few revisions to land there or >> if it should land right away. Nobody would get a compile error if it >> landed in a different tree right away since unknown config symbols are >> silently ignored, but it feels a little weird to me. >> >> ...of course, I'm also OK just dropping the config patch. I personally >> don't use the upstream "defconfig". It just seemed courteous to update >> it for those who do. > > Hmmm, probably should have put Arnd on this thread. Added now in case > he has any opinions. I also did manage to find when this last came up > where I was involved. At that time Will Deacon (who get_maintainer.pl > reports is the official maintainer of this file) said [1]: > >> But yes, although there are a few things I really care about >> in defconfig (e.g. things like page size!), generally speaking we don't >> need to Ack everything that changes in there. > My preferred way of getting arm/arm64 defconfig updates is to have them picked up by the platform maintainer, the same way we handle updates to dts files. The platform maintainers are familiar with the process and will send the patches on to me for integration through the soc tree. If a change is not specific to any particular platform, I recommend to send it to:s...@kernel.org, cc:lakml. This makes it show up in my patchwork, so I will eventually get around to picking it up. When you do this, it's helpful to me if you include an explanation (after the --- line) why this patch does not get picked up by a platform maintainer, and it also helps me to include whether I should include it in the current (6.10) fixes or queue it for the next merge window. Arnd
Re: [PATCH v2 1/1] video: Handle HAS_IOPORT dependencies
On Tue, May 14, 2024, at 13:08, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
> compile time. We thus need to #ifdef functions and their callsites which
> unconditionally use these I/O accessors. In the include/video/vga.h
> these are conveniently all those functions with the vga_io_* prefix.
>
> Co-developed-by: Arnd Bergmann
> Signed-off-by: Arnd Bergmann
> Signed-off-by: Niklas Schnelle
> ---
> Note: This patch does not depend any not-yet-mainline HAS_IOPORT changes
> and may be merged via subsystem specific trees at your earliest
> convenience.
>
> v1 -> v2:
> - Moved vga_mm_r(), vga_mm_w(), vga_mm_w_fast() above #ifdef CONFIG_HAS_IOPORT
> to use them in with or without I/O port variants.
> - Duplicated vga_r(), vga_w(), vga_w_fast() functions as non-I/O port variants
> to get rid of in-code #ifdef (Arnd)
> - Got rid of if (regbase) logic inversion needed for in-code #ifdef
Thanks for preparing the new version!
> diff --git a/include/video/vga.h b/include/video/vga.h
> index 947c0abd04ef..468764d6727a 100644
> --- a/include/video/vga.h
> +++ b/include/video/vga.h
> @@ -197,9 +197,26 @@ struct vgastate {
> extern int save_vga(struct vgastate *state);
> extern int restore_vga(struct vgastate *state);
>
> +static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned
> short port)
> +{
> + return readb (regbase + port);
> +}
My first thought was that this should use the normal whitespace,
but I guess the file is pretty consistent about the style here,
so I agree with your choice here.
Arnd
Re: [PATCH v2 5/5] misc: add ge-addon-connector driver
On Fri, May 10, 2024, at 12:54, Luca Ceresoli wrote: > On Fri, 10 May 2024 12:24:06 +0200 "Arnd Bergmann" wrote: >> On Fri, May 10, 2024, at 09:55, Greg Kroah-Hartman wrote: >> > On Fri, May 10, 2024 at 09:10:41AM +0200, Luca Ceresoli wrote: >> >> >> >> +config GE_SUNH_CONNECTOR >> >> + tristate "GE SUNH hotplug add-on connector" >> >> + depends on OF >> >> + select OF_OVERLAY >> >> + select FW_LOADER >> >> + select NVMEM >> >> + select DRM_HOTPLUG_BRIDGE >> > >> > Can these be depends instead of select? 'select' causes dependencies >> > that are hard, if not almost impossible, to detect at times why >> > something is being enabled. >> >> I think FW_LOADER needs to be 'select' since it is normally >> a hidden symbol and gets selected by its users, all the other >> ones should be 'depends on'. > > I see, makes sense. > > And as you pointed that out, I realize perhaps DRM_HOTPLUG_BRIDGE could > become a hidden symbol as it's not expected to be used alone. It's slightly easier to keep it as a visible symbol with 'depends on' though, since otherwise you have to add 'depends on' statments for anything that DRM_HOTPLUG_BRIDGE in turn depends on, most notably DRM itself. Arnd
Re: [PATCH v2 5/5] misc: add ge-addon-connector driver
On Fri, May 10, 2024, at 09:55, Greg Kroah-Hartman wrote: > On Fri, May 10, 2024 at 09:10:41AM +0200, Luca Ceresoli wrote: >> >> +config GE_SUNH_CONNECTOR >> +tristate "GE SUNH hotplug add-on connector" >> +depends on OF >> +select OF_OVERLAY >> +select FW_LOADER >> +select NVMEM >> +select DRM_HOTPLUG_BRIDGE > > Can these be depends instead of select? 'select' causes dependencies > that are hard, if not almost impossible, to detect at times why > something is being enabled. I think FW_LOADER needs to be 'select' since it is normally a hidden symbol and gets selected by its users, all the other ones should be 'depends on'. Arnd
Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate
On Wed, May 8, 2024, at 22:36, Sam Ravnborg wrote: >> >> I think if you want to do a new version, that is likely to run >> into new problems, given that this part of fbdev is particularly >> fragile and partly wrong. On the other hand, it would be nice to >> have a patch to limit the use of the notifiers to the smallest >> set of kernel configs that actually need it, and leave it turned >> off for everything else. >> >> These are the ones I could find: >> >> - CONFIG_GUMSTIX_AM200EPD (FB_EVENT_FB_REGISTERED) > > I was surprised to see this driver is still around as many other old > drivers was nuked as part of the pxa cleanup. > It is the only user of FB_EVENT_FB_REGISTERED - so a potential cleanup > if the driver is no longer relevant. We kept gumstix at the time to give a chance to do a DT conversion using the qemu model, but so far nobody has worked on this. My feeling is that we'll end up removing it at some point in the future along with the other legacy PXA board files. Even if someone wants to keep working on gumstix DT support for qemu, I agree that we can probably remove AM200EPD, AM300EPD and metronomefb, since those are not even modeled by qemu. Arnd
Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate
On Wed, May 8, 2024, at 20:37, Florian Fainelli wrote: > On 5/7/24 04:44, Arnd Bergmann wrote: >> On Tue, May 7, 2024, at 13:10, Daniel Vetter wrote: >>> On Mon, May 06, 2024 at 04:53:47PM +0200, Arnd Bergmann wrote: >> Right, let's wait for Florian to reply. From what he said earlier >> though, the only reason that the notifiers are getting in the >> way is the link error you get from trying to load a separately >> built fb.ko module on a kernel that was built with FB=n / FB_CORE=n, >> so I don't think he even cares about notifiers, only about >> allowing the recovery application to mmap() the framebuffer. > > Right, we do not really care about notifiers AFAICT. Based upon this > discussion there has been an action on our side to stop making use of > the FB subsystem for recovery and use the full blow DRM driver instead. Ok, sounds good. > While we get there, though I still see some value into this patch (or a > v2, that is). I have a v2 ready if you think there is some value in > pursuing that route, if not, we can stop there. I think if you want to do a new version, that is likely to run into new problems, given that this part of fbdev is particularly fragile and partly wrong. On the other hand, it would be nice to have a patch to limit the use of the notifiers to the smallest set of kernel configs that actually need it, and leave it turned off for everything else. These are the ones I could find: - CONFIG_GUMSTIX_AM200EPD (FB_EVENT_FB_REGISTERED) - CONFIG_LCD_CORGI, CONFIG_LCD_TDO24M (FB_EVENT_MODE_CHANGE) - CONFIG_LEDS_TRIGGER_BACKLIGHT (FB_EVENT_BLANK) - CONFIG_FB_OLPC_DCON (FB_EVENT_BLANK/BL_CORE_FBBLANK) - CONFIG_FB_SH_MOBILE_LCDC, CONFIG_BACKLIGHT_PCF50633, CONFIG_BACKLIGHT_PANDORA, CONFIG_BACKLIGHT_LP855X (BL_CORE_FBBLANK) - CONFIG_FB_CLPS711X, CONFIG_FB_IMX, CONFIG_MACH_AMS_DELTA (lcd BL_CORE_FBBLANK) - CONFIG_LCD_AMS369FG06, CONFIG_LCD_CORGI, CONFIG_LCD_HX8357, CONFIG_LCD_ILI922X, CONFIG_LCD_ILI9320, CONFIG_LCD_HP700, CONFIG_LCD_L4F00242T03, CONFIG_LCD_LMS283GF05, CONFIG_LCD_LMS501KF03 CONFIG_LCD_LTV350QV, CONFIG_LCD_OTM3225A, CONFIG_LCD_PLATFORM, CONFIG_LCD_TDO24M (lcd BL_CORE_FBBLANK) Almost all of these are exclusive to ancient ARMv5 boards or similar, so if we make the notifiers depend on the whole list, this would leave it disabled even for most configurations that enable CONFIG_FB=y. This could be done with a 'select', but I'd prefer the 'default y; depends on LCD_FOO || LCD_BAR || ...' variant because that makes it easier to spot if someone tries to add another one. Arnd
Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate
On Tue, May 7, 2024, at 13:10, Daniel Vetter wrote: > On Mon, May 06, 2024 at 04:53:47PM +0200, Arnd Bergmann wrote: >> On Mon, May 6, 2024, at 15:14, Daniel Vetter wrote: >> > On Fri, May 03, 2024 at 01:22:10PM -0700, Florian Fainelli wrote: >> >> On 5/3/24 12:45, Arnd Bergmann wrote: >> >> This is the current Android GKI config: >> https://android.googlesource.com/kernel/common.git/+/refs/heads/android-mainline/arch/arm64/configs/gki_defconfig >> where I can see that CONFIG_DRM is built-in, but DRM_FBDEV_EMULATION >> CONFIG_VT, CONFIG_FRAMEBUFFER_CONSOLE, CONFIG_FB_DEVICE and >> CONFIG_FB_CORE are all disabled. >> >> So the console won't work at all,I think this means that there >> is no way to get the console working, but building a fb.ko module >> allows using /dev/fb with simplefb.ko (or any other one) happens >> to almost work, but only by dumb luck rather than by design. > > So using /dev/fb chardev without fbcon is very much a real idea. This way > you should be able to run old userspace that uses fbdev directly for > drawing, but your console needs are served by a userspace console running > on top of drm. > > vt switching gets a bit more entertaining, but I thought logind has all > the glue already to make that happen. Worst case you need a tiny launcher > tool to get your userspace console out of the way while you launch a fbdev > using application, but I think correctly implement the vt ioctls to switch > to graphics mode /should/ work automatically. > > I do agree that this is only really a good idea with drm drivers, since > those do not rely on any of the fbdev infrastructure like the notifier > under discussion. I'm pretty sure what Florian is looking for has no dependency on VT, fbcon or logind, but I'm only guessing based on the information I see in the public Android source trees. My understanding is that the Android recovery application is a graphical tool that accesses the framebuffer directly and is controlled using the volume and power buttons on a phone. >> I suppose making CONFIG_FB_NOTIFIER optional for FB (on by >> default if any of the consumers of the notification are turned >> on) would not be a bad direction to go in general and also >> address Florian's link error, but that doesn't solve the >> more general concern about a third-party fb.ko module on a >> kernel that was explicitly built with FB disabled. >> >> The GKI defconfig was initially done at a time where one could >> not have CONFIG_FBDEV_EMULATION and CONFIG_FB_DEVICE without >> pulling in the entire fbdev module, but now that is possible. >> Maybe that is something that Android could now include? >> >> Alternatively, I wonder if that recovery image could be changed >> to access the screen through the /dev/dri/ interfaces? I have >> no experience in using those without Xorg or Wayland, is that >> a sensible thing to do? > > Uh ... I think I'm confused about the requirements. Does android's > recovery image need fbdev (meaning /dev/fb chardevs), or does it need > fbcon? > > Note that fbcon runs (or well, should run) totally fine on top of drm > drivers without the fb notifier. This wasn't the case a few years ago > (because fbcon also used that notifier), but nowadays fb notifiers are > only needed for legacy fbdev drivers. So could it be that this "need fb > notifier" requirement is a leftover from rather old kernel versions and > not actually needed anymore? > > I think we should nail the actual requirements here first before jumping > to solutions ... Right, let's wait for Florian to reply. From what he said earlier though, the only reason that the notifiers are getting in the way is the link error you get from trying to load a separately built fb.ko module on a kernel that was built with FB=n / FB_CORE=n, so I don't think he even cares about notifiers, only about allowing the recovery application to mmap() the framebuffer. Arnd
Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate
On Mon, May 6, 2024, at 16:53, Arnd Bergmann wrote: > On Mon, May 6, 2024, at 15:14, Daniel Vetter wrote: >> >> This one is. And it doesn't need to be simpledrm, just a drm kms driver >> with fbdev emulation. Heck even if you have an fbdev driver you should >> control the corresponding backlight explicitly, and not rely on the fb >> notifier to magical enable/disable some random backlights somewhere. >> >> So please do not encourage using this in any modern code. ... > Alternatively, I wonder if that recovery image could be changed > to access the screen through the /dev/dri/ interfaces? I have > no experience in using those without Xorg or Wayland, is that > a sensible thing to do? Replying to myself here, I think I found the answer in https://android.googlesource.com/platform/bootable/recovery/+/refs/heads/main/minui/ which has separate backends for fbdev and drm, and should just work as long as the drm driver is loaded in the recovery image. Florian, do you know why this is not being used on the machines you are looking at? Arnd
Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate
On Mon, May 6, 2024, at 15:14, Daniel Vetter wrote: > On Fri, May 03, 2024 at 01:22:10PM -0700, Florian Fainelli wrote: >> On 5/3/24 12:45, Arnd Bergmann wrote: >> > On Fri, May 3, 2024, at 21:28, Florian Fainelli wrote: >> > > Android devices in recovery mode make use of a framebuffer device to >> > > provide an user interface. In a GKI configuration that has CONFIG_FB=m, >> > > but CONFIG_FB_NOTIFY=y, loading the fb.ko module will fail with: >> > > >> > > fb: Unknown symbol fb_notifier_call_chain (err -2) >> > > >> > > Have CONFIG_FB_NOTIFY be tristate, just like CONFIG_FB such that both >> > > can be loaded as module with fb_notify.ko first, and fb.ko second. >> > > >> > > Signed-off-by: Florian Fainelli >> > >> > I see two problems here, so I don't think this is the right >> > approach: >> > >> > 1. I don't understand your description: Since fb_notifier_call_chain() >> > is an exported symbol, it should work for both FB_NOTIFY=y and >> > FB_NOTIFY=m, unless something in Android drops the exported >> > symbols for some reason. >> >> The symbol is still exported in the Android tree. The issue is that the GKI >> defconfig does not enable any CONFIG_FB options at all. This is left for SoC >> vendors to do in their own "fragment" which would add CONFIG_FB=m. That >> implies CONFIG_FB_NOTIFY=y which was not part of the original kernel image >> we build/run against, and so we cannot resolve the symbol. I see. >> This could be resolved by having the GKI kernel have CONFIG_FB_NOTIFY=y but >> this is a bit wasteful (not by much since the code is very slim anyway) and >> it does require making changes specifically to the Android tree which could >> be beneficial upstream, hence my attempt at going upstream first. > > Making fbdev (the driver subsystem, not the uapi, we'll still happily > merge patches for that) more useful is really not the upstream direction :-) I'm more worried about the idea of enabling an entire subsystem as a loadable module and expecting that to work with an existing kernel, specifically when the drm.ko and fb.ko interact with one another and are built with different .config files. This is the current Android GKI config: https://android.googlesource.com/kernel/common.git/+/refs/heads/android-mainline/arch/arm64/configs/gki_defconfig where I can see that CONFIG_DRM is built-in, but DRM_FBDEV_EMULATION CONFIG_VT, CONFIG_FRAMEBUFFER_CONSOLE, CONFIG_FB_DEVICE and CONFIG_FB_CORE are all disabled. So the console won't work at all,I think this means that there is no way to get the console working, but building a fb.ko module allows using /dev/fb with simplefb.ko (or any other one) happens to almost work, but only by dumb luck rather than by design. >> > $ git grep -w fb_register_client >> > arch/arm/mach-pxa/am200epd.c: fb_register_client(_fb_notif); >> > drivers/leds/trigger/ledtrig-backlight.c: ret = >> > fb_register_client(>notifier); >> > drivers/video/backlight/backlight.c:return >> > fb_register_client(>fb_notif); >> > drivers/video/backlight/lcd.c: return fb_register_client(>fb_notif); >> > >> > Somewhat related but not directly addressing your patch, I wonder >> > if Android itself could migrate to using FB_CORE=m FB=n FB_NOTIFY=n >> > instead and use simpledrm for the console in place of the legacy >> > fbdev layer. >> >> That is beyond my reach :) > > This one is. And it doesn't need to be simpledrm, just a drm kms driver > with fbdev emulation. Heck even if you have an fbdev driver you should > control the corresponding backlight explicitly, and not rely on the fb > notifier to magical enable/disable some random backlights somewhere. > > So please do not encourage using this in any modern code. I suppose making CONFIG_FB_NOTIFIER optional for FB (on by default if any of the consumers of the notification are turned on) would not be a bad direction to go in general and also address Florian's link error, but that doesn't solve the more general concern about a third-party fb.ko module on a kernel that was explicitly built with FB disabled. The GKI defconfig was initially done at a time where one could not have CONFIG_FBDEV_EMULATION and CONFIG_FB_DEVICE without pulling in the entire fbdev module, but now that is possible. Maybe that is something that Android could now include? Alternatively, I wonder if that recovery image could be changed to access the screen through the /dev/dri/ interfaces? I have no experience in using those without Xorg or Wayland, is that a sensible thing to do? Arnd
Re: [PATCH] fbdev: Have CONFIG_FB_NOTIFY be tristate
On Fri, May 3, 2024, at 21:28, Florian Fainelli wrote: > Android devices in recovery mode make use of a framebuffer device to > provide an user interface. In a GKI configuration that has CONFIG_FB=m, > but CONFIG_FB_NOTIFY=y, loading the fb.ko module will fail with: > > fb: Unknown symbol fb_notifier_call_chain (err -2) > > Have CONFIG_FB_NOTIFY be tristate, just like CONFIG_FB such that both > can be loaded as module with fb_notify.ko first, and fb.ko second. > > Signed-off-by: Florian Fainelli I see two problems here, so I don't think this is the right approach: 1. I don't understand your description: Since fb_notifier_call_chain() is an exported symbol, it should work for both FB_NOTIFY=y and FB_NOTIFY=m, unless something in Android drops the exported symbols for some reason. 2. This breaks if any of the four callers of fb_register_client() are built-in while CONFIG_FB_NOTIFY is set to =m: $ git grep -w fb_register_client arch/arm/mach-pxa/am200epd.c: fb_register_client(_fb_notif); drivers/leds/trigger/ledtrig-backlight.c: ret = fb_register_client(>notifier); drivers/video/backlight/backlight.c:return fb_register_client(>fb_notif); drivers/video/backlight/lcd.c: return fb_register_client(>fb_notif); Somewhat related but not directly addressing your patch, I wonder if Android itself could migrate to using FB_CORE=m FB=n FB_NOTIFY=n instead and use simpledrm for the console in place of the legacy fbdev layer. Arnd
Re: [PATCH v3 0/3] arch: Remove fbdev dependency from video helpers
On Fri, Apr 5, 2024, at 11:04, Thomas Zimmermann wrote: > Hi, > > if there are no further comments, can this series be merged through > asm-generic? Sorry for the delay, I've merged these for asm-generic now. Arnd
Re: nouveau: r535.c:1266:3: error: label at end of compound statement default: with gcc-8
On Mon, Apr 29, 2024, at 19:08, Timur Tabi wrote: > On Mon, 2024-04-29 at 17:30 +0200, Linux regression tracking (Thorsten > Leemhuis) wrote: >> TWIMC, there is another report about this in this thread (sadly some of >> its post did not make it to lore): >> >> https://lore.kernel.org/all/162ef3c0-1d7b-4220-a21f-b0008657f...@redhat.com/ >> >> Ciao, Thorsten > > This doesn't fail on x86-64 when I build it. I also did a cross-compile to > arm64 with the arm64 defconfig, and it doesn't fail there either. > > I'm guessing this is a compiler version thing. I'm using gcc 11.4. Is that > just too old? It's too new: this is valid syntax in c23 and accepted by newer compilers as an extension to gnu11, but older versions don't like it. gcc-11 and clang-16 are fine, while gcc-10 and clang-15 as well as earlier versions produce this warning. Arnd
Re: [PATCH] drm: move DRM-related CONFIG options into DRM submenu
On Tue, Apr 23, 2024, at 12:24, Masahiro Yamada wrote: > When you create a submenu using the 'menu' syntax, there is no > ambiguity about the end of the submenu because the code between > 'menu' and 'endmenu' becomes the submenu. > ... > > Signed-off-by: Masahiro Yamada I think this is a useful cleanup. Acked-by: Arnd Bergmann
Re: [PATCH 00/11] drm: Restore helper usability
On Mon, Apr 22, 2024, at 21:42, Masahiro Yamada wrote: > On Tue, Apr 23, 2024 at 3:24 AM Arnd Bergmann wrote: >> On Mon, Apr 22, 2024, at 18:58, Geert Uytterhoeven wrote: >> > On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann wrote: >> >> On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote: >> >> I think hiding this would make it much harder to get anything >> right. The symbols in question are almost all ones that should >> be enabled in normal configs, and the 'make menuconfig' help >> doesn't make it too hard to figure things out normally, we just >> have to find a way to avoid regressions when converting things >> to 'depends on' that used an incorrect 'select'. > > I am confused because you repeatedly discussed > the missing I2C dependency. > > > Are you talking about DRM drivers, > or is it just "an example" in general? > > > > DRM selects I2C. It's a prominent example because I2C ends up showing up in most circular dependencies. I forgot that CONFIG_DRM itself selects this one, but this is clearly part of the overall problem: $ git grep -w 'select I2C' | wc -l 35 $ git grep -w 'depends on I2C' | wc -l 1068 Attempting to clean up some of the incorrect 'select' statements, such as changing DRM_NOUVEAU to 'depends on ACPI_VIDEO' instead of 'select' tends to run into another 'select I2C' such as this one: drivers/i2c/Kconfig:8: symbol I2C is selected by DRM_NOUVEAU drivers/gpu/drm/nouveau/Kconfig:2: symbol DRM_NOUVEAU depends on ACPI_VIDEO drivers/acpi/Kconfig:214: symbol ACPI_VIDEO depends on BACKLIGHT_CLASS_DEVICE drivers/video/backlight/Kconfig:136:symbol BACKLIGHT_CLASS_DEVICE is selected by FB_BACKLIGHT drivers/video/fbdev/core/Kconfig:184: symbol FB_BACKLIGHT is selected by HT16K33 drivers/auxdisplay/Kconfig:490: symbol HT16K33 depends on I2C Again, I2C was probably not the best example for an urgent problem as it ends up being selected unconditionally anyway, but I think ACPI_VIDEO and BACKLIGHT_CLASS_DEVICE are the ones that we should stop selecting. > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index 5a0c476361c3..6984b3fea271 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -29,6 +29,8 @@ menuconfig DRM > details. You should also select and configure AGP > (/dev/agpgart) support if it is available for your platform. > > +if DRM > + > config DRM_MIPI_DBI > tristate > depends on DRM > @@ -414,3 +416,5 @@ config DRM_LIB_RANDOM > config DRM_PRIVACY_SCREEN > bool >default n > + > +endif This is a probably good idea (aside from DRM_PANEL_ORIENTATION_QUIRKS, which needs to be moved out of the section), but seems completely unrelated to the issue of selecting too many symbols. Arnd
Re: [PATCH 1/1] video: Handle HAS_IOPORT dependencies
On Mon, Apr 22, 2024, at 10:34, Niklas Schnelle wrote:
> On Thu, 2024-04-11 at 16:00 +0200, Helge Deller wrote:
>> * Niklas Schnelle :
>> > In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
>> > compile time. We thus need to #ifdef functions and their callsites which
>> > unconditionally use these I/O accessors. In the include/video/vga.h
>> > these are conveniently all those functions with the vga_io_* prefix.
>>
>> Why don't you code it like in the patch below?
>> inb_p(), outb_p() and outw() would then need to be defined externally
>> without an implementation so that they would generate link time errors
>> (instead of compile time errors).
>
> This may be personal preference but I feel like link time errors would
> be very late to catch a configuration that can't work. Also this would
> bypass the __compiletime_error("inb()) requires CONFIG_HAS_IOPORT");
> added instead of the in*()/out*() helpers to make it easy to spot the
> problem.
>
> I'm not a fan of #ifdeffery either but I think in this case it is
> simple, well enough contained and overall there aren't that many spots
> where we need to exclude just some sections of code vs entire drivers
> with vga.h probably being the worst of them all.
Agreed. I also tried to see if we can move stuff out of vga.h
to have it included in fewer places, as almost everything that
uses this header already has a HAS_IOPORT dependency, but that
would be a lot more work.
The other one that gains a few ugly #ifdefs is the 8250 driver,
everything else is already merged in linux-next or needs a simple
Kconfig dependency.
I think we can make the vga.h file a little more readable
by duplicating the functions and still keep the __compiletime_error()
version in asm/io.h, see below.
Arnd
diff --git a/include/video/vga.h b/include/video/vga.h
index 947c0abd04ef..7e1d8252b732 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -197,6 +197,23 @@ struct vgastate {
extern int save_vga(struct vgastate *state);
extern int restore_vga(struct vgastate *state);
+static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short
port)
+{
+ return readb (regbase + port);
+}
+
+static inline void vga_mm_w (void __iomem *regbase, unsigned short port,
unsigned char val)
+{
+ writeb (val, regbase + port);
+}
+
+static inline void vga_mm_w_fast (void __iomem *regbase, unsigned short port,
+ unsigned char reg, unsigned char val)
+{
+ writew (VGA_OUT16VAL (val, reg), regbase + port);
+}
+
+#ifdef CONFIG_HAS_IOPORT
/*
* generic VGA port read/write
*/
@@ -217,22 +234,6 @@ static inline void vga_io_w_fast (unsigned short port,
unsigned char reg,
outw(VGA_OUT16VAL (val, reg), port);
}
-static inline unsigned char vga_mm_r (void __iomem *regbase, unsigned short
port)
-{
- return readb (regbase + port);
-}
-
-static inline void vga_mm_w (void __iomem *regbase, unsigned short port,
unsigned char val)
-{
- writeb (val, regbase + port);
-}
-
-static inline void vga_mm_w_fast (void __iomem *regbase, unsigned short port,
- unsigned char reg, unsigned char val)
-{
- writew (VGA_OUT16VAL (val, reg), regbase + port);
-}
-
static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
{
if (regbase)
@@ -258,7 +259,25 @@ static inline void vga_w_fast (void __iomem *regbase,
unsigned short port,
else
vga_io_w_fast (port, reg, val);
}
+#else
+static inline unsigned char vga_r (void __iomem *regbase, unsigned short port)
+{
+ return vga_mm_r(regbase, port);
+}
+
+static inline void vga_w(void __iomem *regbase, unsigned short port, unsigned
char val)
+{
+ vga_mm_w (regbase, port, val);
+}
+
+static inline void vga_w_fast (void __iomem *regbase, unsigned short port,
+ unsigned char reg, unsigned char val)
+{
+ vga_mm_w_fast(regbase, port, reg, val);
+}
+
+#endif
/*
* VGA CRTC register read/write
Re: [PATCH 00/11] drm: Restore helper usability
On Mon, Apr 22, 2024, at 18:58, Geert Uytterhoeven wrote: > On Mon, Apr 22, 2024 at 3:55 PM Arnd Bergmann wrote: >> On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote: >> Whereas this one is broken: >> >> config FEATURE_A >>tristate "user visible if I2C is enabled" >>depends on I2C >> >> config HELPER_B >>tristate # hidden >>select FEATURE_A >> >> config DRIVER >>tristate "This driver is broken if I2C is disabled" >>select HELPER_B > > So the DRIVER section should gain a "depends on I2C" statement. That is of course the common workaround, but my point was that nothing should ever 'select I2C' or any of the other subsystems that are user visible. > Yamada-san: would it be difficult to modify Kconfig to ignore symbols > like DRIVER that select other symbols with unmet dependencies? > Currently it already warns about that. > > Handling this implicitly (instead of the current explict "depends > on") would have the disadvantage though: a user who is not aware of > the implicit dependency may wonder why DRIVER is invisible in his > config interface. I think hiding this would make it much harder to get anything right. The symbols in question are almost all ones that should be enabled in normal configs, and the 'make menuconfig' help doesn't make it too hard to figure things out normally, we just have to find a way to avoid regressions when converting things to 'depends on' that used an incorrect 'select'. Arnd
Re: [PATCH 00/11] drm: Restore helper usability
On Mon, Apr 22, 2024, at 15:28, Jani Nikula wrote: > On Mon, 22 Apr 2024, "Arnd Bergmann" wrote: >> On Mon, Apr 22, 2024, at 13:50, Jani Nikula wrote: >> >>> I still disagree with this, because fundamentally the source symbol >>> really should not have to care about the dependencies of the target >>> symbol. >> >> Sorry you missed the IRC discussion on #armlinux, we should have >> included you as well since you applied the original patch. >> >> I think the reason for this revert is a bit more nuanced than >> just the usability problem. Sorry if I'm dragging this out too >> much, but I want to be sure that two points come across: >> >> 1. There is a semantic problem that is mostly subjective, but >>with the naming as "helper", I generally expect it as a hidden >>symbol that gets selected by its users, while calling same module >>"feature" would be something that is user-enabled and that >>other modules depend on. Both ways are commonly used in the >>kernel and are not mistakes on their own. > > Fair enough. I believe for (optional) "feature" the common pattern would > then be depends on FEATURE || FEATURE=n. > >> 2. Using "select" on user visible symbols that have dependencies >>is a common source for bugs, and this is is a problem in >>drivers/gpu/drm more than elsewhere in the kernel, as these >>drivers traditionally select entire subsystems or drivers >>(I2C, VIRTIO, INPUT, ACPI_WMI, BACKLIGHT_CLASS_DEVICE, >>POWER_SUPPLY, SND_PCM, INTERCONNECT, ...). This regularly >>leads to circular dependencies and we should fix all of them. > > What annoys me is that the fixes tend to fall in two categories: > > - Play catch with selecting the dependencies of the selected > symbols. "depends on" handles this recursively, while select does > not. I'm not sure where this misunderstanding comes from, as you seem to be repeating the same incorrect assumption about how select works that Maxime wrote in his changelog. To clarify, this works exactly as one would expect: config HELPER_A tristate config HELPER_B tristate select HELPER_A config DRIVER tristate "Turn on the driver and the helpers it uses" select HELPER_B # this recursively selects HELPER_A Whereas this one is broken: config FEATURE_A tristate "user visible if I2C is enabled" depends on I2C config HELPER_B tristate # hidden select FEATURE_A config DRIVER tristate "This driver is broken if I2C is disabled" select HELPER_B > There is no end to this, it just goes on and on, as the > dependencies of the selected symbols change over time. Often the > selects require unintuitive if patterns that are about the > implementation details of the symbol being selected. Agreed, that is the problem I frequently face with drivers/gpu/drm, and most of the time it can only be solved by rewriting the whole system to not select user-visible symbol at all. Using 'depends on' by itself is unfortunately not enough to avoid /all/ the problems. See e.g. today's failure config DRM_DISPLAY_HELPER tristate "DRM Display Helpers" default y config DRM_DISPLAY_DP_HELPER bool "DRM DisplayPort Helpers" depends on DRM_DISPLAY_HELPER config DRM_PANEL_LG_SW43408 tristate "LG SW43408 panel" depends on DRM_DISPLAY_DP_HELPER This version is still broken for DRM_DISPLAY_HELPER=m, DRM_DISPLAY_DP_HELPER=m, DRM_PANEL_LG_SW43408=y because the dependency on the bool symbol is not enough to ensure that DRM_DISPLAY_HELPER is also built-in, so you still need explicit dependencies on both DRM_DISPLAY_HELPER and DRM_DISPLAY_DP_HELPER in the users. This can be solved by making DRM_DISPLAY_DP_HELPER a tristate symbol and adjusting the #ifdef checks and Makefile logic accordingly, which is exactly what you'd need to do to make it work with 'select' as well. > - Brush the invalid configs under the rug by using IS_REACHABLE(), > switching from a loud link time failure to a silent runtime > failure. (I regularly reject patches adding IS_REACHABLE() to i915, > because from my pov e.g. i915=y backlight=m is an invalid > configuration that the user shouldn't have to debug at runtime. But I > can't express that in kconfig while everyone selects backlight.) Thanks a lot for rejecting the IS_REACHABLE() patches, it is indeed the worst way to handle those (I know, as I introduced IS_REACHABLE() only to replace open-coded versions of the same, not to have it as a feature or to use it in new code). > If you have other ideas how these should be fixed, I'm all ears. > >
Re: [PATCH 00/11] drm: Restore helper usability
On Mon, Apr 22, 2024, at 13:50, Jani Nikula wrote: > On Mon, 22 Apr 2024, Geert Uytterhoeven wrote: >> Hi all, >> >> As discussed on IRC with Maxime and Arnd, this series reverts the >> conversion of select to depends for various DRM helpers in series >> "[PATCH v3 00/13] drm/display: Convert helpers Kconfig symbols to >> depends on"[1], and various fixes for it. This conversion introduced a >> big usability issue when configuring a kernel and enabling DRM drivers >> that use DRM helper code: as drivers now depend on helpers, the user >> needs to know which helpers to enable, before the driver he is >> interested even becomes visible. The user should not need to know that, >> and drivers should select the helpers they need. >> >> Hence revert back to what we had before, where drivers selected the >> helpers (and any of their dependencies, if they can be met) they need. >> In general, when a symbol selects another symbol, it should just make >> sure the dependencies of the target symbol are met, which may mean >> adding dependencies to the source symbol. Thanks for doing this. Acked-by: Arnd Bergmann > I still disagree with this, because fundamentally the source symbol > really should not have to care about the dependencies of the target > symbol. Sorry you missed the IRC discussion on #armlinux, we should have included you as well since you applied the original patch. I think the reason for this revert is a bit more nuanced than just the usability problem. Sorry if I'm dragging this out too much, but I want to be sure that two points come across: 1. There is a semantic problem that is mostly subjective, but with the naming as "helper", I generally expect it as a hidden symbol that gets selected by its users, while calling same module "feature" would be something that is user-enabled and that other modules depend on. Both ways are commonly used in the kernel and are not mistakes on their own. 2. Using "select" on user visible symbols that have dependencies is a common source for bugs, and this is is a problem in drivers/gpu/drm more than elsewhere in the kernel, as these drivers traditionally select entire subsystems or drivers (I2C, VIRTIO, INPUT, ACPI_WMI, BACKLIGHT_CLASS_DEVICE, POWER_SUPPLY, SND_PCM, INTERCONNECT, ...). This regularly leads to circular dependencies and we should fix all of them. The display helpers however don't have this problem because they do not have any dependencies outside of drivers/gpu/ Arnd
[PATCH 2/2] drm/amd/display: fix graphics_object_id size
From: Arnd Bergmann
The graphics_object_id structure is meant to fit into 32 bits, as it's
passed by value in and out of functions. A recent change increased
the size to 128 bits, so it's now always passed by reference, which
is clearly not intended and ends up producing a compile-time warning:
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c: In function
'construct_phy':
drivers/gpu/drm/amd/amdgpu/../display/dc/link/link_factory.c:743:1: error: the
frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Add back the bitfields to revert to the original size, while keeping
the 'enum' type change.
fec85f995a4b ("drm/amd/display: Fix compiler redefinition warnings for certain
configs")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/amd/display/include/grph_object_id.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/include/grph_object_id.h
b/drivers/gpu/drm/amd/display/include/grph_object_id.h
index 08ee0350b31f..54e33062b3c0 100644
--- a/drivers/gpu/drm/amd/display/include/grph_object_id.h
+++ b/drivers/gpu/drm/amd/display/include/grph_object_id.h
@@ -226,8 +226,8 @@ enum dp_alt_mode {
struct graphics_object_id {
uint32_t id:8;
- enum object_enum_id enum_id;
- enum object_type type;
+ enum object_enum_id enum_id :4;
+ enum object_type type :4;
uint32_t reserved:16; /* for padding. total size should be u32 */
};
--
2.39.2
[PATCH 1/2] drm/amd/display: dynamically allocate dml2_configuration_options structures
From: Arnd Bergmann
This structure is too large to fit on a stack, as shown by the
newly introduced warnings from a recent code change:
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn32/dcn32_resource.c: In
function 'dcn32_update_bw_bounding_box':
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn32/dcn32_resource.c:2019:1:
error: the frame size of 1180 bytes is larger than 1024 bytes
[-Werror=frame-larger-than=]
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn321/dcn321_resource.c: In
function 'dcn321_update_bw_bounding_box':
drivers/gpu/drm/amd/amdgpu/../display/dc/resource/dcn321/dcn321_resource.c:1597:1:
error: the frame size of 1180 bytes is larger than 1024 bytes
[-Werror=frame-larger-than=]
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_state.c: In function
'dc_state_create':
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_state.c:219:1: error: the
frame size of 1184 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Instead of open-coding the assignment of a large structure to a stack
variable, use an explicit kmemdup() in each case to move it off the stack.
Fixes: e779f4587f61 ("drm/amd/display: Add handling for DC power mode")
Signed-off-by: Arnd Bergmann
---
.../display/dc/resource/dcn32/dcn32_resource.c | 16 +++-
.../display/dc/resource/dcn321/dcn321_resource.c | 16 +++-
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
index c16e915686fc..b2b95f5abb09 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn32/dcn32_resource.c
@@ -2001,21 +2001,27 @@ void dcn32_calculate_wm_and_dlg(struct dc *dc, struct
dc_state *context,
static void dcn32_update_bw_bounding_box(struct dc *dc, struct clk_bw_params
*bw_params)
{
- struct dml2_configuration_options dml2_opt = dc->dml2_options;
+ struct dml2_configuration_options *dml2_opt;
+
+ dml2_opt = kmemdup(>dml2_options, sizeof(dc->dml2_options),
GFP_KERNEL);
+ if (!dml2_opt)
+ return;
DC_FP_START();
dcn32_update_bw_bounding_box_fpu(dc, bw_params);
- dml2_opt.use_clock_dc_limits = false;
+ dml2_opt->use_clock_dc_limits = false;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2)
- dml2_reinit(dc, _opt, >current_state->bw_ctx.dml2);
+ dml2_reinit(dc, dml2_opt, >current_state->bw_ctx.dml2);
- dml2_opt.use_clock_dc_limits = true;
+ dml2_opt->use_clock_dc_limits = true;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2_dc_power_source)
- dml2_reinit(dc, _opt,
>current_state->bw_ctx.dml2_dc_power_source);
+ dml2_reinit(dc, dml2_opt,
>current_state->bw_ctx.dml2_dc_power_source);
DC_FP_END();
+
+ kfree(dml2_opt);
}
static struct resource_funcs dcn32_res_pool_funcs = {
diff --git a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
index 3816678b044f..ea5768f57138 100644
--- a/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/resource/dcn321/dcn321_resource.c
@@ -1579,21 +1579,27 @@ static struct dc_cap_funcs cap_funcs = {
static void dcn321_update_bw_bounding_box(struct dc *dc, struct clk_bw_params
*bw_params)
{
- struct dml2_configuration_options dml2_opt = dc->dml2_options;
+ struct dml2_configuration_options *dml2_opt;
+
+ dml2_opt = kmemdup(>dml2_options, sizeof(dc->dml2_options),
GFP_KERNEL);
+ if (!dml2_opt)
+ return;
DC_FP_START();
dcn321_update_bw_bounding_box_fpu(dc, bw_params);
- dml2_opt.use_clock_dc_limits = false;
+ dml2_opt->use_clock_dc_limits = false;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2)
- dml2_reinit(dc, _opt, >current_state->bw_ctx.dml2);
+ dml2_reinit(dc, dml2_opt, >current_state->bw_ctx.dml2);
- dml2_opt.use_clock_dc_limits = true;
+ dml2_opt->use_clock_dc_limits = true;
if (dc->debug.using_dml2 && dc->current_state &&
dc->current_state->bw_ctx.dml2_dc_power_source)
- dml2_reinit(dc, _opt,
>current_state->bw_ctx.dml2_dc_power_source);
+ dml2_reinit(dc, dml2_opt,
>current_state->bw_ctx.dml2_dc_power_source);
DC_FP_END();
+
+ kfree(dml2_opt);
}
static struct resource_funcs dcn321_res_pool_funcs = {
--
2.39.2
[PATCH] accel/qaic: mark debugfs stub functions as static inline
From: Arnd Bergmann
The alternative stub functions are listed as global, which produces
a build failure in some configs:
In file included from drivers/accel/qaic/qaic_drv.c:31:
drivers/accel/qaic/qaic_debugfs.h:16:5: error: no previous prototype for
'qaic_bootlog_register' [-Werror=missing-prototypes]
16 | int qaic_bootlog_register(void) { return 0; }
| ^
drivers/accel/qaic/qaic_debugfs.h:17:6: error: no previous prototype for
'qaic_bootlog_unregister' [-Werror=missing-prototypes]
17 | void qaic_bootlog_unregister(void) {}
| ^~~
drivers/accel/qaic/qaic_debugfs.h:18:6: error: no previous prototype for
'qaic_debugfs_init' [-Werror=missing-prototypes]
18 | void qaic_debugfs_init(struct qaic_drm_device *qddev) {}
| ^
Make them static inline as intended.
Fixes: 5f8df5c6def6 ("accel/qaic: Add bootlog debugfs")
Signed-off-by: Arnd Bergmann
---
drivers/accel/qaic/qaic_debugfs.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/accel/qaic/qaic_debugfs.h
b/drivers/accel/qaic/qaic_debugfs.h
index ea3fd1a88405..05e74f84cf9f 100644
--- a/drivers/accel/qaic/qaic_debugfs.h
+++ b/drivers/accel/qaic/qaic_debugfs.h
@@ -13,8 +13,8 @@ int qaic_bootlog_register(void);
void qaic_bootlog_unregister(void);
void qaic_debugfs_init(struct qaic_drm_device *qddev);
#else
-int qaic_bootlog_register(void) { return 0; }
-void qaic_bootlog_unregister(void) {}
-void qaic_debugfs_init(struct qaic_drm_device *qddev) {}
+static inline int qaic_bootlog_register(void) { return 0; }
+static inline void qaic_bootlog_unregister(void) {}
+static inline void qaic_debugfs_init(struct qaic_drm_device *qddev) {}
#endif /* CONFIG_DEBUG_FS */
#endif /* __QAIC_DEBUGFS_H__ */
--
2.39.2
[PATCH] drm/msm: remove an unused-but-set variable
From: Arnd Bergmann
The modification to a6xx_get_shader_block() had no effect other
than causing a warning:
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c:843:6: error: variable 'out' set
but not used [-Werror,-Wunused-but-set-variable]
u64 out = dumper->iova + A6XX_CD_DATA_OFFSET;
Revert this part of the previous patch.
Fixes: 64d6255650d4 ("drm/msm: More fully implement devcoredump for a7xx")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
index 1f5245fc2cdc..d4e1ebfcb021 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c
@@ -840,7 +840,6 @@ static void a6xx_get_shader_block(struct msm_gpu *gpu,
struct a6xx_crashdumper *dumper)
{
u64 *in = dumper->ptr;
- u64 out = dumper->iova + A6XX_CD_DATA_OFFSET;
size_t datasize = block->size * A6XX_NUM_SHADER_BANKS * sizeof(u32);
int i;
@@ -853,8 +852,6 @@ static void a6xx_get_shader_block(struct msm_gpu *gpu,
in += CRASHDUMP_READ(in, REG_A6XX_HLSQ_DBG_AHB_READ_APERTURE,
block->size, dumper->iova + A6XX_CD_DATA_OFFSET);
-
- out += block->size * sizeof(u32);
}
CRASHDUMP_FINI(in);
--
2.39.2
Re: [PATCH 1/1] vgacon: add HAS_IOPORT dependencies
On Fri, Apr 5, 2024, at 17:43, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
> compile time. We thus need to add HAS_IOPORT as dependency for
> those drivers using them.
>
> Co-developed-by: Arnd Bergmann
> Signed-off-by: Arnd Bergmann
> Signed-off-by: Niklas Schnelle
> ---
> Note: This patch does not depend any not-yet-mainline HAS_IOPORT changes
> and may be merged via subsystem specific trees at your earliest
> convenience.
I think this patch can just get dropped now, no need to merge
it because it's already handled by e9e3300b6e77 ("vgacon:
rework Kconfig dependencies").
Arnd
[PATCH] drm: fix DRM_DISPLAY_DP_HELPER dependencies, part 2
From: Arnd Bergmann
After my fix yesterday, I ran into another problem of the same kind:
aarch64-linux-ld: drivers/gpu/drm/bridge/analogix/analogix_dp_core.o: in
function `drm_dp_dpcd_readb':
analogix_dp_core.c:(.text+0x194): undefined reference to `drm_dp_dpcd_read'
aarch64-linux-ld: drivers/gpu/drm/bridge/analogix/analogix_dp_core.o: in
function `drm_dp_dpcd_writeb':
analogix_dp_core.c:(.text+0x214): undefined reference to `drm_dp_dpcd_write'
aarch64-linux-ld: drivers/gpu/drm/bridge/analogix/analogix_dp_core.o: in
function `analogix_dp_stop_crc':
analogix_dp_core.c:(.text+0x4b0): undefined reference to `drm_dp_stop_crc'
aarch64-linux-ld: drivers/gpu/drm/bridge/analogix/analogix_dp_core.o: in
function `analogix_dp_start_crc':
analogix_dp_core.c:(.text+0xbe8): undefined reference to `drm_dp_start_crc'
Add the same dependency again to ROCKCHIP_ANALOGIX_DP after checking that
nothing else selects the analogix driver. Also add a dependency to
DRM_ANALOGIX_DP to make it easier to identifier future problems of
this type when they get introduced.
Fixes: 0323287de87d ("drm: Switch DRM_DISPLAY_DP_HELPER to depends on")
Fixes: d1ef8fc18be6 ("drm: fix DRM_DISPLAY_DP_HELPER dependencies")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/bridge/analogix/Kconfig | 2 +-
drivers/gpu/drm/rockchip/Kconfig| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig
b/drivers/gpu/drm/bridge/analogix/Kconfig
index 12bfea53bf24..5b564fded6d6 100644
--- a/drivers/gpu/drm/bridge/analogix/Kconfig
+++ b/drivers/gpu/drm/bridge/analogix/Kconfig
@@ -28,7 +28,7 @@ config DRM_ANALOGIX_ANX78XX
config DRM_ANALOGIX_DP
tristate
- depends on DRM
+ depends on DRM_DISPLAY_HELPER
config DRM_ANALOGIX_ANX7625
tristate "Analogix Anx7625 MIPI to DP interface support"
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 4b4ad75032fd..4c7072e6e34e 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -36,7 +36,7 @@ config ROCKCHIP_VOP2
config ROCKCHIP_ANALOGIX_DP
bool "Rockchip specific extensions for Analogix DP driver"
depends on DRM_DISPLAY_DP_HELPER
- depends on DRM_DISPLAY_HELPER
+ depends on DRM_DISPLAY_HELPER=y || (DRM_DISPLAY_HELPER=m &&
DRM_ROCKCHIP=m)
depends on ROCKCHIP_VOP
help
This selects support for Rockchip SoC specific extensions
--
2.39.2
[PATCH] [v2] nouveau: fix function cast warning
From: Arnd Bergmann
Calling a function through an incompatible pointer type causes breaks
kcfi, so clang warns about the assignment:
drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c:73:10: error: cast from
'void (*)(const void *)' to 'void (*)(void *)' converts to incompatible
function type [-Werror,-Wcast-function-type-strict]
73 | .fini = (void(*)(void *))kfree,
Avoid this with a trivial wrapper.
Fixes: c39f472e9f14 ("drm/nouveau: remove symlinks, move core/ to nvkm/ (no
code changes)")
Signed-off-by: Arnd Bergmann
---
v2: avoid 'return kfree()' expression returning a void
---
drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
index 4bf486b57101..cb05f7f48a98 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
@@ -66,11 +66,16 @@ of_init(struct nvkm_bios *bios, const char *name)
return ERR_PTR(-EINVAL);
}
+static void of_fini(void *p)
+{
+ kfree(p);
+}
+
const struct nvbios_source
nvbios_of = {
.name = "OpenFirmware",
.init = of_init,
- .fini = (void(*)(void *))kfree,
+ .fini = of_fini,
.read = of_read,
.size = of_size,
.rw = false,
--
2.39.2
[PATCH] drm: fix DRM_DISPLAY_DP_HELPER dependencies
From: Arnd Bergmann
Both the exynos and rockchip drivers ran into link failures after
a Kconfig cleanup:
aarch64-linux-ld: drivers/gpu/drm/exynos/exynos_dp.o: in function
`exynos_dp_resume':
exynos_dp.c:(.text+0xc0): undefined reference to `analogix_dp_resume'
aarch64-linux-ld: drivers/gpu/drm/exynos/exynos_dp.o: in function
`exynos_dp_suspend':
exynos_dp.c:(.text+0xf4): undefined reference to `analogix_dp_suspend'
x86_64-linux-ld: drivers/gpu/drm/rockchip/cdn-dp-core.o: in function
`cdn_dp_connector_mode_valid':
cdn-dp-core.c:(.text+0x13a): undefined reference to
`drm_dp_bw_code_to_link_rate'
x86_64-linux-ld: cdn-dp-core.c:(.text+0x148): undefined reference to
`drm_dp_bw_code_to_link_rate'
x86_64-linux-ld: drivers/gpu/drm/rockchip/cdn-dp-core.o: in function
`cdn_dp_check_link_status':
cdn-dp-core.c:(.text+0x1396): undefined reference to `drm_dp_channel_eq_ok'
In both cases, the problem is that ROCKCHIP_CDN_DP and DRM_EXYNOS_DP
are 'bool' symbols that depend on the the 'tristate' DRM_DISPLAY_HELPER
symbol, but end up not working when the SoC specific part is built-in
but the helper is in a loadable module.
Use the same trick that DRM_ROCKCHIP already uses for the EXTCON
dependency and disallow DP support when it would not work.
Fixes: 0323287de87d ("drm: Switch DRM_DISPLAY_DP_HELPER to depends on")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/exynos/Kconfig | 2 +-
drivers/gpu/drm/rockchip/Kconfig | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 6a26a0b8eff2..58cd77220741 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -68,7 +68,7 @@ config DRM_EXYNOS_DP
bool "Exynos specific extensions for Analogix DP driver"
depends on DRM_EXYNOS_FIMD || DRM_EXYNOS7_DECON
depends on DRM_DISPLAY_DP_HELPER
- depends on DRM_DISPLAY_HELPER
+ depends on DRM_DISPLAY_HELPER=y || (DRM_DISPLAY_HELPER=m &&
DRM_EXYNOS=m)
select DRM_ANALOGIX_DP
default DRM_EXYNOS
select DRM_PANEL
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 4b49a14758fe..4b4ad75032fd 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -46,7 +46,7 @@ config ROCKCHIP_ANALOGIX_DP
config ROCKCHIP_CDN_DP
bool "Rockchip cdn DP"
depends on DRM_DISPLAY_DP_HELPER
- depends on DRM_DISPLAY_HELPER
+ depends on DRM_DISPLAY_HELPER=y || (DRM_DISPLAY_HELPER=m &&
DRM_ROCKCHIP=m)
depends on EXTCON=y || (EXTCON=m && DRM_ROCKCHIP=m)
help
This selects support for Rockchip SoC specific extensions
--
2.39.2
[PATCH 00/34] address all -Wunused-const warnings
From: Arnd Bergmann
Compilers traditionally warn for unused 'static' variables, but not
if they are constant. The reason here is a custom for C++ programmers
to define named constants as 'static const' variables in header files
instead of using macros or enums.
In W=1 builds, we get warnings only static const variables in C
files, but not in headers, which is a good compromise, but this still
produces warning output in at least 30 files. These warnings are
almost all harmless, but also trivial to fix, and there is no
good reason to warn only about the non-const variables being unused.
I've gone through all the files that I found using randconfig and
allmodconfig builds and created patches to avoid these warnings,
with the goal of retaining a clean build once the option is enabled
by default.
Unfortunately, there is one fairly large patch ("drivers: remove
incorrect of_match_ptr/ACPI_PTR annotations") that touches
34 individual drivers that all need the same one-line change.
If necessary, I can split it up by driver or by subsystem,
but at least for reviewing I would keep it as one piece for
the moment.
Please merge the individual patches through subsystem trees.
I expect that some of these will have to go through multiple
revisions before they are picked up, so anything that gets
applied early saves me from resending.
Arnd
Arnd Bergmann (31):
powerpc/fsl-soc: hide unused const variable
ubsan: fix unused variable warning in test module
platform: goldfish: remove ACPI_PTR() annotations
i2c: pxa: hide unused icr_bits[] variable
3c515: remove unused 'mtu' variable
tracing: hide unused ftrace_event_id_fops
Input: synaptics: hide unused smbus_pnp_ids[] array
power: rt9455: hide unused rt9455_boost_voltage_values
efi: sysfb: don't build when EFI is disabled
clk: ti: dpll: fix incorrect #ifdef checks
apm-emulation: hide an unused variable
sisfb: hide unused variables
dma/congiguous: avoid warning about unused size_bytes
leds: apu: remove duplicate DMI lookup data
iio: ad5755: hook up of_device_id lookup to platform driver
greybus: arche-ctrl: move device table to its right location
lib: checksum: hide unused expected_csum_ipv6_magic[]
sunrpc: suppress warnings for unused procfs functions
comedi: ni_atmio: avoid warning for unused device_ids[] table
iwlegacy: don't warn for unused variables with DEBUG_FS=n
drm/komeda: don't warn for unused debugfs files
firmware: qcom_scm: mark qcom_scm_qseecom_allowlist as __maybe_unused
crypto: ccp - drop platform ifdef checks
usb: gadget: omap_udc: remove unused variable
isdn: kcapi: don't build unused procfs code
cpufreq: intel_pstate: hide unused intel_pstate_cpu_oob_ids[]
net: xgbe: remove extraneous #ifdef checks
Input: imagis - remove incorrect ifdef checks
sata: mv: drop unnecessary #ifdef checks
ASoC: remove incorrect of_match_ptr/ACPI_PTR annotations
spi: remove incorrect of_match_ptr annotations
drivers: remove incorrect of_match_ptr/ACPI_PTR annotations
kbuild: always enable -Wunused-const-variable
Krzysztof Kozlowski (1):
Input: stmpe-ts - mark OF related data as maybe unused
arch/powerpc/sysdev/fsl_msi.c | 2 +
drivers/ata/sata_mv.c | 64 +--
drivers/char/apm-emulation.c | 5 +-
drivers/char/ipmi/ipmb_dev_int.c | 2 +-
drivers/char/tpm/tpm_ftpm_tee.c | 2 +-
drivers/clk/ti/dpll.c | 10 ++-
drivers/comedi/drivers/ni_atmio.c | 2 +-
drivers/cpufreq/intel_pstate.c| 2 +
drivers/crypto/ccp/sp-platform.c | 14 +---
drivers/dma/img-mdc-dma.c | 2 +-
drivers/firmware/efi/Makefile | 3 +-
drivers/firmware/efi/sysfb_efi.c | 2 -
drivers/firmware/qcom/qcom_scm.c | 2 +-
drivers/fpga/versal-fpga.c| 2 +-
.../gpu/drm/arm/display/komeda/komeda_dev.c | 8 ---
drivers/hid/hid-google-hammer.c | 6 +-
drivers/i2c/busses/i2c-pxa.c | 2 +-
drivers/i2c/muxes/i2c-mux-ltc4306.c | 2 +-
drivers/i2c/muxes/i2c-mux-reg.c | 2 +-
drivers/iio/dac/ad5755.c | 1 +
drivers/input/mouse/synaptics.c | 2 +
drivers/input/touchscreen/imagis.c| 4 +-
drivers/input/touchscreen/stmpe-ts.c | 2 +-
drivers/input/touchscreen/wdt87xx_i2c.c | 2 +-
drivers/isdn/capi/Makefile| 3 +-
drivers/isdn/capi/kcapi.c | 7 +-
drivers/leds/leds-apu.c | 3 +-
drivers/mux/adg792a.c | 2 +-
drivers/net/ethernet/3com/3c515.c | 3 -
drivers/net/ethernet/amd/xgbe/xgbe-platform.c | 8 ---
drivers/net/ethernet/apm/xgene-v2/main.c | 2 +-
drivers/net/ethernet/hisilicon/hns_mdio.c | 2 +-
drivers/net/wireless/intel/iwlegacy/4965-rs.c | 15 +---
[PATCH 22/34] drm/komeda: don't warn for unused debugfs files
From: Arnd Bergmann
With debugfs disabled, the komeda_register debugfs file is unused:
drivers/gpu/drm/arm/display/komeda/komeda_dev.c:44:1: error: unused variable
'komeda_register_fops' [-Werror,-Wunused-const-variable]
DEFINE_SHOW_ATTRIBUTE(komeda_register);
The komeda_debugfs_init() function already has a call to debugfs_initialized()
that ends up eliminating the file as dead code, so just drop the #ifdef
to get rid of the warning.
Fixes: 576832681891 ("arm/komeda: Compile komeda_debugfs_init() only if
CONFIG_DEBUG_FS is enabled")
Fixes: 7d3cfb70a604 ("drm/komeda: Add debugfs node "register" for register
dump")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/arm/display/komeda/komeda_dev.c | 8
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index 14ee79becacb..5ba62e637a61 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -12,10 +12,8 @@
#include
#include
#include
-#ifdef CONFIG_DEBUG_FS
#include
#include
-#endif
#include
@@ -43,7 +41,6 @@ static int komeda_register_show(struct seq_file *sf, void *x)
DEFINE_SHOW_ATTRIBUTE(komeda_register);
-#ifdef CONFIG_DEBUG_FS
static void komeda_debugfs_init(struct komeda_dev *mdev)
{
if (!debugfs_initialized())
@@ -55,7 +52,6 @@ static void komeda_debugfs_init(struct komeda_dev *mdev)
debugfs_create_x16("err_verbosity", 0664, mdev->debugfs_root,
>err_verbosity);
}
-#endif
static ssize_t
core_id_show(struct device *dev, struct device_attribute *attr, char *buf)
@@ -265,9 +261,7 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
mdev->err_verbosity = KOMEDA_DEV_PRINT_ERR_EVENTS;
-#ifdef CONFIG_DEBUG_FS
komeda_debugfs_init(mdev);
-#endif
return mdev;
@@ -286,9 +280,7 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
sysfs_remove_group(>kobj, _sysfs_attr_group);
-#ifdef CONFIG_DEBUG_FS
debugfs_remove_recursive(mdev->debugfs_root);
-#endif
if (mdev->aclk)
clk_prepare_enable(mdev->aclk);
--
2.39.2
[PATCH 13/34] sisfb: hide unused variables
From: Arnd Bergmann
Building with W=1 shows that a couple of variables in this driver are only
used in certain configurations:
drivers/video/fbdev/sis/init301.c:239:28: error: 'SiS_Part2CLVX_6' defined but
not used [-Werror=unused-const-variable=]
239 | static const unsigned char SiS_Part2CLVX_6[] = { /* 1080i */
|^~~
drivers/video/fbdev/sis/init301.c:230:28: error: 'SiS_Part2CLVX_5' defined but
not used [-Werror=unused-const-variable=]
230 | static const unsigned char SiS_Part2CLVX_5[] = { /* 750p */
|^~~
drivers/video/fbdev/sis/init301.c:211:28: error: 'SiS_Part2CLVX_4' defined but
not used [-Werror=unused-const-variable=]
211 | static const unsigned char SiS_Part2CLVX_4[] = { /* PAL */
|^~~
drivers/video/fbdev/sis/init301.c:192:28: error: 'SiS_Part2CLVX_3' defined but
not used [-Werror=unused-const-variable=]
192 | static const unsigned char SiS_Part2CLVX_3[] = { /* NTSC, 525i, 525p */
|^~~
drivers/video/fbdev/sis/init301.c:184:28: error: 'SiS_Part2CLVX_2' defined but
not used [-Werror=unused-const-variable=]
184 | static const unsigned char SiS_Part2CLVX_2[] = {
|^~~
drivers/video/fbdev/sis/init301.c:176:28: error: 'SiS_Part2CLVX_1' defined but
not used [-Werror=unused-const-variable=]
176 | static const unsigned char SiS_Part2CLVX_1[] = {
|^~~
This started showing up after the definitions were moved into the
source file from the header, which was not flagged by the compiler.
Move the definition into the appropriate #ifdef block that already
exists next to them.
Fixes: 5908986ef348 ("video: fbdev: sis: avoid mismatched prototypes")
Signed-off-by: Arnd Bergmann
---
drivers/video/fbdev/sis/init301.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/sis/init301.c
b/drivers/video/fbdev/sis/init301.c
index a8fb41f1a258..09329072004f 100644
--- a/drivers/video/fbdev/sis/init301.c
+++ b/drivers/video/fbdev/sis/init301.c
@@ -172,7 +172,7 @@ static const unsigned char SiS_HiTVGroup3_2[] = {
};
/* 301C / 302ELV extended Part2 TV registers (4 tap scaler) */
-
+#ifdef CONFIG_FB_SIS_315
static const unsigned char SiS_Part2CLVX_1[] = {
0x00,0x00,
0x00,0x20,0x00,0x00,0x7F,0x20,0x02,0x7F,0x7D,0x20,0x04,0x7F,0x7D,0x1F,0x06,0x7E,
@@ -245,7 +245,6 @@ static const unsigned char SiS_Part2CLVX_6[] = { /* 1080i
*/
0xFF,0xFF,
};
-#ifdef CONFIG_FB_SIS_315
/* 661 et al LCD data structure (2.03.00) */
static const unsigned char SiS_LCDStruct661[] = {
/* 1024x768 */
--
2.39.2
Re: [PATCH v2 3/3] arch: Rename fbdev header and source files
On Thu, Mar 28, 2024, at 13:46, Helge Deller wrote: > On 3/27/24 21:41, Thomas Zimmermann wrote: >> +++ b/arch/arc/include/asm/video.h >> @@ -0,0 +1,8 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> + >> +#ifndef _ASM_VIDEO_H_ >> +#define _ASM_VIDEO_H_ >> + >> +#include >> + >> +#endif /* _ASM_VIDEO_H_ */ > > I wonder, since that file simply #includes the generic version, > wasn't there a possibility that kbuild could symlink > the generic version for us? > Does it need to be mandatory in include/asm-generic/Kbuild ? > Same applies to a few other files below. It should be enough to just remove the files entirely, as kbuild will generate the same wrappers for mandatory files. Arnd
Re: [PATCH 01/12] kbuild: make -Woverride-init warnings more consistent
On Wed, Mar 27, 2024, at 08:50, Jani Nikula wrote:
> On Tue, 26 Mar 2024, "Arnd Bergmann" wrote:
>> On Tue, Mar 26, 2024, at 21:24, Jani Nikula wrote:
>>> On Tue, 26 Mar 2024, Arnd Bergmann wrote:
>>
>> It works now.
>>
>> The original __diag_ignore_all() only did it for gcc-8 and above
>> because that was initially needed to suppress warnings that
>> got added in that version, but this was always a mistake.
>>
>> 689b097a06ba ("compiler-gcc: Suppress -Wmissing-prototypes
>> warning for all supported GCC") made it work correctly.
>
> Oh, nice! Then I think we'd like to go back to __diag_ignore_all() in
> i915 and xe.
>
> The diff is below. I'm fine with you squashing it to your patch, or if
> you want me to turn it into a proper patch for you to pick up in your
> series, that's fine too. Just let me know.
I think I'd prefer to keep my patch simpler for the moment and
get that merged through the kbuild tree, it already touches
too many places at once.
It may be better for me to just drop the drivers/gpu/ part of
my patch so you can just just take your patch through the
drm tree. I actually have a similar patch for the amdgpu driver
that I can send if you like this option better.
Arnd
[PATCH 1/9] fbdev: shmobile: fix snprintf truncation
From: Arnd Bergmann
The name of the overlay does not fit into the fixed-length field:
drivers/video/fbdev/sh_mobile_lcdcfb.c:1577:2: error: 'snprintf' will always be
truncated; specified size is 16, but format string expands to at least 25
Make it short enough by changing the string.
Fixes: c5deac3c9b22 ("fbdev: sh_mobile_lcdc: Implement overlays support")
Signed-off-by: Arnd Bergmann
---
drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c
b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index eb2297b37504..d35d2cf8 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -1575,7 +1575,7 @@ sh_mobile_lcdc_overlay_fb_init(struct
sh_mobile_lcdc_overlay *ovl)
*/
info->fix = sh_mobile_lcdc_overlay_fix;
snprintf(info->fix.id, sizeof(info->fix.id),
-"SH Mobile LCDC Overlay %u", ovl->index);
+"SHMobile ovl %u", ovl->index);
info->fix.smem_start = ovl->dma_handle;
info->fix.smem_len = ovl->fb_size;
info->fix.line_length = ovl->pitch;
--
2.39.2
[PATCH 0/9] enabled -Wformat-truncation for clang
From: Arnd Bergmann With randconfig build testing, I found only eight files that produce warnings with clang when -Wformat-truncation is enabled. This means we can just turn it on by default rather than only enabling it for "make W=1". Unfortunately, gcc produces a lot more warnings when the option is enabled, so it's not yet possible to turn it on both both compilers. I hope that the patches can get picked up by platform maintainers directly, so the final patch can go in later on. Arnd Arnd Bergmann (9): fbdev: shmobile: fix snprintf truncation enetc: avoid truncating error message qed: avoid truncating work queue length mlx5: avoid truncating error message surface3_power: avoid format string truncation warning Input: IMS: fix printf string overflow scsi: mylex: fix sysfs buffer lengths ALSA: aoa: avoid false-positive format truncation warning kbuild: enable -Wformat-truncation on clang drivers/input/misc/ims-pcu.c | 4 ++-- drivers/net/ethernet/freescale/enetc/enetc.c | 2 +- .../ethernet/mellanox/mlx5/core/esw/bridge.c | 2 +- drivers/net/ethernet/qlogic/qed/qed_main.c| 9 --- drivers/platform/surface/surface3_power.c | 2 +- drivers/scsi/myrb.c | 20 drivers/scsi/myrs.c | 24 +-- drivers/video/fbdev/sh_mobile_lcdcfb.c| 2 +- scripts/Makefile.extrawarn| 2 ++ sound/aoa/soundbus/i2sbus/core.c | 2 +- 10 files changed, 35 insertions(+), 34 deletions(-) -- 2.39.2 Cc: Dmitry Torokhov Cc: Claudiu Manoil Cc: Vladimir Oltean Cc: Jakub Kicinski Cc: Saeed Mahameed Cc: Leon Romanovsky Cc: Ariel Elior Cc: Manish Chopra Cc: Hans de Goede Cc: "Ilpo Järvinen" Cc: Maximilian Luz Cc: Hannes Reinecke Cc: "Martin K. Petersen" Cc: Helge Deller Cc: Masahiro Yamada Cc: Nathan Chancellor Cc: Nicolas Schier Cc: Johannes Berg Cc: Jaroslav Kysela Cc: Takashi Iwai Cc: Nick Desaulniers Cc: Bill Wendling Cc: Justin Stitt Cc: linux-in...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: net...@vger.kernel.org Cc: linux-r...@vger.kernel.org Cc: platform-driver-...@vger.kernel.org Cc: linux-s...@vger.kernel.org Cc: linux-fb...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-kbu...@vger.kernel.org Cc: linuxppc-...@lists.ozlabs.org Cc: alsa-de...@alsa-project.org Cc: linux-so...@vger.kernel.org Cc: l...@lists.linux.dev
Re: [PATCH 01/12] kbuild: make -Woverride-init warnings more consistent
On Tue, Mar 26, 2024, at 21:24, Jani Nikula wrote:
> On Tue, 26 Mar 2024, Arnd Bergmann wrote:
>> From: Arnd Bergmann
>> index 475e1e8c1d35..0786eb0da391 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -50,7 +50,7 @@
>> * the macros available to do this only define GCC 8.
>> */
>> __diag_push();
>> -__diag_ignore(GCC, 8, "-Woverride-init",
>> +__diag_ignore_all("-Woverride-init",
>>"logic to initialize all and then override some is OK");
>
> This is nice because it's more localized than the per-file
> disable. However, we tried to do this in i915, but this doesn't work for
> GCC versions < 8, and some defconfigs enabling -Werror forced us to
> revert. See commit 290d16104575 ("Revert "drm/i915: use localized
> __diag_ignore_all() instead of per file"").
It works now.
The original __diag_ignore_all() only did it for gcc-8 and above
because that was initially needed to suppress warnings that
got added in that version, but this was always a mistake.
689b097a06ba ("compiler-gcc: Suppress -Wmissing-prototypes
warning for all supported GCC") made it work correctly.
Arnd
Re: [PATCH 06/12] nouveau: fix function cast warning
On Tue, Mar 26, 2024, at 16:20, Timur Tabi wrote:
> On Tue, 2024-03-26 at 15:51 +0100, Arnd Bergmann wrote:
>> Calling a function through an incompatible pointer type causes breaks
>> kcfi, so clang warns about the assignment:
>>
>
> ...
>
>> +static void of_fini(void *p)
>> +{
>> + return kfree(p);
>> +}
>> +
>
> I don't know anything about kfci, but shouldn't this just be "kfree(p)",
> without the "return"?
You are right, fixed this up locally now, waiting for more
comments before I resend patches from my series.
I think it works because of a gcc extension, but isn't
valid C otherwise, and I did not mean to rely on this.
Arnd
[PATCH 06/12] nouveau: fix function cast warning
From: Arnd Bergmann
Calling a function through an incompatible pointer type causes breaks
kcfi, so clang warns about the assignment:
drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c:73:10: error: cast from
'void (*)(const void *)' to 'void (*)(void *)' converts to incompatible
function type [-Werror,-Wcast-function-type-strict]
73 | .fini = (void(*)(void *))kfree,
Avoid this with a trivial wrapper.
Fixes: c39f472e9f14 ("drm/nouveau: remove symlinks, move core/ to nvkm/ (no
code changes)")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
index 4bf486b57101..0dbcc23305f3 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowof.c
@@ -66,11 +66,16 @@ of_init(struct nvkm_bios *bios, const char *name)
return ERR_PTR(-EINVAL);
}
+static void of_fini(void *p)
+{
+ return kfree(p);
+}
+
const struct nvbios_source
nvbios_of = {
.name = "OpenFirmware",
.init = of_init,
- .fini = (void(*)(void *))kfree,
+ .fini = of_fini,
.read = of_read,
.size = of_size,
.rw = false,
--
2.39.2
[PATCH 01/12] kbuild: make -Woverride-init warnings more consistent
From: Arnd Bergmann
The -Woverride-init warn about code that may be intentional or not,
but the inintentional ones tend to be real bugs, so there is a bit of
disagreement on whether this warning option should be enabled by default
and we have multiple settings in scripts/Makefile.extrawarn as well as
individual subsystems.
Older versions of clang only supported -Wno-initializer-overrides with
the same meaning as gcc's -Woverride-init, though all supported versions
now work with both. Because of this difference, an earlier cleanup of
mine accidentally turned the clang warning off for W=1 builds and only
left it on for W=2, while it's still enabled for gcc with W=1.
There is also one driver that only turns the warning off for newer
versions of gcc but not other compilers, and some but not all the
Makefiles still use a cc-disable-warning conditional that is no
longer needed with supported compilers here.
Address all of the above by removing the special cases for clang
and always turning the warning off unconditionally where it got
in the way, using the syntax that is supported by both compilers.
Fixes: 2cd3271b7a31 ("kbuild: avoid duplicate warning options")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/amd/display/dc/dce110/Makefile | 2 +-
drivers/gpu/drm/amd/display/dc/dce112/Makefile | 2 +-
drivers/gpu/drm/amd/display/dc/dce120/Makefile | 2 +-
drivers/gpu/drm/amd/display/dc/dce60/Makefile | 2 +-
drivers/gpu/drm/amd/display/dc/dce80/Makefile | 2 +-
drivers/gpu/drm/i915/Makefile | 6 +++---
drivers/gpu/drm/xe/Makefile| 4 ++--
drivers/net/ethernet/renesas/sh_eth.c | 2 +-
drivers/pinctrl/aspeed/Makefile| 2 +-
fs/proc/Makefile | 2 +-
kernel/bpf/Makefile| 2 +-
mm/Makefile| 3 +--
scripts/Makefile.extrawarn | 10 +++---
13 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/Makefile
b/drivers/gpu/drm/amd/display/dc/dce110/Makefile
index f0777d61c2cb..c307f040e48f 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce110/Makefile
@@ -23,7 +23,7 @@
# Makefile for the 'controller' sub-component of DAL.
# It provides the control and status of HW CRTC block.
-CFLAGS_$(AMDDALPATH)/dc/dce110/dce110_resource.o = $(call cc-disable-warning,
override-init)
+CFLAGS_$(AMDDALPATH)/dc/dce110/dce110_resource.o = -Wno-override-init
DCE110 = dce110_timing_generator.o \
dce110_compressor.o dce110_opp_regamma_v.o \
diff --git a/drivers/gpu/drm/amd/display/dc/dce112/Makefile
b/drivers/gpu/drm/amd/display/dc/dce112/Makefile
index 7e92effec894..683866797709 100644
--- a/drivers/gpu/drm/amd/display/dc/dce112/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce112/Makefile
@@ -23,7 +23,7 @@
# Makefile for the 'controller' sub-component of DAL.
# It provides the control and status of HW CRTC block.
-CFLAGS_$(AMDDALPATH)/dc/dce112/dce112_resource.o = $(call cc-disable-warning,
override-init)
+CFLAGS_$(AMDDALPATH)/dc/dce112/dce112_resource.o = -Wno-override-init
DCE112 = dce112_compressor.o
diff --git a/drivers/gpu/drm/amd/display/dc/dce120/Makefile
b/drivers/gpu/drm/amd/display/dc/dce120/Makefile
index 1e3ef68a452a..8f508e662748 100644
--- a/drivers/gpu/drm/amd/display/dc/dce120/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce120/Makefile
@@ -24,7 +24,7 @@
# It provides the control and status of HW CRTC block.
-CFLAGS_$(AMDDALPATH)/dc/dce120/dce120_resource.o = $(call cc-disable-warning,
override-init)
+CFLAGS_$(AMDDALPATH)/dc/dce120/dce120_resource.o = -Wno-override-init
DCE120 = dce120_timing_generator.o
diff --git a/drivers/gpu/drm/amd/display/dc/dce60/Makefile
b/drivers/gpu/drm/amd/display/dc/dce60/Makefile
index fee331accc0e..eede83ad91fa 100644
--- a/drivers/gpu/drm/amd/display/dc/dce60/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce60/Makefile
@@ -23,7 +23,7 @@
# Makefile for the 'controller' sub-component of DAL.
# It provides the control and status of HW CRTC block.
-CFLAGS_$(AMDDALPATH)/dc/dce60/dce60_resource.o = $(call cc-disable-warning,
override-init)
+CFLAGS_$(AMDDALPATH)/dc/dce60/dce60_resource.o = -Wno-override-init
DCE60 = dce60_timing_generator.o dce60_hw_sequencer.o \
dce60_resource.o
diff --git a/drivers/gpu/drm/amd/display/dc/dce80/Makefile
b/drivers/gpu/drm/amd/display/dc/dce80/Makefile
index 7eefffbdc925..fba189d26652 100644
--- a/drivers/gpu/drm/amd/display/dc/dce80/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dce80/Makefile
@@ -23,7 +23,7 @@
# Makefile for the 'controller' sub-component of DAL.
# It provides the control and status of HW CRTC block.
-CFLAGS_$(AMDDALPATH)/dc/dce80/dce80_resource.o = $(call cc-disable-warning,
override-init)
+CFLAGS_$(AMDDALPATH)/dc/dce80/dce80_resource.o = -Wno-override-init
DCE80 = dce80_timing_generato
[PATCH 00/12] kbuild: enable some -Wextra warnings by default
From: Arnd Bergmann This is a follow-up on a couple of patch series I sent in the past, enabling -Wextra (aside from stuff that is explicitly disabled), -Wcast-function-pointer-strict and -Wrestrict. I have tested these on 'defconfig' and 'allmodconfig' builds across all architectures, as well as many 'randconfig' builds on x86, arm and arm64. It would be nice to have all the Makefile.extrawarn changes in v6.10, hopefully with the driver fixes going in before that through the respective subsystem trees. Arnd Arnd Bergmann (12): kbuild: make -Woverride-init warnings more consistent [v3] parport: mfc3: avoid empty-body warning kbuild: turn on -Wextra by default kbuild: remove redundant extra warning flags firmware: dmi-id: add a release callback function nouveau: fix function cast warning cxlflash: fix function pointer cast warnings x86: math-emu: fix function cast warnings kbuild: enable -Wcast-function-type-strict unconditionally sata: sx4: fix pdc20621_get_from_dimm() on 64-bit [v4] kallsyms: rework symbol lookup return codes kbuild: turn on -Wrestrict by default arch/x86/math-emu/fpu_etc.c | 9 +++-- arch/x86/math-emu/fpu_trig.c | 6 ++-- arch/x86/math-emu/reg_constant.c | 7 +++- drivers/ata/sata_sx4.c| 6 ++-- drivers/firmware/dmi-id.c | 7 +++- .../gpu/drm/amd/display/dc/dce110/Makefile| 2 +- .../gpu/drm/amd/display/dc/dce112/Makefile| 2 +- .../gpu/drm/amd/display/dc/dce120/Makefile| 2 +- drivers/gpu/drm/amd/display/dc/dce60/Makefile | 2 +- drivers/gpu/drm/amd/display/dc/dce80/Makefile | 2 +- drivers/gpu/drm/i915/Makefile | 6 ++-- .../drm/nouveau/nvkm/subdev/bios/shadowof.c | 7 +++- drivers/gpu/drm/xe/Makefile | 4 +-- drivers/net/ethernet/renesas/sh_eth.c | 2 +- drivers/parport/parport_mfc3.c| 3 +- drivers/pinctrl/aspeed/Makefile | 2 +- drivers/scsi/cxlflash/lunmgt.c| 4 +-- drivers/scsi/cxlflash/main.c | 14 drivers/scsi/cxlflash/superpipe.c | 34 +-- drivers/scsi/cxlflash/superpipe.h | 11 +++--- drivers/scsi/cxlflash/vlun.c | 7 ++-- fs/proc/Makefile | 2 +- include/linux/filter.h| 14 include/linux/ftrace.h| 6 ++-- include/linux/module.h| 14 kernel/bpf/Makefile | 2 +- kernel/bpf/core.c | 7 ++-- kernel/kallsyms.c | 23 +++-- kernel/module/kallsyms.c | 26 +++--- kernel/trace/ftrace.c | 13 +++ mm/Makefile | 3 +- scripts/Makefile.extrawarn| 33 -- 32 files changed, 134 insertions(+), 148 deletions(-) -- 2.39.2 Cc: Bill Metzenthen Cc: Thomas Gleixner Cc: x...@kernel.org Cc: Damien Le Moal Cc: Jean Delvare Cc: Harry Wentland Cc: Jani Nikula Cc: Sergey Shtylyov Cc: Jakub Kicinski Cc: Sudip Mukherjee Cc: Andrew Jeffery Cc: "Manoj N. Kumar" Cc: "Martin K. Petersen" Cc: Alexei Starovoitov Cc: Steven Rostedt Cc: Luis Chamberlain Cc: Andrew Morton Cc: Masahiro Yamada Cc: Nathan Chancellor Cc: Nicolas Schier Cc: Greg Kroah-Hartman Cc: linux-ker...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Cc: intel-...@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: intel...@lists.freedesktop.org Cc: net...@vger.kernel.org Cc: linux-renesas-...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-s...@vger.kernel.org Cc: b...@vger.kernel.org Cc: linux-trace-ker...@vger.kernel.org Cc: linux-modu...@vger.kernel.org Cc: linux...@kvack.org Cc: linux-kbu...@vger.kernel.org Cc: l...@lists.linux.dev
[PATCH] [RESEND] drm/imagination: avoid -Woverflow warning
From: Arnd Bergmann
The array size calculation in pvr_vm_mips_fini() appears to be incorrect based
on
taking the size of the pointer rather than the size of the array, which
manifests
as a warning about signed integer overflow:
In file included from include/linux/kernel.h:16,
from drivers/gpu/drm/imagination/pvr_rogue_fwif.h:10,
from drivers/gpu/drm/imagination/pvr_ccb.h:7,
from drivers/gpu/drm/imagination/pvr_device.h:7,
from drivers/gpu/drm/imagination/pvr_vm_mips.c:4:
drivers/gpu/drm/imagination/pvr_vm_mips.c: In function 'pvr_vm_mips_fini':
include/linux/array_size.h:11:25: error: overflow in conversion from 'long
unsigned int' to 'int' changes value from '18446744073709551615' to '-1'
[-Werror=overflow]
11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
__must_be_array(arr))
| ^
drivers/gpu/drm/imagination/pvr_vm_mips.c:106:24: note: in expansion of macro
'ARRAY_SIZE'
106 | for (page_nr = ARRAY_SIZE(mips_data->pt_pages) - 1; page_nr >=
0; page_nr--) {
|^~
Just use the number of array elements directly here, and in the corresponding
init function for consistency.
Fixes: 927f3e0253c1 ("drm/imagination: Implement MIPS firmware processor and
MMU support")
Reviewed-by: Donald Robson
Link:
https://lore.kernel.org/lkml/9df9e4f87727399928c068dbbf614c9895ae15f9.ca...@imgtec.com/
Signed-off-by: Arnd Bergmann
---
I sent this one last year when the warning appeared, it looks like it
got lost in the meantime, resending it unchanged.
---
drivers/gpu/drm/imagination/pvr_vm_mips.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_vm_mips.c
b/drivers/gpu/drm/imagination/pvr_vm_mips.c
index b7fef3c797e6..4f99b4af871c 100644
--- a/drivers/gpu/drm/imagination/pvr_vm_mips.c
+++ b/drivers/gpu/drm/imagination/pvr_vm_mips.c
@@ -46,7 +46,7 @@ pvr_vm_mips_init(struct pvr_device *pvr_dev)
if (!mips_data)
return -ENOMEM;
- for (page_nr = 0; page_nr < ARRAY_SIZE(mips_data->pt_pages); page_nr++)
{
+ for (page_nr = 0; page_nr < PVR_MIPS_PT_PAGE_COUNT; page_nr++) {
mips_data->pt_pages[page_nr] = alloc_page(GFP_KERNEL |
__GFP_ZERO);
if (!mips_data->pt_pages[page_nr]) {
err = -ENOMEM;
@@ -102,7 +102,7 @@ pvr_vm_mips_fini(struct pvr_device *pvr_dev)
int page_nr;
vunmap(mips_data->pt);
- for (page_nr = ARRAY_SIZE(mips_data->pt_pages) - 1; page_nr >= 0;
page_nr--) {
+ for (page_nr = PVR_MIPS_PT_PAGE_COUNT - 1; page_nr >= 0; page_nr--) {
dma_unmap_page(from_pvr_device(pvr_dev)->dev,
mips_data->pt_dma_addr[page_nr], PAGE_SIZE,
DMA_TO_DEVICE);
--
2.39.2
[PATCH] drm/i915: add intel_opregion_vbt_present() stub function
From: Arnd Bergmann
The newly added function is not available without CONFIG_ACPI, causing
a build failure:
drivers/gpu/drm/i915/display/intel_bios.c:3424:24: error: implicit declaration
of function 'intel_opregion_vbt_present'; did you mean
'intel_opregion_asle_present'? [-Werror=implicit-function-declaration]
Add an empty stub in the same place as the other stubs.
Fixes: 9d9bb71f3e11 ("drm/i915: Extract opregion vbt presence check")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/i915/display/intel_opregion.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h
b/drivers/gpu/drm/i915/display/intel_opregion.h
index 63573c38d735..4b2b8e752632 100644
--- a/drivers/gpu/drm/i915/display/intel_opregion.h
+++ b/drivers/gpu/drm/i915/display/intel_opregion.h
@@ -120,6 +120,11 @@ intel_opregion_get_edid(struct intel_connector *connector)
return NULL;
}
+static inline bool intel_opregion_vbt_present(struct drm_i915_private *i915)
+{
+ return false;
+}
+
static inline const void *
intel_opregion_get_vbt(struct drm_i915_private *i915, size_t *size)
{
--
2.39.2
Re: [PATCH v2 28/28] fbdev/p9100: Drop now unused driver p9100
On Sat, Mar 9, 2024, at 19:15, Sam Ravnborg via B4 Relay wrote: > From: Sam Ravnborg > > The p9100 driver is only relevant for the Sparcbook 3 machine, > and with sun4m support removed this driver is no longer relevant. > > Signed-off-by: Sam Ravnborg > Acked-by: Arnd Bergmann > Acked-by: Thomas Zimmermann > Cc: "David S. Miller" > Cc: Arnd Bergmann > Cc: Andreas Larsson > Cc: Helge Deller > --- > drivers/video/fbdev/Kconfig | 8 - > drivers/video/fbdev/Makefile | 1 - > drivers/video/fbdev/p9100.c | 372 > --- > 3 files changed, 381 deletions(-) I tried to figure out if there are other drivers in the same category and found the list at https://everything2.com/title/Sun+graphics+cards As far as I can tell, the only SBUS graphics that were shipped on sparc64 are FB_FFB and FB_CG6, so we could go further and remove BW2, CG3, TCX, CG14 and LEO as well. No need to change anything here for the moment, dropping p9100 is already a step in the right direction. Arnd
Re: [RFC PATCH net-next v6 05/15] netdev: support binding dma-buf to netdevice
On Tue, Mar 5, 2024, at 21:00, Mina Almasry wrote: > On Tue, Mar 5, 2024 at 1:05 AM Arnd Bergmann wrote: >> On Tue, Mar 5, 2024, at 03:01, Mina Almasry wrote: > > A key goal of this patch series is that the kernel does not try to > parse the skb frags that reside in the dma-buf for that precise > reason. This is achieved using patch "net: add support for skbs with > unreadable frags" which disables the kernel touching the payload in > these skbs, and "tcp: RX path for devmem TCP" which implements a uapi > where the kernel hands the data in the dmabuf to the userspace via a > cmsg that gives the user a pointer to the data in the dmabuf (offset + > size). > > So really AFACT the only restriction here is that the NIC should be > able to DMA into the dmabuf that we're attaching, and dma_buf_attach() > fails in this scenario so we're covered there. Ok, makes sense. Thanks for the clarification. Arnd
Re: [RFC PATCH net-next v6 12/15] tcp: RX path for devmem TCP
On Tue, Mar 5, 2024, at 20:22, Mina Almasry wrote:
> On Tue, Mar 5, 2024 at 12:42 AM Arnd Bergmann wrote:
>> On Tue, Mar 5, 2024, at 03:01, Mina Almasry wrote:
>>
>> This structure requires a special compat handler to run
>> x86-32 binaries on x86-64 because of the different alignment
>> requirements. Any uapi-visible structures should be defined
>> to avoid this and just have no holes in them. Maybe extend
>> one of the __u32 members to __u64 or add another 32-bit padding field?
>>
>
> Honestly the 32-bit fields as-is are somewhat comically large. I don't
> think extending the __u32 -> __u64 is preferred because I don't see us
> needing that much, so maybe I can add another 32-bit padding field.
> Does this look good to you?
Having a reserved field works but requires that you check it for
being zero already, so you can detect an incompatible caller.
> struct dmabuf_cmsg {
> __u64 frag_offset;
> __u32 frag_size;
> __u32 frag_token;
> __u32 dmabuf_id;
> __u32 ext; /* reserved for future flags */
> };
Maybe call it 'flags'?
> Another option is to actually compress frag_token & dmabuf_id to be
> 32-bit combined size if that addresses your concern. I prefer that
> less in case they end up being too small for future use cases.
I don't know what either of those fields is. Is dmabuf_id not a
file descriptor? If it is, it has to be 32 bits wide. Otherwise
having two 16-bit fields and a 32-bit field would indeed add up
to a multiple of the structure alignment on all architectures and
solve the problem.
Arnd
Re: [net-next v1 04/16] gve: implement queue api
On Fri, Dec 8, 2023, at 01:52, Mina Almasry wrote:
> +static void *gve_rx_queue_mem_alloc(struct net_device *dev, int idx)
> +{
> + struct gve_per_rx_queue_mem_dqo *gve_q_mem;
...
> +
> + gve_q_mem = kvcalloc(1, sizeof(*gve_q_mem), GFP_KERNEL);
> + if (!gve_q_mem)
> + goto err;
[minor comment]
The structure does not seem overly large, even if you have
an array here, I don't see why you would need the vmalloc
type allocation for struct gve_per_rx_queue_mem_dqo.
Arnd
Re: [RFC PATCH net-next v6 05/15] netdev: support binding dma-buf to netdevice
On Tue, Mar 5, 2024, at 03:01, Mina Almasry wrote:
> +int netdev_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
> +struct netdev_dmabuf_binding **out)
> +{
> + struct netdev_dmabuf_binding *binding;
> + static u32 id_alloc_next;
> + struct scatterlist *sg;
> + struct dma_buf *dmabuf;
> + unsigned int sg_idx, i;
> + unsigned long virtual;
> + int err;
> +
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
> +
> + dmabuf = dma_buf_get(dmabuf_fd);
> + if (IS_ERR_OR_NULL(dmabuf))
> + return -EBADFD;
You should never need to use IS_ERR_OR_NULL() for a properly
defined kernel interface. This one should always return an
error or a valid pointer, so don't check for NULL.
> + binding->attachment = dma_buf_attach(binding->dmabuf, dev->dev.parent);
> + if (IS_ERR(binding->attachment)) {
> + err = PTR_ERR(binding->attachment);
> + goto err_free_id;
> + }
> +
> + binding->sgt =
> + dma_buf_map_attachment(binding->attachment, DMA_BIDIRECTIONAL);
> + if (IS_ERR(binding->sgt)) {
> + err = PTR_ERR(binding->sgt);
> + goto err_detach;
> + }
Should there be a check to verify that this buffer
is suitable for network data?
In general, dmabuf allows buffers that are uncached or reside
in MMIO space of another device, but I think this would break
when you get an skb with those buffers and try to parse the
data inside of the kernel on architectures where MMIO space
is not a normal pointer or unaligned access is disallowed on
uncached data.
Arnd
Re: [RFC PATCH net-next v6 12/15] tcp: RX path for devmem TCP
On Tue, Mar 5, 2024, at 03:01, Mina Almasry wrote:
> --- a/arch/alpha/include/uapi/asm/socket.h
> +++ b/arch/alpha/include/uapi/asm/socket.h
> #define SO_PEERPIDFD 77
> +#define SO_DEVMEM_LINEAR 79
> +#define SO_DEVMEM_DMABUF 80
> --- a/arch/mips/include/uapi/asm/socket.h
> +++ b/arch/mips/include/uapi/asm/socket.h
> #define SO_PEERPIDFD 77
> +#define SO_DEVMEM_LINEAR 79
> +#define SO_DEVMEM_DMABUF 80
> --- a/arch/parisc/include/uapi/asm/socket.h
> +++ b/arch/parisc/include/uapi/asm/socket.h
> #define SO_PEERPIDFD 0x404B
> +#define SO_DEVMEM_LINEAR 98
> +#define SO_DEVMEM_DMABUF 99
> --- a/arch/sparc/include/uapi/asm/socket.h
> +++ b/arch/sparc/include/uapi/asm/socket.h
> #define SO_PEERPIDFD 0x0056
> +#define SO_DEVMEM_LINEAR 0x0058
> +#define SO_DEVMEM_DMABUF 0x0059
> --- a/include/uapi/asm-generic/socket.h
> +++ b/include/uapi/asm-generic/socket.h
> @@ -135,6 +135,11 @@
> #define SO_PEERPIDFD 77
> +#define SO_DEVMEM_LINEAR 98
> +#define SO_DEVMEM_DMABUF 99
These look inconsistent. I can see how you picked the
alpha and mips numbers, but how did you come up with
the generic and parisc ones? Can you follow the existing
scheme instead?
> diff --git a/include/uapi/linux/uio.h b/include/uapi/linux/uio.h
> index 059b1a9147f4..ad92e37699da 100644
> --- a/include/uapi/linux/uio.h
> +++ b/include/uapi/linux/uio.h
> @@ -20,6 +20,16 @@ struct iovec
> __kernel_size_t iov_len; /* Must be size_t (1003.1g) */
> };
>
> +struct dmabuf_cmsg {
> + __u64 frag_offset; /* offset into the dmabuf where the frag starts.
> + */
> + __u32 frag_size;/* size of the frag. */
> + __u32 frag_token; /* token representing this frag for
> + * DEVMEM_DONTNEED.
> + */
> + __u32 dmabuf_id; /* dmabuf id this frag belongs to. */
> +};
This structure requires a special compat handler to run
x86-32 binaries on x86-64 because of the different alignment
requirements. Any uapi-visible structures should be defined
to avoid this and just have no holes in them. Maybe extend
one of the __u32 members to __u64 or add another 32-bit padding field?
Arnd
Re: arm: ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
On Mon, Mar 4, 2024, at 14:01, Ard Biesheuvel wrote:
> On Mon, 4 Mar 2024 at 13:35, Arnd Bergmann wrote:
>> On Mon, Mar 4, 2024, at 12:45, Andre Przywara wrote:
>> It's not critical if this is called infrequently, and as Maxime
>> just replied, the 64-bit division is in fact required here.
>> Since we are dividing by a constant value (200), there is a good
>> chance that this will be get turned into fairly efficient
>> multiply/shift code.
>>
>
> Clang does not implement that optimization for 64-bit division. That
> is how we ended up with this error in the first place.
I meant it will use the optimization after the patch to convert
the plain '/' to div_u64().
> Perhaps it is worthwhile to make div_u64() check its divisor, e.g.,
>
> --- a/include/linux/math64.h
> +++ b/include/linux/math64.h
> @@ -127,6 +127,9 @@
> static inline u64 div_u64(u64 dividend, u32 divisor)
> {
> u32 remainder;
> +
> + if (IS_ENABLED(CONFIG_CC_IS_GCC) && __builtin_constant_p(divisor))
> + return dividend / divisor;
> return div_u64_rem(dividend, divisor, );
> }
I think the div_u64()->do_div()->__div64_const32()->__arch_xprod_64()
optimization in asm-generic/div64.h already produces what we want
on both compilers. Is there something missing there?
Arnd
Re: arm: ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
On Mon, Mar 4, 2024, at 12:45, Andre Przywara wrote: > On Mon, 04 Mar 2024 12:26:46 +0100 > "Arnd Bergmann" wrote: > >> On Mon, Mar 4, 2024, at 12:24, Andre Przywara wrote: >> > On Mon, 04 Mar 2024 12:11:36 +0100 "Arnd Bergmann" wrote: >> >> >> >> This used to be a 32-bit division. If the rate is never more than >> >> 4.2GHz, clock could be turned back into 'unsigned long' to avoid >> >> the expensive div_u64(). >> > >> > Wouldn't "div_u64(clock, 200)" solve this problem? >> >> Yes, that's why I mentioned it as the worse of the two obvious >> solutions. ;-) > > Argh, should have cleaned my glasses first ;-) > > I guess I was put somehow put off by the word "expensive". While it's > admittedly not trivial, I wonder if we care about the (hidden) complexity > of that function? I mean it's neither core code nor something called > frequently? It's not critical if this is called infrequently, and as Maxime just replied, the 64-bit division is in fact required here. Since we are dividing by a constant value (200), there is a good chance that this will be get turned into fairly efficient multiply/shift code. > I don't think we have any clock exceeding 3GHz at the moment, but it > sounds fishy to rely on that. Right, it's just important to look at each case individually. The cost of 64-bit division is crazy if it gets called repeatedly, which is of course the entire reason we don't provide a __aeabi_uldivmod() function and require developers to think before adding div_u64(). Arnd
Re: arm: ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
On Mon, Mar 4, 2024, at 12:24, Andre Przywara wrote: > On Mon, 04 Mar 2024 12:11:36 +0100 "Arnd Bergmann" wrote: >> >> This used to be a 32-bit division. If the rate is never more than >> 4.2GHz, clock could be turned back into 'unsigned long' to avoid >> the expensive div_u64(). > > Wouldn't "div_u64(clock, 200)" solve this problem? Yes, that's why I mentioned it as the worse of the two obvious solutions. ;-) Arnd
Re: arm: ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
On Mon, Mar 4, 2024, at 09:07, Naresh Kamboju wrote:
> The arm defconfig builds failed on today's Linux next tag next-20240304.
>
> Build log:
> -
> ERROR: modpost: "__aeabi_uldivmod"
> [drivers/gpu/drm/sun4i/sun4i-drm-hdmi.ko] undefined!
>
Apparently caused by the 64-bit division in 358e76fd613a
("drm/sun4i: hdmi: Consolidate atomic_check and mode_valid"):
+static enum drm_mode_status
+sun4i_hdmi_connector_clock_valid(const struct drm_connector *connector,
+const struct drm_display_mode *mode,
+unsigned long long clock)
{
- struct sun4i_hdmi *hdmi = drm_encoder_to_sun4i_hdmi(encoder);
- unsigned long rate = mode->clock * 1000;
- unsigned long diff = rate / 200; /* +-0.5% allowed by HDMI spec */
+ const struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
+ unsigned long diff = clock / 200; /* +-0.5% allowed by HDMI spec */
long rounded_rate;
This used to be a 32-bit division. If the rate is never more than
4.2GHz, clock could be turned back into 'unsigned long' to avoid
the expensive div_u64().
Arnd
Re: [PATCH 1/2] drm/buddy: stop using PAGE_SIZE
On Thu, Feb 29, 2024, at 11:51, Matthew Auld wrote: > The drm_buddy minimum page-size requirements should be distinct from the > CPU PAGE_SIZE. Only restriction is that the minimum page-size is at > least 4K. > > Signed-off-by: Matthew Auld > Cc: Arunpravin Paneer Selvam > Cc: Christian König > Cc: Arnd Bergmann Acked-by: Arnd Bergmann
[PATCH] xe: avoid using writeq() on 32-bit
From: Arnd Bergmann
32-bit kernels do not provide a writeq(), failing the build:
drivers/gpu/drm/xe/xe_ggtt.c:78:2: error: use of undeclared identifier 'writeq'
78 | writeq(pte, >gsm[addr >> XE_PTE_SHIFT]);
Using lo_hi_writeq() instead will write the lower 32 bits to the address
before writing the upper 32 bits to the following word, which is likely
the correct replacement to do on 32-bit targets.
Include the linux/io-64-nonatomic-lo-hi.h header to automatically pick
the regular writeq() on 64-bit machines but fall back to lo_hi_writeq()
on 32-bit ones.
Fixes: 237412e45390 ("drm/xe: Enable 32bits build")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/xe/xe_ggtt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
index 5d46958e3144..1ffcc63ca86d 100644
--- a/drivers/gpu/drm/xe/xe_ggtt.c
+++ b/drivers/gpu/drm/xe/xe_ggtt.c
@@ -6,6 +6,7 @@
#include "xe_ggtt.h"
#include
+#include
#include
#include
--
2.39.2
Re: [PATCH 3/3] [v2] drm/xe/xe2: fix 64-bit division in pte_update_size
On Mon, Feb 26, 2024, at 17:40, Lucas De Marchi wrote:
> On Mon, Feb 26, 2024 at 01:46:38PM +0100, Arnd Bergmann wrote:
>>
>>Fixes: 237412e45390 ("drm/xe: Enable 32bits build")
>>Signed-off-by: Arnd Bergmann
>>---
>>v2: use correct Fixes tag
>
> but what about the other comment? How are we supposed to use
> DIV_ROUND_UP() but then in some places (which?) have to open code it?
The problem is not DIV_ROUND_UP() but the division but the 64-bit
division itself. There is a DIV_ROUND_UP_ULL() macro that would
address the build failure as well, but doing the shift is much
more efficient here since it can be done in a couple of instructions.
> What compiler does this fail on?
I saw it with clang-19 on 32-bit arm, but I assume it happens
on others as well.
>> drivers/gpu/drm/xe/xe_migrate.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
>>index a66fdf2d2991..ee1bb938c493 100644
>>--- a/drivers/gpu/drm/xe/xe_migrate.c
>>+++ b/drivers/gpu/drm/xe/xe_migrate.c
>>@@ -462,7 +462,7 @@ static u32 pte_update_size(struct xe_migrate *m,
>> } else {
>> /* Clip L0 to available size */
>> u64 size = min(*L0, (u64)avail_pts * SZ_2M);
>>- u64 num_4k_pages = DIV_ROUND_UP(size, XE_PAGE_SIZE);
>>+ u32 num_4k_pages = (size + XE_PAGE_SIZE - 1) >> XE_PTE_SHIFT;
>
> also the commit message doesn't seem to match the patch as you are only
> changing one instance.
Not sure what you mean. As I wrote in the changelog, the
second instance is fixed by using a 32-bit division here,
which does not cause link failures.
Arnd
[PATCH 3/3] [v2] drm/xe/xe2: fix 64-bit division in pte_update_size
From: Arnd Bergmann
This function does not build on 32-bit targets when the compiler
fails to reduce DIV_ROUND_UP() into a shift:
ld.lld: error: undefined symbol: __aeabi_uldivmod
>>> referenced by xe_migrate.c
>>> drivers/gpu/drm/xe/xe_migrate.o:(pte_update_size) in archive
>>> vmlinux.a
There are two instances in this function. Change the first to
use an open-coded shift with the same behavior, and the second
one to a 32-bit calculation, which is sufficient here as the size
is never more than 2^32 pages (16TB).
Fixes: 237412e45390 ("drm/xe: Enable 32bits build")
Signed-off-by: Arnd Bergmann
---
v2: use correct Fixes tag
---
drivers/gpu/drm/xe/xe_migrate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
index a66fdf2d2991..ee1bb938c493 100644
--- a/drivers/gpu/drm/xe/xe_migrate.c
+++ b/drivers/gpu/drm/xe/xe_migrate.c
@@ -462,7 +462,7 @@ static u32 pte_update_size(struct xe_migrate *m,
} else {
/* Clip L0 to available size */
u64 size = min(*L0, (u64)avail_pts * SZ_2M);
- u64 num_4k_pages = DIV_ROUND_UP(size, XE_PAGE_SIZE);
+ u32 num_4k_pages = (size + XE_PAGE_SIZE - 1) >> XE_PTE_SHIFT;
*L0 = size;
*L0_ofs = xe_migrate_vm_addr(pt_ofs, 0);
--
2.39.2
[PATCH 2/3] [v2] drm/xe/mmio: fix build warning for BAR resize on 32-bit
From: Arnd Bergmann
clang complains about a nonsensical test on builds with a 32-bit phys_addr_t,
which means resizing will always fail:
drivers/gpu/drm/xe/xe_mmio.c:109:23: error: result of comparison of constant
4294967296 with expression of type 'resource_size_t' (aka 'unsigned int') is
always false [-Werror,-Wtautological-constant-out-of-range-compare]
109 | root_res->start > 0x1ull)
| ~~~ ^ ~~
Previously, BAR resize was always disallowed on 32-bit kernels, but
this apparently changed recently. Since 32-bit machines can in theory
support PAE/LPAE for large address spaces, this may end up useful,
so change the driver to shut up the warning but still work when
phys_addr_t/resource_size_t is 64 bit wide.
Fixes: 9a6e6c14bfde ("drm/xe/mmio: Use non-atomic writeq/readq variant for 32b")
Fixes: 237412e45390 ("drm/xe: Enable 32bits build")
Signed-off-by: Arnd Bergmann
---
v2: use correct Fixes tag
---
drivers/gpu/drm/xe/xe_mmio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index e3db3a178760..7ba2477452d7 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -106,7 +106,7 @@ static void xe_resize_vram_bar(struct xe_device *xe)
pci_bus_for_each_resource(root, root_res, i) {
if (root_res && root_res->flags & (IORESOURCE_MEM |
IORESOURCE_MEM_64) &&
- root_res->start > 0x1ull)
+ (u64)root_res->start > 0x1ul)
break;
}
--
2.39.2
[PATCH 1/3] [v2] drm/xe/kunit: fix link failure with built-in xe
From: Arnd Bergmann
When the driver is built-in but the tests are in loadable modules,
the helpers don't actually get put into the driver:
ERROR: modpost: "xe_kunit_helper_alloc_xe_device"
[drivers/gpu/drm/xe/tests/xe_test.ko] undefined!
Change the Makefile to ensure they are always part of the driver
even when the rest of the kunit tests are in loadable modules.
Fixes: 5095d13d758b ("drm/xe/kunit: Define helper functions to allocate fake xe
device")
Signed-off-by: Arnd Bergmann
---
v2: don't remove KUNIT dependency
---
drivers/gpu/drm/xe/Kconfig | 1 +
drivers/gpu/drm/xe/Kconfig.debug | 1 -
drivers/gpu/drm/xe/Makefile | 6 --
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 6d4428b19a4c..c3a3b204ae5b 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -11,6 +11,7 @@ config DRM_XE
select DRM_BUDDY
select DRM_EXEC
select DRM_KMS_HELPER
+ select DRM_KUNIT_TEST_HELPERS if DRM_XE_KUNIT_TEST != n
select DRM_PANEL
select DRM_SUBALLOC_HELPER
select DRM_DISPLAY_DP_HELPER
diff --git a/drivers/gpu/drm/xe/Kconfig.debug b/drivers/gpu/drm/xe/Kconfig.debug
index 549065f57a78..df02e5d17d26 100644
--- a/drivers/gpu/drm/xe/Kconfig.debug
+++ b/drivers/gpu/drm/xe/Kconfig.debug
@@ -76,7 +76,6 @@ config DRM_XE_KUNIT_TEST
depends on DRM_XE && KUNIT && DEBUG_FS
default KUNIT_ALL_TESTS
select DRM_EXPORT_FOR_TESTS if m
- select DRM_KUNIT_TEST_HELPERS
help
Choose this option to allow the driver to perform selftests under
the kunit framework
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 4c6ffe4b2172..b596e4482a9b 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -158,8 +158,10 @@ xe-$(CONFIG_PCI_IOV) += \
xe_lmtt_2l.o \
xe_lmtt_ml.o
-xe-$(CONFIG_DRM_XE_KUNIT_TEST) += \
- tests/xe_kunit_helpers.o
+# include helpers for tests even when XE is built-in
+ifdef CONFIG_DRM_XE_KUNIT_TEST
+xe-y += tests/xe_kunit_helpers.o
+endif
# i915 Display compat #defines and #includes
subdir-ccflags-$(CONFIG_DRM_XE_DISPLAY) += \
--
2.39.2
Re: [PATCH 1/3] drm/xe/kunit: fix link failure with built-in xe
On Mon, Feb 26, 2024, at 04:42, Lucas De Marchi wrote:
> On Sat, Feb 24, 2024 at 01:14:59PM +0100, Arnd Bergmann wrote:
>>From: Arnd Bergmann
>>
>>When the driver is built-in but the tests are in loadable modules,
>>the helpers don't actually get put into the driver:
>>
>>ERROR: modpost: "xe_kunit_helper_alloc_xe_device"
>>[drivers/gpu/drm/xe/tests/xe_test.ko] undefined!
>>
>>Change the Makefile to ensure they are always part of the driver
>>even when the rest of the kunit tests are in loadable modules.
>>
>>The tests/xe_kunit_helpers.c file depends on DRM_KUNIT_TEST_HELPERS,
>>so this has to always be selected by the main XE module now, rather
>>than the actual tests. In turn, the "depends on (m || (y && KUNIT=y))"
>>doesn't really do what it tried and can just be removed.
>
> it actually did, which was to workaround issues prior to the commit you
> are pointing out. What it did was to make sure xe.ko is m, or if it's
> built-in, kunit is also built-in. Apparently the problem here is that
> the xe_test.ko is missing the symbols.
Ah, I misunderstood the intention, as I was thrown off by the
redundant 'y &&' which sounds like it was trying to force XE
to be built-in rather than modular when Kunit is !=m.
The more common way to write this is 'depends on KUNIT || !KUNIT',
with some drivers writing it as 'depends on m || KUNIT!=m'.
I double-checked now and found that the dependency is indeed
still needed:
WARNING: unmet direct dependencies detected for DRM_KUNIT_TEST_HELPERS
Depends on [m]: HAS_IOMEM [=y] && DRM [=y] && KUNIT [=m]
Selected by [y]:
- DRM_XE [=y] && HAS_IOMEM [=y] && DRM [=y] && PCI [=y] && MMU [=y] &&
(ACPI_VIDEO [=y] || !ACPI [=y]) && DRM_XE_KUNIT_TEST [=m]!=n
Selected by [m]:
- DRM_KUNIT_TEST [=m] && HAS_IOMEM [=y] && DRM [=y] && KUNIT [=m] && MMU [=y]
> See commit 08987a8b6820 ("drm/xe: Fix build with KUNIT=m").
>
> I'm happy to remove it though if it's indeed not needed anymore.
Ideally the xe.ko module should not depend on anything exported
by lib/kunit, but for the moment, the tests/xe_kunit_helpers.o
file is still included in xe.ko and in turn depends on kunit.
Changing this is probably a little more complicated than my
patch, so I'll just send a v2 without the incorrect line.
Arnd
[PATCH 3/3] drm/xe/xe2: fix 64-bit division in pte_update_size
From: Arnd Bergmann
This function does not build on 32-bit targets when the compiler
fails to reduce DIV_ROUND_UP() into a shift:
ld.lld: error: undefined symbol: __aeabi_uldivmod
>>> referenced by xe_migrate.c
>>> drivers/gpu/drm/xe/xe_migrate.o:(pte_update_size) in archive
>>> vmlinux.a
There are two instances in this function. Change the first to
use an open-coded shift with the same behavior, and the second
one to a 32-bit calculation, which is sufficient here as the size
is never more than 2^32 pages (16TB).
Fixes: ea97a66a2218 ("drm/xe: Disable 32bits build")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/xe/xe_migrate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
index a66fdf2d2991..ee1bb938c493 100644
--- a/drivers/gpu/drm/xe/xe_migrate.c
+++ b/drivers/gpu/drm/xe/xe_migrate.c
@@ -462,7 +462,7 @@ static u32 pte_update_size(struct xe_migrate *m,
} else {
/* Clip L0 to available size */
u64 size = min(*L0, (u64)avail_pts * SZ_2M);
- u64 num_4k_pages = DIV_ROUND_UP(size, XE_PAGE_SIZE);
+ u32 num_4k_pages = (size + XE_PAGE_SIZE - 1) >> XE_PTE_SHIFT;
*L0 = size;
*L0_ofs = xe_migrate_vm_addr(pt_ofs, 0);
--
2.39.2
[PATCH 2/3] drm/xe/mmio: fix build warning for BAR resize on 32-bit
From: Arnd Bergmann
clang complains about a nonsensical test on builds with a 32-bit phys_addr_t,
which means resizing will always fail:
drivers/gpu/drm/xe/xe_mmio.c:109:23: error: result of comparison of constant
4294967296 with expression of type 'resource_size_t' (aka 'unsigned int') is
always false [-Werror,-Wtautological-constant-out-of-range-compare]
109 | root_res->start > 0x1ull)
| ~~~ ^ ~~
Previously, BAR resize was always disallowed on 32-bit kernels, but
this apparently changed recently. Since 32-bit machines can in theory
support PAE/LPAE for large address spaces, this may end up useful,
so change the driver to shut up the warning but still work when
phys_addr_t/resource_size_t is 64 bit wide.
Fixes: 9a6e6c14bfde ("drm/xe/mmio: Use non-atomic writeq/readq variant for 32b")
Fixes: ea97a66a2218 ("drm/xe: Disable 32bits build")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/xe/xe_mmio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
index e3db3a178760..7ba2477452d7 100644
--- a/drivers/gpu/drm/xe/xe_mmio.c
+++ b/drivers/gpu/drm/xe/xe_mmio.c
@@ -106,7 +106,7 @@ static void xe_resize_vram_bar(struct xe_device *xe)
pci_bus_for_each_resource(root, root_res, i) {
if (root_res && root_res->flags & (IORESOURCE_MEM |
IORESOURCE_MEM_64) &&
- root_res->start > 0x1ull)
+ (u64)root_res->start > 0x1ul)
break;
}
--
2.39.2
[PATCH 1/3] drm/xe/kunit: fix link failure with built-in xe
From: Arnd Bergmann
When the driver is built-in but the tests are in loadable modules,
the helpers don't actually get put into the driver:
ERROR: modpost: "xe_kunit_helper_alloc_xe_device"
[drivers/gpu/drm/xe/tests/xe_test.ko] undefined!
Change the Makefile to ensure they are always part of the driver
even when the rest of the kunit tests are in loadable modules.
The tests/xe_kunit_helpers.c file depends on DRM_KUNIT_TEST_HELPERS,
so this has to always be selected by the main XE module now, rather
than the actual tests. In turn, the "depends on (m || (y && KUNIT=y))"
doesn't really do what it tried and can just be removed.
Fixes: 5095d13d758b ("drm/xe/kunit: Define helper functions to allocate fake xe
device")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/xe/Kconfig | 3 ++-
drivers/gpu/drm/xe/Kconfig.debug | 1 -
drivers/gpu/drm/xe/Makefile | 6 --
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 6d4428b19a4c..2948650680e1 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config DRM_XE
tristate "Intel Xe Graphics"
- depends on DRM && PCI && MMU && (m || (y && KUNIT=y))
+ depends on DRM && PCI && MMU
depends on ACPI_VIDEO || !ACPI
select INTERVAL_TREE
# we need shmfs for the swappable backing store, and in particular
@@ -11,6 +11,7 @@ config DRM_XE
select DRM_BUDDY
select DRM_EXEC
select DRM_KMS_HELPER
+ select DRM_KUNIT_TEST_HELPERS if DRM_XE_KUNIT_TEST != n
select DRM_PANEL
select DRM_SUBALLOC_HELPER
select DRM_DISPLAY_DP_HELPER
diff --git a/drivers/gpu/drm/xe/Kconfig.debug b/drivers/gpu/drm/xe/Kconfig.debug
index 549065f57a78..df02e5d17d26 100644
--- a/drivers/gpu/drm/xe/Kconfig.debug
+++ b/drivers/gpu/drm/xe/Kconfig.debug
@@ -76,7 +76,6 @@ config DRM_XE_KUNIT_TEST
depends on DRM_XE && KUNIT && DEBUG_FS
default KUNIT_ALL_TESTS
select DRM_EXPORT_FOR_TESTS if m
- select DRM_KUNIT_TEST_HELPERS
help
Choose this option to allow the driver to perform selftests under
the kunit framework
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 4c6ffe4b2172..b596e4482a9b 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -158,8 +158,10 @@ xe-$(CONFIG_PCI_IOV) += \
xe_lmtt_2l.o \
xe_lmtt_ml.o
-xe-$(CONFIG_DRM_XE_KUNIT_TEST) += \
- tests/xe_kunit_helpers.o
+# include helpers for tests even when XE is built-in
+ifdef CONFIG_DRM_XE_KUNIT_TEST
+xe-y += tests/xe_kunit_helpers.o
+endif
# i915 Display compat #defines and #includes
subdir-ccflags-$(CONFIG_DRM_XE_DISPLAY) += \
--
2.39.2
Re: [PATCH] drm/tests/drm_buddy: avoid 64-bit calculation
On Mon, Feb 19, 2024, at 12:22, Christian König wrote:
> Am 17.02.24 um 02:31 schrieb Randy Dunlap:
>> On 2/16/24 12:24, Arnd Bergmann wrote:
>>> From: Arnd Bergmann
>>>
>>> The newly added drm_test_buddy_alloc_contiguous() test fails to link on
>>> 32-bit targets because of inadvertent 64-bit calculations:
>>>
>>> ERROR: modpost: "__aeabi_uldivmod"
>>> [drivers/gpu/drm/tests/drm_buddy_test.ko] undefined!
>>> ERROR: modpost: "__aeabi_ldivmod" [drivers/gpu/drm/tests/drm_buddy_test.ko]
>>> undefined!
>>>
>>> >From what I can tell, the numbers cannot possibly overflow a 32-bit size,
>>> so use different types for these.
>>>
>>> I noticed that the function has another possible flaw in that is mixes
>>> what it calls pages with 4KB units. This is a big confusing at best,
>>> or possibly broken when built on machines with larger pages.
>>>
>>> Fixes: a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test")
>>> Signed-off-by: Arnd Bergmann
>> Tested-by: Randy Dunlap
>
> I've just pushed a similar patch Mathew came up a bit earlier to
> drm-misc-fixes.
>
> Sorry for the noise, I have to catch up on picking up patches for
> misc-fixes and misc-next.
Ok, thanks.
Have you looked at how this code works for larger values of PAGE_SIZE?
Is there any need to change other things or will this work with the
hardcoded 4KB chunks?
Arnd
[PATCH] drm/tests/drm_buddy: avoid 64-bit calculation
From: Arnd Bergmann
The newly added drm_test_buddy_alloc_contiguous() test fails to link on
32-bit targets because of inadvertent 64-bit calculations:
ERROR: modpost: "__aeabi_uldivmod" [drivers/gpu/drm/tests/drm_buddy_test.ko]
undefined!
ERROR: modpost: "__aeabi_ldivmod" [drivers/gpu/drm/tests/drm_buddy_test.ko]
undefined!
>From what I can tell, the numbers cannot possibly overflow a 32-bit size,
so use different types for these.
I noticed that the function has another possible flaw in that is mixes
what it calls pages with 4KB units. This is a big confusing at best,
or possibly broken when built on machines with larger pages.
Fixes: a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/tests/drm_buddy_test.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
b/drivers/gpu/drm/tests/drm_buddy_test.c
index fee6bec757d1..50a5f98cd5bd 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -21,7 +21,8 @@ static inline u64 get_size(int order, u64 chunk_size)
static void drm_test_buddy_alloc_contiguous(struct kunit *test)
{
- u64 mm_size, ps = SZ_4K, i, n_pages, total;
+ u64 mm_size, total;
+ u32 i, ps = SZ_4K, n_pages;
struct drm_buddy_block *block;
struct drm_buddy mm;
LIST_HEAD(left);
@@ -29,7 +30,8 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
*test)
LIST_HEAD(right);
LIST_HEAD(allocated);
- mm_size = 16 * 3 * SZ_4K;
+ n_pages = 16 * 3;
+ mm_size = n_pages * SZ_4K;
KUNIT_EXPECT_FALSE(test, drm_buddy_init(, mm_size, ps));
@@ -42,7 +44,6 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
*test)
*/
i = 0;
- n_pages = mm_size / ps;
do {
struct list_head *list;
int slot = i % 3;
--
2.39.2
Re: [PATCH] drm/xe: avoid function cast warnings
On Wed, Feb 14, 2024, at 11:10, Thomas Hellström wrote: > On Tue, 2024-02-13 at 10:56 +0100, Arnd Bergmann wrote: >> >> +static void xe_range_fence_free(struct xe_range_fence * rfence) > > There's a checkpatch.pl style error above: s/* rfence/*rfence/. I can > fix that up when pushing if it's ok with you. Right, I saw the report. Please fix it up then so I don't have to resubmit. Thanks, Arnd
Re: [PATCH] drm/xe: skip building debugfs code for CONFIG_DEBUG_FS=n
On Tue, Feb 13, 2024, at 15:55, Jani Nikula wrote:
> On Tue, 13 Feb 2024, Arnd Bergmann wrote:
>> From: Arnd Bergmann
>>
>> Some of the debugfs functions are stubbed out in these configurations,
>> so trying to build the .c file with the definition fails:
>>
>> In file included from include/uapi/linux/posix_types.h:5,
>> from drivers/gpu/drm/i915/display/intel_pipe_crc.c:27:
>> drivers/gpu/drm/i915/display/intel_pipe_crc.c: At top level:
>> include/linux/stddef.h:8:16: error: expected identifier or '(' before 'void'
>> 8 | #define NULL ((void *)0)
>> |^~~~
>> drivers/gpu/drm/i915/display/intel_pipe_crc.c:549:20: note: in expansion of
>> macro 'intel_crtc_get_crc_sources'
>> 549 | const char *const *intel_crtc_get_crc_sources(struct drm_crtc *crtc,
>> |^~
>>
>> Stop trying to build them by making the Makefile entries conditional,
>> same as for the i915 driver.
>
> Already fixed by commit 439987f6f471 ("drm/xe: don't build debugfs files
> when CONFIG_DEBUG_FS=n") in drm-xe-next.
>
> Maybe that needs to be picked up for -fixes?
I made sure that this still happens in linux-next today, but
it does not seem to contain 439987f6f471.
Arnd
[PATCH] drm/xe: skip building debugfs code for CONFIG_DEBUG_FS=n
From: Arnd Bergmann
Some of the debugfs functions are stubbed out in these configurations,
so trying to build the .c file with the definition fails:
In file included from include/uapi/linux/posix_types.h:5,
from drivers/gpu/drm/i915/display/intel_pipe_crc.c:27:
drivers/gpu/drm/i915/display/intel_pipe_crc.c: At top level:
include/linux/stddef.h:8:16: error: expected identifier or '(' before 'void'
8 | #define NULL ((void *)0)
|^~~~
drivers/gpu/drm/i915/display/intel_pipe_crc.c:549:20: note: in expansion of
macro 'intel_crtc_get_crc_sources'
549 | const char *const *intel_crtc_get_crc_sources(struct drm_crtc *crtc,
|^~
Stop trying to build them by making the Makefile entries conditional,
same as for the i915 driver.
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/xe/Makefile | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index efcf0ab7a1a6..7c10ffdb7809 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -213,8 +213,6 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
i915-display/intel_ddi.o \
i915-display/intel_ddi_buf_trans.o \
i915-display/intel_display.o \
- i915-display/intel_display_debugfs.o \
- i915-display/intel_display_debugfs_params.o \
i915-display/intel_display_device.o \
i915-display/intel_display_driver.o \
i915-display/intel_display_irq.o \
@@ -258,7 +256,6 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
i915-display/intel_modeset_setup.o \
i915-display/intel_modeset_verify.o \
i915-display/intel_panel.o \
- i915-display/intel_pipe_crc.o \
i915-display/intel_pmdemand.o \
i915-display/intel_pps.o \
i915-display/intel_psr.o \
@@ -275,6 +272,13 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
i915-display/skl_universal_plane.o \
i915-display/skl_watermark.o
+ifdef CONFIG_DEBUG_FS
+xe-$(CONFIG_DRM_XE_DISPLAY) += \
+ i915-display/intel_display_debugfs.o \
+ i915-display/intel_display_debugfs_params.o \
+ i915-display/intel_pipe_crc.o
+endif
+
ifeq ($(CONFIG_ACPI),y)
xe-$(CONFIG_DRM_XE_DISPLAY) += \
i915-display/intel_acpi.o \
--
2.39.2
[PATCH] nouveau: fix function cast warnings
From: Arnd Bergmann
clang-16 warns about casting between incompatible function types:
drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c:161:10: error: cast from
'void (*)(const struct firmware *)' to 'void (*)(void *)' converts to
incompatible function type [-Werror,-Wcast-function-type-strict]
161 | .fini = (void(*)(void *))release_firmware,
This one was done to use the generic shadow_fw_release() function as a
callback for struct nvbios_source. Change it to use the same prototype
as the other five instances, with a trivial helper function that actually
calls release_firmware.
Fixes: 70c0f263cc2e ("drm/nouveau/bios: pull in basic vbios subdev, more to
come later")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
index 19188683c8fc..8c2bf1c16f2a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadow.c
@@ -154,11 +154,17 @@ shadow_fw_init(struct nvkm_bios *bios, const char *name)
return (void *)fw;
}
+static void
+shadow_fw_release(void *fw)
+{
+ release_firmware(fw);
+}
+
static const struct nvbios_source
shadow_fw = {
.name = "firmware",
.init = shadow_fw_init,
- .fini = (void(*)(void *))release_firmware,
+ .fini = shadow_fw_release,
.read = shadow_fw_read,
.rw = false,
};
--
2.39.2
[PATCH] drm/xe: avoid function cast warnings
From: Arnd Bergmann
clang-16 warns about a cast between incompatible function types:
drivers/gpu/drm/xe/xe_range_fence.c:155:10: error: cast from 'void (*)(const
void *)' to 'void (*)(struct xe_range_fence *)' converts to incompatible
function type [-Werror,-Wcast-function-type-strict]
155 | .free = (void (*)(struct xe_range_fence *rfence)) kfree,
| ^~~
Avoid this with a trivial helper function that calls kfree() here.
Fixes: 845f64bdbfc9 ("drm/xe: Introduce a range-fence utility")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/xe/xe_range_fence.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_range_fence.c
b/drivers/gpu/drm/xe/xe_range_fence.c
index d35d9ec58e86..8510be4466eb 100644
--- a/drivers/gpu/drm/xe/xe_range_fence.c
+++ b/drivers/gpu/drm/xe/xe_range_fence.c
@@ -151,6 +151,11 @@ xe_range_fence_tree_next(struct xe_range_fence *rfence,
u64 start, u64 last)
return xe_range_fence_tree_iter_next(rfence, start, last);
}
+static void xe_range_fence_free(struct xe_range_fence * rfence)
+{
+ kfree(rfence);
+}
+
const struct xe_range_fence_ops xe_range_fence_kfree_ops = {
- .free = (void (*)(struct xe_range_fence *rfence)) kfree,
+ .free = xe_range_fence_free,
};
--
2.39.2
Re: [PATCH] backlight: ktd2801: fix LED dependency
On Mon, Feb 12, 2024, at 15:31, Duje Mihanović wrote: > On Monday, February 12, 2024 1:44:28 PM CET Daniel Thompson wrote: >> On Mon, Feb 12, 2024 at 12:18:12PM +0100, Arnd Bergmann wrote: > I believe this would be the best thing to do here. Making LEDS_EXPRESSWIRE > user selectable doesn't make much sense to me as the library is rather low- > level (a quick grep turns up BTREE as an example of something similar) and > IMO > the GPIOLIB dependency should be handled by LEDS_EXPRESSWIRE as it's the one > actually using the GPIO interface (except maybe for KTD2692 as it has some > extra GPIOs not present in the other one and thus handles them itself). Agree, let's do it this way. Maybe the leds-expresswire.c file should not be in drivers/leds either, but it's already there and I can't think of a better place for it.so just adapting Kconfig should be enough. Please add the corresponding Makefile change as well though: --- a/drivers/Makefile +++ b/drivers/Makefile @@ -135,7 +135,7 @@ obj-$(CONFIG_CPU_IDLE) += cpuidle/ obj-y += mmc/ obj-y += ufs/ obj-$(CONFIG_MEMSTICK) += memstick/ -obj-$(CONFIG_NEW_LEDS) += leds/ +obj-y += leds/ obj-$(CONFIG_INFINIBAND) += infiniband/ obj-y += firmware/ obj-$(CONFIG_CRYPTO) += crypto/ Without this, the expresswire library module won't get built unless NEW_LEDS is enabled. Arnd
[PATCH] nouveau/svm: fix kvcalloc() argument order
From: Arnd Bergmann
The conversion to kvcalloc() mixed up the object size and count
arguments, causing a warning:
drivers/gpu/drm/nouveau/nouveau_svm.c: In function
'nouveau_svm_fault_buffer_ctor':
drivers/gpu/drm/nouveau/nouveau_svm.c:1010:40: error: 'kvcalloc' sizes
specified with 'sizeof' in the earlier argument and not in the later argument
[-Werror=calloc-transposed-args]
1010 | buffer->fault = kvcalloc(sizeof(*buffer->fault),
buffer->entries, GFP_KERNEL);
|^
drivers/gpu/drm/nouveau/nouveau_svm.c:1010:40: note: earlier argument should
specify number of elements, later size of each element
The behavior is still correct aside from the warning, but fixing it avoids
the warnings and can help the compiler track the individual objects better.
Fixes: 71e4bbca070e ("nouveau/svm: Use kvcalloc() instead of kvzalloc()")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 4d1008915499..b4da82ddbb6b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -1007,7 +1007,7 @@ nouveau_svm_fault_buffer_ctor(struct nouveau_svm *svm,
s32 oclass, int id)
if (ret)
return ret;
- buffer->fault = kvcalloc(sizeof(*buffer->fault), buffer->entries,
GFP_KERNEL);
+ buffer->fault = kvcalloc(buffer->entries, sizeof(*buffer->fault),
GFP_KERNEL);
if (!buffer->fault)
return -ENOMEM;
--
2.39.2
[PATCH] backlight: ktd2801: fix LED dependency
From: Arnd Bergmann
The new backlight driver unconditionally selects LEDS_EXPRESSWIRE, which
is in a different subsystem that may be disabled here:
WARNING: unmet direct dependencies detected for LEDS_EXPRESSWIRE
Depends on [n]: NEW_LEDS [=n] && GPIOLIB [=y]
Selected by [y]:
- BACKLIGHT_KTD2801 [=y] && HAS_IOMEM [=y] && BACKLIGHT_CLASS_DEVICE [=y]
Change the select to depends, to ensure the indirect dependency is
met as well even when LED support is disabled.
Fixes: 66c76c1cd984 ("backlight: Add Kinetic KTD2801 backlight support")
Signed-off-by: Arnd Bergmann
---
drivers/video/backlight/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 230bca07b09d..f83f9ef037fc 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -185,7 +185,7 @@ config BACKLIGHT_KTD253
config BACKLIGHT_KTD2801
tristate "Backlight Driver for Kinetic KTD2801"
- select LEDS_EXPRESSWIRE
+ depends on LEDS_EXPRESSWIRE
help
Say Y to enable the backlight driver for the Kinetic KTD2801 1-wire
GPIO-controlled backlight found in Samsung Galaxy Core Prime VE LTE.
--
2.39.2
Re: [PATCH] ARM: multi_v7_defconfig: Enable BACKLIGHT_CLASS_DEVICE
On Fri, Feb 2, 2024, at 11:07, Geert Uytterhoeven wrote:
> On Fri, Feb 2, 2024 at 10:51 AM Marek Szyprowski
> wrote:
>> core, because the DRM core is set to be compiled-in in this defconfig.
>> This leaves all DRM display panels without integrated backlight control,
>> even if the needed modules have been properly loaded and probed.
>
> Hmm, that's bad.
>
> Is there any way to fix this in DRM?
> A quick grep shows that DRM is using the full monty of
> IS_{BUILTIN,ENABLED,MODULE,REACHABLE}(CONFIG_BACKLIGHT_CLASS_DEVICE).
> Probably not all of them are in perfect sync?
The IS_REACHABLE() ones are almost certainly bugs, as are the
'select BACKLIGHT_CLASS_DEVICE' ones we have in drivers/gpu.
> Several DRM drivers do select BACKLIGHT_CLASS_DEVICE, but if that
> does not work in the modular case, it should be fixed.
The select should do the right thing in principle, but mixing
it with depends is what causes circular dependencies. Unfortunately
trying to fix it likely also causes those, but I think it's worth
revisiting.
It should be possible to change it like this:
- change all DRM drivers that require the class to 'depends on
BACKLIGHT_CLASS_DEVICE'
- change all those drivers that can optionally use it to
'depends on BACKLIGHT_CLASS_DEVICE || !BACKLIGHT_CLASS_DEVICE'
to avoid the dependency on a loadable module
- Make BACKLIGHT_CLASS_DEVICE itself default to 'DRM' in order
to avoid regressions in defconfig files but still make it
possible to turn it off.
>> Fix this by selecting BACKLIGHT_CLASS_DEVICE to be compiled-in in
>> multi_v7_defconfig.
>>
>> Signed-off-by: Marek Szyprowski
>
> Sounds like a good interim solution.
>
> Acked-by: Geert Uytterhoeven
Thanks, I've applied it to the soc/defconfig branch now.
Arnd
Re: Build regressions/improvements in v6.8-rc1
On Tue, Jan 23, 2024, at 12:45, Geert Uytterhoeven wrote:
>> 68 error regressions:
>
>> + /kisskb/src/arch/powerpc/sysdev/udbg_memcons.c: error: no previous
>> prototype for 'memcons_getc' [-Werror=missing-prototypes]: => 80:5
>> + /kisskb/src/arch/powerpc/sysdev/udbg_memcons.c: error: no previous
>> prototype for 'memcons_getc_poll' [-Werror=missing-prototypes]: => 57:5
>> + /kisskb/src/arch/powerpc/sysdev/udbg_memcons.c: error: no previous
>> prototype for 'memcons_putc' [-Werror=missing-prototypes]: => 44:6
>
> powerpc-gcc{5,12,13}/ppc64_book3e_allmodconfig
I now sent patches for powerpc booke warnings
>> + /kisskb/src/arch/sh/kernel/cpu/init.c: error: no previous prototype for
>> 'l2_cache_init' [-Werror=missing-prototypes]: => 99:29
>
> sh4-gcc1[123]/se7{619,750}_defconfig
> sh4-gcc1[123]/sh-{all{mod,no,yes},def}config
> sh4-gcc11/sh-allnoconfig
I assume the sh maintainers will eventually get to that
>> + /kisskb/src/arch/sparc/include/asm/floppy_64.h: error: no previous
>> prototype for 'sparc_floppy_irq' [-Werror=missing-prototypes]: => 200:13
>> + /kisskb/src/arch/sparc/include/asm/floppy_64.h: error: no previous
>> prototype for 'sun_pci_fd_dma_callback' [-Werror=missing-prototypes]: =>
>> 437:6
>
> sparc64-gcc{5,11,12,13}/sparc64-allmodconfig
Andrew Morton did a patch for the sparc warnings, and Andreas Larsson
is joining as a maintainer, so hopefully he can pick that up soon.
> sparc64-gcc{5,1[123]}/sparc64-allmodconfig
>
>> + /kisskb/src/arch/sparc/vdso/vclock_gettime.c: error: no previous
>> prototype for '__vdso_clock_gettime' [-Werror=missing-prototypes]: => 254:1
>> + /kisskb/src/arch/sparc/vdso/vclock_gettime.c: error: no previous
>> prototype for '__vdso_clock_gettime_stick' [-Werror=missing-prototypes]: =>
>> 282:1
>> + /kisskb/src/arch/sparc/vdso/vclock_gettime.c: error: no previous
>> prototype
There are prototypes in include/vdso/gettime.h that should be
used here, but unfortunately the sparc implementation does
not match the prototypes because sparc is missing the gettime64
support.
> sparc64-gcc{5,12,13}/sparc64-{allno,def}config
> sparc64-gcc11/sparc64-{all{mod,no},def}config
>
>> + /kisskb/src/arch/x86/um/shared/sysdep/kernel-offsets.h: error: no
>> previous prototype for ‘foo’ [-Werror=missing-prototypes]: => 9:6
>
> um-x86_64-gcc12/um-{all{mod,yes},def}config
I made a patch for arch/um yesterday.
> sparc64-gcc1[12]/sparc64-allmodconfig
>
>> + /kisskb/src/drivers/scsi/mpi3mr/mpi3mr_transport.c: error: the frame size
>> of 1680 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]: =>
>> 1818:1
I sent a patch in November when the regression started, missed
the reply about needing another change
https://lore.kernel.org/all/cafdvvoxh4uqjww4124e2ttutgknzkhopxvsfoqglfov_dka...@mail.gmail.com/
>> + {standard input}: Error: displacement to undefined symbol .L105 overflows
>> 8-bit field : => 590, 593
>> + {standard input}: Error: displacement to undefined symbol .L135 overflows
>> 8-bit field : => 603
>> + {standard input}: Error: displacement to undefined symbol .L140 overflows
>> 8-bit field : => 606
>> + {standard input}: Error: displacement to undefined symbol .L76 overflows
>> 12-bit field: => 591, 594
>> + {standard input}: Error: displacement to undefined symbol .L77 overflows
>> 8-bit field : 607 => 607, 582, 585
>> + {standard input}: Error: displacement to undefined symbol .L97 overflows
>> 12-bit field: => 607
>> + {standard input}: Error: pcrel too far: 604, 590, 577, 593, 572, 569,
>> 598, 599, 596, 610 => 610, 574, 599, 569, 598, 596, 601, 590, 604, 595, 572,
>> 577, 593
>
> SH ICE crickets
Linus did a patch for the syscall, and I sent another one for
arch/sh to prevent this from happening again:
https://lore.kernel.org/all/CAHk-=wjh6cypo8wc-mcxgszcaou3uxccxb+7pvesugr8ajc...@mail.gmail.com/
https://lore.kernel.org/all/07d8877b-d933-46f4-8ca4-c10ed602f...@app.fastmail.com/
Resent mine now.
Arnd
Re: [PATCH] drm: apple: mark local functions static
On Mon, Jan 22, 2024, at 21:50, Janne Grunau wrote:
> On Wed, Jan 17, 2024, at 11:44, Arnd Bergmann wrote:
>>
>> -int parse_sample_rate_bit(struct dcp_parse_ctx *handle, unsigned int
>> *ratebit)
>> +static int parse_sample_rate_bit(struct dcp_parse_ctx *handle,
>> unsigned int *ratebit)
>> {
>> s64 rate;
>> int ret = parse_int(handle, );
>> @@ -715,7 +715,7 @@ int parse_sample_rate_bit(struct dcp_parse_ctx
>> *handle, unsigned int *ratebit)
>> return 0;
>> }
>>
>> -int parse_sample_fmtbit(struct dcp_parse_ctx *handle, u64 *fmtbit)
>> +static int parse_sample_fmtbit(struct dcp_parse_ctx *handle, u64 *fmtbit)
>> {
>> s64 sample_size;
>> int ret = parse_int(handle, _size);
>
> thanks, patch included in my dev branch and will be in the next pull
> request I'll send to Hector.
>
> I suppose the recipients are generated by an automated
> get_maintainers.pl invocation. Is that desired for out of tree drivers?
I was wondering about that as well, as I don't usually send
patches for code that isn't at least in linux-next yet.
I ended up using what is in the MAINTAINERS file for this driver
in the branch as that is is all I have at this point:
APPLE DRM DISPLAY DRIVER
M: Alyssa Rosenzweig
L: dri-devel@lists.freedesktop.org
S: Maintained
T: git git://anongit.freedesktop.org/drm/drm-misc
F: drivers/gpu/drm/apple/
I left out the drivers/gpu/ maintainer addresses though.
Arnd
[PATCH] drm: apple: use strscpy() in place of strlcpy()
From: Arnd Bergmann
Since commit d26270061ae6 ("string: Remove strlcpy()"), the strlcpy()
function causes a build failure.
Since the return value is ignored, changing it to the strscpy()
causes no change in behavior but fixes the build failure.
Fixes: f237c83e4302 ("drm: apple: DCP AFK/EPIC support")
Signed-off-by: Arnd Bergmann
---
The apple drm driver is not in mainline linux yet, this patch
is against https://github.com/AsahiLinux/linux/tree/bits/200-dcp
---
drivers/gpu/drm/apple/afk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/apple/afk.c b/drivers/gpu/drm/apple/afk.c
index 99d579d5ce47..9fbcd18878e8 100644
--- a/drivers/gpu/drm/apple/afk.c
+++ b/drivers/gpu/drm/apple/afk.c
@@ -236,7 +236,7 @@ static void afk_recv_handle_init(struct apple_dcp_afkep
*ep, u32 channel,
return;
}
- strlcpy(name, payload, sizeof(name));
+ strscpy(name, payload, sizeof(name));
/*
* in DCP firmware 13.2 DCP reports interface-name as name which starts
--
2.39.2
[PATCH] drm: apple: mark local functions static
From: Arnd Bergmann
With linux-6.8, the kernel warns about functions that have no
extern declaration, so mark both of these static.
Fixes: 2d782b0d007d ("gpu: drm: apple: Add sound mode parsing")
Signed-off-by: Arnd Bergmann
---
This is for the bits/200-dcp branch in https://github.com/AsahiLinux/linux,
the code is not yet upstream.
---
drivers/gpu/drm/apple/parser.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/apple/parser.c b/drivers/gpu/drm/apple/parser.c
index 44aad9a64f9a..ea9f40bb7de2 100644
--- a/drivers/gpu/drm/apple/parser.c
+++ b/drivers/gpu/drm/apple/parser.c
@@ -694,7 +694,7 @@ int parse_epic_service_init(struct dcp_parse_ctx *handle,
const char **name,
return ret;
}
-int parse_sample_rate_bit(struct dcp_parse_ctx *handle, unsigned int *ratebit)
+static int parse_sample_rate_bit(struct dcp_parse_ctx *handle, unsigned int
*ratebit)
{
s64 rate;
int ret = parse_int(handle, );
@@ -715,7 +715,7 @@ int parse_sample_rate_bit(struct dcp_parse_ctx *handle,
unsigned int *ratebit)
return 0;
}
-int parse_sample_fmtbit(struct dcp_parse_ctx *handle, u64 *fmtbit)
+static int parse_sample_fmtbit(struct dcp_parse_ctx *handle, u64 *fmtbit)
{
s64 sample_size;
int ret = parse_int(handle, _size);
--
2.39.2
Re: [PATCH 08/22] [v2] arch: consolidate arch_irq_work_raise prototypes
On Wed, Jan 10, 2024, at 10:03, Geert Uytterhoeven wrote: > On Wed, Nov 8, 2023 at 2:01 PM Arnd Bergmann wrote: >> From: Arnd Bergmann >> >> The prototype was hidden in an #ifdef on x86, which causes a warning: >> >> kernel/irq_work.c:72:13: error: no previous prototype for >> 'arch_irq_work_raise' [-Werror=missing-prototypes] > > This issue is now present upstream. > >> Some architectures have a working prototype, while others don't. >> Fix this by providing it in only one place that is always visible. >> >> Acked-by: Catalin Marinas >> Acked-by: Palmer Dabbelt >> Acked-by: Guo Ren >> Reviewed-by: Alexander Gordeev >> Signed-off-by: Arnd Bergmann > > Tested-by: Geert Uytterhoeven I've sent out the asm-generic pull request now, that contains the fix. Thanks for the reminder. Arnd
[PATCH] drm/xe: circumvent bogus stringop-overflow warning
From: Arnd Bergmann
gcc-13 warns about an array overflow that it sees but that is
prevented by the "asid % NUM_PF_QUEUE" calculation:
drivers/gpu/drm/xe/xe_gt_pagefault.c: In function 'xe_guc_pagefault_handler':
include/linux/fortify-string.h:57:33: error: writing 16 bytes into a region of
size 0 [-Werror=stringop-overflow=]
include/linux/fortify-string.h:689:26: note: in expansion of macro
'__fortify_memcpy_chk'
689 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s,
\
| ^~~~
drivers/gpu/drm/xe/xe_gt_pagefault.c:341:17: note: in expansion of macro
'memcpy'
341 | memcpy(pf_queue->data + pf_queue->tail, msg, len *
sizeof(u32));
| ^~
drivers/gpu/drm/xe/xe_gt_types.h:102:25: note: at offset [1144, 265324] into
destination object 'tile' of size 8
I found that rewriting the assignment using pointer addition rather than the
equivalent array index calculation prevents the warning, so use that instead.
I sent a bug report against gcc for the false positive warning.
Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113214
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/xe/xe_gt_pagefault.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c
b/drivers/gpu/drm/xe/xe_gt_pagefault.c
index 59a70d2e0a7a..78dc08cc2bfe 100644
--- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
@@ -332,7 +332,7 @@ int xe_guc_pagefault_handler(struct xe_guc *guc, u32 *msg,
u32 len)
return -EPROTO;
asid = FIELD_GET(PFD_ASID, msg[1]);
- pf_queue = >usm.pf_queue[asid % NUM_PF_QUEUE];
+ pf_queue = gt->usm.pf_queue + (asid % NUM_PF_QUEUE);
spin_lock_irqsave(_queue->lock, flags);
full = pf_queue_full(pf_queue);
--
2.39.2
Re: [PATCH 00/27] sparc32: sunset sun4m and sun4d
On Wed, Dec 20, 2023, at 09:54, John Paul Adrian Glaubitz wrote: > On Wed, 2023-12-20 at 08:36 +0000, Arnd Bergmann wrote: >> All of these were found through inspection rather than testing, >> so there is a good chance that other fatal kernel bugs prevent >> testing in qemu, at least until the fixes from Andreas' tree >> are included. > > Andreas has fixes for these issues? Not sure, all I know is that - Andreas has some fixes for Leon in his tree - Sam is unable to boot mainline in qemu - There is an unknown set of bugs in sparc32 since it has not been tested for many years without Andreas' patches it appears that the qemu developers are still testing the sun4m model against old Linux and Solaris installations [1], but failure to run the leon3 model could still be any combination of kernel, qemu or configuration problems. Arnd [1] https://wiki.qemu.org/Documentation/Platforms/SPARC#Compatibility
Re: [PATCH 00/27] sparc32: sunset sun4m and sun4d
On Wed, Dec 20, 2023, at 09:34, David Laight wrote: > From: Arnd Bergmann >> Sent: 20 December 2023 08:37 >> >> On Tue, Dec 19, 2023, at 22:03, Sam Ravnborg via B4 Relay wrote: >> > TODO before this can be applied: >> > - Ack from davem - as he is the principal sparc maintainer >> > - Tested-by: preferably on a target or QEMU (see above) >> > I expect bugs as there are some involved changes! >> > >> > Ideas for the future >> > - Apply the most relevant downstream Gaisler patches >> > - The ones introducing CAS should have preference as we then >> > can drop the cmpxchg emulation >> >> One note about the CAS -- as far as I can tell, the absence >> of the futex() syscall on sparc32 kernels means that no glibc >> from the past decade can work correctly as it now requires futex >> for its internal locking, though it does work on sparc64 kernels >> in compat32 mode as well as the LEON3 kernel that adds futex >> support. > > Does the glibc mutex 'fast path' also require a CAS instruction? I think that depends on whether glibc is built for a CPU with CAS or not. If it's built for 32-bit sparcv9 or leon, it should use CAS and crash on sparcv8 without CAS. If it's built for pure sparcv8, it should try to use an emulation that is incompatible with the kernel futex syscall. > Presumably having CAS also removes the 'really horrid (tm)' code > required to make all the bitmap operations atomic? Yes, but I'm not sure this is implemented in the leon3 tree. With CAS enabled, at least asm/atomic.h, asm/bitops.h, asm/cmpxchg.h and asm/spinlock.h can be implemented as efficiently as they are in the 64-bit version. > Seems a shame to lose old sparc32 support when (I think) the sun3 > in my cupboard would still work - if only it had a working psu. > (The 110/220V switch wasn't connected and the FET wasn't rated > for 450V. UK mains can be 240+10% if you are near a substation.) sun3 support has never worked upstream. There is an old series from 20 years ago that made it work but nobody ever tried to get it merged. Arnd
Re: [PATCH 01/27] sparc32: Update defconfig to LEON SMP
On Wed, Dec 20, 2023, at 06:43, Sam Ravnborg wrote: > On Tue, Dec 19, 2023 at 10:23:05PM +0000, Arnd Bergmann wrote: >> On Tue, Dec 19, 2023, at 22:03, Sam Ravnborg via B4 Relay wrote: >> > From: Sam Ravnborg >> > >> > This is a copy of the leon_smp defconfig found in >> > gaisler-buildroot-2023.02-1.0. >> > >> > Signed-off-by: Sam Ravnborg >> > Cc: "David S. Miller" >> > Cc: Arnd Bergmann >> > Cc: Andreas Larsson >> >> I did not get a cover letter for the series, but I looked at > You are listed as a receiver?!? Found it now, I was just blind. Arnd
Re: [PATCH 00/27] sparc32: sunset sun4m and sun4d
On Tue, Dec 19, 2023, at 22:03, Sam Ravnborg via B4 Relay wrote: > TODO before this can be applied: > - Ack from davem - as he is the principal sparc maintainer > - Tested-by: preferably on a target or QEMU (see above) > I expect bugs as there are some involved changes! > > Ideas for the future > - Apply the most relevant downstream Gaisler patches > - The ones introducing CAS should have preference as we then > can drop the cmpxchg emulation One note about the CAS -- as far as I can tell, the absence of the futex() syscall on sparc32 kernels means that no glibc from the past decade can work correctly as it now requires futex for its internal locking, though it does work on sparc64 kernels in compat32 mode as well as the LEON3 kernel that adds futex support. There have been a number of other bugs in sun4m/sun4d support that ended up making the system unusable during the same timeframe, I remember sysvipc being broken in native 32-bit mode (but again not in compat mode) and I think Al Viro has a list of things that he fixed over the years. All of these were found through inspection rather than testing, so there is a good chance that other fatal kernel bugs prevent testing in qemu, at least until the fixes from Andreas' tree are included. Arnd
Re: [PATCH 01/27] sparc32: Update defconfig to LEON SMP
On Tue, Dec 19, 2023, at 22:03, Sam Ravnborg via B4 Relay wrote: > From: Sam Ravnborg > > This is a copy of the leon_smp defconfig found in > gaisler-buildroot-2023.02-1.0. > > Signed-off-by: Sam Ravnborg > Cc: "David S. Miller" > Cc: Arnd Bergmann > Cc: Andreas Larsson I did not get a cover letter for the series, but I looked at all 27 patches and they all look good to me, nice cleanup! Acked-by: Arnd Bergmann
[PATCH] drm/exynos: fix accidental on-stack copy of exynos_drm_plane
From: Arnd Bergmann
gcc rightfully complains about excessive stack usage in the
fimd_win_set_pixfmt()
function:
drivers/gpu/drm/exynos/exynos_drm_fimd.c: In function 'fimd_win_set_pixfmt':
drivers/gpu/drm/exynos/exynos_drm_fimd.c:750:1: error: the frame size of 1032
bytes is larger than 1024 byte
drivers/gpu/drm/exynos/exynos5433_drm_decon.c: In function
'decon_win_set_pixfmt':
drivers/gpu/drm/exynos/exynos5433_drm_decon.c:381:1: error: the frame size of
1032 bytes is larger than 1024 bytes
There is really no reason to copy the large exynos_drm_plane
structure to the stack before using one of its members, so just
use a pointer instead.
Fixes: 6f8ee5c21722 ("drm/exynos: fimd: Make plane alpha configurable")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 4 ++--
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 4d986077738b..bce027552474 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -319,9 +319,9 @@ static void decon_win_set_bldmod(struct decon_context *ctx,
unsigned int win,
static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win,
struct drm_framebuffer *fb)
{
- struct exynos_drm_plane plane = ctx->planes[win];
+ struct exynos_drm_plane *plane = >planes[win];
struct exynos_drm_plane_state *state =
- to_exynos_plane_state(plane.base.state);
+ to_exynos_plane_state(plane->base.state);
unsigned int alpha = state->base.alpha;
unsigned int pixel_alpha;
unsigned long val;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 8dde7b1e9b35..5bdc246f5fad 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -661,9 +661,9 @@ static void fimd_win_set_bldmod(struct fimd_context *ctx,
unsigned int win,
static void fimd_win_set_pixfmt(struct fimd_context *ctx, unsigned int win,
struct drm_framebuffer *fb, int width)
{
- struct exynos_drm_plane plane = ctx->planes[win];
+ struct exynos_drm_plane *plane = >planes[win];
struct exynos_drm_plane_state *state =
- to_exynos_plane_state(plane.base.state);
+ to_exynos_plane_state(plane->base.state);
uint32_t pixel_format = fb->format->format;
unsigned int alpha = state->base.alpha;
u32 val = WINCONx_ENWIN;
--
2.39.2
Re: [PATCH 3/3] drm/amd/display: Support DRM_AMD_DC_FP on RISC-V
On Sat, Dec 9, 2023, at 22:29, Samuel Holland wrote: > On 2023-12-09 2:38 PM, Arnd Bergmann wrote: >> On Fri, Dec 8, 2023, at 06:04, Samuel Holland wrote: >>> On 2023-11-29 6:42 PM, Nathan Chancellor wrote: >>>> >>>> https://lore.kernel.org/20231019205117.GA839902@dev-arch.thelio-3990X/ >>> >>> I also see one of these with clang 17 even with KASAN disabled: >>> >>> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:37:6: >>> warning: stack frame size (2208) exceeds limit (2048) in 'dml32_recalculate' >>> [-Wframe-larger-than] >>> void dml32_recalculate(struct display_mode_lib *mode_lib) >>> >>> ^ >>> 1532/2208 (69.38%) spills, 676/2208 (30.62%) variables >>> >>> So I'm in favor of just raising the limit for these files for clang, like >>> you >>> suggested in the linked thread. >> >> How about just adding a BUG_ON(IS_ENABLED(CONFIG_RISCV)) >> in that function? That should also avoid the build failure >> but give a better indication of where the problem is >> if someone actually runs into that function and triggers >> a runtime stack overflow. > > Won't that break actual users of the driver, trading an unlikely but > theoretically possible stack overflow for a guaranteed crash? The intent of > this > series is that I have one of these GPUs plugged in to a RISC-V board, and I > want > to use it. Ah, I thought you just wanted to get it to compile cleanly so you could use some of the more common cards. If you are trying to run the dcn32 code specifically, then you should definitely fix the stack usage of that function instead. Arnd
Re: [PATCH 3/3] drm/amd/display: Support DRM_AMD_DC_FP on RISC-V
On Fri, Dec 8, 2023, at 06:04, Samuel Holland wrote:
> On 2023-11-29 6:42 PM, Nathan Chancellor wrote:
>> On Thu, Nov 23, 2023 at 02:23:01PM +, Conor Dooley wrote:
>>> On Tue, Nov 21, 2023 at 07:05:15PM -0800, Samuel Holland wrote:
RISC-V uses kernel_fpu_begin()/kernel_fpu_end() like several other
architectures. Enabling hardware FP requires overriding the ISA string
for the relevant compilation units.
>>>
>>> Ah yes, bringing the joy of frame-larger-than warnings to RISC-V:
>>> ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:58:13:
>>> warning: stack frame size (2416) exceeds limit (2048) in
>>> 'DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation'
>>> [-Wframe-larger-than]
>>
>> :(
>>
>>> Nathan, have you given up on these being sorted out?
>>
>> Does your configuration have KASAN (I don't think RISC-V supports
>> KCSAN)? It is possible that dml/dcn32 needs something similar to commit
>> 6740ec97bcdb ("drm/amd/display: Increase frame warning limit with KASAN
>> or KCSAN in dml2")?
>>
>> I am not really interested in playing whack-a-mole with these warnings
>> like I have done in the past for the reasons I outlined here:
>>
>> https://lore.kernel.org/20231019205117.GA839902@dev-arch.thelio-3990X/
>
> I also see one of these with clang 17 even with KASAN disabled:
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:37:6:
> warning: stack frame size (2208) exceeds limit (2048) in 'dml32_recalculate'
> [-Wframe-larger-than]
> void dml32_recalculate(struct display_mode_lib *mode_lib)
>
> ^
> 1532/2208 (69.38%) spills, 676/2208 (30.62%) variables
>
> So I'm in favor of just raising the limit for these files for clang, like you
> suggested in the linked thread.
How about just adding a BUG_ON(IS_ENABLED(CONFIG_RISCV))
in that function? That should also avoid the build failure
but give a better indication of where the problem is
if someone actually runs into that function and triggers
a runtime stack overflow.
Arnd
Re: [DO NOT MERGE v5 22/37] dt-bindings: display: smi, sm501: SMI SM501 binding json-schema
On Tue, Dec 5, 2023, at 10:45, Yoshinori Sato wrote: > Define SM501 functions and modes. > > Signed-off-by: Yoshinori Sato > --- > .../bindings/display/smi,sm501.yaml | 134 ++ > include/dt-bindings/display/sm501.h | 25 It looks like we already have a binding at Documentation/devicetree/bindings/display/sm501fb.txt > + little-endian: > +$ref: /schemas/types.yaml#/definitions/flag > +description: available on big endian systems, to set different > foreign endian. > + big-endian: > +$ref: /schemas/types.yaml#/definitions/flag > +description: available on little endian systems, to set different > foreign endian. > + > + swap-fb-endian: > +$ref: /schemas/types.yaml#/definitions/flag > +description: swap framebuffer byteorder. Why do you need both the "swap" and the specific little/big properties? > + crt: > +description: CRT output control > + > + panel: > +description: Panel output control What type are these? > + smi,misc-timing: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: Miscellaneous Timing reg value. > + > + smi,misc-control: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: Miscellaneous Control reg value. > + > + smi,gpio-low: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: GPIO0 to 31 Control reg value. > + > + smi,gpio-high: > +$ref: /schemas/types.yaml#/definitions/uint32 > +description: GPIO32 to 63 Control reg value. Register values should generally not go into DT > + > + smi,gpio-i2c: > +$ref: /schemas/types.yaml#/definitions/uint32-array > +minItems: 5 > +description: | > + GPIO I2C bus number > + 1st field - I2C bus number > + 2nd Field - GPIO SDA > + 3rd Field - GPIO SCL > + 4th Field - Timeout > + 5th Field - udelay Instead of a bus number and other fields, I think this should reference an i2c device. Arnd
Re: [DO NOT MERGE v5 11/37] pci: pci-sh7751: Add SH7751 PCI driver
On Tue, Dec 5, 2023, at 10:45, Yoshinori Sato wrote:
> +#include
> +#include "pci-sh7751.h"
> +
> +#define pcic_writel(val, base, reg) __raw_writel(val, base + (reg))
> +#define pcic_readl(base, reg) __raw_readl(base + (reg))
__raw_writel()/__raw_readl() has a number of problems with
atomicity (the compiler may split or merge pointer
dereferences), barriers and endianess. You should normally
always use readl()/writel() instead.
> + memset(pci_config, 0, sizeof(pci_config));
> + if (of_property_read_u32_array(np, "renesas,config",
> +pci_config, SH7751_NUM_CONFIG) == 0) {
> + for (i = 0; i < SH7751_NUM_CONFIG; i++) {
> + r = pci_config[i * 2];
> + /* CONFIG0 is read-only, so make it a sentinel. */
> + if (r == 0)
> + break;
> + pcic_writel(pci_config[i * 2 + 1], pcic, r * 4);
> + }
> + }
the config property seems a little too specific to this
implementation of the driver. Instead of encoding register
values in DT, I think these should either be described
in named properties where needed, or hardcoded in the driver
if there is only one sensible value.
> +/*
> + * We need to avoid collisions with `mirrored' VGA ports
> + * and other strange ISA hardware, so we always want the
> + * addresses to be allocated in the 0x000-0x0ff region
> + * modulo 0x400.
> + */
> +#define IO_REGION_BASE 0x1000
> +resource_size_t pcibios_align_resource(void *data, const struct
> resource *res,
> + resource_size_t size, resource_size_t align)
> +{
You can't have these generic functions in a driver, as that
prevents you from building more than one such driver.
The logic you have here is neither architecture nor
driver specific.
> +static int sh7751_pci_probe(struct platform_device *pdev)
> +{
> + struct resource *res, *bscres;
> + void __iomem *pcic;
> + void __iomem *bsc;
> + u32 memory[2];
> + u16 vid, did;
> + u32 word;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (IS_ERR(res))
> + return PTR_ERR(res);
> + pcic = (void __iomem *)res->start;
This cast is invalid, as res->start is a physical address
that you would need to ioremap() to turn into an __iomem
pointer.
> + bscres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + bsc = devm_ioremap_resource(>dev, bscres);
> + if (IS_ERR(bsc))
> + return PTR_ERR(bsc);
> +
> + if (of_property_read_u32_array(pdev->dev.of_node,
> +"renesas,memory", memory, 2) < 0) {
> + /*
> + * If no memory range is specified,
> + * the entire main memory will be targeted for DMA.
> + */
> + memory[0] = memory_start;
> + memory[1] = memory_end - memory_start;
> + }
There is a generic "dma-ranges" proerty for describing
which memory is visible by a bus.
> diff --git a/drivers/pci/controller/pci-sh7751.h
> b/drivers/pci/controller/pci-sh7751.h
> new file mode 100644
> index ..540cee7095c6
> --- /dev/null
> +++ b/drivers/pci/controller/pci-sh7751.h
> @@ -0,0 +1,76 @@
If the header is only included from one file, just removed
it and add the register definitions to the driver directly.
Arnd
Re: [DO NOT MERGE v5 06/37] sh: kernel/setup Update DT support.
On Tue, Dec 5, 2023, at 10:45, Yoshinori Sato wrote:
> Fix extrnal fdt initialize and bootargs.
>
> Signed-off-by: Yoshinori Sato
> ---
> arch/sh/kernel/setup.c | 51 --
> 1 file changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c
> index 3d80515298d2..b299abff68e0 100644
> --- a/arch/sh/kernel/setup.c
> +++ b/arch/sh/kernel/setup.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -74,7 +75,13 @@ extern int root_mountflags;
> #define RAMDISK_PROMPT_FLAG 0x8000
> #define RAMDISK_LOAD_FLAG0x4000
>
> +#if defined(CONFIG_OF) && !defined(CONFIG_USE_BUILTIN_DTB)
> +#define CHOSEN_BOOTARGS
> +#endif
> +
> +#ifndef CHOSEN_BOOTARGS
> static char __initdata command_line[COMMAND_LINE_SIZE] = { 0, };
> +#endif
I think an appended DTB is generally better than a built-in
one, as that allows you to still have a single kernel
image across machines and just pick the dtb when installing it.
With everything else being equal, I would suggest not
actually making this an option for new platforms.
Arnd
Re: [DO NOT MERGE v5 10/37] sh: Common PCI Framework driver support.
On Tue, Dec 5, 2023, at 10:45, Yoshinori Sato wrote:
> +
> +#if defined(CONFIG_PCI) && !defined(CONFIG_GENERIC_IOMAP)
> +void pci_iounmap(struct pci_dev *dev, void __iomem *addr)
> +{
> + iounmap(addr);
> +}
> +EXPORT_SYMBOL(pci_iounmap);
This definition does not work for addresses that are
returned by ioport_map(), include pci_iomap() on
IORESOURCE_IO. However, the definition in lib/pci_iomap.c
should work fine, you just need to #define ARCH_WANTS_GENERIC_PCI_IOUNMAP
to get that.
Arnd
Re: [DO NOT MERGE v5 35/37] sh: RTS7751R2D Plus OF defconfig
On Tue, Dec 5, 2023, at 10:45, Yoshinori Sato wrote: > Signed-off-by: Yoshinori Sato > --- > arch/sh/configs/rts7751r2dplus-of_defconfig | 93 + This is very similar to the landisk config, so it may be easier to just have one of them that works for both, as well as future ones. > +CONFIG_LOG_BUF_SHIFT=14 > +CONFIG_NAMESPACES=y > +CONFIG_EXPERT=y You should not normally need to enable CONFIG_EXPERT in a defconfig. Is there any particular reason for this? Arnd
[PATCH] drm/imagination: move update_logtype() into ifdef section
From: Arnd Bergmann
This function is only used when debugfs is enabled, and otherwise
causes a build warning:
drivers/gpu/drm/imagination/pvr_fw_trace.c:135:1: error: 'update_logtype'
defined but not used [-Werror=unused-function]
Move the #ifdef check to include this function as well.
Fixes: cb56cd610866 ("drm/imagination: Add firmware trace to debugfs")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/imagination/pvr_fw_trace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_fw_trace.c
b/drivers/gpu/drm/imagination/pvr_fw_trace.c
index 87a42fb6ace6..8261fa4f7f83 100644
--- a/drivers/gpu/drm/imagination/pvr_fw_trace.c
+++ b/drivers/gpu/drm/imagination/pvr_fw_trace.c
@@ -121,6 +121,8 @@ void pvr_fw_trace_fini(struct pvr_device *pvr_dev)
pvr_fw_object_unmap_and_destroy(fw_trace->tracebuf_ctrl_obj);
}
+#if defined(CONFIG_DEBUG_FS)
+
/**
* update_logtype() - Send KCCB command to trigger FW to update logtype
* @pvr_dev: Target PowerVR device
@@ -165,8 +167,6 @@ update_logtype(struct pvr_device *pvr_dev, u32 group_mask)
return err;
}
-#if defined(CONFIG_DEBUG_FS)
-
static int fw_trace_group_mask_show(struct seq_file *m, void *data)
{
struct pvr_device *pvr_dev = m->private;
--
2.39.2
[PATCH] drm/bridge: tc358768: select CONFIG_VIDEOMODE_HELPERS
From: Arnd Bergmann
A dependency on this feature was recently introduced:
x86_64-linux-ld: vmlinux.o: in function `tc358768_bridge_pre_enable':
tc358768.c:(.text+0xbe3dae): undefined reference to
`drm_display_mode_to_videomode'
Make sure this is always enabled.
Fixes: e5fb21678136 ("drm/bridge: tc358768: Use struct videomode")
Signed-off-by: Arnd Bergmann
---
drivers/gpu/drm/bridge/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index ba82a1142adf..3e6a4e2044c0 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -313,6 +313,7 @@ config DRM_TOSHIBA_TC358768
select REGMAP_I2C
select DRM_PANEL
select DRM_MIPI_DSI
+ select VIDEOMODE_HELPERS
help
Toshiba TC358768AXBG/TC358778XBG DSI bridge chip driver.
--
2.39.2
Re: [PATCH 1/2] drm/imagination: avoid -Wmissing-prototype warnings
On Wed, Nov 29, 2023, at 13:07, Donald Robson wrote: > Hello Arnd, > > Thanks for this. However, I fixed these in a patch a few minutes > before you sent > yours. I'm not sure what normally happens in these circumstances, but > as my patch has > the kernel robot tags and a few additional fixes, I think we should > probably use that > one. Sure, that's fine. If you don't mind rebasing, just add a "Reported-by: Arnd Bergmann " line as well. I tend to create a bug fix for any build regressions I see as part of building randconfig kernels, but it's normal that the mainainers have already fixed the same bug before I send my patch, or that there is a better solution than what I come up with. Arnd
