[RFC PATCH 17/37] DRM: Atomic: Use pointer for mode in CRTC state
Daniel Stone
daniels at collabora.com
Wed Mar 18 21:33:16 PDT 2015
Holding a pointer to the mode, rather than an embed, allows us to get
towards sharing refcounted modes.
XXX: atomic_destroy_state does _not_ seem to be optional - so we should
remove any fallback paths which compensate for its lack!
the crtc_state->mode handling is particularly ugly here :\
Signed-off-by: Daniel Stone <daniels at collabora.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 35 +++++++++++++++++++++++--------
drivers/gpu/drm/drm_crtc.c | 2 +-
drivers/gpu/drm/drm_crtc_helper.c | 32 ++++++++++++++++++++++++----
drivers/gpu/drm/i915/intel_atomic.c | 18 +++++++++++++++-
drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c | 2 +-
drivers/gpu/drm/tegra/dc.c | 8 +++++++
drivers/gpu/drm/tegra/dsi.c | 4 ++--
drivers/gpu/drm/tegra/hdmi.c | 2 +-
drivers/gpu/drm/tegra/rgb.c | 2 +-
drivers/gpu/drm/tegra/sor.c | 2 +-
include/drm/drm_crtc.h | 2 +-
12 files changed, 88 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 5bcb4ba..962443d 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -260,7 +260,7 @@ mode_fixup(struct drm_atomic_state *state)
if (!crtc_state || !crtc_state->mode_changed)
continue;
- drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode);
+ drm_mode_copy(&crtc_state->adjusted_mode, crtc_state->mode);
}
for (i = 0; i < state->num_connector; i++) {
@@ -289,7 +289,7 @@ mode_fixup(struct drm_atomic_state *state)
if (encoder->bridge && encoder->bridge->funcs->mode_fixup) {
ret = encoder->bridge->funcs->mode_fixup(
- encoder->bridge, &crtc_state->mode,
+ encoder->bridge, crtc_state->mode,
&crtc_state->adjusted_mode);
if (!ret) {
DRM_DEBUG_ATOMIC("Bridge fixup failed\n");
@@ -306,7 +306,7 @@ mode_fixup(struct drm_atomic_state *state)
return ret;
}
} else {
- ret = funcs->mode_fixup(encoder, &crtc_state->mode,
+ ret = funcs->mode_fixup(encoder, crtc_state->mode,
&crtc_state->adjusted_mode);
if (!ret) {
DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] fixup failed\n",
@@ -327,7 +327,7 @@ mode_fixup(struct drm_atomic_state *state)
continue;
funcs = crtc->helper_private;
- ret = funcs->mode_fixup(crtc, &crtc_state->mode,
+ ret = funcs->mode_fixup(crtc, crtc_state->mode,
&crtc_state->adjusted_mode);
if (!ret) {
DRM_DEBUG_ATOMIC("[CRTC:%d] fixup failed\n",
@@ -383,7 +383,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
if (!crtc)
continue;
- if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode)) {
+ if (!drm_mode_equal(crtc->state->mode, crtc_state->mode)) {
DRM_DEBUG_ATOMIC("[CRTC:%d] mode changed\n",
crtc->base.id);
crtc_state->mode_changed = true;
@@ -699,7 +699,7 @@ set_routing_links(struct drm_device *dev, struct drm_atomic_state *old_state)
if (!crtc)
continue;
- crtc->mode = crtc->state->mode;
+ crtc->mode = *crtc->state->mode;
crtc->enabled = crtc->state->enable;
crtc->x = crtc->primary->state->src_x >> 16;
crtc->y = crtc->primary->state->src_y >> 16;
@@ -746,7 +746,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
encoder = connector->state->best_encoder;
funcs = encoder->helper_private;
new_crtc_state = connector->state->crtc->state;
- mode = &new_crtc_state->mode;
+ mode = new_crtc_state->mode;
adjusted_mode = &new_crtc_state->adjusted_mode;
if (!new_crtc_state->mode_changed)
@@ -1643,7 +1643,7 @@ retry:
crtc_state->enable = true;
crtc_state->active = true;
- drm_mode_copy(&crtc_state->mode, set->mode);
+ drm_mode_copy(crtc_state->mode, set->mode);
ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);
if (ret != 0)
@@ -2058,11 +2058,22 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_dpms);
*/
void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
{
+ if (crtc->state)
+ kfree(crtc->state->mode);
+
kfree(crtc->state);
crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
- if (crtc->state)
+ if (crtc->state) {
crtc->state->crtc = crtc;
+ crtc->state->mode =
+ kzalloc(sizeof(*crtc->state->mode), GFP_KERNEL);
+ }
+
+ if (crtc->state && !crtc->state->mode) {
+ kfree(crtc->state);
+ crtc->state = NULL;
+ }
}
EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
@@ -2088,6 +2099,11 @@ drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc)
state->active_changed = false;
state->planes_changed = false;
state->event = NULL;
+ state->mode = drm_mode_duplicate(crtc->dev, crtc->state->mode);
+ if (!state->mode) {
+ kfree(state);
+ state = NULL;
+ }
}
return state;
@@ -2105,6 +2121,7 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
+ kfree(state->mode);
kfree(state);
}
EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 5785336..6023851 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2008,7 +2008,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
crtc_resp->x = crtc->primary->state->src_x >> 16;
crtc_resp->y = crtc->primary->state->src_y >> 16;
if (crtc->state->enable) {
- drm_crtc_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
+ drm_crtc_convert_to_umode(&crtc_resp->mode, crtc->state->mode);
crtc_resp->mode_valid = 1;
} else {
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index c6063ff..8a9a045 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -943,11 +943,32 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc,
if (crtc->funcs->atomic_duplicate_state)
crtc_state = crtc->funcs->atomic_duplicate_state(crtc);
- else if (crtc->state)
+ else if (crtc->state) {
crtc_state = kmemdup(crtc->state, sizeof(*crtc_state),
GFP_KERNEL);
- else
+ /* XXX: this is unpleasant: we should mandate dup instead */
+ if (crtc_state) {
+ crtc_state->mode =
+ drm_mode_duplicate(crtc->dev,
+ crtc->state->mode);
+ if (!crtc_state->mode) {
+ kfree(crtc_state);
+ crtc_state = NULL;
+ }
+ }
+ }
+ else {
crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
+ if (crtc_state) {
+ crtc_state->mode = kzalloc(sizeof(*crtc_state->mode),
+ GFP_KERNEL);
+ /* XXX: as above, but mandate a new_state */
+ if (!crtc_state->mode) {
+ kfree(crtc_state);
+ crtc_state = NULL;
+ }
+ }
+ }
if (!crtc_state)
return -ENOMEM;
crtc_state->crtc = crtc;
@@ -955,12 +976,13 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc,
crtc_state->enable = true;
crtc_state->planes_changed = true;
crtc_state->mode_changed = true;
- drm_mode_copy(&crtc_state->mode, mode);
+ drm_mode_copy(crtc_state->mode, mode);
drm_mode_copy(&crtc_state->adjusted_mode, adjusted_mode);
if (crtc_funcs->atomic_check) {
ret = crtc_funcs->atomic_check(crtc, crtc_state);
if (ret) {
+ kfree(crtc_state->mode);
kfree(crtc_state);
return ret;
@@ -974,8 +996,10 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc,
if (crtc_state) {
if (crtc->funcs->atomic_destroy_state)
crtc->funcs->atomic_destroy_state(crtc, crtc_state);
- else
+ else {
+ kfree(crtc_state->mode);
kfree(crtc_state);
+ }
}
return drm_helper_crtc_mode_set_base(crtc, x, y, old_fb);
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 3903b90..c479386 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -222,9 +222,25 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
crtc_state = kmemdup(intel_crtc->config,
sizeof(*intel_crtc->config), GFP_KERNEL);
- if (crtc_state)
+ if (crtc_state) {
crtc_state->base.crtc = crtc;
+ /* XXX: this is tedious */
+ if (intel_crtc->config) {
+ crtc_state->mode =
+ drm_mode_duplicate(crtc->dev,
+ intel_crtc->config->mode);
+ } else {
+ crtc_state->mode =
+ kzalloc(sizeof(*crtc_state->mode), GFP_KERNEL);
+ }
+
+ if (!crtc_state->mode) {
+ kfree(crtc_state);
+ crtc_state = NULL;
+ }
+ }
+
return &crtc_state->base;
}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
index 0db6a54..14d74b4 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -64,7 +64,7 @@ static int rcar_du_encoder_atomic_check(struct drm_encoder *encoder,
{
struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
- const struct drm_display_mode *mode = &crtc_state->mode;
+ const struct drm_display_mode *mode = crtc_state->mode;
const struct drm_display_mode *panel_mode;
struct drm_connector *connector = conn_state->connector;
struct drm_device *dev = encoder->dev;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
index c000850..1c9b6f0 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_hdmienc.c
@@ -69,7 +69,7 @@ static int rcar_du_hdmienc_atomic_check(struct drm_encoder *encoder,
struct rcar_du_hdmienc *hdmienc = to_rcar_hdmienc(encoder);
struct drm_encoder_slave_funcs *sfuncs = to_slave_funcs(encoder);
struct drm_display_mode *adjusted_mode = &crtc_state->adjusted_mode;
- const struct drm_display_mode *mode = &crtc_state->mode;
+ const struct drm_display_mode *mode = crtc_state->mode;
/* The internal LVDS encoder has a clock frequency operating range of
* 30MHz to 150MHz. Clamp the clock accordingly.
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index b7f7815..40f6e74 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1015,6 +1015,13 @@ tegra_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
if (!copy)
return NULL;
+ /* XXX: tedium */
+ copy->base.mode = drm_mode_duplicate(crtc->dev, state->base.mode);
+ if (!copy->base.mode) {
+ kfree(copy);
+ return NULL;
+ }
+
copy->base.mode_changed = false;
copy->base.active_changed = false;
copy->base.planes_changed = false;
@@ -1026,6 +1033,7 @@ tegra_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
static void tegra_crtc_atomic_destroy_state(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
+ kfree(state->mode);
kfree(state);
}
diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index 4714eb4..cfd4da7 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -887,7 +887,7 @@ tegra_dsi_encoder_atomic_check(struct drm_encoder *encoder,
unsigned long plld;
int err;
- state->pclk = crtc_state->mode.clock * 1000;
+ state->pclk = crtc_state->mode->clock * 1000;
err = tegra_dsi_get_muldiv(dsi->format, &state->mul, &state->div);
if (err < 0)
@@ -899,7 +899,7 @@ tegra_dsi_encoder_atomic_check(struct drm_encoder *encoder,
if (err < 0)
return err;
- state->vrefresh = drm_mode_vrefresh(&crtc_state->mode);
+ state->vrefresh = drm_mode_vrefresh(crtc_state->mode);
/* compute byte clock */
state->bclk = (state->pclk * state->mul) / (state->div * state->lanes);
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 68d315e..d80de91 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -1060,7 +1060,7 @@ tegra_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
{
struct tegra_output *output = encoder_to_output(encoder);
struct tegra_dc *dc = to_tegra_dc(conn_state->crtc);
- unsigned long pclk = crtc_state->mode.clock * 1000;
+ unsigned long pclk = crtc_state->mode->clock * 1000;
struct tegra_hdmi *hdmi = to_hdmi(output);
int err;
diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
index 745759a..5b4d86a 100644
--- a/drivers/gpu/drm/tegra/rgb.c
+++ b/drivers/gpu/drm/tegra/rgb.c
@@ -196,7 +196,7 @@ tegra_rgb_encoder_atomic_check(struct drm_encoder *encoder,
{
struct tegra_output *output = encoder_to_output(encoder);
struct tegra_dc *dc = to_tegra_dc(conn_state->crtc);
- unsigned long pclk = crtc_state->mode.clock * 1000;
+ unsigned long pclk = crtc_state->mode->clock * 1000;
struct tegra_rgb *rgb = to_rgb(output);
unsigned int div;
int err;
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index fe86207..d0eaa48 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -1282,7 +1282,7 @@ tegra_sor_encoder_atomic_check(struct drm_encoder *encoder,
{
struct tegra_output *output = encoder_to_output(encoder);
struct tegra_dc *dc = to_tegra_dc(conn_state->crtc);
- unsigned long pclk = crtc_state->mode.clock * 1000;
+ unsigned long pclk = crtc_state->mode->clock * 1000;
struct tegra_sor *sor = to_sor(output);
int err;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 7b141d837..2bce96e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -294,7 +294,7 @@ struct drm_crtc_state {
/* adjusted_mode: for use by helpers and drivers */
struct drm_display_mode adjusted_mode;
- struct drm_display_mode mode;
+ struct drm_display_mode *mode;
struct drm_pending_vblank_event *event;
--
2.3.2
More information about the dri-devel
mailing list