* [PATCH v2 1/8] drm/client: Constify modes
2025-02-28 21:14 [PATCH v2 0/8] drm/client: Stop using legacy crtc->mode and a bunch of cleanups Ville Syrjala
@ 2025-02-28 21:14 ` Ville Syrjala
2025-03-01 3:28 ` kernel test robot
` (2 more replies)
2025-02-28 21:14 ` [PATCH v2 2/8] drm/client: Use array notation for function arguments Ville Syrjala
` (6 subsequent siblings)
7 siblings, 3 replies; 18+ messages in thread
From: Ville Syrjala @ 2025-02-28 21:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Jani Nikula, Thomas Zimmermann
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The modes used by the client code live on the connectors' mode
lists, which are not owned by the client code, and thus it has
no business modifying the modes. Mark the modes const to make
that fact abundantly clear.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_client_modeset.c | 39 +++++++++++++++-------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index aca442c25209..b114d1b8793b 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -117,10 +117,10 @@ drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc)
return NULL;
}
-static struct drm_display_mode *
+static const struct drm_display_mode *
drm_connector_get_tiled_mode(struct drm_connector *connector)
{
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
list_for_each_entry(mode, &connector->modes, head) {
if (mode->hdisplay == connector->tile_h_size &&
@@ -130,10 +130,10 @@ drm_connector_get_tiled_mode(struct drm_connector *connector)
return NULL;
}
-static struct drm_display_mode *
+static const struct drm_display_mode *
drm_connector_fallback_non_tiled_mode(struct drm_connector *connector)
{
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
list_for_each_entry(mode, &connector->modes, head) {
if (mode->hdisplay == connector->tile_h_size &&
@@ -144,10 +144,10 @@ drm_connector_fallback_non_tiled_mode(struct drm_connector *connector)
return NULL;
}
-static struct drm_display_mode *
+static const struct drm_display_mode *
drm_connector_preferred_mode(struct drm_connector *connector, int width, int height)
{
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
list_for_each_entry(mode, &connector->modes, head) {
if (mode->hdisplay > width ||
@@ -159,16 +159,18 @@ drm_connector_preferred_mode(struct drm_connector *connector, int width, int hei
return NULL;
}
-static struct drm_display_mode *drm_connector_first_mode(struct drm_connector *connector)
+static const struct drm_display_mode *
+drm_connector_first_mode(struct drm_connector *connector)
{
return list_first_entry_or_null(&connector->modes,
struct drm_display_mode, head);
}
-static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_connector *connector)
+static const struct drm_display_mode *
+drm_connector_pick_cmdline_mode(struct drm_connector *connector)
{
- struct drm_cmdline_mode *cmdline_mode;
- struct drm_display_mode *mode;
+ const struct drm_cmdline_mode *cmdline_mode;
+ const struct drm_display_mode *mode;
bool prefer_non_interlace;
/*
@@ -266,13 +268,14 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
struct drm_client_offset *offsets,
bool *enabled, int width, int height)
{
int count, i, j;
bool can_clone = false;
- struct drm_display_mode *dmt_mode, *mode;
+ const struct drm_display_mode *mode;
+ struct drm_display_mode *dmt_mode;
/* only contemplate cloning in the single crtc case */
if (dev->mode_config.num_crtc > 1)
@@ -351,7 +354,7 @@ static bool drm_client_target_cloned(struct drm_device *dev,
static int drm_client_get_tile_offsets(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
struct drm_client_offset *offsets,
int idx,
int h_idx, int v_idx)
@@ -386,7 +389,7 @@ static int drm_client_get_tile_offsets(struct drm_device *dev,
static bool drm_client_target_preferred(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
struct drm_client_offset *offsets,
bool *enabled, int width, int height)
{
@@ -505,7 +508,7 @@ static int drm_client_pick_crtcs(struct drm_client_dev *client,
struct drm_connector **connectors,
unsigned int connector_count,
struct drm_crtc **best_crtcs,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
int n, int width, int height)
{
struct drm_device *dev = client->dev;
@@ -580,7 +583,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
struct drm_connector **connectors,
unsigned int connector_count,
struct drm_crtc **crtcs,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
struct drm_client_offset *offsets,
bool *enabled, int width, int height)
{
@@ -800,7 +803,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
struct drm_client_offset *offsets;
unsigned int connector_count = 0;
/* points to modes protected by mode_config.mutex */
- struct drm_display_mode **modes;
+ const struct drm_display_mode **modes;
struct drm_crtc **crtcs;
int i, ret = 0;
bool *enabled;
@@ -871,7 +874,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
drm_client_modeset_release(client);
for (i = 0; i < connector_count; i++) {
- struct drm_display_mode *mode = modes[i];
+ const struct drm_display_mode *mode = modes[i];
struct drm_crtc *crtc = crtcs[i];
struct drm_client_offset *offset = &offsets[i];
--
2.45.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 1/8] drm/client: Constify modes
2025-02-28 21:14 ` [PATCH v2 1/8] drm/client: Constify modes Ville Syrjala
@ 2025-03-01 3:28 ` kernel test robot
2025-03-01 4:41 ` kernel test robot
2025-03-03 9:38 ` [PATCH v3 " Ville Syrjala
2 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2025-03-01 3:28 UTC (permalink / raw)
To: Ville Syrjala, dri-devel
Cc: llvm, oe-kbuild-all, intel-gfx, Jani Nikula, Thomas Zimmermann
Hi Ville,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-exynos/exynos-drm-next]
[also build test ERROR on linus/master v6.14-rc4 next-20250228]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ville-Syrjala/drm-client-Constify-modes/20250301-051619
base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
patch link: https://lore.kernel.org/r/20250228211454.8138-2-ville.syrjala%40linux.intel.com
patch subject: [PATCH v2 1/8] drm/client: Constify modes
config: hexagon-randconfig-001-20250301 (https://download.01.org/0day-ci/archive/20250301/202503011150.x8nK9fZM-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 14170b16028c087ca154878f5ed93d3089a965c6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250301/202503011150.x8nK9fZM-lkp@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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503011150.x8nK9fZM-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/gpu/drm/drm_client_modeset.c:1275:
>> drivers/gpu/drm/tests/drm_client_modeset_test.c:108:7: error: assigning to 'struct drm_display_mode *' from 'const struct drm_display_mode *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
108 | mode = drm_connector_pick_cmdline_mode(connector);
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
vim +108 drivers/gpu/drm/tests/drm_client_modeset_test.c
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 84
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 85 static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test)
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 86 {
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 87 struct drm_client_modeset_test_priv *priv = test->priv;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 88 struct drm_device *drm = priv->drm;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 89 struct drm_connector *connector = &priv->connector;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 90 struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 91 struct drm_display_mode *expected_mode, *mode;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 92 const char *cmdline = "1920x1080@60";
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 93 int ret;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 94
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 95 expected_mode = drm_mode_find_dmt(priv->drm, 1920, 1080, 60, false);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 96 KUNIT_ASSERT_NOT_NULL(test, expected_mode);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 97
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 98 KUNIT_ASSERT_TRUE(test,
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 99 drm_mode_parse_command_line_for_connector(cmdline,
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 100 connector,
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 101 cmdline_mode));
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 102
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 103 mutex_lock(&drm->mode_config.mutex);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 104 ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 105 mutex_unlock(&drm->mode_config.mutex);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 106 KUNIT_ASSERT_GT(test, ret, 0);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 107
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 @108 mode = drm_connector_pick_cmdline_mode(connector);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 109 KUNIT_ASSERT_NOT_NULL(test, mode);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 110
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 111 KUNIT_EXPECT_TRUE(test, drm_mode_equal(expected_mode, mode));
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 112 }
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 113
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v2 1/8] drm/client: Constify modes
2025-02-28 21:14 ` [PATCH v2 1/8] drm/client: Constify modes Ville Syrjala
2025-03-01 3:28 ` kernel test robot
@ 2025-03-01 4:41 ` kernel test robot
2025-03-03 9:38 ` [PATCH v3 " Ville Syrjala
2 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2025-03-01 4:41 UTC (permalink / raw)
To: Ville Syrjala, dri-devel
Cc: oe-kbuild-all, intel-gfx, Jani Nikula, Thomas Zimmermann
Hi Ville,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-exynos/exynos-drm-next]
[also build test WARNING on linus/master v6.14-rc4 next-20250228]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ville-Syrjala/drm-client-Constify-modes/20250301-051619
base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
patch link: https://lore.kernel.org/r/20250228211454.8138-2-ville.syrjala%40linux.intel.com
patch subject: [PATCH v2 1/8] drm/client: Constify modes
config: arm64-randconfig-001-20250301 (https://download.01.org/0day-ci/archive/20250301/202503011242.ZSTgTEVd-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250301/202503011242.ZSTgTEVd-lkp@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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503011242.ZSTgTEVd-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/drm_client_modeset.c:1275:
drivers/gpu/drm/tests/drm_client_modeset_test.c: In function 'drm_test_pick_cmdline_res_1920_1080_60':
>> drivers/gpu/drm/tests/drm_client_modeset_test.c:108:14: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
108 | mode = drm_connector_pick_cmdline_mode(connector);
| ^
vim +/const +108 drivers/gpu/drm/tests/drm_client_modeset_test.c
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 84
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 85 static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test)
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 86 {
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 87 struct drm_client_modeset_test_priv *priv = test->priv;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 88 struct drm_device *drm = priv->drm;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 89 struct drm_connector *connector = &priv->connector;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 90 struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 91 struct drm_display_mode *expected_mode, *mode;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 92 const char *cmdline = "1920x1080@60";
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 93 int ret;
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 94
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 95 expected_mode = drm_mode_find_dmt(priv->drm, 1920, 1080, 60, false);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 96 KUNIT_ASSERT_NOT_NULL(test, expected_mode);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 97
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 98 KUNIT_ASSERT_TRUE(test,
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 99 drm_mode_parse_command_line_for_connector(cmdline,
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 100 connector,
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 101 cmdline_mode));
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 102
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 103 mutex_lock(&drm->mode_config.mutex);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 104 ret = drm_helper_probe_single_connector_modes(connector, 1920, 1080);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 105 mutex_unlock(&drm->mode_config.mutex);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 106 KUNIT_ASSERT_GT(test, ret, 0);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 107
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 @108 mode = drm_connector_pick_cmdline_mode(connector);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 109 KUNIT_ASSERT_NOT_NULL(test, mode);
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 110
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 111 KUNIT_EXPECT_TRUE(test, drm_mode_equal(expected_mode, mode));
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 112 }
8fc0380f6ba7e9 Maxime Ripard 2022-11-14 113
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v3 1/8] drm/client: Constify modes
2025-02-28 21:14 ` [PATCH v2 1/8] drm/client: Constify modes Ville Syrjala
2025-03-01 3:28 ` kernel test robot
2025-03-01 4:41 ` kernel test robot
@ 2025-03-03 9:38 ` Ville Syrjala
2 siblings, 0 replies; 18+ messages in thread
From: Ville Syrjala @ 2025-03-03 9:38 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Jani Nikula, Thomas Zimmermann
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The modes used by the client code live on the connectors' mode
lists, which are not owned by the client code, and thus it has
no business modifying the modes. Mark the modes const to make
that fact abundantly clear.
v2: Fix up the kunit test
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_client_modeset.c | 39 ++++++++++---------
.../gpu/drm/tests/drm_client_modeset_test.c | 3 +-
2 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index aca442c25209..b114d1b8793b 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -117,10 +117,10 @@ drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc)
return NULL;
}
-static struct drm_display_mode *
+static const struct drm_display_mode *
drm_connector_get_tiled_mode(struct drm_connector *connector)
{
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
list_for_each_entry(mode, &connector->modes, head) {
if (mode->hdisplay == connector->tile_h_size &&
@@ -130,10 +130,10 @@ drm_connector_get_tiled_mode(struct drm_connector *connector)
return NULL;
}
-static struct drm_display_mode *
+static const struct drm_display_mode *
drm_connector_fallback_non_tiled_mode(struct drm_connector *connector)
{
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
list_for_each_entry(mode, &connector->modes, head) {
if (mode->hdisplay == connector->tile_h_size &&
@@ -144,10 +144,10 @@ drm_connector_fallback_non_tiled_mode(struct drm_connector *connector)
return NULL;
}
-static struct drm_display_mode *
+static const struct drm_display_mode *
drm_connector_preferred_mode(struct drm_connector *connector, int width, int height)
{
- struct drm_display_mode *mode;
+ const struct drm_display_mode *mode;
list_for_each_entry(mode, &connector->modes, head) {
if (mode->hdisplay > width ||
@@ -159,16 +159,18 @@ drm_connector_preferred_mode(struct drm_connector *connector, int width, int hei
return NULL;
}
-static struct drm_display_mode *drm_connector_first_mode(struct drm_connector *connector)
+static const struct drm_display_mode *
+drm_connector_first_mode(struct drm_connector *connector)
{
return list_first_entry_or_null(&connector->modes,
struct drm_display_mode, head);
}
-static struct drm_display_mode *drm_connector_pick_cmdline_mode(struct drm_connector *connector)
+static const struct drm_display_mode *
+drm_connector_pick_cmdline_mode(struct drm_connector *connector)
{
- struct drm_cmdline_mode *cmdline_mode;
- struct drm_display_mode *mode;
+ const struct drm_cmdline_mode *cmdline_mode;
+ const struct drm_display_mode *mode;
bool prefer_non_interlace;
/*
@@ -266,13 +268,14 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
struct drm_client_offset *offsets,
bool *enabled, int width, int height)
{
int count, i, j;
bool can_clone = false;
- struct drm_display_mode *dmt_mode, *mode;
+ const struct drm_display_mode *mode;
+ struct drm_display_mode *dmt_mode;
/* only contemplate cloning in the single crtc case */
if (dev->mode_config.num_crtc > 1)
@@ -351,7 +354,7 @@ static bool drm_client_target_cloned(struct drm_device *dev,
static int drm_client_get_tile_offsets(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
struct drm_client_offset *offsets,
int idx,
int h_idx, int v_idx)
@@ -386,7 +389,7 @@ static int drm_client_get_tile_offsets(struct drm_device *dev,
static bool drm_client_target_preferred(struct drm_device *dev,
struct drm_connector **connectors,
unsigned int connector_count,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
struct drm_client_offset *offsets,
bool *enabled, int width, int height)
{
@@ -505,7 +508,7 @@ static int drm_client_pick_crtcs(struct drm_client_dev *client,
struct drm_connector **connectors,
unsigned int connector_count,
struct drm_crtc **best_crtcs,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
int n, int width, int height)
{
struct drm_device *dev = client->dev;
@@ -580,7 +583,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
struct drm_connector **connectors,
unsigned int connector_count,
struct drm_crtc **crtcs,
- struct drm_display_mode **modes,
+ const struct drm_display_mode **modes,
struct drm_client_offset *offsets,
bool *enabled, int width, int height)
{
@@ -800,7 +803,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
struct drm_client_offset *offsets;
unsigned int connector_count = 0;
/* points to modes protected by mode_config.mutex */
- struct drm_display_mode **modes;
+ const struct drm_display_mode **modes;
struct drm_crtc **crtcs;
int i, ret = 0;
bool *enabled;
@@ -871,7 +874,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
drm_client_modeset_release(client);
for (i = 0; i < connector_count; i++) {
- struct drm_display_mode *mode = modes[i];
+ const struct drm_display_mode *mode = modes[i];
struct drm_crtc *crtc = crtcs[i];
struct drm_client_offset *offset = &offsets[i];
diff --git a/drivers/gpu/drm/tests/drm_client_modeset_test.c b/drivers/gpu/drm/tests/drm_client_modeset_test.c
index 7516f6cb36e4..cd43d2a52a2d 100644
--- a/drivers/gpu/drm/tests/drm_client_modeset_test.c
+++ b/drivers/gpu/drm/tests/drm_client_modeset_test.c
@@ -88,7 +88,8 @@ static void drm_test_pick_cmdline_res_1920_1080_60(struct kunit *test)
struct drm_device *drm = priv->drm;
struct drm_connector *connector = &priv->connector;
struct drm_cmdline_mode *cmdline_mode = &connector->cmdline_mode;
- struct drm_display_mode *expected_mode, *mode;
+ struct drm_display_mode *expected_mode;
+ const struct drm_display_mode *mode;
const char *cmdline = "1920x1080@60";
int ret;
--
2.45.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/8] drm/client: Use array notation for function arguments
2025-02-28 21:14 [PATCH v2 0/8] drm/client: Stop using legacy crtc->mode and a bunch of cleanups Ville Syrjala
2025-02-28 21:14 ` [PATCH v2 1/8] drm/client: Constify modes Ville Syrjala
@ 2025-02-28 21:14 ` Ville Syrjala
2025-02-28 21:14 ` [PATCH v2 3/8] drm/client: Streamline mode selection debugs Ville Syrjala
` (5 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Ville Syrjala @ 2025-02-28 21:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Jani Nikula, Thomas Zimmermann
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Use the array notation rather that the pointer notation for
function arguments. This makes it clear to the reader that
we are in fact dealing with an array rather than a single
pointer. Functionally the two are equivalent.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_client_modeset.c | 42 ++++++++++++++--------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index b114d1b8793b..bdd4078e62ad 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -239,9 +239,9 @@ static bool drm_connector_enabled(struct drm_connector *connector, bool strict)
return enable;
}
-static void drm_client_connectors_enabled(struct drm_connector **connectors,
+static void drm_client_connectors_enabled(struct drm_connector *connectors[],
unsigned int connector_count,
- bool *enabled)
+ bool enabled[])
{
bool any_enabled = false;
struct drm_connector *connector;
@@ -266,11 +266,11 @@ static void drm_client_connectors_enabled(struct drm_connector **connectors,
}
static bool drm_client_target_cloned(struct drm_device *dev,
- struct drm_connector **connectors,
+ struct drm_connector *connectors[],
unsigned int connector_count,
- const struct drm_display_mode **modes,
- struct drm_client_offset *offsets,
- bool *enabled, int width, int height)
+ const struct drm_display_mode *modes[],
+ struct drm_client_offset offsets[],
+ bool enabled[], int width, int height)
{
int count, i, j;
bool can_clone = false;
@@ -352,10 +352,10 @@ static bool drm_client_target_cloned(struct drm_device *dev,
}
static int drm_client_get_tile_offsets(struct drm_device *dev,
- struct drm_connector **connectors,
+ struct drm_connector *connectors[],
unsigned int connector_count,
- const struct drm_display_mode **modes,
- struct drm_client_offset *offsets,
+ const struct drm_display_mode *modes[],
+ struct drm_client_offset offsets[],
int idx,
int h_idx, int v_idx)
{
@@ -387,11 +387,11 @@ static int drm_client_get_tile_offsets(struct drm_device *dev,
}
static bool drm_client_target_preferred(struct drm_device *dev,
- struct drm_connector **connectors,
+ struct drm_connector *connectors[],
unsigned int connector_count,
- const struct drm_display_mode **modes,
- struct drm_client_offset *offsets,
- bool *enabled, int width, int height)
+ const struct drm_display_mode *modes[],
+ struct drm_client_offset offsets[],
+ bool enabled[], int width, int height)
{
const u64 mask = BIT_ULL(connector_count) - 1;
struct drm_connector *connector;
@@ -505,10 +505,10 @@ static bool connector_has_possible_crtc(struct drm_connector *connector,
}
static int drm_client_pick_crtcs(struct drm_client_dev *client,
- struct drm_connector **connectors,
+ struct drm_connector *connectors[],
unsigned int connector_count,
- struct drm_crtc **best_crtcs,
- const struct drm_display_mode **modes,
+ struct drm_crtc *best_crtcs[],
+ const struct drm_display_mode *modes[],
int n, int width, int height)
{
struct drm_device *dev = client->dev;
@@ -580,12 +580,12 @@ static int drm_client_pick_crtcs(struct drm_client_dev *client,
/* Try to read the BIOS display configuration and use it for the initial config */
static bool drm_client_firmware_config(struct drm_client_dev *client,
- struct drm_connector **connectors,
+ struct drm_connector *connectors[],
unsigned int connector_count,
- struct drm_crtc **crtcs,
- const struct drm_display_mode **modes,
- struct drm_client_offset *offsets,
- bool *enabled, int width, int height)
+ struct drm_crtc *crtcs[],
+ const struct drm_display_mode *modes[],
+ struct drm_client_offset offsets[],
+ bool enabled[], int width, int height)
{
const int count = min_t(unsigned int, connector_count, BITS_PER_LONG);
unsigned long conn_configured, conn_seq, mask;
--
2.45.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 3/8] drm/client: Streamline mode selection debugs
2025-02-28 21:14 [PATCH v2 0/8] drm/client: Stop using legacy crtc->mode and a bunch of cleanups Ville Syrjala
2025-02-28 21:14 ` [PATCH v2 1/8] drm/client: Constify modes Ville Syrjala
2025-02-28 21:14 ` [PATCH v2 2/8] drm/client: Use array notation for function arguments Ville Syrjala
@ 2025-02-28 21:14 ` Ville Syrjala
2025-02-28 21:14 ` [PATCH v2 4/8] drm/client: Make copies of modes Ville Syrjala
` (4 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Ville Syrjala @ 2025-02-28 21:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Thomas Zimmermann
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Get rid of all the redundant debugs and just wait until the end
to print which mode (and of which type) we picked.
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_client_modeset.c | 70 +++++++++++++---------------
1 file changed, 33 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index bdd4078e62ad..148257287ae4 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))
@@ -441,20 +443,20 @@ static bool drm_client_target_preferred(struct drm_device *dev,
modes, offsets, i,
connector->tile_h_loc, connector->tile_v_loc);
}
- drm_dbg_kms(dev, "[CONNECTOR:%d:%s] looking for cmdline mode\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, "[CONNECTOR:%d:%s] looking for preferred mode, tile %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.
@@ -469,18 +471,22 @@ 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,
- "[CONNECTOR:%d:%s] Falling back to non-tiled mode\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, "[CONNECTOR:%d:%s] Found mode %s\n",
- connector->base.id, connector->name,
- modes[i] ? modes[i]->name : "none");
+ if (modes[i])
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] found %s mode: %s\n",
+ connector->base.id, connector->name,
+ mode_type, modes[i]->name);
+ else
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] no mode found\n",
+ connector->base.id, connector->name);
+
conn_configured |= BIT_ULL(i);
}
@@ -627,6 +633,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
struct drm_connector *connector;
struct drm_encoder *encoder;
struct drm_crtc *new_crtc;
+ const char *mode_type;
connector = connectors[i];
@@ -676,30 +683,22 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
*/
for (j = 0; j < count; j++) {
if (crtcs[j] == new_crtc) {
- drm_dbg_kms(dev, "fallback: cloned configuration\n");
+ drm_dbg_kms(dev, "[CONNECTOR:%d:%s] fallback: cloned configuration\n",
+ connector->base.id, connector->name);
goto bail;
}
}
- drm_dbg_kms(dev, "[CONNECTOR:%d:%s] looking for cmdline mode\n",
- connector->base.id, connector->name);
-
- /* go for command line mode first */
+ mode_type = "cmdline";
modes[i] = drm_connector_pick_cmdline_mode(connector);
- /* try for preferred next */
if (!modes[i]) {
- drm_dbg_kms(dev,
- "[CONNECTOR:%d:%s] looking for preferred mode, has tile: %s\n",
- connector->base.id, connector->name,
- str_yes_no(connector->has_tile));
+ mode_type = "preferred";
modes[i] = drm_connector_preferred_mode(connector, width, height);
}
- /* No preferred mode marked by the EDID? Are there any modes? */
- if (!modes[i] && !list_empty(&connector->modes)) {
- drm_dbg_kms(dev, "[CONNECTOR:%d:%s] using first listed mode\n",
- connector->base.id, connector->name);
+ if (!modes[i]) {
+ mode_type = "first";
modes[i] = drm_connector_first_mode(connector);
}
@@ -716,28 +715,25 @@ 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.
*/
- drm_dbg_kms(dev, "[CONNECTOR:%d:%s] looking for current mode\n",
- connector->base.id, connector->name);
+ mode_type = "current";
modes[i] = &connector->state->crtc->mode;
}
+
/*
* In case of tiled modes, if all tiles are not present
* then fallback to a non tiled mode.
*/
if (connector->has_tile &&
num_tiled_conns < connector->num_h_tile * connector->num_v_tile) {
- drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Falling back to non-tiled mode\n",
- connector->base.id, connector->name);
+ mode_type = "non tiled";
modes[i] = drm_connector_fallback_non_tiled_mode(connector);
}
crtcs[i] = new_crtc;
- drm_dbg_kms(dev, "[CONNECTOR:%d:%s] on [CRTC:%d:%s]: %dx%d%s\n",
+ drm_dbg_kms(dev, "[CONNECTOR::%d:%s] on [CRTC:%d:%s] using %s mode: %s\n",
connector->base.id, connector->name,
- connector->state->crtc->base.id,
- connector->state->crtc->name,
- modes[i]->hdisplay, modes[i]->vdisplay,
- modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" : "");
+ new_crtc->base.id, new_crtc->name,
+ mode_type, modes[i]->name);
fallback = false;
conn_configured |= BIT(i);
--
2.45.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v2 4/8] drm/client: Make copies of modes
2025-02-28 21:14 [PATCH v2 0/8] drm/client: Stop using legacy crtc->mode and a bunch of cleanups Ville Syrjala
` (2 preceding siblings ...)
2025-02-28 21:14 ` [PATCH v2 3/8] drm/client: Streamline mode selection debugs Ville Syrjala
@ 2025-02-28 21:14 ` Ville Syrjala
2025-03-12 14:38 ` Jani Nikula
2025-02-28 21:14 ` [PATCH v2 5/8] drm/client: Stop using the legacy crtc->mode Ville Syrjala
` (3 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2025-02-28 21:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
drm_client_firmware_config() is currently picking up the current
mode of the crtc via the legacy crtc->mode, which is not supposed
to be used by atomic drivers at all. We can't simply switch over
to the proper crtc->state->mode because we drop the crtc->mutex
(which protects crtc->state) before the mode gets used.
The most straightforward solution to extend the lifetime of
modes[] seem to be to make full copies of the modes.
And with this we can undo also commit 3eadd887dbac
("drm/client:Fully protect modes[] with dev->mode_config.mutex")
as the lifetime of modes[] no longer has anything to do with
that lock.
v2: Don't try to copy NULL modes
v3: Keep storing pointers and use drm_mode_{duplicate,destroy}()
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_client_modeset.c | 62 +++++++++++++++++++++-------
1 file changed, 47 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 148257287ae4..ff034359f063 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -265,6 +265,25 @@ static void drm_client_connectors_enabled(struct drm_connector *connectors[],
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static void mode_replace(struct drm_device *dev,
+ const struct drm_display_mode **dst,
+ const struct drm_display_mode *src)
+{
+ drm_mode_destroy(dev, (struct drm_display_mode *)*dst);
+
+ *dst = src ? drm_mode_duplicate(dev, src) : NULL;
+}
+
+static void modes_destroy(struct drm_device *dev,
+ const struct drm_display_mode *modes[],
+ int count)
+{
+ int i;
+
+ for (i = 0; i < count; i++)
+ mode_replace(dev, &modes[i], NULL);
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector *connectors[],
unsigned int connector_count,
@@ -296,7 +315,9 @@ static bool drm_client_target_cloned(struct drm_device *dev,
for (i = 0; i < connector_count; i++) {
if (!enabled[i])
continue;
- modes[i] = drm_connector_pick_cmdline_mode(connectors[i]);
+
+ mode_replace(dev, &modes[i],
+ drm_connector_pick_cmdline_mode(connectors[i]));
if (!modes[i]) {
can_clone = false;
break;
@@ -335,7 +356,7 @@ static bool drm_client_target_cloned(struct drm_device *dev,
DRM_MODE_MATCH_CLOCK |
DRM_MODE_MATCH_FLAGS |
DRM_MODE_MATCH_3D_FLAGS))
- modes[i] = mode;
+ mode_replace(dev, &modes[i], mode);
}
if (!modes[i])
can_clone = false;
@@ -445,16 +466,19 @@ static bool drm_client_target_preferred(struct drm_device *dev,
}
mode_type = "cmdline";
- modes[i] = drm_connector_pick_cmdline_mode(connector);
+ mode_replace(dev, &modes[i],
+ drm_connector_pick_cmdline_mode(connector));
if (!modes[i]) {
mode_type = "preferred";
- modes[i] = drm_connector_preferred_mode(connector, width, height);
+ mode_replace(dev, &modes[i],
+ drm_connector_preferred_mode(connector, width, height));
}
if (!modes[i]) {
mode_type = "first";
- modes[i] = drm_connector_first_mode(connector);
+ mode_replace(dev, &modes[i],
+ drm_connector_first_mode(connector));
}
/*
@@ -472,10 +496,12 @@ static bool drm_client_target_preferred(struct drm_device *dev,
connector->tile_v_loc == 0 &&
!drm_connector_get_tiled_mode(connector))) {
mode_type = "non tiled";
- modes[i] = drm_connector_fallback_non_tiled_mode(connector);
+ mode_replace(dev, &modes[i],
+ drm_connector_fallback_non_tiled_mode(connector));
} else {
mode_type = "tiled";
- modes[i] = drm_connector_get_tiled_mode(connector);
+ mode_replace(dev, &modes[i],
+ drm_connector_get_tiled_mode(connector));
}
}
@@ -690,16 +716,19 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
}
mode_type = "cmdline";
- modes[i] = drm_connector_pick_cmdline_mode(connector);
+ mode_replace(dev, &modes[i],
+ drm_connector_pick_cmdline_mode(connector));
if (!modes[i]) {
mode_type = "preferred";
- modes[i] = drm_connector_preferred_mode(connector, width, height);
+ mode_replace(dev, &modes[i],
+ drm_connector_preferred_mode(connector, width, height));
}
if (!modes[i]) {
mode_type = "first";
- modes[i] = drm_connector_first_mode(connector);
+ mode_replace(dev, &modes[i],
+ drm_connector_first_mode(connector));
}
/* last resort: use current mode */
@@ -716,7 +745,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
* fastboot check to work correctly.
*/
mode_type = "current";
- modes[i] = &connector->state->crtc->mode;
+ mode_replace(dev, &modes[i],
+ &connector->state->crtc->mode);
}
/*
@@ -726,7 +756,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
if (connector->has_tile &&
num_tiled_conns < connector->num_h_tile * connector->num_v_tile) {
mode_type = "non tiled";
- modes[i] = drm_connector_fallback_non_tiled_mode(connector);
+ mode_replace(dev, &modes[i],
+ drm_connector_fallback_non_tiled_mode(connector));
}
crtcs[i] = new_crtc;
@@ -798,7 +829,6 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
unsigned int total_modes_count = 0;
struct drm_client_offset *offsets;
unsigned int connector_count = 0;
- /* points to modes protected by mode_config.mutex */
const struct drm_display_mode **modes;
struct drm_crtc **crtcs;
int i, ret = 0;
@@ -850,7 +880,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
if (!drm_client_firmware_config(client, connectors, connector_count, crtcs,
modes, offsets, enabled, width, height)) {
- memset(modes, 0, connector_count * sizeof(*modes));
+ modes_destroy(dev, modes, connector_count);
memset(crtcs, 0, connector_count * sizeof(*crtcs));
memset(offsets, 0, connector_count * sizeof(*offsets));
@@ -867,6 +897,8 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
crtcs, modes, 0, width, height);
}
+ mutex_unlock(&dev->mode_config.mutex);
+
drm_client_modeset_release(client);
for (i = 0; i < connector_count; i++) {
@@ -901,11 +933,11 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
modeset->y = offset->y;
}
}
- mutex_unlock(&dev->mode_config.mutex);
mutex_unlock(&client->modeset_mutex);
out:
kfree(crtcs);
+ modes_destroy(dev, modes, connector_count);
kfree(modes);
kfree(offsets);
kfree(enabled);
--
2.45.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 4/8] drm/client: Make copies of modes
2025-02-28 21:14 ` [PATCH v2 4/8] drm/client: Make copies of modes Ville Syrjala
@ 2025-03-12 14:38 ` Jani Nikula
0 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2025-03-12 14:38 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Fri, 28 Feb 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drm_client_firmware_config() is currently picking up the current
> mode of the crtc via the legacy crtc->mode, which is not supposed
> to be used by atomic drivers at all. We can't simply switch over
> to the proper crtc->state->mode because we drop the crtc->mutex
> (which protects crtc->state) before the mode gets used.
>
> The most straightforward solution to extend the lifetime of
> modes[] seem to be to make full copies of the modes.
>
> And with this we can undo also commit 3eadd887dbac
> ("drm/client:Fully protect modes[] with dev->mode_config.mutex")
> as the lifetime of modes[] no longer has anything to do with
> that lock.
>
> v2: Don't try to copy NULL modes
> v3: Keep storing pointers and use drm_mode_{duplicate,destroy}()
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/drm_client_modeset.c | 62 +++++++++++++++++++++-------
> 1 file changed, 47 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 148257287ae4..ff034359f063 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -265,6 +265,25 @@ static void drm_client_connectors_enabled(struct drm_connector *connectors[],
> enabled[i] = drm_connector_enabled(connectors[i], false);
> }
>
> +static void mode_replace(struct drm_device *dev,
> + const struct drm_display_mode **dst,
> + const struct drm_display_mode *src)
> +{
> + drm_mode_destroy(dev, (struct drm_display_mode *)*dst);
Arguably drm_mode_destroy() should be changed to const too.
Anyway, I think I was able to wrap my head around this.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> +
> + *dst = src ? drm_mode_duplicate(dev, src) : NULL;
> +}
> +
> +static void modes_destroy(struct drm_device *dev,
> + const struct drm_display_mode *modes[],
> + int count)
> +{
> + int i;
> +
> + for (i = 0; i < count; i++)
> + mode_replace(dev, &modes[i], NULL);
> +}
> +
> static bool drm_client_target_cloned(struct drm_device *dev,
> struct drm_connector *connectors[],
> unsigned int connector_count,
> @@ -296,7 +315,9 @@ static bool drm_client_target_cloned(struct drm_device *dev,
> for (i = 0; i < connector_count; i++) {
> if (!enabled[i])
> continue;
> - modes[i] = drm_connector_pick_cmdline_mode(connectors[i]);
> +
> + mode_replace(dev, &modes[i],
> + drm_connector_pick_cmdline_mode(connectors[i]));
> if (!modes[i]) {
> can_clone = false;
> break;
> @@ -335,7 +356,7 @@ static bool drm_client_target_cloned(struct drm_device *dev,
> DRM_MODE_MATCH_CLOCK |
> DRM_MODE_MATCH_FLAGS |
> DRM_MODE_MATCH_3D_FLAGS))
> - modes[i] = mode;
> + mode_replace(dev, &modes[i], mode);
> }
> if (!modes[i])
> can_clone = false;
> @@ -445,16 +466,19 @@ static bool drm_client_target_preferred(struct drm_device *dev,
> }
>
> mode_type = "cmdline";
> - modes[i] = drm_connector_pick_cmdline_mode(connector);
> + mode_replace(dev, &modes[i],
> + drm_connector_pick_cmdline_mode(connector));
>
> if (!modes[i]) {
> mode_type = "preferred";
> - modes[i] = drm_connector_preferred_mode(connector, width, height);
> + mode_replace(dev, &modes[i],
> + drm_connector_preferred_mode(connector, width, height));
> }
>
> if (!modes[i]) {
> mode_type = "first";
> - modes[i] = drm_connector_first_mode(connector);
> + mode_replace(dev, &modes[i],
> + drm_connector_first_mode(connector));
> }
>
> /*
> @@ -472,10 +496,12 @@ static bool drm_client_target_preferred(struct drm_device *dev,
> connector->tile_v_loc == 0 &&
> !drm_connector_get_tiled_mode(connector))) {
> mode_type = "non tiled";
> - modes[i] = drm_connector_fallback_non_tiled_mode(connector);
> + mode_replace(dev, &modes[i],
> + drm_connector_fallback_non_tiled_mode(connector));
> } else {
> mode_type = "tiled";
> - modes[i] = drm_connector_get_tiled_mode(connector);
> + mode_replace(dev, &modes[i],
> + drm_connector_get_tiled_mode(connector));
> }
> }
>
> @@ -690,16 +716,19 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
> }
>
> mode_type = "cmdline";
> - modes[i] = drm_connector_pick_cmdline_mode(connector);
> + mode_replace(dev, &modes[i],
> + drm_connector_pick_cmdline_mode(connector));
>
> if (!modes[i]) {
> mode_type = "preferred";
> - modes[i] = drm_connector_preferred_mode(connector, width, height);
> + mode_replace(dev, &modes[i],
> + drm_connector_preferred_mode(connector, width, height));
> }
>
> if (!modes[i]) {
> mode_type = "first";
> - modes[i] = drm_connector_first_mode(connector);
> + mode_replace(dev, &modes[i],
> + drm_connector_first_mode(connector));
> }
>
> /* last resort: use current mode */
> @@ -716,7 +745,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
> * fastboot check to work correctly.
> */
> mode_type = "current";
> - modes[i] = &connector->state->crtc->mode;
> + mode_replace(dev, &modes[i],
> + &connector->state->crtc->mode);
> }
>
> /*
> @@ -726,7 +756,8 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
> if (connector->has_tile &&
> num_tiled_conns < connector->num_h_tile * connector->num_v_tile) {
> mode_type = "non tiled";
> - modes[i] = drm_connector_fallback_non_tiled_mode(connector);
> + mode_replace(dev, &modes[i],
> + drm_connector_fallback_non_tiled_mode(connector));
> }
> crtcs[i] = new_crtc;
>
> @@ -798,7 +829,6 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
> unsigned int total_modes_count = 0;
> struct drm_client_offset *offsets;
> unsigned int connector_count = 0;
> - /* points to modes protected by mode_config.mutex */
> const struct drm_display_mode **modes;
> struct drm_crtc **crtcs;
> int i, ret = 0;
> @@ -850,7 +880,7 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
>
> if (!drm_client_firmware_config(client, connectors, connector_count, crtcs,
> modes, offsets, enabled, width, height)) {
> - memset(modes, 0, connector_count * sizeof(*modes));
> + modes_destroy(dev, modes, connector_count);
> memset(crtcs, 0, connector_count * sizeof(*crtcs));
> memset(offsets, 0, connector_count * sizeof(*offsets));
>
> @@ -867,6 +897,8 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
> crtcs, modes, 0, width, height);
> }
>
> + mutex_unlock(&dev->mode_config.mutex);
> +
> drm_client_modeset_release(client);
>
> for (i = 0; i < connector_count; i++) {
> @@ -901,11 +933,11 @@ int drm_client_modeset_probe(struct drm_client_dev *client, unsigned int width,
> modeset->y = offset->y;
> }
> }
> - mutex_unlock(&dev->mode_config.mutex);
>
> mutex_unlock(&client->modeset_mutex);
> out:
> kfree(crtcs);
> + modes_destroy(dev, modes, connector_count);
> kfree(modes);
> kfree(offsets);
> kfree(enabled);
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 5/8] drm/client: Stop using the legacy crtc->mode
2025-02-28 21:14 [PATCH v2 0/8] drm/client: Stop using legacy crtc->mode and a bunch of cleanups Ville Syrjala
` (3 preceding siblings ...)
2025-02-28 21:14 ` [PATCH v2 4/8] drm/client: Make copies of modes Ville Syrjala
@ 2025-02-28 21:14 ` Ville Syrjala
2025-03-12 14:44 ` Jani Nikula
2025-02-28 21:14 ` [PATCH v2 6/8] drm/client: s/new_crtc/crtc/ Ville Syrjala
` (2 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2025-02-28 21:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
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 now that the modes[] lifetime issues
have been sorted out we can just switch over to the
proper crtc->state->mode.
v2: Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_client_modeset.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index ff034359f063..4c64535fb82c 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -733,20 +733,9 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
/* last resort: use current mode */
if (!modes[i]) {
- /*
- * IMPORTANT: We want to use the adjusted mode (i.e.
- * after the panel fitter upscaling) as the initial
- * config, not the input mode, which is what crtc->mode
- * usually contains. But since our current
- * code puts a mode derived from the post-pfit timings
- * into crtc->mode this works out correctly.
- *
- * This is crtc->mode and not crtc->state->mode for the
- * fastboot check to work correctly.
- */
mode_type = "current";
mode_replace(dev, &modes[i],
- &connector->state->crtc->mode);
+ &new_crtc->state->mode);
}
/*
--
2.45.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 5/8] drm/client: Stop using the legacy crtc->mode
2025-02-28 21:14 ` [PATCH v2 5/8] drm/client: Stop using the legacy crtc->mode Ville Syrjala
@ 2025-03-12 14:44 ` Jani Nikula
0 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2025-03-12 14:44 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Fri, 28 Feb 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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 now that the modes[] lifetime issues
> have been sorted out we can just switch over to the
> proper crtc->state->mode.
>
> v2: Rebase
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_client_modeset.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index ff034359f063..4c64535fb82c 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -733,20 +733,9 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
>
> /* last resort: use current mode */
> if (!modes[i]) {
> - /*
> - * IMPORTANT: We want to use the adjusted mode (i.e.
> - * after the panel fitter upscaling) as the initial
> - * config, not the input mode, which is what crtc->mode
> - * usually contains. But since our current
> - * code puts a mode derived from the post-pfit timings
> - * into crtc->mode this works out correctly.
> - *
> - * This is crtc->mode and not crtc->state->mode for the
> - * fastboot check to work correctly.
> - */
> mode_type = "current";
> mode_replace(dev, &modes[i],
> - &connector->state->crtc->mode);
> + &new_crtc->state->mode);
> }
>
> /*
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 6/8] drm/client: s/new_crtc/crtc/
2025-02-28 21:14 [PATCH v2 0/8] drm/client: Stop using legacy crtc->mode and a bunch of cleanups Ville Syrjala
` (4 preceding siblings ...)
2025-02-28 21:14 ` [PATCH v2 5/8] drm/client: Stop using the legacy crtc->mode Ville Syrjala
@ 2025-02-28 21:14 ` Ville Syrjala
2025-03-12 14:45 ` Jani Nikula
2025-02-28 21:14 ` [PATCH v2 7/8] drm/client: Move variables to tighter scope Ville Syrjala
2025-02-28 21:14 ` [PATCH v2 8/8] drm/client: s/unsigned int i/int i/ Ville Syrjala
7 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2025-02-28 21:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Rename the 'new_crtc' variable to just 'crtc' in
drm_client_firmware_config(). We don't call any of the other
stuff in here new or old so this feels out of place.
v2: Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_client_modeset.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 4c64535fb82c..a0caa2b229dd 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -658,7 +658,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
for (i = 0; i < count; i++) {
struct drm_connector *connector;
struct drm_encoder *encoder;
- struct drm_crtc *new_crtc;
+ struct drm_crtc *crtc;
const char *mode_type;
connector = connectors[i];
@@ -700,7 +700,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
num_connectors_enabled++;
- new_crtc = connector->state->crtc;
+ crtc = connector->state->crtc;
/*
* Make sure we're not trying to drive multiple connectors
@@ -708,7 +708,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
* match the BIOS.
*/
for (j = 0; j < count; j++) {
- if (crtcs[j] == new_crtc) {
+ if (crtcs[j] == crtc) {
drm_dbg_kms(dev, "[CONNECTOR:%d:%s] fallback: cloned configuration\n",
connector->base.id, connector->name);
goto bail;
@@ -735,7 +735,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
if (!modes[i]) {
mode_type = "current";
mode_replace(dev, &modes[i],
- &new_crtc->state->mode);
+ &crtc->state->mode);
}
/*
@@ -748,11 +748,11 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
mode_replace(dev, &modes[i],
drm_connector_fallback_non_tiled_mode(connector));
}
- crtcs[i] = new_crtc;
+ crtcs[i] = crtc;
drm_dbg_kms(dev, "[CONNECTOR::%d:%s] on [CRTC:%d:%s] using %s mode: %s\n",
connector->base.id, connector->name,
- new_crtc->base.id, new_crtc->name,
+ crtc->base.id, crtc->name,
mode_type, modes[i]->name);
fallback = false;
--
2.45.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 6/8] drm/client: s/new_crtc/crtc/
2025-02-28 21:14 ` [PATCH v2 6/8] drm/client: s/new_crtc/crtc/ Ville Syrjala
@ 2025-03-12 14:45 ` Jani Nikula
0 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2025-03-12 14:45 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Fri, 28 Feb 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rename the 'new_crtc' variable to just 'crtc' in
> drm_client_firmware_config(). We don't call any of the other
> stuff in here new or old so this feels out of place.
>
> v2: Rebase
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_client_modeset.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 4c64535fb82c..a0caa2b229dd 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -658,7 +658,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
> for (i = 0; i < count; i++) {
> struct drm_connector *connector;
> struct drm_encoder *encoder;
> - struct drm_crtc *new_crtc;
> + struct drm_crtc *crtc;
> const char *mode_type;
>
> connector = connectors[i];
> @@ -700,7 +700,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
>
> num_connectors_enabled++;
>
> - new_crtc = connector->state->crtc;
> + crtc = connector->state->crtc;
>
> /*
> * Make sure we're not trying to drive multiple connectors
> @@ -708,7 +708,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
> * match the BIOS.
> */
> for (j = 0; j < count; j++) {
> - if (crtcs[j] == new_crtc) {
> + if (crtcs[j] == crtc) {
> drm_dbg_kms(dev, "[CONNECTOR:%d:%s] fallback: cloned configuration\n",
> connector->base.id, connector->name);
> goto bail;
> @@ -735,7 +735,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
> if (!modes[i]) {
> mode_type = "current";
> mode_replace(dev, &modes[i],
> - &new_crtc->state->mode);
> + &crtc->state->mode);
> }
>
> /*
> @@ -748,11 +748,11 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
> mode_replace(dev, &modes[i],
> drm_connector_fallback_non_tiled_mode(connector));
> }
> - crtcs[i] = new_crtc;
> + crtcs[i] = crtc;
>
> drm_dbg_kms(dev, "[CONNECTOR::%d:%s] on [CRTC:%d:%s] using %s mode: %s\n",
> connector->base.id, connector->name,
> - new_crtc->base.id, new_crtc->name,
> + crtc->base.id, crtc->name,
> mode_type, modes[i]->name);
>
> fallback = false;
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 7/8] drm/client: Move variables to tighter scope
2025-02-28 21:14 [PATCH v2 0/8] drm/client: Stop using legacy crtc->mode and a bunch of cleanups Ville Syrjala
` (5 preceding siblings ...)
2025-02-28 21:14 ` [PATCH v2 6/8] drm/client: s/new_crtc/crtc/ Ville Syrjala
@ 2025-02-28 21:14 ` Ville Syrjala
2025-03-12 14:45 ` Jani Nikula
2025-02-28 21:14 ` [PATCH v2 8/8] drm/client: s/unsigned int i/int i/ Ville Syrjala
7 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2025-02-28 21:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Bunch of variables are only needed inside loops and whatnot.
Move them to a tighter scope to make the code less confusing.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_client_modeset.c | 33 +++++++++++++++-------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index a0caa2b229dd..54cbcaa476e2 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -73,9 +73,10 @@ int drm_client_modeset_create(struct drm_client_dev *client)
static void drm_client_modeset_release(struct drm_client_dev *client)
{
struct drm_mode_set *modeset;
- unsigned int i;
drm_client_for_each_modeset(modeset, client) {
+ unsigned int i;
+
drm_mode_destroy(client->dev, modeset->mode);
modeset->mode = NULL;
modeset->fb = NULL;
@@ -291,9 +292,8 @@ static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_client_offset offsets[],
bool enabled[], int width, int height)
{
- int count, i, j;
+ int count, i;
bool can_clone = false;
- const struct drm_display_mode *mode;
struct drm_display_mode *dmt_mode;
/* only contemplate cloning in the single crtc case */
@@ -313,6 +313,8 @@ static bool drm_client_target_cloned(struct drm_device *dev,
/* check the command line or if nothing common pick 1024x768 */
can_clone = true;
for (i = 0; i < connector_count; i++) {
+ int j;
+
if (!enabled[i])
continue;
@@ -347,6 +349,8 @@ static bool drm_client_target_cloned(struct drm_device *dev,
goto fail;
for (i = 0; i < connector_count; i++) {
+ const struct drm_display_mode *mode;
+
if (!enabled[i])
continue;
@@ -380,12 +384,12 @@ static int drm_client_get_tile_offsets(struct drm_device *dev,
int idx,
int h_idx, int v_idx)
{
- struct drm_connector *connector;
int i;
int hoffset = 0, voffset = 0;
for (i = 0; i < connector_count; i++) {
- connector = connectors[i];
+ struct drm_connector *connector = connectors[i];
+
if (!connector->has_tile)
continue;
@@ -415,7 +419,6 @@ static bool drm_client_target_preferred(struct drm_device *dev,
bool enabled[], int width, int height)
{
const u64 mask = BIT_ULL(connector_count) - 1;
- struct drm_connector *connector;
u64 conn_configured = 0;
int tile_pass = 0;
int num_tiled_conns = 0;
@@ -429,9 +432,9 @@ static bool drm_client_target_preferred(struct drm_device *dev,
retry:
for (i = 0; i < connector_count; i++) {
+ struct drm_connector *connector = connectors[i];
const char *mode_type;
- connector = connectors[i];
if (conn_configured & BIT_ULL(i))
continue;
@@ -546,9 +549,8 @@ static int drm_client_pick_crtcs(struct drm_client_dev *client,
struct drm_device *dev = client->dev;
struct drm_connector *connector;
int my_score, best_score, score;
- struct drm_crtc **crtcs, *crtc;
+ struct drm_crtc **crtcs;
struct drm_mode_set *modeset;
- int o;
if (n == connector_count)
return 0;
@@ -578,7 +580,8 @@ static int drm_client_pick_crtcs(struct drm_client_dev *client,
* remaining connectors
*/
drm_client_for_each_modeset(modeset, client) {
- crtc = modeset->crtc;
+ struct drm_crtc *crtc = modeset->crtc;
+ int o;
if (!connector_has_possible_crtc(connector, crtc))
continue;
@@ -622,7 +625,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
const int count = min_t(unsigned int, connector_count, BITS_PER_LONG);
unsigned long conn_configured, conn_seq, mask;
struct drm_device *dev = client->dev;
- int i, j;
+ int i;
bool *save_enabled;
bool fallback = true, ret = true;
int num_connectors_enabled = 0;
@@ -656,12 +659,11 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
retry:
conn_seq = conn_configured;
for (i = 0; i < count; i++) {
- struct drm_connector *connector;
+ struct drm_connector *connector = connectors[i];
struct drm_encoder *encoder;
struct drm_crtc *crtc;
const char *mode_type;
-
- connector = connectors[i];
+ int j;
if (conn_configured & BIT(i))
continue;
@@ -1239,11 +1241,12 @@ static void drm_client_modeset_dpms_legacy(struct drm_client_dev *client, int dp
struct drm_connector *connector;
struct drm_mode_set *modeset;
struct drm_modeset_acquire_ctx ctx;
- int j;
int ret;
DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
drm_client_for_each_modeset(modeset, client) {
+ int j;
+
if (!modeset->crtc->enabled)
continue;
--
2.45.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 7/8] drm/client: Move variables to tighter scope
2025-02-28 21:14 ` [PATCH v2 7/8] drm/client: Move variables to tighter scope Ville Syrjala
@ 2025-03-12 14:45 ` Jani Nikula
0 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2025-03-12 14:45 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Fri, 28 Feb 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Bunch of variables are only needed inside loops and whatnot.
> Move them to a tighter scope to make the code less confusing.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_client_modeset.c | 33 +++++++++++++++-------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index a0caa2b229dd..54cbcaa476e2 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -73,9 +73,10 @@ int drm_client_modeset_create(struct drm_client_dev *client)
> static void drm_client_modeset_release(struct drm_client_dev *client)
> {
> struct drm_mode_set *modeset;
> - unsigned int i;
>
> drm_client_for_each_modeset(modeset, client) {
> + unsigned int i;
> +
> drm_mode_destroy(client->dev, modeset->mode);
> modeset->mode = NULL;
> modeset->fb = NULL;
> @@ -291,9 +292,8 @@ static bool drm_client_target_cloned(struct drm_device *dev,
> struct drm_client_offset offsets[],
> bool enabled[], int width, int height)
> {
> - int count, i, j;
> + int count, i;
> bool can_clone = false;
> - const struct drm_display_mode *mode;
> struct drm_display_mode *dmt_mode;
>
> /* only contemplate cloning in the single crtc case */
> @@ -313,6 +313,8 @@ static bool drm_client_target_cloned(struct drm_device *dev,
> /* check the command line or if nothing common pick 1024x768 */
> can_clone = true;
> for (i = 0; i < connector_count; i++) {
> + int j;
> +
> if (!enabled[i])
> continue;
>
> @@ -347,6 +349,8 @@ static bool drm_client_target_cloned(struct drm_device *dev,
> goto fail;
>
> for (i = 0; i < connector_count; i++) {
> + const struct drm_display_mode *mode;
> +
> if (!enabled[i])
> continue;
>
> @@ -380,12 +384,12 @@ static int drm_client_get_tile_offsets(struct drm_device *dev,
> int idx,
> int h_idx, int v_idx)
> {
> - struct drm_connector *connector;
> int i;
> int hoffset = 0, voffset = 0;
>
> for (i = 0; i < connector_count; i++) {
> - connector = connectors[i];
> + struct drm_connector *connector = connectors[i];
> +
> if (!connector->has_tile)
> continue;
>
> @@ -415,7 +419,6 @@ static bool drm_client_target_preferred(struct drm_device *dev,
> bool enabled[], int width, int height)
> {
> const u64 mask = BIT_ULL(connector_count) - 1;
> - struct drm_connector *connector;
> u64 conn_configured = 0;
> int tile_pass = 0;
> int num_tiled_conns = 0;
> @@ -429,9 +432,9 @@ static bool drm_client_target_preferred(struct drm_device *dev,
>
> retry:
> for (i = 0; i < connector_count; i++) {
> + struct drm_connector *connector = connectors[i];
> const char *mode_type;
>
> - connector = connectors[i];
>
> if (conn_configured & BIT_ULL(i))
> continue;
> @@ -546,9 +549,8 @@ static int drm_client_pick_crtcs(struct drm_client_dev *client,
> struct drm_device *dev = client->dev;
> struct drm_connector *connector;
> int my_score, best_score, score;
> - struct drm_crtc **crtcs, *crtc;
> + struct drm_crtc **crtcs;
> struct drm_mode_set *modeset;
> - int o;
>
> if (n == connector_count)
> return 0;
> @@ -578,7 +580,8 @@ static int drm_client_pick_crtcs(struct drm_client_dev *client,
> * remaining connectors
> */
> drm_client_for_each_modeset(modeset, client) {
> - crtc = modeset->crtc;
> + struct drm_crtc *crtc = modeset->crtc;
> + int o;
>
> if (!connector_has_possible_crtc(connector, crtc))
> continue;
> @@ -622,7 +625,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
> const int count = min_t(unsigned int, connector_count, BITS_PER_LONG);
> unsigned long conn_configured, conn_seq, mask;
> struct drm_device *dev = client->dev;
> - int i, j;
> + int i;
> bool *save_enabled;
> bool fallback = true, ret = true;
> int num_connectors_enabled = 0;
> @@ -656,12 +659,11 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
> retry:
> conn_seq = conn_configured;
> for (i = 0; i < count; i++) {
> - struct drm_connector *connector;
> + struct drm_connector *connector = connectors[i];
> struct drm_encoder *encoder;
> struct drm_crtc *crtc;
> const char *mode_type;
> -
> - connector = connectors[i];
> + int j;
>
> if (conn_configured & BIT(i))
> continue;
> @@ -1239,11 +1241,12 @@ static void drm_client_modeset_dpms_legacy(struct drm_client_dev *client, int dp
> struct drm_connector *connector;
> struct drm_mode_set *modeset;
> struct drm_modeset_acquire_ctx ctx;
> - int j;
> int ret;
>
> DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
> drm_client_for_each_modeset(modeset, client) {
> + int j;
> +
> if (!modeset->crtc->enabled)
> continue;
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 8/8] drm/client: s/unsigned int i/int i/
2025-02-28 21:14 [PATCH v2 0/8] drm/client: Stop using legacy crtc->mode and a bunch of cleanups Ville Syrjala
` (6 preceding siblings ...)
2025-02-28 21:14 ` [PATCH v2 7/8] drm/client: Move variables to tighter scope Ville Syrjala
@ 2025-02-28 21:14 ` Ville Syrjala
2025-03-12 14:45 ` Jani Nikula
7 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjala @ 2025-02-28 21:14 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Replace the 'unsigned int i' footguns with plain old signed
int. Avoids accidents if/when someone decides they need
to iterate backwards.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/drm_client_modeset.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 54cbcaa476e2..0f9d5ba36c81 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -39,7 +39,7 @@ int drm_client_modeset_create(struct drm_client_dev *client)
unsigned int max_connector_count = 1;
struct drm_mode_set *modeset;
struct drm_crtc *crtc;
- unsigned int i = 0;
+ int i = 0;
/* Add terminating zero entry to enable index less iteration */
client->modesets = kcalloc(num_crtc + 1, sizeof(*client->modesets), GFP_KERNEL);
@@ -75,7 +75,7 @@ static void drm_client_modeset_release(struct drm_client_dev *client)
struct drm_mode_set *modeset;
drm_client_for_each_modeset(modeset, client) {
- unsigned int i;
+ int i;
drm_mode_destroy(client->dev, modeset->mode);
modeset->mode = NULL;
@@ -960,7 +960,7 @@ bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation)
struct drm_plane *plane = modeset->crtc->primary;
struct drm_cmdline_mode *cmdline;
u64 valid_mask = 0;
- unsigned int i;
+ int i;
if (!modeset->num_connectors)
return false;
--
2.45.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v2 8/8] drm/client: s/unsigned int i/int i/
2025-02-28 21:14 ` [PATCH v2 8/8] drm/client: s/unsigned int i/int i/ Ville Syrjala
@ 2025-03-12 14:45 ` Jani Nikula
0 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2025-03-12 14:45 UTC (permalink / raw)
To: Ville Syrjala, dri-devel; +Cc: intel-gfx
On Fri, 28 Feb 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Replace the 'unsigned int i' footguns with plain old signed
> int. Avoids accidents if/when someone decides they need
> to iterate backwards.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/drm_client_modeset.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 54cbcaa476e2..0f9d5ba36c81 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -39,7 +39,7 @@ int drm_client_modeset_create(struct drm_client_dev *client)
> unsigned int max_connector_count = 1;
> struct drm_mode_set *modeset;
> struct drm_crtc *crtc;
> - unsigned int i = 0;
> + int i = 0;
>
> /* Add terminating zero entry to enable index less iteration */
> client->modesets = kcalloc(num_crtc + 1, sizeof(*client->modesets), GFP_KERNEL);
> @@ -75,7 +75,7 @@ static void drm_client_modeset_release(struct drm_client_dev *client)
> struct drm_mode_set *modeset;
>
> drm_client_for_each_modeset(modeset, client) {
> - unsigned int i;
> + int i;
>
> drm_mode_destroy(client->dev, modeset->mode);
> modeset->mode = NULL;
> @@ -960,7 +960,7 @@ bool drm_client_rotation(struct drm_mode_set *modeset, unsigned int *rotation)
> struct drm_plane *plane = modeset->crtc->primary;
> struct drm_cmdline_mode *cmdline;
> u64 valid_mask = 0;
> - unsigned int i;
> + int i;
>
> if (!modeset->num_connectors)
> return false;
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 18+ messages in thread