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

Reply via email to