From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
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 instead
of just storing pointers. We do have to replace the NULL checks
with something else though. Checking that mode->clock!=0
should be sufficient.
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
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/drm_client_modeset.c | 96 +++++++++++++++++-----------
1 file changed, 59 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index 888323137a6a..730ed0d4bfa9 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -265,10 +265,21 @@ static void drm_client_connectors_enabled(struct drm_connector *connectors[],
enabled[i] = drm_connector_enabled(connectors[i], false);
}
+static bool mode_valid(const struct drm_display_mode *mode)
+{
+ return mode->clock != 0;
+}
+
+static void mode_copy_if_not_null(struct drm_display_mode *dst, const struct drm_display_mode *src)
+{
+ if (src)
+ drm_mode_copy(dst, src);
+}
+
static bool drm_client_target_cloned(struct drm_device *dev,
struct drm_connector *connectors[],
unsigned int connector_count,
- const struct drm_display_mode *modes[],
+ struct drm_display_mode modes[],
struct drm_client_offset offsets[],
bool enabled[], int width, int height)
{
@@ -296,15 +307,16 @@ 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]);
- if (!modes[i]) {
+
+ mode_copy_if_not_null(&modes[i], drm_connector_pick_cmdline_mode(connectors[i]));
+ if (!mode_valid(&modes[i])) {
can_clone = false;
break;
}
for (j = 0; j < i; j++) {
if (!enabled[j])
continue;
- if (!drm_mode_match(modes[j], modes[i],
+ if (!drm_mode_match(&modes[j], &modes[i],
DRM_MODE_MATCH_TIMINGS |
DRM_MODE_MATCH_CLOCK |
DRM_MODE_MATCH_FLAGS |
@@ -335,9 +347,9 @@ 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_copy_if_not_null(&modes[i], mode);
}
- if (!modes[i])
+ if (!mode_valid(&modes[i]))
can_clone = false;
}
drm_mode_destroy(dev, dmt_mode);
@@ -354,7 +366,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,
- const struct drm_display_mode *modes[],
+ const struct drm_display_mode modes[],
struct drm_client_offset offsets[],
int idx,
int h_idx, int v_idx)
@@ -368,17 +380,17 @@ static int drm_client_get_tile_offsets(struct drm_device *dev,
if (!connector->has_tile)
continue;
- if (!modes[i] && (h_idx || v_idx)) {
+ if (!mode_valid(&modes[i]) && (h_idx || v_idx)) {
drm_dbg_kms(dev,
"[CONNECTOR:%d:%s] no modes for connector tiled %d\n",
connector->base.id, connector->name, i);
continue;
}
if (connector->tile_h_loc < h_idx)
- hoffset += modes[i]->hdisplay;
+ hoffset += modes[i].hdisplay;
if (connector->tile_v_loc < v_idx)
- voffset += modes[i]->vdisplay;
+ voffset += modes[i].vdisplay;
}
offsets[idx].x = hoffset;
offsets[idx].y = voffset;
@@ -389,7 +401,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,
- const struct drm_display_mode *modes[],
+ struct drm_display_mode modes[],
struct drm_client_offset offsets[],
bool enabled[], int width, int height)
{
@@ -445,16 +457,19 @@ static bool drm_client_target_preferred(struct drm_device *dev,
}
mode_type = "cmdline";
- modes[i] = drm_connector_pick_cmdline_mode(connector);
+ mode_copy_if_not_null(&modes[i],
+ drm_connector_pick_cmdline_mode(connector));
- if (!modes[i]) {
+ if (!mode_valid(&modes[i])) {
mode_type = "preferred";
- modes[i] = drm_connector_preferred_mode(connector, width, height);
+ mode_copy_if_not_null(&modes[i],
+ drm_connector_preferred_mode(connector, width, height));
}
- if (!modes[i]) {
+ if (!mode_valid(&modes[i])) {
mode_type = "first";
- modes[i] = drm_connector_first_mode(connector);
+ mode_copy_if_not_null(&modes[i],
+ drm_connector_first_mode(connector));
}
/*
@@ -472,17 +487,19 @@ 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_copy_if_not_null(&modes[i],
+ drm_connector_fallback_non_tiled_mode(connector));
} else {
mode_type = "tiled";
- modes[i] = drm_connector_get_tiled_mode(connector);
+ mode_copy_if_not_null(&modes[i],
+ drm_connector_get_tiled_mode(connector));
}
}
- if (modes[i])
+ if (mode_valid(&modes[i]))
drm_dbg_kms(dev, "[CONNECTOR:%d:%s] found %s mode: %s\n",
connector->base.id, connector->name,
- mode_type, modes[i]->name);
+ mode_type, modes[i].name);
else
drm_dbg_kms(dev, "[CONNECTOR:%d:%s] no mode found\n",
connector->base.id, connector->name);
@@ -514,7 +531,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[],
- const struct drm_display_mode *modes[],
+ const struct drm_display_mode modes[],
int n, int width, int height)
{
struct drm_device *dev = client->dev;
@@ -532,7 +549,7 @@ static int drm_client_pick_crtcs(struct drm_client_dev *client,
best_crtcs[n] = NULL;
best_score = drm_client_pick_crtcs(client, connectors, connector_count,
best_crtcs, modes, n + 1, width, height);
- if (modes[n] == NULL)
+ if (!mode_valid(&modes[n]))
return best_score;
crtcs = kcalloc(connector_count, sizeof(*crtcs), GFP_KERNEL);
@@ -566,7 +583,7 @@ static int drm_client_pick_crtcs(struct drm_client_dev *client,
if (dev->mode_config.num_crtc > 1)
continue;
- if (!drm_mode_equal(modes[o], modes[n]))
+ if (!drm_mode_equal(&modes[o], &modes[n]))
continue;
}
@@ -589,7 +606,7 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
struct drm_connector *connectors[],
unsigned int connector_count,
struct drm_crtc *crtcs[],
- const struct drm_display_mode *modes[],
+ struct drm_display_mode modes[],
struct drm_client_offset offsets[],
bool enabled[], int width, int height)
{
@@ -690,20 +707,23 @@ static bool drm_client_firmware_config(struct drm_client_dev *client,
}
mode_type = "cmdline";
- modes[i] = drm_connector_pick_cmdline_mode(connector);
+ mode_copy_if_not_null(&modes[i],
+ drm_connector_pick_cmdline_mode(connector));
- if (!modes[i]) {
+ if (!mode_valid(&modes[i])) {
mode_type = "preferred";
- modes[i] = drm_connector_preferred_mode(connector, width, height);
+ mode_copy_if_not_null(&modes[i],
+ drm_connector_preferred_mode(connector, width, height));
}
- if (!modes[i]) {
+ if (!mode_valid(&modes[i])) {
mode_type = "first";
- modes[i] = drm_connector_first_mode(connector);
+ mode_copy_if_not_null(&modes[i],
+ drm_connector_first_mode(connector));
}
/* last resort: use current mode */
- if (!modes[i]) {
+ if (!mode_valid(&modes[i])) {
/*
* IMPORTANT: We want to use the adjusted mode (i.e.
* after the panel fitter upscaling) as the initial
@@ -716,7 +736,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_copy_if_not_null(&modes[i],
+ &connector->state->crtc->mode);
}
/*
@@ -726,14 +747,15 @@ 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_copy_if_not_null(&modes[i],
+ drm_connector_fallback_non_tiled_mode(connector));
}
crtcs[i] = new_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,
- mode_type, modes[i]->name);
+ mode_type, modes[i].name);
fallback = false;
conn_configured |= BIT(i);
@@ -789,8 +811,7 @@ 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_display_mode *modes;
struct drm_crtc **crtcs;
int i, ret = 0;
bool *enabled;
@@ -858,10 +879,12 @@ 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++) {
- const 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];
@@ -892,7 +915,6 @@ 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:
--
2.45.2
