Re: [PATCH 00/34] address all -Wunused-const warnings
Hello: This series was applied to netdev/net-next.git (main) by Jakub Kicinski : On Wed, 3 Apr 2024 10:06:18 +0200 you wrote: > 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. > > [...] Here is the summary with links: - [05/34] 3c515: remove unused 'mtu' variable https://git.kernel.org/netdev/net-next/c/17b35355c2c6 - [19/34] sunrpc: suppress warnings for unused procfs functions (no matching commit) - [26/34] isdn: kcapi: don't build unused procfs code https://git.kernel.org/netdev/net-next/c/91188544af06 - [28/34] net: xgbe: remove extraneous #ifdef checks https://git.kernel.org/netdev/net-next/c/0ef416e045ad - [33/34] drivers: remove incorrect of_match_ptr/ACPI_PTR annotations (no matching commit) You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH v2] drm/msm/dp: Remove now unused connector_type from desc
On Fri, Apr 05, 2024 at 08:14:11PM -0700, Bjorn Andersson wrote: > Now that the connector_type is dynamically determined, the > connector_type of the struct msm_dp_desc is unused. Clean it up. > > Remaining duplicate entries are squashed. > > Signed-off-by: Bjorn Andersson > --- > This cleans up after, and hence depends on, > https://lore.kernel.org/all/20240324-x1e80100-display-refactor-connector-v4-1-e0ebaea66...@linaro.org/ > --- > Changes in v2: > - Squashed now duplicate entries > - Link to v1: > https://lore.kernel.org/r/20240328-dp-connector-type-cleanup-v1-1-9bf84c5a6...@quicinc.com > --- > drivers/gpu/drm/msm/dp/dp_display.c | 48 > + > 1 file changed, 17 insertions(+), 31 deletions(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 0/6] Add SMEM-based speedbin matching
On Fri, Apr 05, 2024 at 10:41:28AM +0200, Konrad Dybcio wrote: > Newer (SM8550+) SoCs don't seem to have a nice speedbin fuse anymore, > but instead rely on a set of combinations of "feature code" (FC) and > "product code" (PC) identifiers to match the bins. This series adds > support for that. > > I suppose a qcom/for-soc immutable branch would be in order if we want > to land this in the upcoming cycle. > > FWIW I preferred the fuses myself.. > > Signed-off-by: Konrad Dybcio > --- > Konrad Dybcio (5): > soc: qcom: Move some socinfo defines to the header, expand them > soc: qcom: smem: Add pcode/fcode getters > drm/msm/adreno: Implement SMEM-based speed bin > drm/msm/adreno: Add speedbin data for SM8550 / A740 > arm64: dts: qcom: sm8550: Wire up GPU speed bin & more OPPs > > Neil Armstrong (1): > drm/msm/adreno: Allow specifying default speedbin value Generic comment: as you are reworking speed bins implementaiton, could you please take a broader look. A5xx just reads nvmem manually. A6xx uses adreno_read_speedbin(). And then we call adreno_read_speedbin second time from from adreno_gpu_init(). Can we get to the point where the function is called only once for all the platforms which implements speed binning? -- With best wishes Dmitry
Re: [PATCH 5/6] drm/msm/adreno: Add speedbin data for SM8550 / A740
On Fri, Apr 05, 2024 at 10:41:33AM +0200, Konrad Dybcio wrote:
> Add speebin data for A740, as found on SM8550 and derivative SoCs.
>
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 901ef767e491..c976a485aef2 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -570,6 +570,20 @@ static const struct adreno_info gpulist[] = {
> .zapfw = "a740_zap.mdt",
> .hwcg = a740_hwcg,
> .address_space_size = SZ_16G,
> + .speedbins = ADRENO_SPEEDBINS(
I think this deserves either a comment or some info in the commit
message.
> + { ADRENO_SKU_ID(SOCINFO_PC_UNKNOWN, SOCINFO_FC_AC), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PC_UNKNOWN, SOCINFO_FC_AF), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(1), SOCINFO_FC_UNKNOWN), 1
> },
> + { ADRENO_SKU_ID(SOCINFO_PCn(2), SOCINFO_FC_Yn(0x0)), 0
> },
> + { ADRENO_SKU_ID(SOCINFO_PCn(2), SOCINFO_FC_Yn(0x2)), 0
> },
> + { ADRENO_SKU_ID(SOCINFO_PCn(4), SOCINFO_FC_Yn(0x0)), 0
> },
> + { ADRENO_SKU_ID(SOCINFO_PCn(4), SOCINFO_FC_Yn(0x2)), 0
> },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0x0)), 0
> },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0x1)), 0
> },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0xd)), 0
> },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0xe)), 0
> },
> + ),
> + .default_speedbin = 1,
> }, {
> .chip_ids = ADRENO_CHIP_IDS(0x43051401), /* "C520v2" */
> .family = ADRENO_7XX_GEN3,
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
Re: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
On Fri, Apr 05, 2024 at 10:41:32AM +0200, Konrad Dybcio wrote:
> On recent (SM8550+) Snapdragon platforms, the GPU speed bin data is
> abstracted through SMEM, instead of being directly available in a fuse.
>
> Add support for SMEM-based speed binning, which includes getting
> "feature code" and "product code" from said source and parsing them
> to form something that lets us match OPPs against.
>
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 +++---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 2 ++
> drivers/gpu/drm/msm/adreno/adreno_gpu.c| 39
> +++---
> drivers/gpu/drm/msm/adreno/adreno_gpu.h| 12 ++---
> 4 files changed, 51 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 4cbdfabbcee5..6776fd80f7a6 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2890,13 +2890,15 @@ static u32 fuse_to_supp_hw(const struct adreno_info
> *info, u32 fuse)
> return UINT_MAX;
> }
>
> -static int a6xx_set_supported_hw(struct device *dev, const struct
> adreno_info *info)
> +static int a6xx_set_supported_hw(struct adreno_gpu *adreno_gpu,
> + struct device *dev,
> + const struct adreno_info *info)
> {
> u32 supp_hw;
> u32 speedbin;
> int ret;
>
> - ret = adreno_read_speedbin(dev, );
> + ret = adreno_read_speedbin(adreno_gpu, dev, );
> /*
>* -ENOENT means that the platform doesn't support speedbin which is
>* fine
> @@ -3056,7 +3058,7 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
>
> a6xx_llc_slices_init(pdev, a6xx_gpu, is_a7xx);
>
> - ret = a6xx_set_supported_hw(>dev, config->info);
> + ret = a6xx_set_supported_hw(adreno_gpu, >dev, config->info);
> if (ret) {
> a6xx_destroy(&(a6xx_gpu->base.base));
> return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index c3703a51287b..901ef767e491 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -6,6 +6,8 @@
> * Copyright (c) 2014,2017 The Linux Foundation. All rights reserved.
> */
>
> +#include
> +
> #include "adreno_gpu.h"
>
> bool hang_debug = false;
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 074fb498706f..0e4ff532ac3c 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -21,6 +21,9 @@
> #include "msm_gem.h"
> #include "msm_mmu.h"
>
> +#include
> +#include
> +
> static u64 address_space_size = 0;
> MODULE_PARM_DESC(address_space_size, "Override for size of processes private
> GPU address space");
> module_param(address_space_size, ullong, 0600);
> @@ -1057,9 +1060,37 @@ void adreno_gpu_ocmem_cleanup(struct adreno_ocmem
> *adreno_ocmem)
> adreno_ocmem->hdl);
> }
>
> -int adreno_read_speedbin(struct device *dev, u32 *speedbin)
> +int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
> + struct device *dev, u32 *speedbin)
> {
> - return nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
> + u32 fcode, pcode;
> + int ret;
> +
> + /* Try reading the speedbin via a nvmem cell first */
> + ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
> + if (!ret && ret != -EINVAL)
This is always false.
> + return ret;
> +
> + ret = qcom_smem_get_feature_code();
> + if (ret) {
> + dev_err(dev, "Couldn't get feature code from SMEM!\n");
> + return ret;
This brings in QCOM_SMEM dependency (which is not mentioned in the
Kconfig). Please keep iMX5 hardware in mind, so the dependency should be
optional. Respective functions should be stubbed in the header.
> + }
> +
> + ret = qcom_smem_get_product_code();
> + if (ret) {
> + dev_err(dev, "Couldn't get product code from SMEM!\n");
> + return ret;
> + }
> +
> + /* Don't consider fcode for external feature codes */
> + if (fcode <= SOCINFO_FC_EXT_RESERVE)
> + fcode = SOCINFO_FC_UNKNOWN;
> +
> + *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
> + FIELD_PREP(ADRENO_SKU_ID_FCODE, fcode);
What about just asking the qcom_smem for the 'gpu_bin' and hiding gory
details there? It almost feels that handling raw PCODE / FCODE here is
too low-level and a subject to change depending on the socinfo format.
> +
> + return ret;
> }
>
> int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> @@ -1098,9 +1129,9 @@ int adreno_gpu_init(struct drm_device *drm, struct
> platform_device *pdev,
>
[PATCH v3] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD
From: Kuogee Hsieh
For HPD events coming from external modules using drm_bridge_hpd_notify(),
the sequence of calls leading to dp_bridge_hpd_notify() is like below:
dp_bridge_hpd_notify+0x18/0x70 [msm]
drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
drm_helper_probe_detect+0x94/0xc0 [drm_kms_helper]
drm_helper_probe_single_connector_modes+0x43c/0x53c [drm_kms_helper]
drm_client_modeset_probe+0x240/0x1114 [drm]
drm_fb_helper_hotplug_event.part.26+0x9c/0xe8 [drm_kms_helper]
drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
msm_fbdev_client_hotplug+0x24/0xd4 [msm]
drm_client_dev_hotplug+0xd8/0x148 [drm]
drm_kms_helper_connector_hotplug_event+0x30/0x3c [drm_kms_helper]
drm_bridge_connector_handle_hpd+0x84/0x94 [drm_kms_helper]
drm_bridge_connector_hpd_cb+0xc/0x14 [drm_kms_helper]
drm_bridge_hpd_notify+0x38/0x50 [drm]
drm_aux_hpd_bridge_notify+0x14/0x20 [aux_hpd_bridge]
pmic_glink_altmode_worker+0xec/0x27c [pmic_glink_altmode]
process_scheduled_works+0x17c/0x2cc
worker_thread+0x2ac/0x2d0
kthread+0xfc/0x120
There are three notifications delivered to DP driver for each notification
event.
1) From the drm_aux_hpd_bridge_notify() itself as shown above
2) From output_poll_execute() thread which arises due to
drm_helper_probe_single_connector_modes() call of the above stacktrace
as shown in more detail here.
dp_bridge_hpd_notify+0x18/0x70 [msm]
drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
drm_helper_probe_detect+0x94/0xc0 [drm_kms_helper]
drm_helper_probe_single_connector_modes+0x43c/0x53c [drm_kms_helper]
drm_client_modeset_probe+0x240/0x1114 [drm]
drm_fb_helper_hotplug_event.part.26+0x9c/0xe8 [drm_kms_helper]
drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
msm_fbdev_client_hotplug+0x24/0xd4 [msm]
drm_client_dev_hotplug+0xd8/0x148 [drm]
drm_kms_helper_hotplug_event+0x30/0x3c [drm_kms_helper]
output_poll_execute+0xe0/0x210 [drm_kms_helper]
3) From the DP driver as the dp_bridge_hpd_notify() callback today triggers
the hpd_event_thread for connect and disconnect events respectively via below
stack
dp_bridge_hpd_notify+0x18/0x70 [msm]
drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
drm_helper_probe_detect_ctx+0x98/0x110 [drm_kms_helper]
check_connector_changed+0x4c/0x20c [drm_kms_helper]
drm_helper_hpd_irq_event+0x98/0x120 [drm_kms_helper]
hpd_event_thread+0x478/0x5bc [msm]
dp_bridge_hpd_notify() delivered from output_poll_execute() thread
returns the incorrect HPD status as the MSM DP driver returns the value
of link_ready and not the HPD status currently in the .detect() callback.
And because the HPD event thread has not run yet, this results in two
complementary
events.
To address this, fix dp_bridge_hpd_notify() to call
dp_hpd_plug_handle/unplug_handle()
directly to return consistent values for the above scenarios.
changes in v3:
- Fix the commit message as per submitting guidelines.
- remove extra line added
changes in v2:
- Fix the commit message to explain the scenario
- Fix the subject a little as well
Fixes: 542b37efc20e ("drm/msm/dp: Implement hpd_notify()")
Signed-off-by: Kuogee Hsieh
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/dp/dp_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index d80f89581760..bfb6dfff27e8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
return;
if (!dp_display->link_ready && status == connector_status_connected)
- dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+ dp_hpd_plug_handle(dp, 0);
else if (dp_display->link_ready && status ==
connector_status_disconnected)
- dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+ dp_hpd_unplug_handle(dp, 0);
}
--
2.43.2
[PATCH v2] drm/msm/dp: Remove now unused connector_type from desc
Now that the connector_type is dynamically determined, the
connector_type of the struct msm_dp_desc is unused. Clean it up.
Remaining duplicate entries are squashed.
Signed-off-by: Bjorn Andersson
---
This cleans up after, and hence depends on,
https://lore.kernel.org/all/20240324-x1e80100-display-refactor-connector-v4-1-e0ebaea66...@linaro.org/
---
Changes in v2:
- Squashed now duplicate entries
- Link to v1:
https://lore.kernel.org/r/20240328-dp-connector-type-cleanup-v1-1-9bf84c5a6...@quicinc.com
---
drivers/gpu/drm/msm/dp/dp_display.c | 48 +
1 file changed, 17 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 521cba76d2a0..12c01625c551 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -119,55 +119,41 @@ struct dp_display_private {
struct msm_dp_desc {
phys_addr_t io_start;
unsigned int id;
- unsigned int connector_type;
bool wide_bus_supported;
};
static const struct msm_dp_desc sc7180_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0 },
{}
};
static const struct msm_dp_desc sc7280_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0,
.wide_bus_supported = true },
+ { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_1,
.wide_bus_supported = true },
{}
};
static const struct msm_dp_desc sc8180x_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
- { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
- { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_eDP },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0 },
+ { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1 },
+ { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2 },
{}
};
static const struct msm_dp_desc sc8280xp_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x2209, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x22098000, .id = MSM_DP_CONTROLLER_1, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x2209a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- { .io_start = 0x220a, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_supported = true },
- {}
-};
-
-static const struct msm_dp_desc sc8280xp_edp_descs[] = {
- { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- { .io_start = 0x2209a000, .id = MSM_DP_CONTROLLER_2, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- { .io_start = 0x220a, .id = MSM_DP_CONTROLLER_3, .connector_type =
DRM_MODE_CONNECTOR_eDP, .wide_bus_supported = true },
- {}
-};
-
-static const struct msm_dp_desc sm8350_dp_descs[] = {
- { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0, .connector_type =
DRM_MODE_CONNECTOR_DisplayPort },
+ { .io_start = 0x0ae9, .id = MSM_DP_CONTROLLER_0,
.wide_bus_supported = true },
+ { .io_start = 0x0ae98000, .id = MSM_DP_CONTROLLER_1,
.wide_bus_supported = true },
+ { .io_start = 0x0ae9a000, .id = MSM_DP_CONTROLLER_2,
.wide_bus_supported = true },
+ { .io_start = 0x0aea, .id = MSM_DP_CONTROLLER_3,
.wide_bus_supported = true },
+ { .io_start = 0x2209, .id = MSM_DP_CONTROLLER_0,
.wide_bus_supported = true },
+ { .io_start = 0x22098000, .id = MSM_DP_CONTROLLER_1,
.wide_bus_supported = true },
+ { .io_start
Re: [PATCH 3/6] drm/msm/adreno: Allow specifying default speedbin value
On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> From: Neil Armstrong
>
> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> the highest. Falling back to it when things go wrong is largely
> suboptimal, as more often than not, the top frequencies are not
> supposed to work on other bins.
Isn't it better to just return an error here instead of trying to guess
which speedbin to use?
If that's not the case, I think the commit should be expanded with
actually setting default_speedbin for the existing GPUs.
>
> Let the developer specify the intended "lowest common denominator" bin
> in struct adreno_info. If not specified, partial struct initialization
> will ensure it's set to zero, retaining previous behavior.
>
> Signed-off-by: Neil Armstrong
> [Konrad: clean up, add commit message]
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 0674aca0f8a3..4cbdfabbcee5 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2915,7 +2915,7 @@ static int a6xx_set_supported_hw(struct device *dev,
> const struct adreno_info *i
> DRM_DEV_ERROR(dev,
> "missing support for speed-bin: %u. Some OPPs may not
> be supported by hardware\n",
> speedbin);
> - supp_hw = BIT(0); /* Default */
> + supp_hw = BIT(info->default_speedbin); /* Default */
> }
>
> ret = devm_pm_opp_set_supported_hw(dev, _hw, 1);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 77526892eb8c..460b399be37b 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -110,6 +110,7 @@ struct adreno_info {
>* {SHRT_MAX, 0} sentinal.
>*/
> struct adreno_speedbin *speedbins;
> + unsigned int default_speedbin;
> };
>
> #define ADRENO_CHIP_IDS(tbl...) (uint32_t[]) { tbl, 0 }
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
Re: [PATCH 1/6] soc: qcom: Move some socinfo defines to the header, expand them
On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote:
> In preparation for parsing the chip "feature code" (FC) and "product
> code" (PC) (essentially the parameters that let us conclusively
> characterize the sillicon we're running on, including various speed
> bins), move the socinfo version defines to the public header and
> include some more FC/PC defines.
>
> Signed-off-by: Konrad Dybcio
> ---
> drivers/soc/qcom/socinfo.c | 8
> include/linux/soc/qcom/socinfo.h | 36
> 2 files changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> index 277c07a6603d..cf4616a468f2 100644
> --- a/drivers/soc/qcom/socinfo.c
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -21,14 +21,6 @@
>
> #include
>
> -/*
> - * SoC version type with major number in the upper 16 bits and minor
> - * number in the lower 16 bits.
> - */
> -#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0x)
> -#define SOCINFO_MINOR(ver) ((ver) & 0x)
> -#define SOCINFO_VERSION(maj, min) maj) & 0x) << 16)|((min) &
> 0x))
> -
> /* Helper macros to create soc_id table */
> #define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
> #define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name)
> diff --git a/include/linux/soc/qcom/socinfo.h
> b/include/linux/soc/qcom/socinfo.h
> index e78777bb0f4a..ba7f683bd32c 100644
> --- a/include/linux/soc/qcom/socinfo.h
> +++ b/include/linux/soc/qcom/socinfo.h
> @@ -3,6 +3,8 @@
> #ifndef __QCOM_SOCINFO_H__
> #define __QCOM_SOCINFO_H__
>
> +#include
> +
> /*
> * SMEM item id, used to acquire handles to respective
> * SMEM region.
> @@ -12,6 +14,14 @@
> #define SMEM_SOCINFO_BUILD_ID_LENGTH 32
> #define SMEM_SOCINFO_CHIP_ID_LENGTH 32
>
> +/*
> + * SoC version type with major number in the upper 16 bits and minor
> + * number in the lower 16 bits.
> + */
> +#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0x)
> +#define SOCINFO_MINOR(ver) ((ver) & 0x)
> +#define SOCINFO_VERSION(maj, min) maj) & 0x) << 16)|((min) &
> 0x))
> +
> /* Socinfo SMEM item structure */
> struct socinfo {
> __le32 fmt;
> @@ -74,4 +84,30 @@ struct socinfo {
> __le32 boot_core;
> };
>
> +/* Internal feature codes */
> +enum feature_code {
> + /* External feature codes */
> + SOCINFO_FC_UNKNOWN = 0x0,
> + SOCINFO_FC_AA,
> + SOCINFO_FC_AB,
> + SOCINFO_FC_AC,
> + SOCINFO_FC_AD,
> + SOCINFO_FC_AE,
> + SOCINFO_FC_AF,
> + SOCINFO_FC_AG,
> + SOCINFO_FC_AH,
> + SOCINFO_FC_EXT_RESERVE,
> +};
> +
> +/* Internal feature codes */
> +/* Valid values: 0 <= n <= 0xf */
> +#define SOCINFO_FC_Yn(n) (0xf1 + n)
> +#define SOCINFO_FC_INT_RESERVE SOCINFO_FC_Yn(0x10)
> +
> +/* Product codes */
> +#define SOCINFO_PC_UNKNOWN 0
> +/* Valid values: 0 <= n <= 8, the rest is reserved */
> +#define SOCINFO_PCn(n) (n + 1)
> +#define SOCINFO_PC_RESERVE (BIT(31) - 1)
Please move these defines into the next patch.
> +
> #endif
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
Re: [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
On Fri, Apr 05, 2024 at 10:41:30AM +0200, Konrad Dybcio wrote:
> Introduce getters for SoC product and feature codes and export them.
>
> Signed-off-by: Konrad Dybcio
> ---
> drivers/soc/qcom/smem.c | 66
> +++
> include/linux/soc/qcom/smem.h | 2 ++
> 2 files changed, 68 insertions(+)
>
> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index 7191fa0c087f..e89b4d26877a 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c
> @@ -795,6 +795,72 @@ int qcom_smem_get_soc_id(u32 *id)
> }
> EXPORT_SYMBOL_GPL(qcom_smem_get_soc_id);
>
> +/**
> + * qcom_smem_get_feature_code() - return the feature code
> + * @id: On success, we return the feature code here.
> + *
> + * Look up the feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_feature_code(u32 *code)
> +{
> + struct socinfo *info;
> + u32 raw_code;
> +
> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> + if (IS_ERR(info))
> + return PTR_ERR(info);
> +
> + /* This only makes sense for socinfo >= 16 */
> + if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> + return -EINVAL;
> +
> + raw_code = __le32_to_cpu(info->feature_code);
> +
> + /* Ensure the value makes sense */
> + if (raw_code >= SOCINFO_FC_INT_RESERVE)
> + raw_code = SOCINFO_FC_UNKNOWN;
> +
> + *code = raw_code;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_feature_code);
> +
> +/**
> + * qcom_smem_get_product_code() - return the product code
> + * @id: On success, we return the product code here.
> + *
> + * Look up feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_product_code(u32 *code)
> +{
> + struct socinfo *info;
> + u32 raw_code;
> +
> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> + if (IS_ERR(info))
> + return PTR_ERR(info);
> +
> + /* This only makes sense for socinfo >= 16 */
> + if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> + return -EINVAL;
> +
> + raw_code = __le32_to_cpu(info->pcode);
> +
> + /* Ensure the value makes sense */
> + if (raw_code >= SOCINFO_FC_INT_RESERVE)
> + raw_code = SOCINFO_FC_UNKNOWN;
This looks like a c from the previous function. Should we be comparing
the raw_code with a SOCINFO_PC_ constant?
> +
> + *code = raw_code;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_product_code);
> +
> static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
> {
> struct smem_header *header;
> diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
> index a36a3b9d4929..aef8c9fc6c08 100644
> --- a/include/linux/soc/qcom/smem.h
> +++ b/include/linux/soc/qcom/smem.h
> @@ -13,5 +13,7 @@ int qcom_smem_get_free_space(unsigned host);
> phys_addr_t qcom_smem_virt_to_phys(void *p);
>
> int qcom_smem_get_soc_id(u32 *id);
> +int qcom_smem_get_feature_code(u32 *code);
> +int qcom_smem_get_product_code(u32 *code);
>
> #endif
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
Re: [PATCH v2] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD
On Fri, Apr 05, 2024 at 05:17:14PM -0700, Abhinav Kumar wrote:
> From: Kuogee Hsieh
>
> In the external HPD case, hotplug event is delivered by pmic glink to DP
> driver
> using drm_aux_hpd_bridge_notify().
There can be other drivers in front of the DP chain. For example,
altmode driver uses drm_connector_oob_hotplug_event() to deliver HPD
events.
>
> The stacktrace showing the sequence of events is below:
>
> dp_bridge_hpd_notify+0x18/0x70 [msm]
> drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
> drm_helper_probe_detect+0x94/0xc0 [drm_kms_helper]
> drm_helper_probe_single_connector_modes+0x43c/0x53c [drm_kms_helper]
> drm_client_modeset_probe+0x240/0x1114 [drm]
> drm_fb_helper_hotplug_event.part.26+0x9c/0xe8 [drm_kms_helper]
> drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
> msm_fbdev_client_hotplug+0x24/0xd4 [msm]
> drm_client_dev_hotplug+0xd8/0x148 [drm]
> drm_kms_helper_connector_hotplug_event+0x30/0x3c [drm_kms_helper]
> drm_bridge_connector_handle_hpd+0x84/0x94 [drm_kms_helper]
> drm_bridge_connector_hpd_cb+0xc/0x14 [drm_kms_helper]
> drm_bridge_hpd_notify+0x38/0x50 [drm]
> drm_aux_hpd_bridge_notify+0x14/0x20 [aux_hpd_bridge]
> pmic_glink_altmode_worker+0xec/0x27c [pmic_glink_altmode]
> process_scheduled_works+0x17c/0x2cc
> worker_thread+0x2ac/0x2d0
> kthread+0xfc/0x120
>
> There are three notifications delivered to DP driver for each notification
> event.
>
> 1) From the drm_aux_hpd_bridge_notify() itself as shown above
>
> 2) From output_poll_execute() thread which arises due to
> drm_helper_probe_single_connector_modes() call of the above stacktrace
> as shown in more detail here.
>
> dp_bridge_hpd_notify+0x18/0x70 [msm]
> drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
> drm_helper_probe_detect+0x94/0xc0 [drm_kms_helper]
> drm_helper_probe_single_connector_modes+0x43c/0x53c [drm_kms_helper]
> drm_client_modeset_probe+0x240/0x1114 [drm]
> drm_fb_helper_hotplug_event.part.26+0x9c/0xe8 [drm_kms_helper]
> drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
> msm_fbdev_client_hotplug+0x24/0xd4 [msm]
> drm_client_dev_hotplug+0xd8/0x148 [drm]
> drm_kms_helper_hotplug_event+0x30/0x3c [drm_kms_helper]
> output_poll_execute+0xe0/0x210 [drm_kms_helper]
>
> 3) From the DP driver as the dp_bridge_hpd_notify() callback today triggers
> the hpd_event_thread for connect and disconnect events respectively via below
> stack
>
> dp_bridge_hpd_notify+0x18/0x70 [msm]
> drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
> drm_helper_probe_detect_ctx+0x98/0x110 [drm_kms_helper]
> check_connector_changed+0x4c/0x20c [drm_kms_helper]
> drm_helper_hpd_irq_event+0x98/0x120 [drm_kms_helper]
> hpd_event_thread+0x478/0x5bc [msm]
>
> We have to address why we end up with 3 events for every single event so
> something
> is broken with how we work with the drm_bridge_connector.
>
> But, the dp_bridge_hpd_notify() delivered from output_poll_execute() thread
> will
> return the incorrect HPD status DP driver because the .detect() returns the
> value
> of link_ready and not the HPD status currently.
>
> And because the HPD event thread has not run yet and this results in the two
> complementary
> events.
>
> To fix this problem lets have dp_bridge_hpd_notify() call
> dp_hpd_plug_handle/unplug_handle() directly instead of going through the
> event thread.
>
> Then the following .detect() called by
> drm_kms_helper_connector_hotplug_event()
> will return correct value of HPD status since it uses the correct link_ready
> value.
>
> With this change, the HPD status delivered by both
> drm_bridge_connector_hpd_notify()
> and drm_kms_helper_connector_hotplug_event() will match each other
> consistently.
Please take a look at Documentation/process/submitting-patches.rst
With the commit message fixed, the change LGTM. Thanks a lot for
describing the call chains leading to this issue.
I must admit, initially I thought that the change should be rejected on
a basis of being a band-aid, but after studying the call graphs and the
locking within the DP driver, the change looks correct to me.
>
> changes in v2:
> - Fix the commit message to explain the scenario
> - Fix the subject a little as well
>
> Fixes: 542b37efc20e ("drm/msm/dp: Implement hpd_notify()")
> Signed-off-by: Kuogee Hsieh
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index d80f89581760..dfb10584ff97 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1665,7 +1665,8 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
> return;
>
> if (!dp_display->link_ready && status == connector_status_connected)
> - dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
> + dp_hpd_plug_handle(dp, 0);
> else if (dp_display->link_ready && status
Re: [PATCH v2 23/25] scsi: virtio: drop owner assignment
Krzysztof, > virtio core already sets the .owner, so driver does not need to. virtio_scsi looks OK to me. Acked-by: Martin K. Petersen -- Martin K. Petersen Oracle Linux Engineering
[PATCH v2] drm/msm/dp: call dp_hpd_plug_handle()/unplug_handle() directly for external HPD
From: Kuogee Hsieh
In the external HPD case, hotplug event is delivered by pmic glink to DP driver
using drm_aux_hpd_bridge_notify().
The stacktrace showing the sequence of events is below:
dp_bridge_hpd_notify+0x18/0x70 [msm]
drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
drm_helper_probe_detect+0x94/0xc0 [drm_kms_helper]
drm_helper_probe_single_connector_modes+0x43c/0x53c [drm_kms_helper]
drm_client_modeset_probe+0x240/0x1114 [drm]
drm_fb_helper_hotplug_event.part.26+0x9c/0xe8 [drm_kms_helper]
drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
msm_fbdev_client_hotplug+0x24/0xd4 [msm]
drm_client_dev_hotplug+0xd8/0x148 [drm]
drm_kms_helper_connector_hotplug_event+0x30/0x3c [drm_kms_helper]
drm_bridge_connector_handle_hpd+0x84/0x94 [drm_kms_helper]
drm_bridge_connector_hpd_cb+0xc/0x14 [drm_kms_helper]
drm_bridge_hpd_notify+0x38/0x50 [drm]
drm_aux_hpd_bridge_notify+0x14/0x20 [aux_hpd_bridge]
pmic_glink_altmode_worker+0xec/0x27c [pmic_glink_altmode]
process_scheduled_works+0x17c/0x2cc
worker_thread+0x2ac/0x2d0
kthread+0xfc/0x120
There are three notifications delivered to DP driver for each notification
event.
1) From the drm_aux_hpd_bridge_notify() itself as shown above
2) From output_poll_execute() thread which arises due to
drm_helper_probe_single_connector_modes() call of the above stacktrace
as shown in more detail here.
dp_bridge_hpd_notify+0x18/0x70 [msm]
drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
drm_helper_probe_detect+0x94/0xc0 [drm_kms_helper]
drm_helper_probe_single_connector_modes+0x43c/0x53c [drm_kms_helper]
drm_client_modeset_probe+0x240/0x1114 [drm]
drm_fb_helper_hotplug_event.part.26+0x9c/0xe8 [drm_kms_helper]
drm_fb_helper_hotplug_event+0x24/0x38 [drm_kms_helper]
msm_fbdev_client_hotplug+0x24/0xd4 [msm]
drm_client_dev_hotplug+0xd8/0x148 [drm]
drm_kms_helper_hotplug_event+0x30/0x3c [drm_kms_helper]
output_poll_execute+0xe0/0x210 [drm_kms_helper]
3) From the DP driver as the dp_bridge_hpd_notify() callback today triggers
the hpd_event_thread for connect and disconnect events respectively via below
stack
dp_bridge_hpd_notify+0x18/0x70 [msm]
drm_bridge_connector_detect+0x60/0xe8 [drm_kms_helper]
drm_helper_probe_detect_ctx+0x98/0x110 [drm_kms_helper]
check_connector_changed+0x4c/0x20c [drm_kms_helper]
drm_helper_hpd_irq_event+0x98/0x120 [drm_kms_helper]
hpd_event_thread+0x478/0x5bc [msm]
We have to address why we end up with 3 events for every single event so
something
is broken with how we work with the drm_bridge_connector.
But, the dp_bridge_hpd_notify() delivered from output_poll_execute() thread will
return the incorrect HPD status DP driver because the .detect() returns the
value
of link_ready and not the HPD status currently.
And because the HPD event thread has not run yet and this results in the two
complementary
events.
To fix this problem lets have dp_bridge_hpd_notify() call
dp_hpd_plug_handle/unplug_handle() directly instead of going through the
event thread.
Then the following .detect() called by drm_kms_helper_connector_hotplug_event()
will return correct value of HPD status since it uses the correct link_ready
value.
With this change, the HPD status delivered by both
drm_bridge_connector_hpd_notify()
and drm_kms_helper_connector_hotplug_event() will match each other consistently.
changes in v2:
- Fix the commit message to explain the scenario
- Fix the subject a little as well
Fixes: 542b37efc20e ("drm/msm/dp: Implement hpd_notify()")
Signed-off-by: Kuogee Hsieh
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/dp/dp_display.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index d80f89581760..dfb10584ff97 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1665,7 +1665,8 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
return;
if (!dp_display->link_ready && status == connector_status_connected)
- dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+ dp_hpd_plug_handle(dp, 0);
else if (dp_display->link_ready && status ==
connector_status_disconnected)
- dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+ dp_hpd_unplug_handle(dp, 0);
+
}
--
2.43.2
Re: [PATCH v1] drm/msm/dp: use dp_hpd_plug_handle() and dp_hpd_unplug_handle() directly
On 3/28/2024 10:47 PM, Abhinav Kumar wrote:
On 3/28/2024 8:23 PM, Dmitry Baryshkov wrote:
On Fri, 29 Mar 2024 at 04:16, Abhinav Kumar
wrote:
On 3/28/2024 5:10 PM, Dmitry Baryshkov wrote:
On Fri, 29 Mar 2024 at 01:42, Abhinav Kumar
wrote:
On 3/28/2024 3:50 PM, Dmitry Baryshkov wrote:
On Thu, 28 Mar 2024 at 23:21, Abhinav Kumar
wrote:
On 3/28/2024 1:58 PM, Stephen Boyd wrote:
Quoting Abhinav Kumar (2024-03-28 13:24:34)
+ Johan and Bjorn for FYI
On 3/28/2024 1:04 PM, Kuogee Hsieh wrote:
For internal HPD case, hpd_event_thread is created to handle HPD
interrupts generated by HPD block of DP controller. It converts
HPD interrupts into events and executed them under
hpd_event_thread
context. For external HPD case, HPD events is delivered by way of
dp_bridge_hpd_notify() under thread context. Since they are
executed
under thread context already, there is no reason to hand over
those
events to hpd_event_thread. Hence dp_hpd_plug_handle() and
dp_hpd_unplug_hanlde() are called directly at
dp_bridge_hpd_notify().
Signed-off-by: Kuogee Hsieh
---
drivers/gpu/drm/msm/dp/dp_display.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Fixes: 542b37efc20e ("drm/msm/dp: Implement hpd_notify()")
Is this a bug fix or an optimization? The commit text doesn't
tell me.
I would say both.
optimization as it avoids the need to go through the hpd_event
thread
processing.
bug fix because once you go through the hpd event thread
processing it
exposes and often breaks the already fragile hpd handling state
machine
which can be avoided in this case.
Please add a description for the particular issue that was observed
and how it is fixed by the patch.
Otherwise consider there to be an implicit NAK for all HPD-related
patches unless it is a series that moves link training to the enable
path and drops the HPD state machine completely.
I really mean it. We should stop beating a dead horse unless there is
a grave bug that must be fixed.
I think the commit message is explaining the issue well enough.
This was not fixing any issue we saw to explain you the exact scenario
of things which happened but this is just from code walkthrough.
Like kuogee wrote, hpd event thread was there so handle events coming
out of the hpd_isr for internal hpd cases. For the hpd_notify coming
from pmic_glink or any other extnernal hpd cases, there is no need to
put this through the hpd event thread because this will only make
things
worse of exposing the race conditions of the state machine.
Moving link training to enable and removal of hpd event thread will be
worked on but delaying obvious things we can fix does not make sense.
From the commit message this feels like an optimisation rather than a
fix. And granted the fragility of the HPD state machine, I'd prefer to
stay away from optimisations. As far as I understood from the history
of the last revert, we'd better make sure that HPD handling goes only
through the HPD event thread.
I think you are mixing the two. We tried to send the events through
DRM's hpd_notify which ended up in a bad way and btw, thats still not
resolved even though I have seen reports that things are fine with the
revert, we are consistently able to see us ending up in a disconnected
state with all the reverts and fixes in our x1e80100 DP setup.
I plan to investigate that issue properly in the next week and try to
make some sense of it all.
In fact, this patch is removing one more user of the hpd event thread
which is the direction in which we all want to head towards.
As I stated earlier, from my point of view it doesn't make sense to
rework the HPD thread in small steps.
On whether this is an optimization or a bug fix. I think by avoiding hpd
event thread (which should have never been used for hpd_notify updates,
hence a bug) we are avoiding the possibility of more race conditions.
I think that the HPD event thread serializes handling of events, so
avoiding it increases the possibility of a race condition.
So, this has my R-b and it holds. Upto you.
I'd wait for a proper description of the issue that was observed and
how it is solved by this patch.
This was a code walkthrough fix as I wrote a few times. If there no
merit in pushing this, lets ignore it and stop discussing.
Ok, so after we debugged the HPD issue on we have found the issue and
why actually this change will help. I am going to post a V2 with more
details on the commit text. We can discuss after that.
Re: [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
Hi Konrad,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd]
url:
https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/soc-qcom-Move-some-socinfo-defines-to-the-header-expand-them/20240405-164231
base: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
patch link:
https://lore.kernel.org/r/20240405-topic-smem_speedbin-v1-2-ce2b864251b1%40linaro.org
patch subject: [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
config: arm-defconfig
(https://download.01.org/0day-ci/archive/20240406/202404060648.dojoyusf-...@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git
f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240406/202404060648.dojoyusf-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202404060648.dojoyusf-...@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/soc/qcom/smem.c:807: warning: Function parameter or struct member
>> 'code' not described in 'qcom_smem_get_feature_code'
>> drivers/soc/qcom/smem.c:807: warning: Excess function parameter 'id'
>> description in 'qcom_smem_get_feature_code'
>> drivers/soc/qcom/smem.c:840: warning: Function parameter or struct member
>> 'code' not described in 'qcom_smem_get_product_code'
>> drivers/soc/qcom/smem.c:840: warning: Excess function parameter 'id'
>> description in 'qcom_smem_get_product_code'
vim +807 drivers/soc/qcom/smem.c
797
798 /**
799 * qcom_smem_get_feature_code() - return the feature code
800 * @id: On success, we return the feature code here.
801 *
802 * Look up the feature code identifier from SMEM and return it.
803 *
804 * Return: 0 on success, negative errno on failure.
805 */
806 int qcom_smem_get_feature_code(u32 *code)
> 807 {
808 struct socinfo *info;
809 u32 raw_code;
810
811 info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID,
NULL);
812 if (IS_ERR(info))
813 return PTR_ERR(info);
814
815 /* This only makes sense for socinfo >= 16 */
816 if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
817 return -EINVAL;
818
819 raw_code = __le32_to_cpu(info->feature_code);
820
821 /* Ensure the value makes sense */
822 if (raw_code >= SOCINFO_FC_INT_RESERVE)
823 raw_code = SOCINFO_FC_UNKNOWN;
824
825 *code = raw_code;
826
827 return 0;
828 }
829 EXPORT_SYMBOL_GPL(qcom_smem_get_feature_code);
830
831 /**
832 * qcom_smem_get_product_code() - return the product code
833 * @id: On success, we return the product code here.
834 *
835 * Look up feature code identifier from SMEM and return it.
836 *
837 * Return: 0 on success, negative errno on failure.
838 */
839 int qcom_smem_get_product_code(u32 *code)
> 840 {
841 struct socinfo *info;
842 u32 raw_code;
843
844 info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID,
NULL);
845 if (IS_ERR(info))
846 return PTR_ERR(info);
847
848 /* This only makes sense for socinfo >= 16 */
849 if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
850 return -EINVAL;
851
852 raw_code = __le32_to_cpu(info->pcode);
853
854 /* Ensure the value makes sense */
855 if (raw_code >= SOCINFO_FC_INT_RESERVE)
856 raw_code = SOCINFO_FC_UNKNOWN;
857
858 *code = raw_code;
859
860 return 0;
861 }
862 EXPORT_SYMBOL_GPL(qcom_smem_get_product_code);
863
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 1/5] drm/vmwgfx: Implement virtual kms
On Fri, Apr 5, 2024 at 5:53 PM Maaz Mombasawala wrote: > > On 4/2/24 16:28, Zack Rusin wrote: > > > > @@ -541,6 +518,8 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, > > unsigned unit) > >dev_priv->implicit_placement_property, > >1); > > > > + vmw_du_init(>base); > > + > > return 0; > > > > err_free_unregister: > > > @@ -905,6 +900,9 @@ static int vmw_sou_init(struct vmw_private *dev_priv, > > unsigned unit) > > dev->mode_config.suggested_x_property, 0); > > drm_object_attach_property(>base, > > dev->mode_config.suggested_y_property, 0); > > + > > + vmw_du_init(>base); > > + > > return 0; > > > > err_free_unregister: > > > @@ -1575,6 +1576,9 @@ static int vmw_stdu_init(struct vmw_private > > *dev_priv, unsigned unit) > > dev->mode_config.suggested_x_property, 0); > > drm_object_attach_property(>base, > > dev->mode_config.suggested_y_property, 0); > > + > > + vmw_du_init(>base); > > + > > return 0; > > > > err_free_unregister: > > Shouldn't calls to vmw_du_init() be behind an if(vkms_enabled) condition? So the vmw_du_init is supposed to initialize the base, so that's unconditional. To match the unconditional vmw_du_cleanup. There's an argument to be made whether both of those should unconditionally call vmw_vkms_crtc_init and vmw_vkms_crtc_cleanup. My opinion was that they're not doing anything costly and just initialize members and having the members of vmw_display_unit initialized whether vkms is enabled or not still makes sense. z
Re: [PATCH 1/5] drm/vmwgfx: Implement virtual kms
On 4/2/24 16:28, Zack Rusin wrote: > > @@ -541,6 +518,8 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, > unsigned unit) >dev_priv->implicit_placement_property, >1); > > + vmw_du_init(>base); > + > return 0; > > err_free_unregister: > @@ -905,6 +900,9 @@ static int vmw_sou_init(struct vmw_private *dev_priv, > unsigned unit) > dev->mode_config.suggested_x_property, 0); > drm_object_attach_property(>base, > dev->mode_config.suggested_y_property, 0); > + > + vmw_du_init(>base); > + > return 0; > > err_free_unregister: > @@ -1575,6 +1576,9 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, > unsigned unit) > dev->mode_config.suggested_x_property, 0); > drm_object_attach_property(>base, > dev->mode_config.suggested_y_property, 0); > + > + vmw_du_init(>base); > + > return 0; > > err_free_unregister: Shouldn't calls to vmw_du_init() be behind an if(vkms_enabled) condition? Thanks, Maaz Mombasawala
Re: [PATCH 2/6] drm/panel: novatek-nt36682e: don't unregister DSI device
On 4/4/2024 3:08 AM, Dmitry Baryshkov wrote:
The DSI device for the panel was registered by the DSI host, so it is an
error to unregister it from the panel driver. Drop the call to
mipi_dsi_device_unregister().
Fixes: ea4f9975625a ("drm/panel: Add support for Novatek NT36672E panel driver")
Signed-off-by: Dmitry Baryshkov
Reviewed-by: Jessica Zhang
---
drivers/gpu/drm/panel/panel-novatek-nt36672e.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
index cb7406d74466..c39fe0fc5d69 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672e.c
@@ -614,8 +614,6 @@ static void nt36672e_panel_remove(struct mipi_dsi_device
*dsi)
struct nt36672e_panel *ctx = mipi_dsi_get_drvdata(dsi);
mipi_dsi_detach(ctx->dsi);
- mipi_dsi_device_unregister(ctx->dsi);
-
drm_panel_remove(>panel);
}
--
2.39.2
Re: [PATCH 1/6] drm/panel: visionox-rm69299: don't unregister DSI device
On 4/4/2024 3:07 AM, Dmitry Baryshkov wrote:
The DSI device for the panel was registered by the DSI host, so it is an
error to unregister it from the panel driver. Drop the call to
mipi_dsi_device_unregister().
Hi Dmitry,
Reviewed-by: Jessica Zhang
Thanks,
Jessica Zhang
Fixes: c7f66d32dd43 ("drm/panel: add support for rm69299 visionox panel")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/panel/panel-visionox-rm69299.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c
b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
index 775144695283..b15ca56a09a7 100644
--- a/drivers/gpu/drm/panel/panel-visionox-rm69299.c
+++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
@@ -253,8 +253,6 @@ static void visionox_rm69299_remove(struct mipi_dsi_device
*dsi)
struct visionox_rm69299 *ctx = mipi_dsi_get_drvdata(dsi);
mipi_dsi_detach(ctx->dsi);
- mipi_dsi_device_unregister(ctx->dsi);
-
drm_panel_remove(>panel);
}
--
2.39.2
Re: [PATCH 01/12] drm/client: Fully protect modes[] with dev->mode_config.mutex
On Fri, Apr 05, 2024 at 11:39:33PM +0300, Dmitry Baryshkov wrote:
> On Fri, 5 Apr 2024 at 22:17, Ville Syrjälä
> wrote:
> >
> > On Fri, Apr 05, 2024 at 06:24:01AM +0300, Dmitry Baryshkov wrote:
> > > On Thu, Apr 04, 2024 at 11:33:25PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä
> > > >
> > > > The modes[] array contains pointers to modes on the connectors'
> > > > mode lists, which are protected by dev->mode_config.mutex.
> > > > Thus we need to extend modes[] the same protection or by the
> > > > time we use it the elements may already be pointing to
> > > > freed/reused memory.
> > > >
> > > > Cc: sta...@vger.kernel.org
> > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10583
> > > > Signed-off-by: Ville Syrjälä
> > >
> > > Reviewed-by: Dmitry Baryshkov
> > >
> > > I tried looking for the proper Fixes tag, but it looks like it might be
> > > something like 386516744ba4 ("drm/fb: fix fbdev object model + cleanup
> > > properly.")
> >
> > The history is rather messy. I think it was originally completely
> > lockless and broken, and got fixed piecemeal later in these:
> > commit 7394371d8569 ("drm: Take lock around probes for
> > drm_fb_helper_hotplug_event")
> > commit 966a6a13c666 ("drm: Hold mode_config.lock to prevent hotplug whilst
> > setting up crtcs")
> >
> > commit e13a05831050 ("drm/fb-helper: Stop using mode_config.mutex for
> > internals")
> > looks to me like where the race might have been re-introduced.
> > But didn't do a thorough analysis so not 100% sure. It's all
> > rather ancient history by now so a Fixes tag doesn't seem all
> > that useful anyway.
>
> Well, you have added stable to cc list, so you expect to have this
> patch backported. Then it should either have a kernel version as a
> 'starting' point or a Fixes tag to assist the sable team.
It'll get backported just fine without either.
--
Ville Syrjälä
Intel
Re: [PATCH 01/12] drm/client: Fully protect modes[] with dev->mode_config.mutex
On Fri, 5 Apr 2024 at 22:17, Ville Syrjälä
wrote:
>
> On Fri, Apr 05, 2024 at 06:24:01AM +0300, Dmitry Baryshkov wrote:
> > On Thu, Apr 04, 2024 at 11:33:25PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä
> > >
> > > The modes[] array contains pointers to modes on the connectors'
> > > mode lists, which are protected by dev->mode_config.mutex.
> > > Thus we need to extend modes[] the same protection or by the
> > > time we use it the elements may already be pointing to
> > > freed/reused memory.
> > >
> > > Cc: sta...@vger.kernel.org
> > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10583
> > > Signed-off-by: Ville Syrjälä
> >
> > Reviewed-by: Dmitry Baryshkov
> >
> > I tried looking for the proper Fixes tag, but it looks like it might be
> > something like 386516744ba4 ("drm/fb: fix fbdev object model + cleanup
> > properly.")
>
> The history is rather messy. I think it was originally completely
> lockless and broken, and got fixed piecemeal later in these:
> commit 7394371d8569 ("drm: Take lock around probes for
> drm_fb_helper_hotplug_event")
> commit 966a6a13c666 ("drm: Hold mode_config.lock to prevent hotplug whilst
> setting up crtcs")
>
> commit e13a05831050 ("drm/fb-helper: Stop using mode_config.mutex for
> internals")
> looks to me like where the race might have been re-introduced.
> But didn't do a thorough analysis so not 100% sure. It's all
> rather ancient history by now so a Fixes tag doesn't seem all
> that useful anyway.
Well, you have added stable to cc list, so you expect to have this
patch backported. Then it should either have a kernel version as a
'starting' point or a Fixes tag to assist the sable team.
--
With best wishes
Dmitry
[PATCH v2 6/6] drm/v3d: Enable big and super pages
The V3D MMU also supports 64KB and 1MB pages, called big and super pages,
respectively. In order to set a 64KB page or 1MB page in the MMU, we need
to make sure that page table entries for all 4KB pages within a big/super
page must be correctly configured.
In order to create a big/super page, we need a contiguous memory region.
That's why we use a separate mountpoint with THP enabled. In order to
place the page table entries in the MMU, we iterate over the 16 4KB pages
(for big pages) or 256 4KB pages (for super pages) and insert the PTE.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/v3d/v3d_bo.c| 21 +--
drivers/gpu/drm/v3d/v3d_drv.c | 8 ++
drivers/gpu/drm/v3d/v3d_drv.h | 2 ++
drivers/gpu/drm/v3d/v3d_gemfs.c | 6 +
drivers/gpu/drm/v3d/v3d_mmu.c | 46 ++---
5 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 79e31c5299b1..cfe82232886a 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -94,6 +94,7 @@ v3d_bo_create_finish(struct drm_gem_object *obj)
struct v3d_dev *v3d = to_v3d_dev(obj->dev);
struct v3d_bo *bo = to_v3d_bo(obj);
struct sg_table *sgt;
+ u64 align;
int ret;
/* So far we pin the BO in the MMU for its lifetime, so use
@@ -103,6 +104,15 @@ v3d_bo_create_finish(struct drm_gem_object *obj)
if (IS_ERR(sgt))
return PTR_ERR(sgt);
+ if (!v3d->super_pages)
+ align = SZ_4K;
+ else if (obj->size >= SZ_1M)
+ align = SZ_1M;
+ else if (obj->size >= SZ_64K)
+ align = SZ_64K;
+ else
+ align = SZ_4K;
+
spin_lock(>mm_lock);
/* Allocate the object's space in the GPU's page tables.
* Inserting PTEs will happen later, but the offset is for the
@@ -110,7 +120,7 @@ v3d_bo_create_finish(struct drm_gem_object *obj)
*/
ret = drm_mm_insert_node_generic(>mm, >node,
obj->size >> V3D_MMU_PAGE_SHIFT,
-SZ_4K >> V3D_MMU_PAGE_SHIFT, 0, 0);
+align >> V3D_MMU_PAGE_SHIFT, 0, 0);
spin_unlock(>mm_lock);
if (ret)
return ret;
@@ -130,10 +140,17 @@ struct v3d_bo *v3d_bo_create(struct drm_device *dev,
struct drm_file *file_priv,
size_t unaligned_size)
{
struct drm_gem_shmem_object *shmem_obj;
+ struct v3d_dev *v3d = to_v3d_dev(dev);
struct v3d_bo *bo;
int ret;
- shmem_obj = drm_gem_shmem_create(dev, unaligned_size);
+ /* Let the user opt out of allocating the BOs with THP */
+ if (v3d->super_pages)
+ shmem_obj = drm_gem_shmem_create_with_mnt(dev, unaligned_size,
+ v3d->gemfs);
+ else
+ shmem_obj = drm_gem_shmem_create(dev, unaligned_size);
+
if (IS_ERR(shmem_obj))
return ERR_CAST(shmem_obj);
bo = to_v3d_bo(_obj->base);
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 3debf37e7d9b..3dbd29560be4 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -36,6 +36,12 @@
#define DRIVER_MINOR 0
#define DRIVER_PATCHLEVEL 0
+static bool super_pages = true;
+module_param_named(super_pages, super_pages, bool, 0400);
+MODULE_PARM_DESC(super_pages, "Enable/Disable Super Pages support. Note: \
+ To enable Super Pages, you need support to \
+ enable THP.");
+
static int v3d_get_param_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
@@ -308,6 +314,8 @@ static int v3d_platform_drm_probe(struct platform_device
*pdev)
return -ENOMEM;
}
+ v3d->super_pages = super_pages;
+
ret = v3d_gem_init(drm);
if (ret)
goto dma_free;
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 17236ee23490..0a7aacf51164 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -18,6 +18,7 @@ struct platform_device;
struct reset_control;
#define V3D_MMU_PAGE_SHIFT 12
+#define V3D_PAGE_FACTOR (PAGE_SIZE >> V3D_MMU_PAGE_SHIFT)
#define V3D_MAX_QUEUES (V3D_CPU + 1)
@@ -121,6 +122,7 @@ struct v3d_dev {
* tmpfs instance used for shmem backed objects
*/
struct vfsmount *gemfs;
+ bool super_pages;
struct work_struct overflow_mem_work;
diff --git a/drivers/gpu/drm/v3d/v3d_gemfs.c b/drivers/gpu/drm/v3d/v3d_gemfs.c
index 31cf5bd11e39..7ee55b32c36e 100644
--- a/drivers/gpu/drm/v3d/v3d_gemfs.c
+++ b/drivers/gpu/drm/v3d/v3d_gemfs.c
@@ -12,6 +12,10 @@ void v3d_gemfs_init(struct v3d_dev *v3d)
struct file_system_type *type;
struct
[PATCH v2 5/6] drm/v3d: Reduce the alignment of the node allocation
Currently, we are using an alignment of 128 kB to insert a node, which ends up wasting memory as we perform plenty of small BOs allocations (<= 4 kB). We require that allocations are aligned to 128Kb so for any allocation smaller than that, we are wasting the difference. This implies that we cannot effectively use the whole 4 GB address space available for the GPU in the RPi 4. Currently, we can allocate up to 32000 BOs of 4 kB (~140 MB) and 3000 BOs of 400 kB (~1,3 GB). This can be quite limiting for applications that have a high memory requirement, such as vkoverhead [1]. By reducing the page alignment to 4 kB, we can allocate up to 100 BOs of 4 kB (~4 GB) and 1 BOs of 400 kB (~4 GB). Moreover, by performing benchmarks, we were able to attest that reducing the page alignment to 4 kB can provide a general performance improvement in OpenGL applications (e.g. glmark2). Therefore, this patch reduces the alignment of the node allocation to 4 kB, which will allow RPi users to explore the whole 4GB virtual address space provided by the hardware. Also, this patch allow users to fully run vkoverhead in the RPi 4/5, solving the issue reported in [1]. [1] https://github.com/zmike/vkoverhead/issues/14 Signed-off-by: Maíra Canal --- drivers/gpu/drm/v3d/v3d_bo.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.h | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c index a07ede668cc1..79e31c5299b1 100644 --- a/drivers/gpu/drm/v3d/v3d_bo.c +++ b/drivers/gpu/drm/v3d/v3d_bo.c @@ -110,7 +110,7 @@ v3d_bo_create_finish(struct drm_gem_object *obj) */ ret = drm_mm_insert_node_generic(>mm, >node, obj->size >> V3D_MMU_PAGE_SHIFT, -GMP_GRANULARITY >> V3D_MMU_PAGE_SHIFT, 0, 0); +SZ_4K >> V3D_MMU_PAGE_SHIFT, 0, 0); spin_unlock(>mm_lock); if (ret) return ret; diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h index d2ce8222771a..17236ee23490 100644 --- a/drivers/gpu/drm/v3d/v3d_drv.h +++ b/drivers/gpu/drm/v3d/v3d_drv.h @@ -17,8 +17,6 @@ struct clk; struct platform_device; struct reset_control; -#define GMP_GRANULARITY (128 * 1024) - #define V3D_MMU_PAGE_SHIFT 12 #define V3D_MAX_QUEUES (V3D_CPU + 1) -- 2.44.0
[PATCH v2 4/6] drm/gem: Create shmem GEM object in a given mountpoint
Create a function `drm_gem_shmem_create_with_mnt()`, similar to
`drm_gem_shmem_create()`, that has a mountpoint as a argument. This
function will create a shmem GEM object in a given tmpfs mountpoint.
This function will be useful for drivers that have a special mountpoint
with flags enabled.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/drm_gem_shmem_helper.c | 30 ++
include/drm/drm_gem_shmem_helper.h | 3 +++
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 13bcdbfd..10b7c4c769a3 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -49,7 +49,8 @@ static const struct drm_gem_object_funcs drm_gem_shmem_funcs
= {
};
static struct drm_gem_shmem_object *
-__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)
+__drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private,
+ struct vfsmount *gemfs)
{
struct drm_gem_shmem_object *shmem;
struct drm_gem_object *obj;
@@ -76,7 +77,7 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size,
bool private)
drm_gem_private_object_init(dev, obj, size);
shmem->map_wc = false; /* dma-buf mappings use always
writecombine */
} else {
- ret = drm_gem_object_init(dev, obj, size);
+ ret = drm_gem_object_init_with_mnt(dev, obj, size, gemfs);
}
if (ret) {
drm_gem_private_object_fini(obj);
@@ -123,10 +124,31 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t
size, bool private)
*/
struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev,
size_t size)
{
- return __drm_gem_shmem_create(dev, size, false);
+ return __drm_gem_shmem_create(dev, size, false, NULL);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_create);
+/**
+ * drm_gem_shmem_create_with_mnt - Allocate an object with the given size in a
+ * given mountpoint
+ * @dev: DRM device
+ * @size: Size of the object to allocate
+ * @gemfs: tmpfs mount where the GEM object will be created
+ *
+ * This function creates a shmem GEM object in a given tmpfs mountpoint.
+ *
+ * Returns:
+ * A struct drm_gem_shmem_object * on success or an ERR_PTR()-encoded negative
+ * error code on failure.
+ */
+struct drm_gem_shmem_object *drm_gem_shmem_create_with_mnt(struct drm_device
*dev,
+ size_t size,
+ struct vfsmount
*gemfs)
+{
+ return __drm_gem_shmem_create(dev, size, false, gemfs);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_create_with_mnt);
+
/**
* drm_gem_shmem_free - Free resources associated with a shmem GEM object
* @shmem: shmem GEM object to free
@@ -760,7 +782,7 @@ drm_gem_shmem_prime_import_sg_table(struct drm_device *dev,
size_t size = PAGE_ALIGN(attach->dmabuf->size);
struct drm_gem_shmem_object *shmem;
- shmem = __drm_gem_shmem_create(dev, size, true);
+ shmem = __drm_gem_shmem_create(dev, size, true, NULL);
if (IS_ERR(shmem))
return ERR_CAST(shmem);
diff --git a/include/drm/drm_gem_shmem_helper.h
b/include/drm/drm_gem_shmem_helper.h
index efbc9f27312b..d22e3fb53631 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -97,6 +97,9 @@ struct drm_gem_shmem_object {
container_of(obj, struct drm_gem_shmem_object, base)
struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev,
size_t size);
+struct drm_gem_shmem_object *drm_gem_shmem_create_with_mnt(struct drm_device
*dev,
+ size_t size,
+ struct vfsmount
*gemfs);
void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem);
void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem);
--
2.44.0
[PATCH v2 3/6] drm/v3d: Introduce gemfs
Create a separate "tmpfs" kernel mount for V3D. This will allow us to
move away from the shmemfs `shm_mnt` and gives the flexibility to do
things like set our own mount options. Here, the interest is to use
"huge=", which should allow us to enable the use of THP for our
shmem-backed objects.
Signed-off-by: Maíra Canal
Reviewed-by: Iago Toral Quiroga
---
drivers/gpu/drm/v3d/Makefile| 3 ++-
drivers/gpu/drm/v3d/v3d_drv.h | 9 +++
drivers/gpu/drm/v3d/v3d_gem.c | 3 +++
drivers/gpu/drm/v3d/v3d_gemfs.c | 46 +
4 files changed, 60 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/v3d/v3d_gemfs.c
diff --git a/drivers/gpu/drm/v3d/Makefile b/drivers/gpu/drm/v3d/Makefile
index b7d673f1153b..fcf710926057 100644
--- a/drivers/gpu/drm/v3d/Makefile
+++ b/drivers/gpu/drm/v3d/Makefile
@@ -13,7 +13,8 @@ v3d-y := \
v3d_trace_points.o \
v3d_sched.o \
v3d_sysfs.o \
- v3d_submit.o
+ v3d_submit.o \
+ v3d_gemfs.o
v3d-$(CONFIG_DEBUG_FS) += v3d_debugfs.o
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 1950c723dde1..d2ce8222771a 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -119,6 +119,11 @@ struct v3d_dev {
struct drm_mm mm;
spinlock_t mm_lock;
+ /*
+* tmpfs instance used for shmem backed objects
+*/
+ struct vfsmount *gemfs;
+
struct work_struct overflow_mem_work;
struct v3d_bin_job *bin_job;
@@ -519,6 +524,10 @@ void v3d_reset(struct v3d_dev *v3d);
void v3d_invalidate_caches(struct v3d_dev *v3d);
void v3d_clean_caches(struct v3d_dev *v3d);
+/* v3d_gemfs.c */
+void v3d_gemfs_init(struct v3d_dev *v3d);
+void v3d_gemfs_fini(struct v3d_dev *v3d);
+
/* v3d_submit.c */
void v3d_job_cleanup(struct v3d_job *job);
void v3d_job_put(struct v3d_job *job);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index 66f4b78a6b2e..faefbe497e8d 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -287,6 +287,8 @@ v3d_gem_init(struct drm_device *dev)
v3d_init_hw_state(v3d);
v3d_mmu_set_page_table(v3d);
+ v3d_gemfs_init(v3d);
+
ret = v3d_sched_init(v3d);
if (ret) {
drm_mm_takedown(>mm);
@@ -304,6 +306,7 @@ v3d_gem_destroy(struct drm_device *dev)
struct v3d_dev *v3d = to_v3d_dev(dev);
v3d_sched_fini(v3d);
+ v3d_gemfs_fini(v3d);
/* Waiting for jobs to finish would need to be done before
* unregistering V3D.
diff --git a/drivers/gpu/drm/v3d/v3d_gemfs.c b/drivers/gpu/drm/v3d/v3d_gemfs.c
new file mode 100644
index ..31cf5bd11e39
--- /dev/null
+++ b/drivers/gpu/drm/v3d/v3d_gemfs.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0+
+/* Copyright (C) 2024 Raspberry Pi */
+
+#include
+#include
+
+#include "v3d_drv.h"
+
+void v3d_gemfs_init(struct v3d_dev *v3d)
+{
+ char huge_opt[] = "huge=within_size";
+ struct file_system_type *type;
+ struct vfsmount *gemfs;
+
+ /*
+* By creating our own shmemfs mountpoint, we can pass in
+* mount flags that better match our usecase. However, we
+* only do so on platforms which benefit from it.
+*/
+ if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
+ goto err;
+
+ type = get_fs_type("tmpfs");
+ if (!type)
+ goto err;
+
+ gemfs = vfs_kern_mount(type, SB_KERNMOUNT, type->name, huge_opt);
+ if (IS_ERR(gemfs))
+ goto err;
+
+ v3d->gemfs = gemfs;
+ drm_info(>drm, "Using Transparent Hugepages\n");
+
+ return;
+
+err:
+ v3d->gemfs = NULL;
+ drm_notice(>drm,
+ "Transparent Hugepage support is recommended for optimal
performance on this platform!\n");
+}
+
+void v3d_gemfs_fini(struct v3d_dev *v3d)
+{
+ if (v3d->gemfs)
+ kern_unmount(v3d->gemfs);
+}
--
2.44.0
[PATCH v2 2/6] drm/gem: Create a drm_gem_object_init_with_mnt() function
For some applications, such as applications that uses huge pages, we might
want to have a different mountpoint, for which we pass mount flags that
better match our usecase.
Therefore, create a new function `drm_gem_object_init_with_mnt()` that
allow us to define the tmpfs mountpoint where the GEM object will be
created. If this parameter is NULL, then we fallback to `shmem_file_setup()`.
Signed-off-by: Maíra Canal
---
drivers/gpu/drm/drm_gem.c | 34 ++
include/drm/drm_gem.h | 3 +++
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index d4bbc5d109c8..74ebe68e3d61 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -114,22 +114,32 @@ drm_gem_init(struct drm_device *dev)
}
/**
- * drm_gem_object_init - initialize an allocated shmem-backed GEM object
+ * drm_gem_object_init_with_mnt - initialize an allocated shmem-backed GEM
+ * object in a given shmfs mountpoint
+ *
* @dev: drm_device the object should be initialized for
* @obj: drm_gem_object to initialize
* @size: object size
+ * @gemfs: tmpfs mount where the GEM object will be created. If NULL, use
+ * the usual tmpfs mountpoint (`shm_mnt`).
*
* Initialize an already allocated GEM object of the specified size with
* shmfs backing store.
*/
-int drm_gem_object_init(struct drm_device *dev,
- struct drm_gem_object *obj, size_t size)
+int drm_gem_object_init_with_mnt(struct drm_device *dev,
+struct drm_gem_object *obj, size_t size,
+struct vfsmount *gemfs)
{
struct file *filp;
drm_gem_private_object_init(dev, obj, size);
- filp = shmem_file_setup("drm mm object", size, VM_NORESERVE);
+ if (gemfs)
+ filp = shmem_file_setup_with_mnt(gemfs, "drm mm object", size,
+VM_NORESERVE);
+ else
+ filp = shmem_file_setup("drm mm object", size, VM_NORESERVE);
+
if (IS_ERR(filp))
return PTR_ERR(filp);
@@ -137,6 +147,22 @@ int drm_gem_object_init(struct drm_device *dev,
return 0;
}
+EXPORT_SYMBOL(drm_gem_object_init_with_mnt);
+
+/**
+ * drm_gem_object_init - initialize an allocated shmem-backed GEM object
+ * @dev: drm_device the object should be initialized for
+ * @obj: drm_gem_object to initialize
+ * @size: object size
+ *
+ * Initialize an already allocated GEM object of the specified size with
+ * shmfs backing store.
+ */
+int drm_gem_object_init(struct drm_device *dev, struct drm_gem_object *obj,
+ size_t size)
+{
+ return drm_gem_object_init_with_mnt(dev, obj, size, NULL);
+}
EXPORT_SYMBOL(drm_gem_object_init);
/**
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index bae4865b2101..2ebf6e10cc44 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -472,6 +472,9 @@ void drm_gem_object_release(struct drm_gem_object *obj);
void drm_gem_object_free(struct kref *kref);
int drm_gem_object_init(struct drm_device *dev,
struct drm_gem_object *obj, size_t size);
+int drm_gem_object_init_with_mnt(struct drm_device *dev,
+struct drm_gem_object *obj, size_t size,
+struct vfsmount *gemfs);
void drm_gem_private_object_init(struct drm_device *dev,
struct drm_gem_object *obj, size_t size);
void drm_gem_private_object_fini(struct drm_gem_object *obj);
--
2.44.0
[PATCH v2 0/6] drm/v3d: Enable Big and Super Pages
This series introduces support for big and super pages in V3D. The V3D MMU has support for 64KB and 1MB pages, called big pages and super pages, which are currently not used. Therefore, this patchset has the intention to enable big and super pages in V3D. The advantage of enabling big and super pages is that if any entry for a page within a big/super page is cached in the MMU, it will be used for the translation of all virtual addresses in the range of that super page without requiring fetching any other entries. Big/Super pages essentially mean a slightly better performance for users, especially in applications with high memory requirements (e.g. applications that use multiple large BOs). Using a Raspberry Pi 4 (with a PAGE_SIZE=4KB downstream kernel), when running traces from multiple applications, we were able to see the following improvements: fps_avg helped: warzone2100.70secs.1024x768.trace:1.98 -> 2.42 (22.19%) (v1: 2.56) fps_avg helped: warzone2100.30secs.1024x768.trace:2 -> 2.45(21.96%) (v1: 2.39) fps_avg helped: supertuxkart-menus_1024x768.trace:123.12 -> 128.39 (4.28%) (v1: 125.50) fps_avg helped: vkQuake_capture_frames_1_through_1200_1280x720.gfxr: 61.26 -> 62.89 (2.67%) (v1: 61.36) fps_avg helped: quake2-gles3-1280x720.trace: 63.42 -> 64.86 (2.27%) (v1: 64.29) fps_avg helped: ue4_sun_temple_640x480.gfxr: 25.15 -> 25.63 (1.89%) (v1: 24.94) fps_avg helped: ue4_shooter_game_high_quality_640x480.gfxr: 19.28 -> 19.61 (1.72%) (v1: 19.02) fps_avg helped: ue4_shooter_game_shooting_high_quality_640x480.gfxr: 23.58 -> 23.91 (1.39%) (v1: 23.34) fps_avg helped: quake3e_capture_frames_1_through_1800_1920x1080.gfxr: 61.40 -> 62.00 (0.96%) (v1: -) fps_avg helped: serious_sam_trace02_1280x720.gfxr:49.41 -> 49.84 (0.86%) (v1: 47.74) fps_avg helped: supertuxkart-racing_1024x768.trace: 8.69 -> 8.74 (0.56%) (v1: -) fps_avg helped: sponza_demo01_800x600.gfxr: 17.55 -> 17.64 (0.50%) (v1: -) fps_avg helped: quake2-gl1.4-1280x720.trace: 36.43 -> 36.58 (0.43%) (v1: 36.57) fps_avg helped: quake3e-1280x720.trace: 94.49 -> 94.87 (0.40%) (v1: -) fps_avg helped: sponza_demo02_800x600.gfxr: 19.79 -> 19.87 (0.39%) (v1: -) Using a Raspberry Pi 5 (with a PAGE_SIZE=16KB downstream kernel), when running traces from multiple applications, we were able to see the following improvements: fps_avg helped: warzone2100.30secs.1024x768.trace: 4.40 -> 4.95 (12.58%) (v1: 4.49) fps_avg helped: vkQuake_capture_frames_1_through_1200_1280x720.gfxr: 135.05 -> 141.21 (4.56%) (v1: 139.45) fps_avg helped: supertuxkart-menus_1024x768.trace: 291.73 -> 302.05 (3.54%) (v1: 303.80) fps_avg helped: quake2-gles3-1280x720.trace: 148.90 -> 153.57 (3.13%) (v1: 156.13) fps_avg helped: quake2-gl1.4-1280x720.trace: 82.60 -> 84.46 (2.25%) (v1: -) fps_avg helped: ue4_shooter_game_shooting_low_quality_640x480.gfxr: 49.59 -> 50.54 (1.92%) (v1: 47.30) fps_avg helped: ue4_shooter_game_shooting_high_quality_640x480.gfxr: 51.03 -> 51.93 (1.76%) (v1: 50.46) fps_avg helped: ue4_shooter_game_high_quality_640x480.gfxr: 31.15 -> 31.64 (1.60%) (v1: 31.05) fps_avg helped: ue4_sun_temple_640x480.gfxr: 40.26 -> 40.88 (1.54%) (v1: 40.23) fps_avg helped: sponza_demo01_800x600.gfxr: 43.23 -> 43.84 (1.40%) (v1: 43.68) fps_avg helped: warzone2100.70secs.1024x768.trace: 4.36 -> 4.42 (1.39%) (v1: -) fps_avg helped: sponza_demo02_800x600.gfxr: 48.94 -> 49.51 (1.17%) (v1: 49.34) fps_avg helped: quake3e_capture_frames_1_through_1800_1920x1080.gfxr: 162.11 -> 163.13 (0.63%) (v1: 165.71) fps_avg helped: quake3e-1280x720.trace: 229.82 -> 231.00 (0.51%) (v1: 234.51) fps_avg helped: supertuxkart-racing_1024x768.trace: 20.42 -> 20.48 (0.30%) (v1: 20.59) fps_avg helped: quake3e_capture_frames_1800_through_2400_1920x1080.gfxr: 157.45 -> 157.61 (0.10%) (v1: 160.35) fps_avg helped: serious_sam_trace02_1280x720.gfxr: 59.99 -> 60.02 (0.05%) (v1: -) When glancing at the percentage improvement, one might initially perceive v2 as causing a performance downgrade. However, that assumption doesn't hold true. In the case of v1, we were using downstream kernel version 6.1, whereas now, with the kernel updated to 6.6 for v2, there's a small uptick in performance. This indicates an enhancement in the baseline scenario, rather than any detriment
[PATCH v2 1/6] drm/v3d: Fix return if scheduler initialization fails
If the scheduler initialization fails, GEM initialization must fail as
well. Therefore, if `v3d_sched_init()` fails, free the DMA memory
allocated and return the error value in `v3d_gem_init()`.
Signed-off-by: Maíra Canal
Reviewed-by: Iago Toral Quiroga
---
drivers/gpu/drm/v3d/v3d_gem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index afc565078c78..66f4b78a6b2e 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -290,8 +290,9 @@ v3d_gem_init(struct drm_device *dev)
ret = v3d_sched_init(v3d);
if (ret) {
drm_mm_takedown(>mm);
- dma_free_coherent(v3d->drm.dev, 4096 * 1024, (void *)v3d->pt,
+ dma_free_coherent(v3d->drm.dev, pt_size, (void *)v3d->pt,
v3d->pt_paddr);
+ return ret;
}
return 0;
--
2.44.0
Re: [PATCH 10/12] drm/client: Use [CONNECTOR:%d:%s] formatting
On Fri, Apr 05, 2024 at 11:23:01AM +0300, Jani Nikula wrote:
> On Thu, 04 Apr 2024, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > Switch to the canonical [CONNECTOR:%d:%s] etc. format for
> > printing out kms objects.
>
> I've been pinging for reviews on [1] for a while now. :/
>
> I'm just doing what you do in patches 9-10 in one go, and I very much
> prefer having the [CONNECTOR:%d:%s] bit as the first thing in the
> debug. For an individual line your style might read better, but for
> reading a log with a bunch of consecutive lines, I think having it as a
> prefix reads better.
>
> BR,
> Jani.
>
>
> [1]
> https://lore.kernel.org/r/f580f7a20bdea45178cef3940b636d491ae3dd92.1709843865.git.jani.nik...@intel.com
>
Looks like you have rbs now. I can rebase this
(and see what's left) after your stuff lands.
>
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/drm_client_modeset.c | 65 +++-
> > 1 file changed, 35 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_client_modeset.c
> > b/drivers/gpu/drm/drm_client_modeset.c
> > index 1751162b7d5c..415d1799337b 100644
> > --- a/drivers/gpu/drm/drm_client_modeset.c
> > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > @@ -251,8 +251,10 @@ static void drm_client_connectors_enabled(struct
> > drm_device *dev,
> > for (i = 0; i < connector_count; i++) {
> > connector = connectors[i];
> > enabled[i] = drm_connector_enabled(connector, true);
> > - drm_dbg_kms(dev, "connector %d enabled? %s\n",
> > connector->base.id,
> > - connector->display_info.non_desktop ? "non desktop"
> > : str_yes_no(enabled[i]));
> > + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] enabled? %s\n",
> > + connector->base.id, connector->name,
> > + connector->display_info.non_desktop ?
> > + "non desktop" : str_yes_no(enabled[i]));
> >
> > any_enabled |= enabled[i];
> > }
> > @@ -368,8 +370,8 @@ static int drm_client_get_tile_offsets(struct
> > drm_device *dev,
> > continue;
> >
> > if (!modes[i] && (h_idx || v_idx)) {
> > - drm_dbg_kms(dev, "no modes for connector tiled %d %d\n",
> > - i, connector->base.id);
> > + drm_dbg_kms(dev, "no modes for tiled
> > [CONNECTOR:%d:%s]\n",
> > + connector->base.id, connector->name);
> > continue;
> > }
> > if (connector->tile_h_loc < h_idx)
> > @@ -438,14 +440,15 @@ static bool drm_client_target_preferred(struct
> > drm_device *dev,
> > drm_client_get_tile_offsets(dev, connectors,
> > connector_count, modes, offsets, i,
> > connector->tile_h_loc,
> > connector->tile_v_loc);
> > }
> > - drm_dbg_kms(dev, "looking for cmdline mode on connector %d\n",
> > - connector->base.id);
> > + drm_dbg_kms(dev, "looking for cmdline mode on
> > [CONNECTOR:%d:%s]\n",
> > + connector->base.id, connector->name);
> >
> > /* got for command line mode first */
> > modes[i] = drm_connector_pick_cmdline_mode(connector);
> > if (!modes[i]) {
> > - drm_dbg_kms(dev, "looking for preferred mode on
> > connector %d %d\n",
> > - connector->base.id, connector->tile_group ?
> > connector->tile_group->id : 0);
> > + drm_dbg_kms(dev, "looking for preferred mode on
> > [CONNECTOR:%d:%s] (tile group: %d)\n",
> > + connector->base.id, connector->name,
> > + connector->tile_group ?
> > connector->tile_group->id : 0);
> > modes[i] = drm_connector_preferred_mode(connector,
> > width, height);
> > }
> > /* No preferred modes, pick one off the list */
> > @@ -465,8 +468,8 @@ static bool drm_client_target_preferred(struct
> > drm_device *dev,
> > (connector->tile_h_loc == 0 &&
> > connector->tile_v_loc == 0 &&
> > !drm_connector_get_tiled_mode(connector))) {
> > - drm_dbg_kms(dev, "Falling back to non tiled
> > mode on Connector %d\n",
> > - connector->base.id);
> > + drm_dbg_kms(dev, "Falling back to non tiled
> > mode on [CONNECTOR:%d:%s]\n",
> > + connector->base.id,
> > connector->name);
> > modes[i] =
> > drm_connector_fallback_non_tiled_mode(connector);
> > } else {
> > modes[i] =
> > drm_connector_get_tiled_mode(connector);
> > @@ -634,15 +637,15 @@ static bool
Re: [git pull] drm fixes for 6.9-rc3
The pull request you sent on Fri, 5 Apr 2024 13:41:06 +1000: > https://gitlab.freedesktop.org/drm/kernel.git tags/drm-fixes-2024-04-05 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/89103a164210f1c88caedf880ac9ab9576a1190d Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH] drm: nv04: Add check to avoid out of bounds access
On Fri, 2024-04-05 at 17:53 +0200, Danilo Krummrich wrote:
> On 3/31/24 08:45, Mikhail Kobuk wrote:
> > Output Resource (dcb->or) value is not guaranteed to be non-zero
> > (i.e.
> > in drivers/gpu/drm/nouveau/nouveau_bios.c, in
> > 'fabricate_dcb_encoder_table()'
> > 'dcb->or' is assigned value '0' in call to
> > 'fabricate_dcb_output()').
>
> I don't really know much about the semantics of this code.
>
> Looking at fabricate_dcb_output() though I wonder if the intention
> was to assign
> BIT(or) to entry->or.
>
> @Lyude, can you help here?
This code is definitely a bit before my time as well - but I think
you're completely correct. Especially considering this bit I found in
nouveau_bios.h:
enum nouveau_or {
DCB_OUTPUT_A = (1 << 0),
DCB_OUTPUT_B = (1 << 1),
DCB_OUTPUT_C = (1 << 2)
};
>
> Otherwise, for parsing the DCB entries, it seems that the bound
> checks are
> happening in olddcb_outp_foreach() [1].
>
> [1]
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/nouveau/nouveau_bios.c#L1331
>
> >
> > Add check to validate 'dcb->or' before it's used.
> >
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> >
> > Fixes: 2e5702aff395 ("drm/nouveau: fabricate DCB encoder table for
> > iMac G4")
> > Signed-off-by: Mikhail Kobuk
> > ---
> > drivers/gpu/drm/nouveau/dispnv04/dac.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/dispnv04/dac.c
> > b/drivers/gpu/drm/nouveau/dispnv04/dac.c
> > index d6b8e0cce2ac..0c8d4fc95ff3 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv04/dac.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv04/dac.c
> > @@ -428,7 +428,7 @@ void nv04_dac_update_dacclk(struct drm_encoder
> > *encoder, bool enable)
> > struct drm_device *dev = encoder->dev;
> > struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
> >
> > - if (nv_gf4_disp_arch(dev)) {
> > + if (nv_gf4_disp_arch(dev) && ffs(dcb->or)) {
> > uint32_t *dac_users = _display(dev)-
> > >dac_users[ffs(dcb->or) - 1];
> > int dacclk_off = NV_PRAMDAC_DACCLK +
> > nv04_dac_output_offset(encoder);
> > uint32_t dacclk = NVReadRAMDAC(dev, 0,
> > dacclk_off);
> > @@ -453,7 +453,7 @@ bool nv04_dac_in_use(struct drm_encoder
> > *encoder)
> > struct drm_device *dev = encoder->dev;
> > struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
> >
> > - return nv_gf4_disp_arch(encoder->dev) &&
> > + return nv_gf4_disp_arch(encoder->dev) && ffs(dcb->or) &&
> > (nv04_display(dev)->dac_users[ffs(dcb->or) - 1] &
> > ~(1 << dcb->index));
> > }
> >
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [PATCH 11/12] drm/client: Streamline mode selection debugs
On Fri, Apr 05, 2024 at 09:57:07AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 04.04.24 um 22:33 schrieb Ville Syrjala:
> > From: Ville Syrjälä
> >
> > Get rid of all the redundant debugs and just wait until the end
> > to print which mode (and of which type) we picked.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/drm_client_modeset.c | 65 +---
> > 1 file changed, 31 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_client_modeset.c
> > b/drivers/gpu/drm/drm_client_modeset.c
> > index 415d1799337b..ad88c11037d8 100644
> > --- a/drivers/gpu/drm/drm_client_modeset.c
> > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > @@ -408,6 +408,8 @@ static bool drm_client_target_preferred(struct
> > drm_device *dev,
> >
> > retry:
> > for (i = 0; i < connector_count; i++) {
> > + const char *mode_type;
> > +
> > connector = connectors[i];
> >
> > if (conn_configured & BIT_ULL(i))
> > @@ -440,20 +442,20 @@ static bool drm_client_target_preferred(struct
> > drm_device *dev,
> > drm_client_get_tile_offsets(dev, connectors,
> > connector_count, modes, offsets, i,
> > connector->tile_h_loc,
> > connector->tile_v_loc);
> > }
> > - drm_dbg_kms(dev, "looking for cmdline mode on
> > [CONNECTOR:%d:%s]\n",
> > - connector->base.id, connector->name);
> >
> > - /* got for command line mode first */
> > + mode_type = "cmdline";
> > modes[i] = drm_connector_pick_cmdline_mode(connector);
> > +
> > if (!modes[i]) {
> > - drm_dbg_kms(dev, "looking for preferred mode on
> > [CONNECTOR:%d:%s] (tile group: %d)\n",
> > - connector->base.id, connector->name,
> > - connector->tile_group ?
> > connector->tile_group->id : 0);
> > + mode_type = "preferred";
> > modes[i] = drm_connector_preferred_mode(connector,
> > width, height);
> > }
> > - /* No preferred modes, pick one off the list */
> > - if (!modes[i])
> > +
> > + if (!modes[i]) {
> > + mode_type = "first";
> > modes[i] = drm_connector_first_mode(connector);
> > + }
> > +
> > /*
> > * In case of tiled mode if all tiles not present fallback to
> > * first available non tiled mode.
> > @@ -468,16 +470,20 @@ static bool drm_client_target_preferred(struct
> > drm_device *dev,
> > (connector->tile_h_loc == 0 &&
> > connector->tile_v_loc == 0 &&
> > !drm_connector_get_tiled_mode(connector))) {
> > - drm_dbg_kms(dev, "Falling back to non tiled
> > mode on [CONNECTOR:%d:%s]\n",
> > - connector->base.id,
> > connector->name);
> > + mode_type = "non tiled";
> > modes[i] =
> > drm_connector_fallback_non_tiled_mode(connector);
> > } else {
> > + mode_type = "tiled";
> > modes[i] =
> > drm_connector_get_tiled_mode(connector);
> > }
> > }
> >
> > - drm_dbg_kms(dev, "found mode %s\n",
> > - modes[i] ? modes[i]->name : "none");
> > + if (!modes[i])
> > + mode_type = "no";
> > +
> > + drm_dbg_kms(dev, "[CONNECTOR:%d:%s] found %s mode: %s\n",
> > + connector->base.id, connector->name,
> > + mode_type, modes[i] ? modes[i]->name : "none");
>
> Instead of tracking the whole mode_type thing, maybe just do
>
> if (!modes[i])
> drm_dbg_kms(dev, "[CONNECTOR:%d:%s] found mode: " DRM_MODE_FMT,
> DRM_MODE_ARG(modes[i]) );
>
> to print the full mode.
The point of the mode_type is to indicate how we derived
that mode. Printing the full modeline doesn't help with that.
--
Ville Syrjälä
Intel
Re: [PATCH 04/12] drm/client: Add a FIXME around crtc->mode usage
On Fri, Apr 05, 2024 at 06:32:46AM +0300, Dmitry Baryshkov wrote:
> On Thu, Apr 04, 2024 at 11:33:28PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > crtc->mode is legacy junk and shouldn't really be used with
> > atomic drivers.
> >
> > Most (all?) atomic drivers do end up still calling
> > drm_atomic_helper_update_legacy_modeset_state() at some
> > point, so crtc->mode does still get populated, and this
> > does work for now. But eventually would be nice to eliminate
> > all the legacy stuff from atomic drivers.
> >
> > Switching to crtc->state->mode would require some bigger
> > changes however, as we currently drop the crtc->mutex
> > before we're done using the mode. So leave the junk in
> > for now and just add a FIXME to remind us that this
> > needs fixing.
>
>
> What about using allocated duplicate modes to fill modes[] array? This
> requires additional allocations, but it will solve most if not all modes
> lifetime issues.
I think there are two obvious solutions:
1. drm_mode_duplicate() as you suggest
upside: existing 'modes[i] != NULL' checks work as is
downside: introduces more error paths due to potential kmalloc() fails
2. Make modes[] and array of structs rather than pointers
up/downsides: the opposite of option 1
So neither is a trivial search and replace job.
>
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/drm_client_modeset.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_client_modeset.c
> > b/drivers/gpu/drm/drm_client_modeset.c
> > index 2b7d0be04911..8ef03608b424 100644
> > --- a/drivers/gpu/drm/drm_client_modeset.c
> > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > @@ -699,6 +699,10 @@ static bool drm_client_firmware_config(struct
> > drm_client_dev *client,
> > *
> > * This is crtc->mode and not crtc->state->mode for the
> > * fastboot check to work correctly.
> > +*
> > +* FIXME using legacy crtc->mode with atomic drivers
> > +* is dodgy. Switch to crtc->state->mode, after taking
> > +* care of the resulting locking/lifetime issues.
> > */
> > DRM_DEBUG_KMS("looking for current mode on connector
> > %s\n",
> > connector->name);
> > --
> > 2.43.2
> >
>
> --
> With best wishes
> Dmitry
--
Ville Syrjälä
Intel
Re: (subset) [PATCH v3 0/4] arm64: dts: fix several display-related schema warnings
On Tue, 02 Apr 2024 05:57:14 +0300, Dmitry Baryshkov wrote: > Fix several warnings produced by the display nodes. > > Please excuse me for the spam for sending v3 soon after v2. > > Applied, thanks! [1/4] dt-bindings: display/msm: sm8150-mdss: add DP node https://gitlab.freedesktop.org/drm/msm/-/commit/be1b7acb9291 Best regards, -- Abhinav Kumar
Re: [PATCH] drm/msm/dp: fix typo in dp_display_handle_port_status_changed()
On Wed, 06 Mar 2024 11:35:15 -0800, Abhinav Kumar wrote: > Fix the typo in the name of dp_display_handle_port_status_changed(). > > Applied, thanks! [1/1] drm/msm/dp: fix typo in dp_display_handle_port_status_changed() (no commit info) https://gitlab.freedesktop.org/drm/msm/-/commit/cd49cca222bc Best regards, -- Abhinav Kumar
Re: [PATCH] drm/msm/dpu: make error messages at dpu_core_irq_register_callback() more sensible
On Sat, 30 Mar 2024 05:53:22 +0200, Dmitry Baryshkov wrote: > There is little point in using %ps to print a value known to be NULL. On > the other hand it makes sense to print the callback symbol in the > 'invalid IRQ' message. Correct those two error messages to make more > sense. > > Applied, thanks! [1/1] drm/msm/dpu: make error messages at dpu_core_irq_register_callback() more sensible https://gitlab.freedesktop.org/drm/msm/-/commit/8844f467d6a5 Best regards, -- Abhinav Kumar
Re: [PATCH v3] drm/msm/dp: assign correct DP controller ID to x1e80100 interface table
On Fri, 29 Mar 2024 12:46:26 -0700, Kuogee Hsieh wrote: > At current x1e80100 interface table, interface #3 is wrongly > connected to DP controller #0 and interface #4 wrongly connected > to DP controller #2. Fix this problem by connect Interface #3 to > DP controller #0 and interface #4 connect to DP controller #1. > Also add interface #6, #7 and #8 connections to DP controller to > complete x1e80100 interface table. > > [...] Applied, thanks! [1/1] drm/msm/dp: assign correct DP controller ID to x1e80100 interface table https://gitlab.freedesktop.org/drm/msm/-/commit/ee15c8bf5d77 Best regards, -- Abhinav Kumar
Re: (subset) [PATCH v3 0/5] drm/msm/dpu: rework debugfs interface of dpu_core_perf
On Thu, 14 Mar 2024 03:10:40 +0200, Dmitry Baryshkov wrote: > Bring back a set of patches extracted from [1] per Abhinav's suggestion. > > Rework debugging overrides for the bandwidth and clock settings. Instead > of specifying the 'mode' and some values, allow one to set the affected > value directly. > > [1] https://patchwork.freedesktop.org/series/119552/#rev2 > > [...] Applied, thanks! [1/5] drm/msm/dpu: don't allow overriding data from catalog https://gitlab.freedesktop.org/drm/msm/-/commit/4f3b77ae5ff5 Best regards, -- Abhinav Kumar
Re: [PATCH 01/12] drm/client: Fully protect modes[] with dev->mode_config.mutex
On Fri, Apr 05, 2024 at 06:24:01AM +0300, Dmitry Baryshkov wrote:
> On Thu, Apr 04, 2024 at 11:33:25PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > The modes[] array contains pointers to modes on the connectors'
> > mode lists, which are protected by dev->mode_config.mutex.
> > Thus we need to extend modes[] the same protection or by the
> > time we use it the elements may already be pointing to
> > freed/reused memory.
> >
> > Cc: sta...@vger.kernel.org
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10583
> > Signed-off-by: Ville Syrjälä
>
> Reviewed-by: Dmitry Baryshkov
>
> I tried looking for the proper Fixes tag, but it looks like it might be
> something like 386516744ba4 ("drm/fb: fix fbdev object model + cleanup
> properly.")
The history is rather messy. I think it was originally completely
lockless and broken, and got fixed piecemeal later in these:
commit 7394371d8569 ("drm: Take lock around probes for
drm_fb_helper_hotplug_event")
commit 966a6a13c666 ("drm: Hold mode_config.lock to prevent hotplug whilst
setting up crtcs")
commit e13a05831050 ("drm/fb-helper: Stop using mode_config.mutex for
internals")
looks to me like where the race might have been re-introduced.
But didn't do a thorough analysis so not 100% sure. It's all
rather ancient history by now so a Fixes tag doesn't seem all
that useful anyway.
--
Ville Syrjälä
Intel
Re: [PATCH] drm/msm: Add newlines to some debug prints
On Mon, 25 Mar 2024 14:08:09 -0700, Stephen Boyd wrote: > These debug prints are missing newlines, leading to multiple messages > being printed on one line and hard to read logs. Add newlines to have > the debug prints on separate lines. The DBG macro used to add a newline, > but I missed that while migrating to drm_dbg wrappers. > > Applied, thanks! [1/1] drm/msm: Add newlines to some debug prints https://gitlab.freedesktop.org/drm/msm/-/commit/c588f7d67044 Best regards, -- Abhinav Kumar
Re: [PATCH 4/5] drm/vmwgfx: Fix crtc's atomic check conditional
Makes sense.
Reviewed-by: Ian Forbes
On Tue, Apr 2, 2024 at 6:28 PM Zack Rusin wrote:
>
> The conditional was supposed to prevent enabling of a crtc state
> without a set primary plane. Accidently it also prevented disabling
> crtc state with a set primary plane. Neither is correct.
>
> Fix the conditional and just driver-warn when a crtc state has been
> enabled without a primary plane which will help debug broken userspace.
>
> Fixes IGT's kms_atomic_interruptible and kms_atomic_transition tests.
>
> Signed-off-by: Zack Rusin
> Fixes: 06ec41909e31 ("drm/vmwgfx: Add and connect CRTC helper functions")
> Cc: Broadcom internal kernel review list
>
> Cc: dri-devel@lists.freedesktop.org
> Cc: # v4.12+
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e33e5993d8fc..13b2820cae51 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -931,6 +931,7 @@ int vmw_du_cursor_plane_atomic_check(struct drm_plane
> *plane,
> int vmw_du_crtc_atomic_check(struct drm_crtc *crtc,
> struct drm_atomic_state *state)
> {
> + struct vmw_private *vmw = vmw_priv(crtc->dev);
> struct drm_crtc_state *new_state =
> drm_atomic_get_new_crtc_state(state,
>
> crtc);
> struct vmw_display_unit *du = vmw_crtc_to_du(new_state->crtc);
> @@ -938,9 +939,13 @@ int vmw_du_crtc_atomic_check(struct drm_crtc *crtc,
> bool has_primary = new_state->plane_mask &
>drm_plane_mask(crtc->primary);
>
> - /* We always want to have an active plane with an active CRTC */
> - if (has_primary != new_state->enable)
> - return -EINVAL;
> + /*
> +* This is fine in general, but broken userspace might expect
> +* some actual rendering so give a clue as why it's blank.
> +*/
> + if (new_state->enable && !has_primary)
> + drm_dbg_driver(>drm,
> + "CRTC without a primary plane will be
> blank.\n");
>
>
> if (new_state->connector_mask != connector_mask &&
> --
> 2.40.1
>
Re: [PATCH 0/2] drm/msm/dp: fix runtime PM leaks on hotplug
On Wed, 13 Mar 2024 17:43:04 +0100, Johan Hovold wrote: > As I've reported elsewhere, I've been hitting runtime PM usage count > leaks when investigated a DisplayPort hotplug regression on the Lenovo > ThinkPad X13s. [1] > > This series addresses two obvious leaks on disconnect and on connect > failures, respectively. > > [...] Applied, thanks! [1/2] drm/msm/dp: fix runtime PM leak on disconnect https://gitlab.freedesktop.org/drm/msm/-/commit/0640f47b7426 [2/2] drm/msm/dp: fix runtime PM leak on connect failure https://gitlab.freedesktop.org/drm/msm/-/commit/e86750b01a15 Best regards, -- Abhinav Kumar
Re: [PATCH 1/3] drm/msm/dsi: remove the drm_bridge_attach fallback
On 4/5/2024 11:19 AM, Dmitry Baryshkov wrote:
On Fri, 5 Apr 2024 at 21:17, Abhinav Kumar wrote:
On 4/5/2024 11:16 AM, Dmitry Baryshkov wrote:
On Fri, 5 Apr 2024 at 20:20, Abhinav Kumar wrote:
On 3/9/2024 7:09 AM, Dmitry Baryshkov wrote:
All the bridges that are being used with the MSM DSI hosts have been
converted to support DRM_BRIDGE_ATTACH_NO_CONNECTOR. Drop the fallback
code and require DRM_BRIDGE_ATTACH_NO_CONNECTOR to be supported by the
downstream bridges.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi_manager.c | 36
+++
1 file changed, 11 insertions(+), 25 deletions(-)
There are the bridges I checked by looking at the dts:
1) lontium,lt9611
2) lontium,lt9611uxc
3) adi,adv7533
4) ti,sn65dsi86
Are there any more?
If not, this LGTM
Reviewed-by: Abhinav Kumar
From your message it looks more like Tested-by rather than just Reviewed-by
No, I only cross-checked the dts.
So, its only Reviewed-by :)
But I wanted to list down all the bridges
Then I'd also nominate the panel bridge to the list of bridges for
cross-checking. It is created automatically when we request a bridge,
but DT has only a panel.
Yes, that one is fine too
58 static int panel_bridge_attach(struct drm_bridge *bridge,
59 enum drm_bridge_attach_flags flags)
60 {
61 struct panel_bridge *panel_bridge = drm_bridge_to_panel_bridge(bridge);
62 struct drm_connector *connector = _bridge->connector;
63 int ret;
64
65 if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
66 return 0;
67
Re: [PATCH 1/6] drm/modes: add drm_mode_print() to dump mode in drm_printer
On Fri, Apr 05, 2024 at 10:45:42AM +0200, Thomas Zimmermann wrote: > Hi > > Am 07.03.24 um 21:39 schrieb Jani Nikula: > > Add a printer based function for dumping the modeline, so it's not > > limited to KMS debug. > > > > Note: The printed output intentionally does not have the "Modeline" > > prefix. Prefix, if any, is for the caller to decide when initializing > > drm_printer. > > > > Signed-off-by: Jani Nikula > > --- > > drivers/gpu/drm/drm_modes.c | 13 + > > include/drm/drm_modes.h | 2 ++ > > 2 files changed, 15 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > index c4f88c3a93b7..711750ab57c7 100644 > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -49,6 +49,19 @@ > > > > #include "drm_crtc_internal.h" > > > > +/** > > + * drm_mode_print - print a mode to drm printer > > + * @p: drm printer > > + * @mode: mode to print > > + * > > + * Write @mode description to struct drm_printer @p. > > + */ > > +void drm_mode_print(struct drm_printer *p, const struct drm_display_mode > > *mode) > > Could this be a printf function with a trailing format string as final > argument? The printed mode could then be part of another string instead > of just at the end of it. All this seems pretty much redundant. We already have DRM_MODE_FMT/ARGS so people can include the mode in any kind of format string they want. I would just nuke drm_mode_print() and all its ilk and let people format things themselves. -- Ville Syrjälä Intel
Re: [PATCH v0 13/14] drm/nouveau: Make I2C terminology more inclusive
On Fri, 2024-04-05 at 09:30 -0700, Easwar Hariharan wrote: > > Thanks for the review, and for the appetite to go further! So we are > on the same page, you would prefer > renaming to controller/target like the feedback on other drm drivers > (i915, gma500, radeon)? FWIW I'm in support of this as well! As long as we make sure it gets renamed everywhere :) > > Thanks, > Easwar > -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
Re: [PATCH v3 13/15] sh: Move defines needed for suppressing warning backtraces
On Wed, Apr 03, 2024 at 06:19:34AM -0700, Guenter Roeck wrote: > Declaring the defines needed for suppressing warning inside > '#ifdef CONFIG_DEBUG_BUGVERBOSE' results in a kerneldoc warning. > > .../bug.h:29: warning: expecting prototype for _EMIT_BUG_ENTRY(). > Prototype was for HAVE_BUG_FUNCTION() instead > > Move the defines above the kerneldoc entry for _EMIT_BUG_ENTRY > to make kerneldoc happy. > > Reported-by: Simon Horman > Cc: Simon Horman > Cc: Yoshinori Sato > Cc: Rich Felker > Cc: John Paul Adrian Glaubitz > Signed-off-by: Guenter Roeck > --- > v3: Added patch. Possibly squash into previous patch. FWIIW, this looks good to me. > arch/sh/include/asm/bug.h | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h > index 470ce6567d20..bf4947d51d69 100644 > --- a/arch/sh/include/asm/bug.h > +++ b/arch/sh/include/asm/bug.h > @@ -11,6 +11,15 @@ > #define HAVE_ARCH_BUG > #define HAVE_ARCH_WARN_ON > > +#ifdef CONFIG_DEBUG_BUGVERBOSE > +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE > +# define HAVE_BUG_FUNCTION > +# define __BUG_FUNC_PTR "\t.long %O2\n" > +#else > +# define __BUG_FUNC_PTR > +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */ > +#endif /* CONFIG_DEBUG_BUGVERBOSE */ > + > /** > * _EMIT_BUG_ENTRY > * %1 - __FILE__ > @@ -25,13 +34,6 @@ > */ > #ifdef CONFIG_DEBUG_BUGVERBOSE > > -#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE > -# define HAVE_BUG_FUNCTION > -# define __BUG_FUNC_PTR "\t.long %O2\n" > -#else > -# define __BUG_FUNC_PTR > -#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */ > - > #define _EMIT_BUG_ENTRY \ > "\t.pushsection __bug_table,\"aw\"\n" \ > "2:\t.long 1b, %O1\n" \ > -- > 2.39.2 >
Re: [PATCH] drm/msm: remove an unused-but-set variable
On Fri, 5 Apr 2024 at 18:59, Arnd Bergmann wrote:
>
> 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
Unfortunately this fix is not correct. The proper patch is present at
https://patchwork.freedesktop.org/patch/584955/?series=131663=1
> ---
> 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
>
--
With best wishes
Dmitry
Re: [PATCH 1/3] drm/msm/dsi: remove the drm_bridge_attach fallback
On Fri, 5 Apr 2024 at 21:17, Abhinav Kumar wrote: > > > > On 4/5/2024 11:16 AM, Dmitry Baryshkov wrote: > > On Fri, 5 Apr 2024 at 20:20, Abhinav Kumar > > wrote: > >> > >> > >> > >> On 3/9/2024 7:09 AM, Dmitry Baryshkov wrote: > >>> All the bridges that are being used with the MSM DSI hosts have been > >>> converted to support DRM_BRIDGE_ATTACH_NO_CONNECTOR. Drop the fallback > >>> code and require DRM_BRIDGE_ATTACH_NO_CONNECTOR to be supported by the > >>> downstream bridges. > >>> > >>> Signed-off-by: Dmitry Baryshkov > >>> --- > >>>drivers/gpu/drm/msm/dsi/dsi_manager.c | 36 > >>> +++ > >>>1 file changed, 11 insertions(+), 25 deletions(-) > >>> > >> > >> There are the bridges I checked by looking at the dts: > >> > >> 1) lontium,lt9611 > >> 2) lontium,lt9611uxc > >> 3) adi,adv7533 > >> 4) ti,sn65dsi86 > >> > >> Are there any more? > >> > >> If not, this LGTM > >> > >> Reviewed-by: Abhinav Kumar > > > > From your message it looks more like Tested-by rather than just Reviewed-by > > > > No, I only cross-checked the dts. > > So, its only Reviewed-by :) > > But I wanted to list down all the bridges Then I'd also nominate the panel bridge to the list of bridges for cross-checking. It is created automatically when we request a bridge, but DT has only a panel. -- With best wishes Dmitry
Re: [PATCH 1/3] drm/msm/dsi: remove the drm_bridge_attach fallback
On 4/5/2024 11:16 AM, Dmitry Baryshkov wrote: On Fri, 5 Apr 2024 at 20:20, Abhinav Kumar wrote: On 3/9/2024 7:09 AM, Dmitry Baryshkov wrote: All the bridges that are being used with the MSM DSI hosts have been converted to support DRM_BRIDGE_ATTACH_NO_CONNECTOR. Drop the fallback code and require DRM_BRIDGE_ATTACH_NO_CONNECTOR to be supported by the downstream bridges. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dsi/dsi_manager.c | 36 +++ 1 file changed, 11 insertions(+), 25 deletions(-) There are the bridges I checked by looking at the dts: 1) lontium,lt9611 2) lontium,lt9611uxc 3) adi,adv7533 4) ti,sn65dsi86 Are there any more? If not, this LGTM Reviewed-by: Abhinav Kumar From your message it looks more like Tested-by rather than just Reviewed-by No, I only cross-checked the dts. So, its only Reviewed-by :) But I wanted to list down all the bridges
Re: [PATCH 1/3] drm/msm/dsi: remove the drm_bridge_attach fallback
On Fri, 5 Apr 2024 at 20:20, Abhinav Kumar wrote: > > > > On 3/9/2024 7:09 AM, Dmitry Baryshkov wrote: > > All the bridges that are being used with the MSM DSI hosts have been > > converted to support DRM_BRIDGE_ATTACH_NO_CONNECTOR. Drop the fallback > > code and require DRM_BRIDGE_ATTACH_NO_CONNECTOR to be supported by the > > downstream bridges. > > > > Signed-off-by: Dmitry Baryshkov > > --- > > drivers/gpu/drm/msm/dsi/dsi_manager.c | 36 > > +++ > > 1 file changed, 11 insertions(+), 25 deletions(-) > > > > There are the bridges I checked by looking at the dts: > > 1) lontium,lt9611 > 2) lontium,lt9611uxc > 3) adi,adv7533 > 4) ti,sn65dsi86 > > Are there any more? > > If not, this LGTM > > Reviewed-by: Abhinav Kumar >From your message it looks more like Tested-by rather than just Reviewed-by -- With best wishes Dmitry
Re: [PATCH 2/3] drm/msm/dsi: move next bridge acquisition to dsi_bind
On Fri, 5 Apr 2024 at 20:35, Abhinav Kumar wrote:
>
>
>
> On 3/9/2024 7:09 AM, Dmitry Baryshkov wrote:
> > Currently the MSM DSI driver looks for the next bridge during
> > msm_dsi_modeset_init(). If the bridge is not registered at that point,
> > this might result in -EPROBE_DEFER, which can be problematic that late
> > during the device probe process. Move next bridge acquisition to the
> > dsi_bind state so that probe deferral is returned as early as possible.
> >
>
> But msm_dsi_modeset)init() is also called during msm_drm_bind() only.
>
> What issue are you suggesting will be fixed by moving this from
> msm_drm_bind() to dsi_bind()?
The goal is to return as early as possible as not not cause
probe-deferral loops. See commit fbc35b45f9f6 ("Add documentation on
meaning of -EPROBE_DEFER"). It discusses returning from probe() but
the same logic applies to bind().
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/dsi/dsi.c | 16
> > drivers/gpu/drm/msm/dsi/dsi.h | 2 ++
> > drivers/gpu/drm/msm/dsi/dsi_manager.c | 8 +---
> > 3 files changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
> > index 37c4c07005fe..38f10f7a10d3 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi.c
> > @@ -120,6 +120,22 @@ static int dsi_bind(struct device *dev, struct device
> > *master, void *data)
> > struct msm_drm_private *priv = dev_get_drvdata(master);
> > struct msm_dsi *msm_dsi = dev_get_drvdata(dev);
> >
> > + /*
> > + * Next bridge doesn't exist for the secondary DSI host in a bonded
> > + * pair.
> > + */
> > + if (!msm_dsi_is_bonded_dsi(msm_dsi) ||
> > + msm_dsi_is_master_dsi(msm_dsi)) {
> > + struct drm_bridge *ext_bridge;
> > +
> > + ext_bridge = devm_drm_of_get_bridge(_dsi->pdev->dev,
> > +
> > msm_dsi->pdev->dev.of_node, 1, 0);
> > + if (IS_ERR(ext_bridge))
> > + return PTR_ERR(ext_bridge);
> > +
> > + msm_dsi->next_bridge = ext_bridge;
> > + }
> > +
> > priv->dsi[msm_dsi->id] = msm_dsi;
> >
> > return 0;
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
> > index 2ad9a842c678..0adef65be1de 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi.h
> > +++ b/drivers/gpu/drm/msm/dsi/dsi.h
> > @@ -38,6 +38,8 @@ struct msm_dsi {
> > struct mipi_dsi_host *host;
> > struct msm_dsi_phy *phy;
> >
> > + struct drm_bridge *next_bridge;
> > +
> > struct device *phy_dev;
> > bool phy_enabled;
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > index a7c7f85b73e4..b7c52b14c790 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > @@ -464,18 +464,12 @@ int msm_dsi_manager_ext_bridge_init(u8 id, struct
> > drm_bridge *int_bridge)
> > struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
> > struct drm_device *dev = msm_dsi->dev;
> > struct drm_encoder *encoder;
> > - struct drm_bridge *ext_bridge;
> > struct drm_connector *connector;
> > int ret;
> >
> > - ext_bridge = devm_drm_of_get_bridge(_dsi->pdev->dev,
> > - msm_dsi->pdev->dev.of_node, 1, 0);
> > - if (IS_ERR(ext_bridge))
> > - return PTR_ERR(ext_bridge);
> > -
> > encoder = int_bridge->encoder;
> >
> > - ret = drm_bridge_attach(encoder, ext_bridge, int_bridge,
> > + ret = drm_bridge_attach(encoder, msm_dsi->next_bridge, int_bridge,
> > DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> > if (ret)
> > return ret;
> >
--
With best wishes
Dmitry
Re: [PATCH 06/23] drm/xe/svm: Introduce a helper to build sg table from hmm range
On Fri, Apr 05, 2024 at 04:42:14PM +, Zeng, Oak wrote: > > > Above codes deal with a case where dma map is not needed. As I > > > understand it, whether we need a dma map depends on the devices > > > topology. For example, when device access host memory or another > > > device's memory through pcie, we need dma mapping; if the connection > > > b/t devices is xelink (similar to nvidia's nvlink), all device's > > > memory can be in same address space, so no dma mapping is needed. > > > > Then you call dma_map_page to do your DMA side and you avoid it for > > the DEVICE_PRIVATE side. SG list doesn't help this anyhow. > > When dma map is needed, we used dma_map_sgtable, a different flavor > of the dma-map-page function. I saw, I am saying this should not be done. You cannot unmap bits of a sgl mapping if an invalidation comes in. > The reason we also used (mis-used) sg list for non-dma-map cases, is > because we want to re-use some data structure. Our goal here is, for > a hmm_range, build a list of device physical address (can be > dma-mapped or not), which will be used later on to program the > device page table. We re-used the sg list structure for the > non-dma-map cases so those two cases can share the same page table > programming codes. Since sg list was originally designed for > dma-map, it does look like this is mis-used here. Please don't use sg list at all for this. > Need to mention, even for some DEVICE_PRIVATE memory, we also need > dma-mapping. For example, if you have two devices connected to each > other through PCIe, both devices memory are registered as > DEVICE_PRIVATE to hmm. Yes, but you don't ever dma map DEVICE_PRIVATE. > I believe we need a dma-map when one device access another device's > memory. Two devices' memory doesn't belong to same address space in > this case. For modern GPU with xeLink/nvLink/XGMI, this is not > needed. Review my emails here: https://lore.kernel.org/dri-devel/20240403125712.ga1744...@nvidia.com/ Which explain how it should work. > > A 1:1 SVA mapping is a special case of this where there is a single > > GPU VMA that spans the entire process address space with a 1:1 VA (no > > offset). > > From implementation perspective, we can have one device page table > for one process for such 1:1 va mapping, but it is not necessary to > have a single gpu vma. We can have many gpu vma each cover a segment > of this address space. This is not what I'm talking about. The GPU VMA is bound to a specific MM VA, it should not be created on demand. If you want the full 1:1 SVA case to optimize invalidations you don't need something like a VMA, a simple bitmap reducing the address space into 1024 faulted in chunks or something would be much cheaper than some dynamic VMA ranges. Jason
Re: [PATCH] drm/sysfs: Add drm class-wide attribute to get active device clients
On Wed, Apr 3, 2024 at 11:37 AM Adrián Larumbe
wrote:
>
> Up to this day, all fdinfo-based GPU profilers must traverse the entire
> /proc directory structure to find open DRM clients with fdinfo file
> descriptors. This is inefficient and time-consuming.
>
> This patch adds a new device class attribute that will install a sysfs file
> per DRM device, which can be queried by profilers to get a list of PIDs for
> their open clients. This file isn't human-readable, and it's meant to be
> queried only by GPU profilers like gputop and nvtop.
>
> Cc: Boris Brezillon
> Cc: Tvrtko Ursulin
> Cc: Christopher Healy
> Signed-off-by: Adrián Larumbe
It does seem like a good idea.. idk if there is some precedent to
prefer binary vs ascii in sysfs, but having a way to avoid walking
_all_ processes is a good idea.
BR,
-R
> ---
> drivers/gpu/drm/drm_internal.h | 2 +-
> drivers/gpu/drm/drm_privacy_screen.c | 2 +-
> drivers/gpu/drm/drm_sysfs.c | 89 ++--
> 3 files changed, 74 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 2215baef9a3e..9a399b03d11c 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -145,7 +145,7 @@ bool drm_master_internal_acquire(struct drm_device *dev);
> void drm_master_internal_release(struct drm_device *dev);
>
> /* drm_sysfs.c */
> -extern struct class *drm_class;
> +extern struct class drm_class;
>
> int drm_sysfs_init(void);
> void drm_sysfs_destroy(void);
> diff --git a/drivers/gpu/drm/drm_privacy_screen.c
> b/drivers/gpu/drm/drm_privacy_screen.c
> index 6cc39e30781f..2fbd24ba5818 100644
> --- a/drivers/gpu/drm/drm_privacy_screen.c
> +++ b/drivers/gpu/drm/drm_privacy_screen.c
> @@ -401,7 +401,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
> mutex_init(>lock);
> BLOCKING_INIT_NOTIFIER_HEAD(>notifier_head);
>
> - priv->dev.class = drm_class;
> + priv->dev.class = _class;
> priv->dev.type = _privacy_screen_type;
> priv->dev.parent = parent;
> priv->dev.release = drm_privacy_screen_device_release;
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index a953f69a34b6..56ca9e22c720 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -58,8 +58,6 @@ static struct device_type drm_sysfs_device_connector = {
> .name = "drm_connector",
> };
>
> -struct class *drm_class;
> -
> #ifdef CONFIG_ACPI
> static bool drm_connector_acpi_bus_match(struct device *dev)
> {
> @@ -128,6 +126,62 @@ static const struct component_ops typec_connector_ops = {
>
> static CLASS_ATTR_STRING(version, S_IRUGO, "drm 1.1.0 20060810");
>
> +static ssize_t clients_show(struct device *cd, struct device_attribute
> *attr, char *buf)
> +{
> + struct drm_minor *minor = cd->driver_data;
> + struct drm_device *ddev = minor->dev;
> + struct drm_file *priv;
> + ssize_t offset = 0;
> + void *pid_buf;
> +
> + if (minor->type != DRM_MINOR_RENDER)
> + return 0;
> +
> + pid_buf = kvmalloc(PAGE_SIZE, GFP_KERNEL);
> + if (!pid_buf)
> + return 0;
> +
> + mutex_lock(>filelist_mutex);
> + list_for_each_entry_reverse(priv, >filelist, lhead) {
> + struct pid *pid;
> +
> + if (drm_WARN_ON(ddev, (PAGE_SIZE - offset) < sizeof(pid_t)))
> + break;
> +
> + rcu_read_lock();
> + pid = rcu_dereference(priv->pid);
> + (*(pid_t *)(pid_buf + offset)) = pid_vnr(pid);
> + rcu_read_unlock();
> +
> + offset += sizeof(pid_t);
> + }
> + mutex_unlock(>filelist_mutex);
> +
> + if (offset < PAGE_SIZE)
> + (*(pid_t *)(pid_buf + offset)) = 0;
> +
> + memcpy(buf, pid_buf, offset);
> +
> + kvfree(pid_buf);
> +
> + return offset;
> +
> +}
> +static DEVICE_ATTR_RO(clients);
> +
> +static struct attribute *drm_device_attrs[] = {
> + _attr_clients.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(drm_device);
> +
> +struct class drm_class = {
> + .name = "drm",
> + .dev_groups = drm_device_groups,
> +};
> +
> +static bool drm_class_initialised;
> +
> /**
> * drm_sysfs_init - initialize sysfs helpers
> *
> @@ -142,18 +196,19 @@ int drm_sysfs_init(void)
> {
> int err;
>
> - drm_class = class_create("drm");
> - if (IS_ERR(drm_class))
> - return PTR_ERR(drm_class);
> + err = class_register(_class);
> + if (err)
> + return err;
>
> - err = class_create_file(drm_class, _attr_version.attr);
> + err = class_create_file(_class, _attr_version.attr);
> if (err) {
> - class_destroy(drm_class);
> - drm_class = NULL;
> + class_destroy(_class);
> return err;
> }
>
> -
Re: [PATCH 2/3] drm/msm/dsi: move next bridge acquisition to dsi_bind
On 3/9/2024 7:09 AM, Dmitry Baryshkov wrote:
Currently the MSM DSI driver looks for the next bridge during
msm_dsi_modeset_init(). If the bridge is not registered at that point,
this might result in -EPROBE_DEFER, which can be problematic that late
during the device probe process. Move next bridge acquisition to the
dsi_bind state so that probe deferral is returned as early as possible.
But msm_dsi_modeset)init() is also called during msm_drm_bind() only.
What issue are you suggesting will be fixed by moving this from
msm_drm_bind() to dsi_bind()?
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi.c | 16
drivers/gpu/drm/msm/dsi/dsi.h | 2 ++
drivers/gpu/drm/msm/dsi/dsi_manager.c | 8 +---
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index 37c4c07005fe..38f10f7a10d3 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -120,6 +120,22 @@ static int dsi_bind(struct device *dev, struct device
*master, void *data)
struct msm_drm_private *priv = dev_get_drvdata(master);
struct msm_dsi *msm_dsi = dev_get_drvdata(dev);
+ /*
+* Next bridge doesn't exist for the secondary DSI host in a bonded
+* pair.
+*/
+ if (!msm_dsi_is_bonded_dsi(msm_dsi) ||
+ msm_dsi_is_master_dsi(msm_dsi)) {
+ struct drm_bridge *ext_bridge;
+
+ ext_bridge = devm_drm_of_get_bridge(_dsi->pdev->dev,
+ msm_dsi->pdev->dev.of_node,
1, 0);
+ if (IS_ERR(ext_bridge))
+ return PTR_ERR(ext_bridge);
+
+ msm_dsi->next_bridge = ext_bridge;
+ }
+
priv->dsi[msm_dsi->id] = msm_dsi;
return 0;
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 2ad9a842c678..0adef65be1de 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -38,6 +38,8 @@ struct msm_dsi {
struct mipi_dsi_host *host;
struct msm_dsi_phy *phy;
+ struct drm_bridge *next_bridge;
+
struct device *phy_dev;
bool phy_enabled;
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index a7c7f85b73e4..b7c52b14c790 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -464,18 +464,12 @@ int msm_dsi_manager_ext_bridge_init(u8 id, struct
drm_bridge *int_bridge)
struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
struct drm_device *dev = msm_dsi->dev;
struct drm_encoder *encoder;
- struct drm_bridge *ext_bridge;
struct drm_connector *connector;
int ret;
- ext_bridge = devm_drm_of_get_bridge(_dsi->pdev->dev,
- msm_dsi->pdev->dev.of_node, 1, 0);
- if (IS_ERR(ext_bridge))
- return PTR_ERR(ext_bridge);
-
encoder = int_bridge->encoder;
- ret = drm_bridge_attach(encoder, ext_bridge, int_bridge,
+ ret = drm_bridge_attach(encoder, msm_dsi->next_bridge, int_bridge,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret)
return ret;
Re: [PATCH 1/3] drm/msm/dsi: remove the drm_bridge_attach fallback
On 3/9/2024 7:09 AM, Dmitry Baryshkov wrote: All the bridges that are being used with the MSM DSI hosts have been converted to support DRM_BRIDGE_ATTACH_NO_CONNECTOR. Drop the fallback code and require DRM_BRIDGE_ATTACH_NO_CONNECTOR to be supported by the downstream bridges. Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/dsi/dsi_manager.c | 36 +++ 1 file changed, 11 insertions(+), 25 deletions(-) There are the bridges I checked by looking at the dts: 1) lontium,lt9611 2) lontium,lt9611uxc 3) adi,adv7533 4) ti,sn65dsi86 Are there any more? If not, this LGTM Reviewed-by: Abhinav Kumar
Re: [PATCH v0 00/14] Make I2C terminology more inclusive for I2C Algobit and consumers
Hi Wolfram, On 4/5/2024 3:18 AM, Wolfram Sang wrote: > Hello Easwar, > > On Fri, Mar 29, 2024 at 05:00:24PM +, Easwar Hariharan wrote: >> I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave" >> with more appropriate terms. Inspired by and following on to Wolfram's >> series to fix drivers/i2c/[1], fix the terminology for users of the >> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists >> in the specification. > > I really appreciate that you want to assist in this task to improve the > I2C core. I do. I am afraid, however, that you took the second step > before the first one, though. As I mentioned in my original cover > letter, this is not only about renaming but also improving the I2C API > (splitting up header files...). So, drivers are not a priority right > now. They can be better fixed once the core is ready. > Sorry, got excited. :) There were drivers I'd been part of that I specifically wanted to fixup, but then the scope grew to other users of algobit. > It is true that I changed quite some controller drivers within the i2c > realm. I did this to gain experience. As you also noticed quite some > questions came up. We need to agree on answers first. And once we are > happy with the answers we found, then IMO we can go outside of the i2c > realm and send patches to other subsystems referencing agreed > precedence. I intentionally did not go outside i2c yet. Since your > patches are already there, you probably want to foster them until they > are ready for inclusion. Sorry, I don't quite follow what you mean by foster in this context. Are you asking me to hold off on merging the series, or to follow through on getting it merged? Yet, regarding further patches, my suggestion > is to wait until the core is ready. That might take a while, though. > However, there is enough to discuss until the core is ready. So, your > collaboration there is highly appreciated! > >> The last patch updating the .master_xfer method to .xfer depends on >> patch 1 of Wolfram's series below, but the series is otherwise >> independent. It may make sense for the last patch to go in with > > Please drop the last patch from this series. It will nicely remove the > dependency. Also, like above, I first want to gain experience with i2c > before going to other subsystems. That was intended. > Will do, thanks! > All the best and happy hacking, > >Wolfram >
Re: [PATCH] drm: ci: fix the xfails for apq8016
On Mon, Apr 01, 2024 at 01:48:58PM -0700, Abhinav Kumar wrote: > After IGT migrating to dynamic sub-tests, the pipe prefixes > in the expected fails list are incorrect. Lets drop those > to accurately match the expected fails. > > In addition, update the xfails list to match the current passing > list. This should have ideally failed in the CI run because some > tests were marked as fail even though they passed but due to the > mismatch in test names, the matching didn't correctly work and was > resulting in those failures not being seen. > > Here is the passing pipeline for apq8016 with this change: > > https://gitlab.freedesktop.org/drm/msm/-/jobs/57050562 > > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/ci/xfails/msm-apq8016-fails.txt | 13 + > 1 file changed, 1 insertion(+), 12 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH] drm/msm: convert all pixel format logging to use %p4cc
On Fri, Apr 05, 2024 at 12:29:07PM +0300, Jani Nikula wrote: > Logging u32 pixel formats using %4.4s format string with a pointer to > the u32 is somewhat questionable, as well as dependent on byte > order. There's a kernel extension format specifier %p4cc to format 4cc > codes. Use it across the board in msm for pixel format logging. > > This should also fix the reported build warning: > > include/drm/drm_print.h:536:35: warning: '%4.4s' directive argument is > null [-Wformat-overflow=] > > Reported-by: Aishwarya TCV > Closes: https://lore.kernel.org/r/2ac758ce-a196-4e89-a397-488ba3101...@arm.com > Signed-off-by: Jani Nikula > > --- > > Tip: 'git show --color-words -w' might be the easiest way to review. > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 +++ > .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 2 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 4 ++-- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 24 +-- > drivers/gpu/drm/msm/msm_fb.c | 10 > 5 files changed, 24 insertions(+), 24 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
RE: [PATCH 06/23] drm/xe/svm: Introduce a helper to build sg table from hmm range
> -Original Message- > From: dri-devel On Behalf Of Jason > Gunthorpe > Sent: Friday, April 5, 2024 8:37 AM > To: Zeng, Oak > Cc: dri-devel@lists.freedesktop.org; intel...@lists.freedesktop.org; Brost, > Matthew ; thomas.hellst...@linux.intel.com; > Welty, Brian ; Ghimiray, Himal Prasad > ; Bommu, Krishnaiah > ; Vishwanathapura, Niranjana > ; Leon Romanovsky > Subject: Re: [PATCH 06/23] drm/xe/svm: Introduce a helper to build sg table > from > hmm range > > On Fri, Apr 05, 2024 at 03:33:10AM +, Zeng, Oak wrote: > > > > > > I didn't look at this series a lot but I wanted to make a few > > > remarks.. This I don't like quite a lot. Yes, the DMA API interaction > > > with hmm_range_fault is pretty bad, but it should not be hacked > > > around like this. Leon is working on a series to improve it: > > > > > > https://lore.kernel.org/linux-rdma/cover.1709635535.git.l...@kernel.org/ > > > > > > I completely agree above codes are really ugly. We definitely want > > to adapt to a better way. I will spend some time on above series. > > > > > > > > Please participate there too. In the mean time you should just call > > > dma_map_page for every single page like ODP does. > > > > Above codes deal with a case where dma map is not needed. As I > > understand it, whether we need a dma map depends on the devices > > topology. For example, when device access host memory or another > > device's memory through pcie, we need dma mapping; if the connection > > b/t devices is xelink (similar to nvidia's nvlink), all device's > > memory can be in same address space, so no dma mapping is needed. > > Then you call dma_map_page to do your DMA side and you avoid it for > the DEVICE_PRIVATE side. SG list doesn't help this anyhow. When dma map is needed, we used dma_map_sgtable, a different flavor of the dma-map-page function. The reason we also used (mis-used) sg list for non-dma-map cases, is because we want to re-use some data structure. Our goal here is, for a hmm_range, build a list of device physical address (can be dma-mapped or not), which will be used later on to program the device page table. We re-used the sg list structure for the non-dma-map cases so those two cases can share the same page table programming codes. Since sg list was originally designed for dma-map, it does look like this is mis-used here. Need to mention, even for some DEVICE_PRIVATE memory, we also need dma-mapping. For example, if you have two devices connected to each other through PCIe, both devices memory are registered as DEVICE_PRIVATE to hmm. I believe we need a dma-map when one device access another device's memory. Two devices' memory doesn't belong to same address space in this case. For modern GPU with xeLink/nvLink/XGMI, this is not needed. > > > > Also, I tried to follow the large discussion in the end but it was > > > quite hard to read the text in Lore for some reason. > > > > Did you mean this discussion: https://lore.kernel.org/dri- > devel/?q=Making+drm_gpuvm+work+across+gpu+devices? This link works good > for me with chrome browser. > > That is the one I am referring to > > > > I would just opine some general points on how I see hmm_range_fault > > > being used by drivers. > > > > > > First of all, the device should have a private page table. At least > > > one, but ideally many. Obviously it should work, so I found it a bit > > > puzzling the talk about problems with virtualization. Either the > > > private page table works virtualized, including faults, or it should > > > not be available.. > > > > To be very honest, I was also very confused. In this series, I had > > one very fundamental assumption that with hmm any valid cpu virtual > > address is also a valid gpu virtual address. But Christian had a > > very strong opinion that the gpu va can have an offset to cpu va. He > > mentioned a failed use case with amdkfd and claimed an offset can > > solve their problem. > > Offset is something different, I said the VM's view of the page table > should fully work. You shouldn't get into a weird situation where the > VM is populating the page table and can't handle faults or something. > We don't have such weird situation. There are two layers of translations when run under virtualized environment. From guest VM's perspective, the first level page table is in the guest device physical address space. It is nothing different from bare-metal situation. Our driver doesn't need to know it runs under virtualized or bare-metal for first level page table programming and page fault handling. > If the VMM has a weird design where there is only one page table and > it needs to partition space based on slicing it into regions then > fine, but the delegated region to the guest OS should still work > fully. Agree. > > > > Or it is a selective mirror where it copies part of the mm page table > > > into a "vma" of a possibly shared device page table. The > > > hmm_range_fault bit would exclusively own
Re: [PATCH] [v2] nouveau: fix function cast warning
On 4/4/24 18:02, Arnd Bergmann wrote:
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
Applied to drm-misc-fixes, thanks!
---
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,
Re: [PATCH] nouveau/gsp: Avoid addressing beyond end of rpc->entries
On 3/30/24 15:12, Kees Cook wrote:
Using the end of rpc->entries[] for addressing runs into both compile-time
and run-time detection of accessing beyond the end of the array. Use the
base pointer instead, since was allocated with the additional bytes for
storing the strings. Avoids the following warning in future GCC releases
with support for __counted_by:
In function 'fortify_memcpy_chk',
inlined from 'r535_gsp_rpc_set_registry' at
../drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1123:3:
../include/linux/fortify-string.h:553:25: error: call to
'__write_overflow_field' declared with attribute warning: detected write beyond
size of field (1st parameter); maybe use struct_group()?
[-Werror=attribute-warning]
553 | __write_overflow_field(p_size_field, size);
| ^~
for this code:
strings = (char *)>entries[NV_GSP_REG_NUM_ENTRIES];
...
memcpy(strings, r535_registry_entries[i].name, name_len);
Signed-off-by: Kees Cook
Applied to drm-misc-fixes, thanks!
---
Cc: Karol Herbst
Cc: Lyude Paul
Cc: Danilo Krummrich
Cc: David Airlie
Cc: Daniel Vetter
Cc: Dave Airlie
Cc: Ben Skeggs
Cc: Timur Tabi
Cc: dri-devel@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
---
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 9994cbd6f1c4..9858c1438aa7 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -1112,7 +1112,7 @@ r535_gsp_rpc_set_registry(struct nvkm_gsp *gsp)
rpc->numEntries = NV_GSP_REG_NUM_ENTRIES;
str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]);
- strings = (char *)>entries[NV_GSP_REG_NUM_ENTRIES];
+ strings = (char *)rpc + str_offset;
for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) {
int name_len = strlen(r535_registry_entries[i].name) + 1;
Re: [PATCH v0 13/14] drm/nouveau: Make I2C terminology more inclusive
On 4/5/2024 9:15 AM, Danilo Krummrich wrote:
> Hi Easwar,
>
> On 3/29/24 18:00, Easwar Hariharan wrote:
>> I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave"
>> with more appropriate terms. Inspired by and following on to Wolfram's
>> series to fix drivers/i2c/[1], fix the terminology for users of
>> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists
>> in the specification.
>>
>> Compile tested, no functionality changes intended
>>
>> [1]:
>> https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/
>>
>> Signed-off-by: Easwar Hariharan
>> ---
>> drivers/gpu/drm/nouveau/dispnv04/dfp.c | 14 +++---
>> .../gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h | 2 +-
>> drivers/gpu/drm/nouveau/nouveau_bios.c | 4 ++--
>> 3 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> index d5b129dc623b..65b791006b19 100644
>> --- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> +++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
>> @@ -149,7 +149,7 @@ void nv04_dfp_update_fp_control(struct drm_encoder
>> *encoder, int mode)
>> }
>> }
>> -static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
>> +static struct drm_encoder *get_tmds_client(struct drm_encoder *encoder)
>> {
>> struct drm_device *dev = encoder->dev;
>> struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
>> @@ -172,7 +172,7 @@ static struct drm_encoder *get_tmds_slave(struct
>> drm_encoder *encoder)
>> struct dcb_output *slave_dcb = nouveau_encoder(slave)->dcb;
>> if (slave_dcb->type == DCB_OUTPUT_TMDS && get_slave_funcs(slave)
>> &&
>> - slave_dcb->tmdsconf.slave_addr == dcb->tmdsconf.slave_addr)
>> + slave_dcb->tmdsconf.client_addr == dcb->tmdsconf.client_addr)
>> return slave;
>
> While, personally, I think master/slave was well suiting for the device
> relationship
> on those busses, I think that if we change it up to conform with the change in
> specification, we should make sure to update drivers consistently.
>
> We should make sure to also change the names of the sourrounding structures
> and variable
> names, otherwise we just make this code harder to read.
>
> - Danilo
>
Thanks for the review, and for the appetite to go further! So we are on the
same page, you would prefer
renaming to controller/target like the feedback on other drm drivers (i915,
gma500, radeon)?
Thanks,
Easwar
Re: [Intel-gfx] [PATCH v5 0/7] Introduce __xchg, non-atomic xchg
On 05.04.2024 16:47, Jani Nikula wrote: On Mon, 27 Feb 2023, Peter Zijlstra wrote: On Thu, Feb 23, 2023 at 10:24:19PM +0100, Andrzej Hajda wrote: On 22.02.2023 18:04, Peter Zijlstra wrote: On Wed, Jan 18, 2023 at 04:35:22PM +0100, Andrzej Hajda wrote: Andrzej Hajda (7): arch: rename all internal names __xchg to __arch_xchg linux/include: add non-atomic version of xchg arch/*/uprobes: simplify arch_uretprobe_hijack_return_addr llist: simplify __llist_del_all io_uring: use __xchg if possible qed: use __xchg if possible drm/i915/gt: use __xchg instead of internal helper Nothing crazy in here I suppose, I somewhat wonder why you went through the trouble, but meh. If you are asking why I have proposed this patchset, then the answer is simple, 1st I've tried to find a way to move internal i915 helper to core (see patch 7). Then I was looking for possible other users of this helper. And apparently there are many of them, patches 3-7 shows some. You want me to take this through te locking tree (for the next cycle, not this one) where I normally take atomic things or does someone else want this? If you could take it I will be happy. OK, I'll go queue it in tip/locking/core after -rc1. Thanks! Is this where the series fell between the cracks, or was there some follow-up that I missed? I think this would still be useful. Andrzej, would you mind rebasing and resending if there are no objections? The patchset was rejected/dropped by Linus at the pull-request stage. He didn't like many things, but the most __xchg name. However he was quite positive about i915 name fetch_and_zero. I can try to revive patchset with fetch_and_zero, and maybe fetch_and_set, instead of __xchg. Regards Andrzej BR, Jani.
Re: [PATCH v0 13/14] drm/nouveau: Make I2C terminology more inclusive
Hi Easwar,
On 3/29/24 18:00, Easwar Hariharan wrote:
I2C v7, SMBus 3.2, and I3C specifications have replaced "master/slave"
with more appropriate terms. Inspired by and following on to Wolfram's
series to fix drivers/i2c/[1], fix the terminology for users of
I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists
in the specification.
Compile tested, no functionality changes intended
[1]:
https://lore.kernel.org/all/20240322132619.6389-1-wsa+rene...@sang-engineering.com/
Signed-off-by: Easwar Hariharan
---
drivers/gpu/drm/nouveau/dispnv04/dfp.c | 14 +++---
.../gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h | 2 +-
drivers/gpu/drm/nouveau/nouveau_bios.c | 4 ++--
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
index d5b129dc623b..65b791006b19 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
@@ -149,7 +149,7 @@ void nv04_dfp_update_fp_control(struct drm_encoder
*encoder, int mode)
}
}
-static struct drm_encoder *get_tmds_slave(struct drm_encoder *encoder)
+static struct drm_encoder *get_tmds_client(struct drm_encoder *encoder)
{
struct drm_device *dev = encoder->dev;
struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
@@ -172,7 +172,7 @@ static struct drm_encoder *get_tmds_slave(struct
drm_encoder *encoder)
struct dcb_output *slave_dcb = nouveau_encoder(slave)->dcb;
if (slave_dcb->type == DCB_OUTPUT_TMDS && get_slave_funcs(slave) &&
- slave_dcb->tmdsconf.slave_addr == dcb->tmdsconf.slave_addr)
+ slave_dcb->tmdsconf.client_addr ==
dcb->tmdsconf.client_addr)
return slave;
While, personally, I think master/slave was well suiting for the device
relationship
on those busses, I think that if we change it up to conform with the change in
specification, we should make sure to update drivers consistently.
We should make sure to also change the names of the sourrounding structures and
variable
names, otherwise we just make this code harder to read.
- Danilo
}
@@ -471,7 +471,7 @@ static void nv04_dfp_commit(struct drm_encoder *encoder)
NVWriteRAMDAC(dev, 0, NV_PRAMDAC_TEST_CONTROL +
nv04_dac_output_offset(encoder), 0x0010);
/* Init external transmitters */
- slave_encoder = get_tmds_slave(encoder);
+ slave_encoder = get_tmds_client(encoder);
if (slave_encoder)
get_slave_funcs(slave_encoder)->mode_set(
slave_encoder, _encoder->mode, _encoder->mode);
@@ -621,7 +621,7 @@ static void nv04_dfp_destroy(struct drm_encoder *encoder)
kfree(nv_encoder);
}
-static void nv04_tmds_slave_init(struct drm_encoder *encoder)
+static void nv04_tmds_client_init(struct drm_encoder *encoder)
{
struct drm_device *dev = encoder->dev;
struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
@@ -632,7 +632,7 @@ static void nv04_tmds_slave_init(struct drm_encoder
*encoder)
{
{
.type = "sil164",
- .addr = (dcb->tmdsconf.slave_addr == 0x7 ? 0x3a : 0x38),
+ .addr = (dcb->tmdsconf.client_addr == 0x7 ? 0x3a :
0x38),
.platform_data = &(struct sil164_encoder_params) {
SIL164_INPUT_EDGE_RISING
}
@@ -642,7 +642,7 @@ static void nv04_tmds_slave_init(struct drm_encoder
*encoder)
};
int type;
- if (!nv_gf4_disp_arch(dev) || !bus || get_tmds_slave(encoder))
+ if (!nv_gf4_disp_arch(dev) || !bus || get_tmds_client(encoder))
return;
type = nvkm_i2c_bus_probe(bus, "TMDS transmitter", info, NULL, NULL);
@@ -716,7 +716,7 @@ nv04_dfp_create(struct drm_connector *connector, struct
dcb_output *entry)
if (entry->type == DCB_OUTPUT_TMDS &&
entry->location != DCB_LOC_ON_CHIP)
- nv04_tmds_slave_init(encoder);
+ nv04_tmds_client_init(encoder);
drm_connector_attach_encoder(connector, encoder);
return 0;
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
index 73f9d9947e7e..5da40cf74b1a 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/bios/dcb.h
@@ -50,7 +50,7 @@ struct dcb_output {
} dpconf;
struct {
struct sor_conf sor;
- int slave_addr;
+ int client_addr;
} tmdsconf;
};
bool i2c_upper_default;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c
b/drivers/gpu/drm/nouveau/nouveau_bios.c
index 479effcf607e..a91a5d3df3ca 100644
---
[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] drm: nv04: Add check to avoid out of bounds access
On 3/31/24 08:45, Mikhail Kobuk wrote:
Output Resource (dcb->or) value is not guaranteed to be non-zero (i.e.
in drivers/gpu/drm/nouveau/nouveau_bios.c, in 'fabricate_dcb_encoder_table()'
'dcb->or' is assigned value '0' in call to 'fabricate_dcb_output()').
I don't really know much about the semantics of this code.
Looking at fabricate_dcb_output() though I wonder if the intention was to assign
BIT(or) to entry->or.
@Lyude, can you help here?
Otherwise, for parsing the DCB entries, it seems that the bound checks are
happening in olddcb_outp_foreach() [1].
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/nouveau/nouveau_bios.c#L1331
Add check to validate 'dcb->or' before it's used.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 2e5702aff395 ("drm/nouveau: fabricate DCB encoder table for iMac G4")
Signed-off-by: Mikhail Kobuk
---
drivers/gpu/drm/nouveau/dispnv04/dac.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/dac.c
b/drivers/gpu/drm/nouveau/dispnv04/dac.c
index d6b8e0cce2ac..0c8d4fc95ff3 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/dac.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/dac.c
@@ -428,7 +428,7 @@ void nv04_dac_update_dacclk(struct drm_encoder *encoder,
bool enable)
struct drm_device *dev = encoder->dev;
struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
- if (nv_gf4_disp_arch(dev)) {
+ if (nv_gf4_disp_arch(dev) && ffs(dcb->or)) {
uint32_t *dac_users =
_display(dev)->dac_users[ffs(dcb->or) - 1];
int dacclk_off = NV_PRAMDAC_DACCLK +
nv04_dac_output_offset(encoder);
uint32_t dacclk = NVReadRAMDAC(dev, 0, dacclk_off);
@@ -453,7 +453,7 @@ bool nv04_dac_in_use(struct drm_encoder *encoder)
struct drm_device *dev = encoder->dev;
struct dcb_output *dcb = nouveau_encoder(encoder)->dcb;
- return nv_gf4_disp_arch(encoder->dev) &&
+ return nv_gf4_disp_arch(encoder->dev) && ffs(dcb->or) &&
(nv04_display(dev)->dac_users[ffs(dcb->or) - 1] & ~(1 <<
dcb->index));
}
Re: [PATCH v2 0/3] accel/qaic: Add debugfs entries
On 3/22/2024 11:57 AM, Jeffrey Hugo wrote: Add 3 debugfs entries that can be useful in debugging a variety of issues. bootlog - output the device bootloader log fifo_size - output the configured dbc fifo size queued - output how many requests are queued in the dbc fifo Bootlog is unique to the device, where as fifo_size/queued is per-dbc. v2: -Use size_add() for bounds check -Move locking into reset_bootlog() -Clamp num dbcs supported to 256 to address a sprintf warning Jeffrey Hugo (3): accel/qaic: Add bootlog debugfs accel/qaic: Add fifo size debugfs accel/qaic: Add fifo queued debugfs drivers/accel/qaic/Makefile | 2 + drivers/accel/qaic/qaic.h | 9 + drivers/accel/qaic/qaic_data.c| 9 + drivers/accel/qaic/qaic_debugfs.c | 338 ++ drivers/accel/qaic/qaic_debugfs.h | 20 ++ drivers/accel/qaic/qaic_drv.c | 16 +- 6 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 drivers/accel/qaic/qaic_debugfs.c create mode 100644 drivers/accel/qaic/qaic_debugfs.h Pushed to drm-misc-next.
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 0/1] vgacon: Handle HAS_IOPORT dependencies
Hi Greg, Helge, This is a follow up in my ongoing effort of making inb()/outb() and similar I/O port accessors compile-time optional. Previously I sent this as a treewide series titled "treewide: Remove I/O port accessors for HAS_IOPORT=n" with the latest being its 5th version[0]. With a significant subset of patches merged I've changed over to per-subsystem series. These series are stand alone and should be merged via the relevant tree such that with all subsystems complete we can follow this up with the final patch that will make the I/O port accessors compile-time optional. The current state of the full series with changes to the remaining subsystems and the aforementioned final patch can be found for your convenience on my git.kernel.org tree in the has_ioport branch[1]. As for compile-time vs runtime see Linus' reply to my first attempt[2]. Thanks, Niklas [0] https://lore.kernel.org/all/20230522105049.1467313-1-schne...@linux.ibm.com/ [1] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/log/?h=has_ioport [2] https://lore.kernel.org/lkml/CAHk-=wg80je=k7madf4e7wrrnp37e3qh6y10svhdc7o8sz_...@mail.gmail.com/ Niklas Schnelle (1): vgacon: add HAS_IOPORT dependencies drivers/video/console/Kconfig | 1 + 1 file changed, 1 insertion(+) -- 2.40.1
[PATCH 1/1] vgacon: add HAS_IOPORT dependencies
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. drivers/video/console/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig index bc31db6ef7d2..a053a2de4432 100644 --- a/drivers/video/console/Kconfig +++ b/drivers/video/console/Kconfig @@ -10,6 +10,7 @@ config VGA_CONSOLE depends on ALPHA || X86 || \ (ARM && ARCH_FOOTBRIDGE) || \ (MIPS && (MIPS_MALTA || SIBYTE_BCM112X || SIBYTE_SB1250 || SIBYTE_BCM1x80 || SNI_RM)) + depends on HAS_IOPORT select APERTURE_HELPERS if (DRM || FB || VFIO_PCI_CORE) default y help -- 2.40.1
Re: [PATCH v9 1/3] drm/buddy: Implement tracking clear page feature
On 01/04/2024 12:07, Paneer Selvam, Arunpravin wrote:
Hi Matthew,
On 3/28/2024 10:18 PM, Matthew Auld wrote:
On 28/03/2024 16:07, Paneer Selvam, Arunpravin wrote:
Hi Matthew,
On 3/26/2024 11:39 PM, Matthew Auld wrote:
On 18/03/2024 21:40, Arunpravin Paneer Selvam wrote:
- Add tracking clear page feature.
- Driver should enable the DRM_BUDDY_CLEARED flag if it
successfully clears the blocks in the free path. On the otherhand,
DRM buddy marks each block as cleared.
- Track the available cleared pages size
- If driver requests cleared memory we prefer cleared memory
but fallback to uncleared if we can't find the cleared blocks.
when driver requests uncleared memory we try to use uncleared but
fallback to cleared memory if necessary.
- When a block gets freed we clear it and mark the freed block as
cleared,
when there are buddies which are cleared as well we can merge them.
Otherwise, we prefer to keep the blocks as separated.
- Add a function to support defragmentation.
v1:
- Depends on the flag check DRM_BUDDY_CLEARED, enable the block as
cleared. Else, reset the clear flag for each block in the
list(Christian)
- For merging the 2 cleared blocks compare as below,
drm_buddy_is_clear(block) != drm_buddy_is_clear(buddy)(Christian)
- Defragment the memory beginning from min_order
till the required memory space is available.
v2: (Matthew)
- Add a wrapper drm_buddy_free_list_internal for the freeing of
blocks
operation within drm buddy.
- Write a macro block_incompatible() to allocate the required
blocks.
- Update the xe driver for the drm_buddy_free_list change in
arguments.
- add a warning if the two blocks are incompatible on
defragmentation
- call full defragmentation in the fini() function
- place a condition to test if min_order is equal to 0
- replace the list with safe_reverse() variant as we might
remove the block from the list.
v3:
- fix Gitlab user reported lockup issue.
- Keep DRM_BUDDY_HEADER_CLEAR define sorted(Matthew)
- modify to pass the root order instead max_order in fini()
function(Matthew)
- change bool 1 to true(Matthew)
- add check if min_block_size is power of 2(Matthew)
- modify the min_block_size datatype to u64(Matthew)
v4:
- rename the function drm_buddy_defrag with __force_merge.
- Include __force_merge directly in drm buddy file and remove
the defrag use in amdgpu driver.
- Remove list_empty() check(Matthew)
- Remove unnecessary space, headers and placement of new
variables(Matthew)
- Add a unit test case(Matthew)
Signed-off-by: Arunpravin Paneer Selvam
Signed-off-by: Matthew Auld
Suggested-by: Christian König
Suggested-by: Matthew Auld
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 6 +-
drivers/gpu/drm/drm_buddy.c | 427
++
drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 6 +-
drivers/gpu/drm/tests/drm_buddy_test.c | 18 +-
drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 4 +-
include/drm/drm_buddy.h | 16 +-
6 files changed, 360 insertions(+), 117 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 8db880244324..c0c851409241 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -571,7 +571,7 @@ static int amdgpu_vram_mgr_new(struct
ttm_resource_manager *man,
return 0;
error_free_blocks:
- drm_buddy_free_list(mm, >blocks);
+ drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
error_fini:
ttm_resource_fini(man, >base);
@@ -604,7 +604,7 @@ static void amdgpu_vram_mgr_del(struct
ttm_resource_manager *man,
amdgpu_vram_mgr_do_reserve(man);
- drm_buddy_free_list(mm, >blocks);
+ drm_buddy_free_list(mm, >blocks, 0);
mutex_unlock(>lock);
atomic64_sub(vis_usage, >vis_usage);
@@ -912,7 +912,7 @@ void amdgpu_vram_mgr_fini(struct amdgpu_device
*adev)
kfree(rsv);
list_for_each_entry_safe(rsv, temp, >reserved_pages,
blocks) {
- drm_buddy_free_list(>mm, >allocated);
+ drm_buddy_free_list(>mm, >allocated, 0);
kfree(rsv);
}
if (!adev->gmc.is_app_apu)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index c4222b886db7..625a30a6b855 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -38,8 +38,8 @@ static void drm_block_free(struct drm_buddy *mm,
kmem_cache_free(slab_blocks, block);
}
-static void list_insert_sorted(struct drm_buddy *mm,
- struct drm_buddy_block *block)
+static void list_insert(struct drm_buddy *mm,
+ struct drm_buddy_block *block)
{
struct drm_buddy_block *node;
struct list_head *head;
@@ -57,6 +57,16 @@ static void list_insert_sorted(struct drm_buddy
*mm,
__list_add(>link,
Re: [linux-next:master] BUILD REGRESSION 8568bb2ccc278f344e6ac44af6ed010a90aa88dc
On Fri, Apr 5, 2024 at 8:37 AM kernel test robot wrote: > > tree/branch: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > branch HEAD: 8568bb2ccc278f344e6ac44af6ed010a90aa88dc Add linux-next > specific files for 20240405 > > Error/Warning reports: > > https://lore.kernel.org/oe-kbuild-all/202404051333.7und7ppw-...@intel.com > https://lore.kernel.org/oe-kbuild-all/202404051423.eiaxlwhx-...@intel.com > https://lore.kernel.org/oe-kbuild-all/202404051659.aawukguq-...@intel.com > https://lore.kernel.org/oe-kbuild-all/202404052022.cwf2ilbp-...@intel.com > > Error/Warning: (recently discovered and may have been fixed) > > aarch64-linux-ld: kernel/bpf/verifier.c:20223:(.text+0xdbb4): undefined > reference to `__SCK__perf_snapshot_branch_stack' > aarch64-linux-ld: verifier.c:(.text+0x17c3c): undefined reference to > `__SCK__perf_snapshot_branch_stack' > drivers/i2c/busses/i2c-i801.c:1407:(.text+0x1d2ef4a): undefined reference to > `i2c_root_adapter' > kernel/bpf/verifier.c:20223:(.text+0xdba4): dangerous relocation: unsupported > relocation > loongarch64-linux-ld: kernel/bpf/verifier.c:20223:(.text+0xa818): undefined > reference to `__SCK__perf_snapshot_branch_stack' > loongarch64-linux-ld: verifier.c:(.text+0xa964): undefined reference to > `__SCK__perf_snapshot_branch_stack' > mips64el-linux-ld: verifier.c:(.text.do_misc_fixups+0xd9c): undefined > reference to `__SCK__perf_snapshot_branch_stack' > riscv32-linux-ld: section .data LMA [00369000,00907967] overlaps section > .text LMA [0007899c,01a6a6af] > s390-linux-ld: verifier.c:(.text+0x13038): undefined reference to > `__SCK__perf_snapshot_branch_stack' > verifier.c:(.text+0x17c14): relocation truncated to fit: > R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol > `__SCK__perf_snapshot_branch_stack' > verifier.c:(.text+0xa960): undefined reference to > `__SCK__perf_snapshot_branch_stack' > verifier.c:(.text+0xadd0): dangerous relocation: unsupported relocation > verifier.c:(.text.do_misc_fixups+0xd98): undefined reference to > `__SCK__perf_snapshot_branch_stack' Fixed in bpf-next with commit: https://lore.kernel.org/all/20240405142637.577046-1-a...@kernel.org/
[linux-next:master] BUILD REGRESSION 8568bb2ccc278f344e6ac44af6ed010a90aa88dc
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 8568bb2ccc278f344e6ac44af6ed010a90aa88dc Add linux-next specific files for 20240405 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202404051333.7und7ppw-...@intel.com https://lore.kernel.org/oe-kbuild-all/202404051423.eiaxlwhx-...@intel.com https://lore.kernel.org/oe-kbuild-all/202404051659.aawukguq-...@intel.com https://lore.kernel.org/oe-kbuild-all/202404052022.cwf2ilbp-...@intel.com Error/Warning: (recently discovered and may have been fixed) aarch64-linux-ld: kernel/bpf/verifier.c:20223:(.text+0xdbb4): undefined reference to `__SCK__perf_snapshot_branch_stack' aarch64-linux-ld: verifier.c:(.text+0x17c3c): undefined reference to `__SCK__perf_snapshot_branch_stack' drivers/i2c/busses/i2c-i801.c:1407:(.text+0x1d2ef4a): undefined reference to `i2c_root_adapter' kernel/bpf/verifier.c:20223:(.text+0xdba4): dangerous relocation: unsupported relocation loongarch64-linux-ld: kernel/bpf/verifier.c:20223:(.text+0xa818): undefined reference to `__SCK__perf_snapshot_branch_stack' loongarch64-linux-ld: verifier.c:(.text+0xa964): undefined reference to `__SCK__perf_snapshot_branch_stack' mips64el-linux-ld: verifier.c:(.text.do_misc_fixups+0xd9c): undefined reference to `__SCK__perf_snapshot_branch_stack' riscv32-linux-ld: section .data LMA [00369000,00907967] overlaps section .text LMA [0007899c,01a6a6af] s390-linux-ld: verifier.c:(.text+0x13038): undefined reference to `__SCK__perf_snapshot_branch_stack' verifier.c:(.text+0x17c14): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `__SCK__perf_snapshot_branch_stack' verifier.c:(.text+0xa960): undefined reference to `__SCK__perf_snapshot_branch_stack' verifier.c:(.text+0xadd0): dangerous relocation: unsupported relocation verifier.c:(.text.do_misc_fixups+0xd98): undefined reference to `__SCK__perf_snapshot_branch_stack' Unverified Error/Warning (likely false positive, please contact us if interested): lib/alloc_tag.c:142 alloc_tag_init() warn: passing zero to 'PTR_ERR' Error/Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- alpha-randconfig-r123-20240405 | `-- kernel-bpf-verifier.c:sparse:sparse:cast-truncates-bits-from-constant-value-(fffc-becomes-) |-- arm-allmodconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- arm-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- arm-hisi_defconfig | |-- drm_dp_mst_topology.c:(.text):undefined-reference-to-__drm_atomic_helper_private_obj_duplicate_state | `-- drm_dp_mst_topology.c:(.text):undefined-reference-to-drm_kms_helper_hotplug_event |-- arm64-randconfig-c041-20221104 | |-- aarch64-linux-ld:kernel-bpf-verifier.c:(.text):undefined-reference-to-__SCK__perf_snapshot_branch_stack | `-- kernel-bpf-verifier.c:(.text):dangerous-relocation:unsupported-relocation |-- arm64-randconfig-r013-20230703 | |-- aarch64-linux-ld:verifier.c:(.text):undefined-reference-to-__SCK__perf_snapshot_branch_stack | `-- verifier.c:(.text):relocation-truncated-to-fit:R_AARCH64_ADR_PREL_PG_HI21-against-undefined-symbol-__SCK__perf_snapshot_branch_stack |-- arm64-randconfig-r032-20220702 | `-- verifier.c:(.text):dangerous-relocation:unsupported-relocation |-- csky-allmodconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- csky-allyesconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- loongarch-allmodconfig | |-- drivers-gpu-drm-imx-ipuv3-imx-ldb.c:error:_sel-directive-output-may-be-truncated-writing-bytes-into-a-region-of-size-between-and | `-- drivers-gpu-drm-nouveau-nouveau_backlight.c:error:d-directive-output-may-be-truncated-writing-between-and-bytes-into-a-region-of-size |-- loongarch
[PATCH v2 1/3] drm/lima: add mask irq callback to gp and pp
This is needed because we want to reset those devices in device-agnostic
code such as lima_sched.
In particular, masking irqs will be useful before a hard reset to
prevent race conditions.
Signed-off-by: Erico Nunes
---
drivers/gpu/drm/lima/lima_bcast.c | 12
drivers/gpu/drm/lima/lima_bcast.h | 3 +++
drivers/gpu/drm/lima/lima_gp.c| 8
drivers/gpu/drm/lima/lima_pp.c| 18 ++
drivers/gpu/drm/lima/lima_sched.h | 1 +
5 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/lima/lima_bcast.c
b/drivers/gpu/drm/lima/lima_bcast.c
index fbc43f243c54..6d000504e1a4 100644
--- a/drivers/gpu/drm/lima/lima_bcast.c
+++ b/drivers/gpu/drm/lima/lima_bcast.c
@@ -43,6 +43,18 @@ void lima_bcast_suspend(struct lima_ip *ip)
}
+int lima_bcast_mask_irq(struct lima_ip *ip)
+{
+ bcast_write(LIMA_BCAST_BROADCAST_MASK, 0);
+ bcast_write(LIMA_BCAST_INTERRUPT_MASK, 0);
+ return 0;
+}
+
+int lima_bcast_reset(struct lima_ip *ip)
+{
+ return lima_bcast_hw_init(ip);
+}
+
int lima_bcast_init(struct lima_ip *ip)
{
int i;
diff --git a/drivers/gpu/drm/lima/lima_bcast.h
b/drivers/gpu/drm/lima/lima_bcast.h
index 465ee587bceb..cd08841e4787 100644
--- a/drivers/gpu/drm/lima/lima_bcast.h
+++ b/drivers/gpu/drm/lima/lima_bcast.h
@@ -13,4 +13,7 @@ void lima_bcast_fini(struct lima_ip *ip);
void lima_bcast_enable(struct lima_device *dev, int num_pp);
+int lima_bcast_mask_irq(struct lima_ip *ip);
+int lima_bcast_reset(struct lima_ip *ip);
+
#endif
diff --git a/drivers/gpu/drm/lima/lima_gp.c b/drivers/gpu/drm/lima/lima_gp.c
index 6b354e2fb61d..e15295071533 100644
--- a/drivers/gpu/drm/lima/lima_gp.c
+++ b/drivers/gpu/drm/lima/lima_gp.c
@@ -233,6 +233,13 @@ static void lima_gp_task_mmu_error(struct lima_sched_pipe
*pipe)
lima_sched_pipe_task_done(pipe);
}
+static void lima_gp_task_mask_irq(struct lima_sched_pipe *pipe)
+{
+ struct lima_ip *ip = pipe->processor[0];
+
+ gp_write(LIMA_GP_INT_MASK, 0);
+}
+
static int lima_gp_task_recover(struct lima_sched_pipe *pipe)
{
struct lima_ip *ip = pipe->processor[0];
@@ -365,6 +372,7 @@ int lima_gp_pipe_init(struct lima_device *dev)
pipe->task_error = lima_gp_task_error;
pipe->task_mmu_error = lima_gp_task_mmu_error;
pipe->task_recover = lima_gp_task_recover;
+ pipe->task_mask_irq = lima_gp_task_mask_irq;
return 0;
}
diff --git a/drivers/gpu/drm/lima/lima_pp.c b/drivers/gpu/drm/lima/lima_pp.c
index d0d2db0ef1ce..a4a2ffe6527c 100644
--- a/drivers/gpu/drm/lima/lima_pp.c
+++ b/drivers/gpu/drm/lima/lima_pp.c
@@ -429,6 +429,9 @@ static void lima_pp_task_error(struct lima_sched_pipe *pipe)
lima_pp_hard_reset(ip);
}
+
+ if (pipe->bcast_processor)
+ lima_bcast_reset(pipe->bcast_processor);
}
static void lima_pp_task_mmu_error(struct lima_sched_pipe *pipe)
@@ -437,6 +440,20 @@ static void lima_pp_task_mmu_error(struct lima_sched_pipe
*pipe)
lima_sched_pipe_task_done(pipe);
}
+static void lima_pp_task_mask_irq(struct lima_sched_pipe *pipe)
+{
+ int i;
+
+ for (i = 0; i < pipe->num_processor; i++) {
+ struct lima_ip *ip = pipe->processor[i];
+
+ pp_write(LIMA_PP_INT_MASK, 0);
+ }
+
+ if (pipe->bcast_processor)
+ lima_bcast_mask_irq(pipe->bcast_processor);
+}
+
static struct kmem_cache *lima_pp_task_slab;
static int lima_pp_task_slab_refcnt;
@@ -468,6 +485,7 @@ int lima_pp_pipe_init(struct lima_device *dev)
pipe->task_fini = lima_pp_task_fini;
pipe->task_error = lima_pp_task_error;
pipe->task_mmu_error = lima_pp_task_mmu_error;
+ pipe->task_mask_irq = lima_pp_task_mask_irq;
return 0;
}
diff --git a/drivers/gpu/drm/lima/lima_sched.h
b/drivers/gpu/drm/lima/lima_sched.h
index 6bd4f3b70109..85b23ba901d5 100644
--- a/drivers/gpu/drm/lima/lima_sched.h
+++ b/drivers/gpu/drm/lima/lima_sched.h
@@ -80,6 +80,7 @@ struct lima_sched_pipe {
void (*task_error)(struct lima_sched_pipe *pipe);
void (*task_mmu_error)(struct lima_sched_pipe *pipe);
int (*task_recover)(struct lima_sched_pipe *pipe);
+ void (*task_mask_irq)(struct lima_sched_pipe *pipe);
struct work_struct recover_work;
};
--
2.44.0
[PATCH v2 3/3] drm/lima: mask irqs in timeout path before hard reset
There is a race condition in which a rendering job might take just long
enough to trigger the drm sched job timeout handler but also still
complete before the hard reset is done by the timeout handler.
This runs into race conditions not expected by the timeout handler.
In some very specific cases it currently may result in a refcount
imbalance on lima_pm_idle, with a stack dump such as:
[10136.669170] WARNING: CPU: 0 PID: 0 at
drivers/gpu/drm/lima/lima_devfreq.c:205 lima_devfreq_record_idle+0xa0/0xb0
...
[10136.669459] pc : lima_devfreq_record_idle+0xa0/0xb0
...
[10136.669628] Call trace:
[10136.669634] lima_devfreq_record_idle+0xa0/0xb0
[10136.669646] lima_sched_pipe_task_done+0x5c/0xb0
[10136.669656] lima_gp_irq_handler+0xa8/0x120
[10136.669666] __handle_irq_event_percpu+0x48/0x160
[10136.669679] handle_irq_event+0x4c/0xc0
We can prevent that race condition entirely by masking the irqs at the
beginning of the timeout handler, at which point we give up on waiting
for that job entirely.
The irqs will be enabled again at the next hard reset which is already
done as a recovery by the timeout handler.
Signed-off-by: Erico Nunes
Reviewed-by: Qiang Yu
---
drivers/gpu/drm/lima/lima_sched.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/lima/lima_sched.c
b/drivers/gpu/drm/lima/lima_sched.c
index 66841503a618..bbf3f8feab94 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -430,6 +430,13 @@ static enum drm_gpu_sched_stat
lima_sched_timedout_job(struct drm_sched_job *job
return DRM_GPU_SCHED_STAT_NOMINAL;
}
+ /*
+* The task might still finish while this timeout handler runs.
+* To prevent a race condition on its completion, mask all irqs
+* on the running core until the next hard reset completes.
+*/
+ pipe->task_mask_irq(pipe);
+
if (!pipe->error)
DRM_ERROR("%s job timeout\n", lima_ip_name(ip));
--
2.44.0
[PATCH v2 2/3] drm/lima: include pp bcast irq in timeout handler check
In commit 53cb55b20208 ("drm/lima: handle spurious timeouts due to high
irq latency") a check was added to detect an unexpectedly high interrupt
latency timeout.
With further investigation it was noted that on Mali-450 the pp bcast
irq may also be a trigger of race conditions against the timeout
handler, so add it to this check too.
Signed-off-by: Erico Nunes
---
drivers/gpu/drm/lima/lima_sched.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/lima/lima_sched.c
b/drivers/gpu/drm/lima/lima_sched.c
index 00b19adfc888..66841503a618 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -422,6 +422,8 @@ static enum drm_gpu_sched_stat
lima_sched_timedout_job(struct drm_sched_job *job
*/
for (i = 0; i < pipe->num_processor; i++)
synchronize_irq(pipe->processor[i]->irq);
+ if (pipe->bcast_processor)
+ synchronize_irq(pipe->bcast_processor->irq);
if (dma_fence_is_signaled(task->fence)) {
DRM_WARN("%s unexpectedly high interrupt latency\n",
lima_ip_name(ip));
--
2.44.0
[PATCH v2 0/3] drm/lima: fix devfreq refcount imbalance for job timeouts
v1 reference: https://patchwork.freedesktop.org/series/131902/ Changes v1 -> v2: - Split synchronize_irq of pp bcast irq change into (new) patch 2. Erico Nunes (3): drm/lima: add mask irq callback to gp and pp drm/lima: include pp bcast irq in timeout handler check drm/lima: mask irqs in timeout path before hard reset drivers/gpu/drm/lima/lima_bcast.c | 12 drivers/gpu/drm/lima/lima_bcast.h | 3 +++ drivers/gpu/drm/lima/lima_gp.c| 8 drivers/gpu/drm/lima/lima_pp.c| 18 ++ drivers/gpu/drm/lima/lima_sched.c | 9 + drivers/gpu/drm/lima/lima_sched.h | 1 + 6 files changed, 51 insertions(+) -- 2.44.0
Re: [PATCH 8/8] accel/ivpu: Fix deadlock in context_xa
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
ivpu_device->context_xa is locked both in kernel thread and IRQ context.
It requires XA_FLAGS_LOCK_IRQ flag to be passed during initialization
otherwise the lock could be acquired from a thread and interrupted by
an IRQ that locks it for the second time causing the deadlock.
This deadlock was reported by lockdep and observed in internal tests.
Fixes: 35b137630f08 ("accel/ivpu: Introduce a new DRM driver for Intel VPU")
Cc: # v6.3+
Signed-off-by: Jacek Lawrynowicz
Reviewed-by: Jeffrey Hugo
Re: [PATCH 7/8] accel/ivpu: Fix missed error message after VPU rename
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote: Change "VPU" to "NPU" in ivpu_suspend() so it matches all other error messages. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 6/8] accel/ivpu: Return max freq for DRM_IVPU_PARAM_CORE_CLOCK_RATE
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
DRM_IVPU_PARAM_CORE_CLOCK_RATE returned current NPU frequency which
Commit text should be present tense, so returned->returns
could be 0 if device was sleeping. This value wasn't really useful to
also wasn't->isn't
the user space, so return max freq instead which can be used to estimate
NPU performance.
Fixes: c39dc15191c4 ("accel/ivpu: Read clock rate only if device is up")
Cc: # v6.7
Signed-off-by: Jacek Lawrynowicz
With the above,
Reviewed-by: Jeffrey Hugo
Re: [PATCH 5/8] accel/ivpu: Improve clarity of MMU error messages
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote: From: "Wachowski, Karol" This patch improves readability and clarity of MMU error messages. Previously, the error strings were somewhat confusing and could lead to ambiguous interpretations, making it difficult to diagnose issues. Signed-off-by: Wachowski, Karol Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 4/8] accel/ivpu: Put NPU back to D3hot after failed resume
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
Put NPU in D3hot after ivpu_resume() fails to power up the device.
This will assure that D3->D0 power cycle will be performed before
the next resume and also will minimize power usage in this corner case.
Fixes: 28083ff18d3f ("accel/ivpu: Fix DevTLB errors on suspend/resume and
recovery")
Cc: # v6.8+
Signed-off-by: Jacek Lawrynowicz
Reviewed-by: Jeffrey Hugo
Re: [PATCH 3/8] accel/ivpu: Fix PCI D0 state entry in resume
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
From: "Wachowski, Karol"
In case of failed power up we end up left in PCI D3hot
state making it impossible to access NPU registers on retry.
Enter D0 state on retry before proceeding with power up sequence.
Fixes: 28083ff18d3f ("accel/ivpu: Fix DevTLB errors on suspend/resume and
recovery")
Cc: # v6.8+
Signed-off-by: Wachowski, Karol
Signed-off-by: Jacek Lawrynowicz
Reviewed_by: Jeffrey Hugo
Re: [PATCH 2/8] accel/ivpu: Remove d3hot_after_power_off WA
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote: Always enter D3hot after entering D0i3 an all platforms. This minimizes power usage. Signed-off-by: Jacek Lawrynowicz Reviewed-by: Jeffrey Hugo
Re: [PATCH 1/8] accel/ivpu: Check return code of ipc->lock init
On 4/2/2024 4:49 AM, Jacek Lawrynowicz wrote:
From: "Wachowski, Karol"
Return value of drmm_mutex_init(ipc->lock) was unchecked.
Fixes: 5d7422cfb498 ("accel/ivpu: Add IPC driver and JSM messages")
Cc: # v6.3+
Signed-off-by: Wachowski, Karol
Signed-off-by: Jacek Lawrynowicz
Reviewed-by: Jeffrey Hugo
Re: [PATCH v15 2/8] phy: Add HDMI configuration options
On Fri, Apr 05, 2024 at 07:54:46PM +0530, Vinod Koul wrote:
> On 06-03-24, 15:48, Maxime Ripard wrote:
> > Hi Alexander,
> >
> > On Wed, Mar 06, 2024 at 11:16:19AM +0100, Alexander Stein wrote:
> > > From: Sandor Yu
> > >
> > > Allow HDMI PHYs to be configured through the generic
> > > functions through a custom structure added to the generic union.
> > >
> > > The parameters added here are based on HDMI PHY
> > > implementation practices. The current set of parameters
> > > should cover the potential users.
> > >
> > > Signed-off-by: Sandor Yu
> > > Reviewed-by: Dmitry Baryshkov
> > > Acked-by: Vinod Koul
> > > ---
> > > include/linux/phy/phy-hdmi.h | 24
> > > include/linux/phy/phy.h | 7 ++-
> > > 2 files changed, 30 insertions(+), 1 deletion(-)
> > > create mode 100644 include/linux/phy/phy-hdmi.h
> > >
> > > diff --git a/include/linux/phy/phy-hdmi.h b/include/linux/phy/phy-hdmi.h
> > > new file mode 100644
> > > index 0..b7de88e9090f0
> > > --- /dev/null
> > > +++ b/include/linux/phy/phy-hdmi.h
> > > @@ -0,0 +1,24 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Copyright 2022 NXP
> > > + */
> > > +
> > > +#ifndef __PHY_HDMI_H_
> > > +#define __PHY_HDMI_H_
> > > +
> > > +#include
> > > +/**
> > > + * struct phy_configure_opts_hdmi - HDMI configuration set
> > > + * @pixel_clk_rate: Pixel clock of video modes in KHz.
> > > + * @bpc: Maximum bits per color channel.
> > > + * @color_space: Colorspace in enum hdmi_colorspace.
> > > + *
> > > + * This structure is used to represent the configuration state of a HDMI
> > > phy.
> > > + */
> > > +struct phy_configure_opts_hdmi {
> > > + unsigned int pixel_clk_rate;
> > > + unsigned int bpc;
> > > + enum hdmi_colorspace color_space;
> > > +};
> >
> > Does the PHY actually care about the pixel clock rate, color space and
> > formats, or does it only care about the character rate?
>
> Nope it should not
After taking a look at the Cadence PHY driver, I share the feeling that
hdptx_hdmi_feedback_factor() should be reworked into drm_display helper
and then the struct phy_configure_opts_hdmi can be limited to having a
single unsigned long char_freq or bit_rate field.
I'm not sure whether we need anything corresponding to the TMDS Bit
Clock Ratio control. As far as I understand, it can be deduced from the
Bit Rate.
--
With best wishes
Dmitry
[PATCH v2] drm/vmwgfx: Don't memcmp equivalent pointers
These pointers are frequently the same and memcmp does not compare the
pointers before comparing their contents so this was wasting cycles
comparing 16 KiB of memory which will always be equal.
Fixes: bb6780aa5a1d ("drm/vmwgfx: Diff cursors when using cmds")
Signed-off-by: Ian Forbes
Cc: # v6.2+
---
v2: Fix code and commit message formatting.
--
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index cd4925346ed4..ef0af10c4968 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -216,7 +216,7 @@ static bool vmw_du_cursor_plane_has_changed(struct
vmw_plane_state *old_vps,
new_image = vmw_du_cursor_plane_acquire_image(new_vps);
changed = false;
- if (old_image && new_image)
+ if (old_image && new_image && old_image != new_image)
changed = memcmp(old_image, new_image, size) != 0;
return changed;
--
2.34.1
Re: [PATCH v15 6/8] phy: freescale: Add DisplayPort/HDMI Combo-PHY driver for i.MX8MQ
On 06-03-24, 11:16, Alexander Stein wrote: > From: Sandor Yu > > Add Cadence HDP-TX DisplayPort and HDMI PHY driver for i.MX8MQ. > > Cadence HDP-TX PHY could be put in either DP mode or > HDMI mode base on the configuration chosen. > DisplayPort or HDMI PHY mode is configured in the driver. > > Signed-off-by: Sandor Yu > Signed-off-by: Alexander Stein > --- > drivers/phy/freescale/Kconfig| 10 + > drivers/phy/freescale/Makefile |1 + > drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c | 1402 ++ > 3 files changed, 1413 insertions(+) > create mode 100644 drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c > > diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig > index 853958fb2c063..abacbe8ba8f46 100644 > --- a/drivers/phy/freescale/Kconfig > +++ b/drivers/phy/freescale/Kconfig > @@ -35,6 +35,16 @@ config PHY_FSL_IMX8M_PCIE > Enable this to add support for the PCIE PHY as found on > i.MX8M family of SOCs. > > +config PHY_FSL_IMX8MQ_HDPTX > + tristate "Freescale i.MX8MQ DP/HDMI PHY support" > + depends on OF && HAS_IOMEM > + depends on COMMON_CLK > + select GENERIC_PHY > + select CDNS_MHDP_HELPER > + help > + Enable this to support the Cadence HDPTX DP/HDMI PHY driver > + on i.MX8MQ SOC. > + > endif > > config PHY_FSL_LYNX_28G > diff --git a/drivers/phy/freescale/Makefile b/drivers/phy/freescale/Makefile > index cedb328bc4d28..17546a4da840a 100644 > --- a/drivers/phy/freescale/Makefile > +++ b/drivers/phy/freescale/Makefile > @@ -1,4 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0-only > +obj-$(CONFIG_PHY_FSL_IMX8MQ_HDPTX) += phy-fsl-imx8mq-hdptx.o > obj-$(CONFIG_PHY_FSL_IMX8MQ_USB) += phy-fsl-imx8mq-usb.o > obj-$(CONFIG_PHY_MIXEL_LVDS_PHY) += phy-fsl-imx8qm-lvds-phy.o > obj-$(CONFIG_PHY_MIXEL_MIPI_DPHY)+= phy-fsl-imx8-mipi-dphy.o > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c > b/drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c > new file mode 100644 > index 0..3bf92718f826a > --- /dev/null > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-hdptx.c > @@ -0,0 +1,1402 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Cadence DP/HDMI PHY driver > + * > + * Copyright (C) 2022-2024 NXP Semiconductor, Inc. > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define ADDR_PHY_AFE 0x8 > + > +/* PHY registers */ > +#define CMN_SSM_BIAS_TMR 0x0022 > +#define CMN_PLLSM0_PLLEN_TMR 0x0029 > +#define CMN_PLLSM0_PLLPRE_TMR0x002a > +#define CMN_PLLSM0_PLLVREF_TMR 0x002b > +#define CMN_PLLSM0_PLLLOCK_TMR 0x002c > +#define CMN_PLLSM0_USER_DEF_CTRL 0x002f > +#define CMN_PSM_CLK_CTRL 0x0061 > +#define CMN_CDIAG_REFCLK_CTRL0x0062 > +#define CMN_PLL0_VCOCAL_START0x0081 > +#define CMN_PLL0_VCOCAL_INIT_TMR 0x0084 > +#define CMN_PLL0_VCOCAL_ITER_TMR 0x0085 > +#define CMN_PLL0_INTDIV 0x0094 > +#define CMN_PLL0_FRACDIV 0x0095 > +#define CMN_PLL0_HIGH_THR0x0096 > +#define CMN_PLL0_DSM_DIAG0x0097 > +#define CMN_PLL0_SS_CTRL20x0099 > +#define CMN_ICAL_INIT_TMR0x00c4 > +#define CMN_ICAL_ITER_TMR0x00c5 > +#define CMN_RXCAL_INIT_TMR 0x00d4 > +#define CMN_RXCAL_ITER_TMR 0x00d5 > +#define CMN_TXPUCAL_CTRL 0x00e0 > +#define CMN_TXPUCAL_INIT_TMR 0x00e4 > +#define CMN_TXPUCAL_ITER_TMR 0x00e5 > +#define CMN_TXPDCAL_CTRL 0x00f0 > +#define CMN_TXPDCAL_INIT_TMR 0x00f4 > +#define CMN_TXPDCAL_ITER_TMR 0x00f5 > +#define CMN_ICAL_ADJ_INIT_TMR0x0102 > +#define CMN_ICAL_ADJ_ITER_TMR0x0103 > +#define CMN_RX_ADJ_INIT_TMR 0x0106 > +#define CMN_RX_ADJ_ITER_TMR 0x0107 > +#define CMN_TXPU_ADJ_CTRL0x0108 > +#define CMN_TXPU_ADJ_INIT_TMR0x010a > +#define CMN_TXPU_ADJ_ITER_TMR0x010b > +#define CMN_TXPD_ADJ_CTRL0x010c > +#define CMN_TXPD_ADJ_INIT_TMR0x010e > +#define CMN_TXPD_ADJ_ITER_TMR0x010f > +#define CMN_DIAG_PLL0_FBH_OVRD 0x01c0 > +#define CMN_DIAG_PLL0_FBL_OVRD 0x01c1 > +#define CMN_DIAG_PLL0_OVRD 0x01c2 > +#define CMN_DIAG_PLL0_TEST_MODE 0x01c4 > +#define CMN_DIAG_PLL0_V2I_TUNE 0x01c5 > +#define CMN_DIAG_PLL0_CP_TUNE0x01c6 > +#define CMN_DIAG_PLL0_LF_PROG0x01c7 > +#define
Re: [Intel-gfx] [PATCH v5 0/7] Introduce __xchg, non-atomic xchg
On Mon, 27 Feb 2023, Peter Zijlstra wrote: > On Thu, Feb 23, 2023 at 10:24:19PM +0100, Andrzej Hajda wrote: >> On 22.02.2023 18:04, Peter Zijlstra wrote: >> > On Wed, Jan 18, 2023 at 04:35:22PM +0100, Andrzej Hajda wrote: >> > >> > > Andrzej Hajda (7): >> > >arch: rename all internal names __xchg to __arch_xchg >> > >linux/include: add non-atomic version of xchg >> > >arch/*/uprobes: simplify arch_uretprobe_hijack_return_addr >> > >llist: simplify __llist_del_all >> > >io_uring: use __xchg if possible >> > >qed: use __xchg if possible >> > >drm/i915/gt: use __xchg instead of internal helper >> > >> > Nothing crazy in here I suppose, I somewhat wonder why you went through >> > the trouble, but meh. >> >> If you are asking why I have proposed this patchset, then the answer is >> simple, 1st I've tried to find a way to move internal i915 helper to core >> (see patch 7). >> Then I was looking for possible other users of this helper. And apparently >> there are many of them, patches 3-7 shows some. >> >> >> > >> > You want me to take this through te locking tree (for the next cycle, >> > not this one) where I normally take atomic things or does someone else >> > want this? >> >> If you could take it I will be happy. > > OK, I'll go queue it in tip/locking/core after -rc1. Thanks! Is this where the series fell between the cracks, or was there some follow-up that I missed? I think this would still be useful. Andrzej, would you mind rebasing and resending if there are no objections? BR, Jani. -- Jani Nikula, Intel
[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
Re: [PATCH v15 2/8] phy: Add HDMI configuration options
On 06-03-24, 15:48, Maxime Ripard wrote:
> Hi Alexander,
>
> On Wed, Mar 06, 2024 at 11:16:19AM +0100, Alexander Stein wrote:
> > From: Sandor Yu
> >
> > Allow HDMI PHYs to be configured through the generic
> > functions through a custom structure added to the generic union.
> >
> > The parameters added here are based on HDMI PHY
> > implementation practices. The current set of parameters
> > should cover the potential users.
> >
> > Signed-off-by: Sandor Yu
> > Reviewed-by: Dmitry Baryshkov
> > Acked-by: Vinod Koul
> > ---
> > include/linux/phy/phy-hdmi.h | 24
> > include/linux/phy/phy.h | 7 ++-
> > 2 files changed, 30 insertions(+), 1 deletion(-)
> > create mode 100644 include/linux/phy/phy-hdmi.h
> >
> > diff --git a/include/linux/phy/phy-hdmi.h b/include/linux/phy/phy-hdmi.h
> > new file mode 100644
> > index 0..b7de88e9090f0
> > --- /dev/null
> > +++ b/include/linux/phy/phy-hdmi.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright 2022 NXP
> > + */
> > +
> > +#ifndef __PHY_HDMI_H_
> > +#define __PHY_HDMI_H_
> > +
> > +#include
> > +/**
> > + * struct phy_configure_opts_hdmi - HDMI configuration set
> > + * @pixel_clk_rate: Pixel clock of video modes in KHz.
> > + * @bpc: Maximum bits per color channel.
> > + * @color_space: Colorspace in enum hdmi_colorspace.
> > + *
> > + * This structure is used to represent the configuration state of a HDMI
> > phy.
> > + */
> > +struct phy_configure_opts_hdmi {
> > + unsigned int pixel_clk_rate;
> > + unsigned int bpc;
> > + enum hdmi_colorspace color_space;
> > +};
>
> Does the PHY actually care about the pixel clock rate, color space and
> formats, or does it only care about the character rate?
Nope it should not
--
~Vinod
Re: [RESEND v7 21/37] dt-bindings: serial: renesas, scif: Add scif-sh7751.
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> Add Renesas SH7751 SCIF.
>
> Signed-off-by: Yoshinori Sato
> Reviewed-by: Geert Uytterhoeven
> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> @@ -18,6 +18,7 @@ properties:
>- items:
>- enum:
>- renesas,scif-r7s72100 # RZ/A1H
> + - renesas,scif-sh7751 # SH7751
>- const: renesas,scif # generic SCIF compatible UART
>
>- items:
If this is applied after "[PATCH v2 2/2] dt-bindings: serial:
renesas,scif: Validate 'interrupts' and 'interrupt-names'"[1], an extra
"- renesas,scif-sh7751" line should be added to the 4-interrupt section
(below "- renesas,scif-r7s72100").
[1]
https://lore.kernel.org/all/20240307114217.34784-3-prabhakar.mahadev-lad...@bp.renesas.com/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [RESEND v7 28/37] dt-bindings: soc: renesas: sh: Add SH7751 based target
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> Signed-off-by: Yoshinori Sato
Thanks for the update!
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/renesas/sh.yaml
> + compatible:
> +oneOf:
As adding more SoCs is expected, having oneOf from the start is fine.
> + - description: SH7751R based platform
> +items:
> + - enum:
> + - renesas,rts7751r2d # Renesas SH4 2D graphics board
> + - iodata,landisk # LANDISK HDL-U
> + - iodata,usl-5p # USL-5P
> + - const: renesas,sh7751r
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [rebase 1/3] drm: Add drm_vblank_work_flush_all().
what does "rebase" instead of "PATCH" is supposed to mean here?
And then we have a "PATCH v2" as reply to this one.
Shouldn't this go to dri-devel (too)?
Lucas De Marchi
On Thu, Apr 04, 2024 at 12:48:11PM +0200, Maarten Lankhorst wrote:
From: Maarten Lankhorst
In some cases we want to flush all vblank work, right before vblank_off
for example. Add a simple function to make this possible.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/drm_vblank_work.c | 22 ++
include/drm/drm_vblank_work.h | 2 ++
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank_work.c
b/drivers/gpu/drm/drm_vblank_work.c
index 43cd5c0f4f6f..ff86f2b2e052 100644
--- a/drivers/gpu/drm/drm_vblank_work.c
+++ b/drivers/gpu/drm/drm_vblank_work.c
@@ -232,6 +232,28 @@ void drm_vblank_work_flush(struct drm_vblank_work *work)
}
EXPORT_SYMBOL(drm_vblank_work_flush);
+/**
+ * drm_vblank_work_flush_all - flush all currently pending vblank work on crtc.
+ * @crtc: crtc for which vblank work to flush
+ *
+ * Wait until all currently queued vblank work on @crtc
+ * has finished executing once.
+ */
+void drm_vblank_work_flush_all(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_vblank_crtc *vblank = >vblank[drm_crtc_index(crtc)];
+
+ spin_lock_irq(>event_lock);
+ wait_event_lock_irq(vblank->work_wait_queue,
+ waitqueue_active(>work_wait_queue),
+ dev->event_lock);
+ spin_unlock_irq(>event_lock);
+
+ kthread_flush_worker(vblank->worker);
+}
+EXPORT_SYMBOL(drm_vblank_work_flush_all);
+
/**
* drm_vblank_work_init - initialize a vblank work item
* @work: vblank work item
diff --git a/include/drm/drm_vblank_work.h b/include/drm/drm_vblank_work.h
index eb41d0810c4f..e04d436b7297 100644
--- a/include/drm/drm_vblank_work.h
+++ b/include/drm/drm_vblank_work.h
@@ -17,6 +17,7 @@ struct drm_crtc;
* drm_vblank_work_init()
* drm_vblank_work_cancel_sync()
* drm_vblank_work_flush()
+ * drm_vblank_work_flush_all()
*/
struct drm_vblank_work {
/**
@@ -67,5 +68,6 @@ void drm_vblank_work_init(struct drm_vblank_work *work,
struct drm_crtc *crtc,
void (*func)(struct kthread_work *work));
bool drm_vblank_work_cancel_sync(struct drm_vblank_work *work);
void drm_vblank_work_flush(struct drm_vblank_work *work);
+void drm_vblank_work_flush_all(struct drm_crtc *crtc);
#endif /* !_DRM_VBLANK_WORK_H_ */
--
2.43.0
Re: [RESEND v7 23/37] dt-bindings: display: sm501 register definition helper
Hi Sato-san,
Thanks for your patch!
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> Miscellaneous Timing and Miscellaneous Control registers definition.
Please do not put raw register value definitions into DT bindings.
> Signed-off-by: Yoshinori Sato
> --- /dev/null
> +++ b/include/dt-bindings/display/sm501.h
> +/* Miscellaneous timing */
> +#define SM501_MISC_TIMING_EX_HOLD_00
> +#define SM501_MISC_TIMING_EX_HOLD_16 1
> +#define SM501_MISC_TIMING_EX_HOLD_32 2
> +#define SM501_MISC_TIMING_EX_HOLD_48 3
> +#define SM501_MISC_TIMING_EX_HOLD_64 4
> +#define SM501_MISC_TIMING_EX_HOLD_80 5
> +#define SM501_MISC_TIMING_EX_HOLD_96 6
> +#define SM501_MISC_TIMING_EX_HOLD_112 7
> +#define SM501_MISC_TIMING_EX_HOLD_128 8
> +#define SM501_MISC_TIMING_EX_HOLD_144 9
> +#define SM501_MISC_TIMING_EX_HOLD_160 10
> +#define SM501_MISC_TIMING_EX_HOLD_176 11
> +#define SM501_MISC_TIMING_EX_HOLD_192 12
> +#define SM501_MISC_TIMING_EX_HOLD_208 13
> +#define SM501_MISC_TIMING_EX_HOLD_224 14
> +#define SM501_MISC_TIMING_EX_HOLD_240 15
E.g. these are used by the (not very descriptive) "ex" property:
ex:
$ref: /schemas/types.yaml#/definitions/uint32
description: Extend bus holding time.
Please instead use an enum for the actual holding time ([ 0, 16, 32,
...]) in the DT bindings, and convert from actual holding time to
register value in the driver.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: (subset) [PATCH v2 1/1] Revert "drm/qxl: simplify qxl_fence_wait"
On Thu, 04 Apr 2024 19:14:48 +0100, Alex Constantino wrote:
> This reverts commit 5a838e5d5825c85556011478abde708251cc0776.
>
> Changes from commit 5a838e5d5825 ("drm/qxl: simplify qxl_fence_wait") would
> result in a '[TTM] Buffer eviction failed' exception whenever it reached a
> timeout.
> Due to a dependency to DMA_FENCE_WARN this also restores some code deleted
> by commit d72277b6c37d ("dma-buf: nuke DMA_FENCE_TRACE macros v2").
>
> [...]
Applied to misc/kernel.git (drm-misc-fixes).
Thanks!
Maxime
Re: (subset) [PATCH 2/7] drm/bridge: dw-hdmi: Make DRM_DW_HDMI selectable
On Wed, 03 Apr 2024 12:56:20 +0200, Maxime Ripard wrote:
> Commit c0e0f139354c ("drm: Make drivers depends on DRM_DW_HDMI") turned
> select dependencies into depends on ones. However, DRM_DW_HDMI was not
> manually selectable which resulted in no way to enable the drivers that
> were now depending on it.
>
>
Applied to misc/kernel.git (drm-misc-next).
Thanks!
Maxime
Re: (subset) [PATCH 1/7] drm/display: Select DRM_KMS_HELPER for DP helpers
On Wed, 03 Apr 2024 12:56:19 +0200, Maxime Ripard wrote:
> The DisplayPort helpers rely on some
> (__drm_atomic_helper_private_obj_duplicate_state,
> drm_kms_helper_hotplug_event) helpers found in files compiled by
> DRM_KMS_HELPER.
>
> Prior to commit d674858ff979 ("drm/display: Make all helpers visible and
> switch to depends on"), DRM_DISPLAY_DP_HELPER was only selectable so it
> wasn't really a big deal. However, since that commit, it's now something
> that can be enabled as is, and since there's no expressed dependency
> with DRM_KMS_HELPER, it can break too.
>
> [...]
Applied to misc/kernel.git (drm-misc-next).
Thanks!
Maxime
Re: [RESEND v7 14/37] clk: Compatible with narrow registers
On 4/5/24 21:56, Geert Uytterhoeven wrote:
> Hi Sato-san,
>
> On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
> wrote:
>> divider and gate only support 32-bit registers.
>> Older hardware uses narrower registers, so I want to be able to handle
>> 8-bit and 16-bit wide registers.
>>
>> Seven clk_divider flags are used, and if I add flags for 8bit access and
>> 16bit access, 8bit will not be enough, so I expanded it to u16.
>>
>> Signed-off-by: Yoshinori Sato
>
> Thanks for the update!
>
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -26,20 +26,38 @@
>> * parent - fixed parent. No clk_set_parent support
>> */
>>
>> -static inline u32 clk_div_readl(struct clk_divider *divider)
>> -{
>> - if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> - return ioread32be(divider->reg);
>> -
>> - return readl(divider->reg);
>> +static inline u32 clk_div_read(struct clk_divider *divider)
>> +{
>> + if (divider->flags & CLK_DIVIDER_REG_8BIT)
>
> When you need curly braces in one branch of an if/else statement,
> please use curly braces in all branches (everywhere).
>
>> + return readb(divider->reg);
>> + else if (divider->flags & CLK_DIVIDER_REG_16BIT) {
And no need for an else after a return...
>> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> + return ioread16be(divider->reg);
>> + else
and here.
>> + return readw(divider->reg);
>> + } else {
>> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
>> + return ioread32be(divider->reg);
>> + else
here too.
>> + return readl(divider->reg);
>> + }
>> }
>
>> --- a/drivers/clk/clk-gate.c
>> +++ b/drivers/clk/clk-gate.c
>
>> @@ -137,12 +155,30 @@ struct clk_hw *__clk_hw_register_gate(struct device
>> *dev,
>> struct clk_init_data init = {};
>> int ret = -EINVAL;
>>
>> + /* validate register size option and bit_idx */
>> if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
>> if (bit_idx > 15) {
>> pr_err("gate bit exceeds LOWORD field\n");
>> return ERR_PTR(-EINVAL);
>> }
>> }
>> + if (clk_gate_flags & CLK_GATE_REG_16BIT) {
>> + if (bit_idx > 15) {
>> + pr_err("gate bit exceeds 16 bits\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> + }
>> + if (clk_gate_flags & CLK_GATE_REG_8BIT) {
>> + if (bit_idx > 7) {
>> + pr_err("gate bit exceeds 8 bits\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> + }
>> + if ((clk_gate_flags & CLK_GATE_HIWORD_MASK) &&
>
> If you use parentheses around "a & b" here...
>
>> + clk_gate_flags & (CLK_GATE_REG_8BIT | CLK_GATE_REG_16BIT)) {
>
> please add parentheses here, too.
>
>> + pr_err("HIWORD_MASK required 32-bit register\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>>
>> /* allocate the gate */
>> gate = kzalloc(sizeof(*gate), GFP_KERNEL);
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 4a537260f655..eaa6ff1d0b2e 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -508,12 +508,16 @@ void of_fixed_clk_setup(struct device_node *np);
>> * CLK_GATE_BIG_ENDIAN - by default little endian register accesses are
>> used for
>> * the gate register. Setting this flag makes the register accesses big
>> * endian.
>> + * CLK_GATE_REG_8BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses
>> 8bit.
>> + * CLK_GATE_REG_16BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses
>> 16bit.
>> */
>> struct clk_gate {
>> struct clk_hw hw;
>> void __iomem*reg;
>> u8 bit_idx;
>> - u8 flags;
>> + u32 flags;
>
> (from my comments on v6)
> There is no need to increase the size of the flags field for the gate clock.
>
>
>> spinlock_t *lock;
>> };
>>
>
>> @@ -675,13 +681,17 @@ struct clk_div_table {
>> * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are
>> used
>> * for the divider register. Setting this flag makes the register
>> accesses
>> * big endian.
>> + * CLK_DIVIDER_REG_8BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses
>> 8bit.
>> + * CLK_DIVIDER_REG_16BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses
>> 16bit.
>> */
>> struct clk_divider {
>>
Re: [RESEND v7 14/37] clk: Compatible with narrow registers
Hi Sato-san,
On Thu, Apr 4, 2024 at 7:15 AM Yoshinori Sato
wrote:
> divider and gate only support 32-bit registers.
> Older hardware uses narrower registers, so I want to be able to handle
> 8-bit and 16-bit wide registers.
>
> Seven clk_divider flags are used, and if I add flags for 8bit access and
> 16bit access, 8bit will not be enough, so I expanded it to u16.
>
> Signed-off-by: Yoshinori Sato
Thanks for the update!
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -26,20 +26,38 @@
> * parent - fixed parent. No clk_set_parent support
> */
>
> -static inline u32 clk_div_readl(struct clk_divider *divider)
> -{
> - if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
> - return ioread32be(divider->reg);
> -
> - return readl(divider->reg);
> +static inline u32 clk_div_read(struct clk_divider *divider)
> +{
> + if (divider->flags & CLK_DIVIDER_REG_8BIT)
When you need curly braces in one branch of an if/else statement,
please use curly braces in all branches (everywhere).
> + return readb(divider->reg);
> + else if (divider->flags & CLK_DIVIDER_REG_16BIT) {
> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
> + return ioread16be(divider->reg);
> + else
> + return readw(divider->reg);
> + } else {
> + if (divider->flags & CLK_DIVIDER_BIG_ENDIAN)
> + return ioread32be(divider->reg);
> + else
> + return readl(divider->reg);
> + }
> }
> --- a/drivers/clk/clk-gate.c
> +++ b/drivers/clk/clk-gate.c
> @@ -137,12 +155,30 @@ struct clk_hw *__clk_hw_register_gate(struct device
> *dev,
> struct clk_init_data init = {};
> int ret = -EINVAL;
>
> + /* validate register size option and bit_idx */
> if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
> if (bit_idx > 15) {
> pr_err("gate bit exceeds LOWORD field\n");
> return ERR_PTR(-EINVAL);
> }
> }
> + if (clk_gate_flags & CLK_GATE_REG_16BIT) {
> + if (bit_idx > 15) {
> + pr_err("gate bit exceeds 16 bits\n");
> + return ERR_PTR(-EINVAL);
> + }
> + }
> + if (clk_gate_flags & CLK_GATE_REG_8BIT) {
> + if (bit_idx > 7) {
> + pr_err("gate bit exceeds 8 bits\n");
> + return ERR_PTR(-EINVAL);
> + }
> + }
> + if ((clk_gate_flags & CLK_GATE_HIWORD_MASK) &&
If you use parentheses around "a & b" here...
> + clk_gate_flags & (CLK_GATE_REG_8BIT | CLK_GATE_REG_16BIT)) {
please add parentheses here, too.
> + pr_err("HIWORD_MASK required 32-bit register\n");
> + return ERR_PTR(-EINVAL);
> + }
>
> /* allocate the gate */
> gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 4a537260f655..eaa6ff1d0b2e 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -508,12 +508,16 @@ void of_fixed_clk_setup(struct device_node *np);
> * CLK_GATE_BIG_ENDIAN - by default little endian register accesses are used
> for
> * the gate register. Setting this flag makes the register accesses big
> * endian.
> + * CLK_GATE_REG_8BIT - by default 32bit register accesses are used for
> + * the gate register. Setting this flag makes the register accesses
> 8bit.
> + * CLK_GATE_REG_16BIT - by default 32bit register accesses are used for
> + * the gate register. Setting this flag makes the register accesses
> 16bit.
> */
> struct clk_gate {
> struct clk_hw hw;
> void __iomem*reg;
> u8 bit_idx;
> - u8 flags;
> + u32 flags;
(from my comments on v6)
There is no need to increase the size of the flags field for the gate clock.
> spinlock_t *lock;
> };
>
> @@ -675,13 +681,17 @@ struct clk_div_table {
> * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are
> used
> * for the divider register. Setting this flag makes the register
> accesses
> * big endian.
> + * CLK_DIVIDER_REG_8BIT - by default 32bit register accesses are used for
> + * the gate register. Setting this flag makes the register accesses
> 8bit.
> + * CLK_DIVIDER_REG_16BIT - by default 32bit register accesses are used for
> + * the gate register. Setting this flag makes the register accesses
> 16bit.
> */
> struct clk_divider {
> struct clk_hw hw;
> void __iomem*reg;
> u8 shift;
> u8 width;
> - u8 flags;
> + u16 flags;
> const struct clk_div_table *table;
> spinlock_t *lock;
> };
