From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [CI 07/12] drm/i915: Complete crtc hw/uapi split, v4.
Date: Tue, 29 Oct 2019 16:33:15 +0200 [thread overview]
Message-ID: <20191029143315.GW1208@intel.com> (raw)
In-Reply-To: <20191029072229.27092-7-maarten.lankhorst@linux.intel.com>
On Tue, Oct 29, 2019 at 08:22:24AM +0100, Maarten Lankhorst wrote:
> Now that we separated everything into uapi and hw, it's
> time to make the split definitive. Remove the union and
> make a copy of the hw state on modeset and fastset.
>
> Color blobs are copied in crtc atomic_check(), right
> before color management is checked.
>
> Changes since v1:
> - Copy all blobs immediately after drm_atomic_helper_check_modeset().
> - Clear crtc_state->hw on disable, instead of using clear_intel_crtc_state().
> Changes since v2:
> - Use intel_crtc_free_hw_state + clear in intel_crtc_disable_noatomic().
> - Make a intel_crtc_prepare_state() function that clears the crtc_state
> and copies hw members.
> - Remove setting uapi.adjusted_mode, we now have a direct call to
> drm_calc_timestamping_constants().
> Changes since v3:
> - Rename prefix copy_hw_to_uapi_state() with intel_crtc.
> - Copy color blobs to uapi as well.
> - Add a intel_crtc_copy_uapi_to_hw_state_nomodeset() function for clarity.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_atomic.c | 44 ++++++++++
> drivers/gpu/drm/i915/display/intel_atomic.h | 2 +
> drivers/gpu/drm/i915/display/intel_display.c | 80 ++++++++++++++++---
> .../drm/i915/display/intel_display_types.h | 9 ++-
> 4 files changed, 118 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> index 366275dc113d..557178906ccf 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -195,6 +195,14 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>
> __drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->uapi);
>
> + /* copy color blobs */
> + if (crtc_state->hw.degamma_lut)
> + drm_property_blob_get(crtc_state->hw.degamma_lut);
> + if (crtc_state->hw.ctm)
> + drm_property_blob_get(crtc_state->hw.ctm);
> + if (crtc_state->hw.gamma_lut)
> + drm_property_blob_get(crtc_state->hw.gamma_lut);
> +
> crtc_state->update_pipe = false;
> crtc_state->disable_lp_wm = false;
> crtc_state->disable_cxsr = false;
> @@ -208,6 +216,41 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
> return &crtc_state->uapi;
> }
>
> +static void intel_crtc_put_color_blobs(struct intel_crtc_state *crtc_state)
> +{
> + drm_property_blob_put(crtc_state->hw.degamma_lut);
> + drm_property_blob_put(crtc_state->hw.gamma_lut);
> + drm_property_blob_put(crtc_state->hw.ctm);
> +}
> +
> +void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state)
> +{
> + intel_crtc_put_color_blobs(crtc_state);
> +}
> +
> +void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state)
> +{
> + intel_crtc_put_color_blobs(crtc_state);
> +
> + if (crtc_state->uapi.degamma_lut)
> + crtc_state->hw.degamma_lut =
> + drm_property_blob_get(crtc_state->uapi.degamma_lut);
> + else
> + crtc_state->hw.degamma_lut = NULL;
> +
> + if (crtc_state->uapi.gamma_lut)
> + crtc_state->hw.gamma_lut =
> + drm_property_blob_get(crtc_state->uapi.gamma_lut);
> + else
> + crtc_state->hw.gamma_lut = NULL;
> +
> + if (crtc_state->uapi.ctm)
> + crtc_state->hw.ctm =
> + drm_property_blob_get(crtc_state->uapi.ctm);
> + else
> + crtc_state->hw.ctm = NULL;
> +}
> +
> /**
> * intel_crtc_destroy_state - destroy crtc state
> * @crtc: drm crtc
> @@ -223,6 +266,7 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
> struct intel_crtc_state *crtc_state = to_intel_crtc_state(state);
>
> __drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi);
> + intel_crtc_free_hw_state(crtc_state);
> kfree(crtc_state);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h b/drivers/gpu/drm/i915/display/intel_atomic.h
> index 49d5cb1b9e0a..7b49623419ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.h
> @@ -36,6 +36,8 @@ intel_digital_connector_duplicate_state(struct drm_connector *connector);
> struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
> void intel_crtc_destroy_state(struct drm_crtc *crtc,
> struct drm_crtc_state *state);
> +void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state);
> +void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state);
> struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
> void intel_atomic_state_clear(struct drm_atomic_state *state);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 1bc008c094e4..a765794597ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7185,6 +7185,8 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
> crtc->enabled = false;
> crtc->state->connector_mask = 0;
> crtc->state->encoder_mask = 0;
> + intel_crtc_free_hw_state(crtc_state);
> + memset(&crtc_state->hw, 0, sizeof(crtc_state->hw));
>
> for_each_encoder_on_crtc(crtc->dev, crtc, encoder)
> encoder->base.crtc = NULL;
> @@ -12560,8 +12562,48 @@ static bool check_digital_port_conflicts(struct intel_atomic_state *state)
> return ret;
> }
>
> +static void
> +intel_crtc_copy_uapi_to_hw_state_nomodeset(struct intel_crtc_state *crtc_state)
> +{
> + intel_crtc_copy_color_blobs(crtc_state);
> +}
> +
> +static void
> +intel_crtc_copy_uapi_to_hw_state(struct intel_crtc_state *crtc_state)
> +{
> + crtc_state->hw.enable = crtc_state->uapi.enable;
> + crtc_state->hw.active = crtc_state->uapi.active;
> + crtc_state->hw.mode = crtc_state->uapi.mode;
> + crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
> + intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
> +}
> +
> +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state)
> +{
> + crtc_state->uapi.enable = crtc_state->hw.enable;
> + crtc_state->uapi.active = crtc_state->hw.active;
> + crtc_state->uapi.mode = crtc_state->hw.mode;
> + crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
> +
> + /* copy color blobs to uapi */
> + drm_property_blob_put(crtc_state->hw.degamma_lut);
uapi.degamma_lut
These look like inlined versions of drm_property_replace_blob()
actually.
Otherwise lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> + crtc_state->uapi.degamma_lut = crtc_state->hw.degamma_lut;
> + if (crtc_state->uapi.degamma_lut)
> + drm_property_blob_get(crtc_state->uapi.degamma_lut);
> +
> + drm_property_blob_put(crtc_state->uapi.ctm);
> + crtc_state->uapi.ctm = crtc_state->hw.ctm;
> + if (crtc_state->uapi.ctm)
> + drm_property_blob_get(crtc_state->uapi.ctm);
> +
> + drm_property_blob_put(crtc_state->uapi.gamma_lut);
> + crtc_state->uapi.gamma_lut = crtc_state->hw.gamma_lut;
> + if (crtc_state->uapi.gamma_lut)
> + drm_property_blob_get(crtc_state->uapi.gamma_lut);
> +}
> +
> static int
> -clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> +intel_crtc_prepare_cleared_state(struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *dev_priv =
> to_i915(crtc_state->uapi.crtc->dev);
> @@ -12571,11 +12613,15 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> if (!saved_state)
> return -ENOMEM;
>
> + /* free the old crtc_state->hw members */
> + intel_crtc_free_hw_state(crtc_state);
> +
> /* FIXME: before the switch to atomic started, a new pipe_config was
> * kzalloc'd. Code that depends on any field being zero should be
> * fixed, so that the crtc_state can be safely duplicated. For now,
> * only fields that are know to not cause problems are preserved. */
>
> + saved_state->uapi = crtc_state->uapi;
> saved_state->scaler_state = crtc_state->scaler_state;
> saved_state->shared_dpll = crtc_state->shared_dpll;
> saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
> @@ -12593,14 +12639,11 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> saved_state->sync_mode_slaves_mask =
> crtc_state->sync_mode_slaves_mask;
>
> - /* Keep base drm_crtc_state intact, only clear our extended struct */
> - BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
> - BUILD_BUG_ON(offsetof(struct intel_crtc_state, uapi));
> - BUILD_BUG_ON(offsetof(struct intel_crtc_state, hw));
> - memcpy(&crtc_state->uapi + 1, &saved_state->uapi + 1,
> - sizeof(*crtc_state) - sizeof(crtc_state->uapi));
> -
> + memcpy(crtc_state, saved_state, sizeof(*crtc_state));
> kfree(saved_state);
> +
> + intel_crtc_copy_uapi_to_hw_state(crtc_state);
> +
> return 0;
> }
>
> @@ -12616,10 +12659,6 @@ intel_modeset_pipe_config(struct intel_crtc_state *pipe_config)
> int i;
> bool retry = true;
>
> - ret = clear_intel_crtc_state(pipe_config);
> - if (ret)
> - return ret;
> -
> pipe_config->cpu_transcoder =
> (enum transcoder) to_intel_crtc(crtc)->pipe;
>
> @@ -13476,6 +13515,8 @@ verify_crtc_state(struct intel_crtc *crtc,
>
> state = old_crtc_state->uapi.state;
> __drm_atomic_helper_crtc_destroy_state(&old_crtc_state->uapi);
> + intel_crtc_free_hw_state(old_crtc_state);
> +
> pipe_config = old_crtc_state;
> memset(pipe_config, 0, sizeof(*pipe_config));
> pipe_config->uapi.crtc = &crtc->base;
> @@ -14010,14 +14051,24 @@ static int intel_atomic_check(struct drm_device *dev,
>
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> - if (!needs_modeset(new_crtc_state))
> + if (!needs_modeset(new_crtc_state)) {
> + /* Light copy */
> + intel_crtc_copy_uapi_to_hw_state_nomodeset(new_crtc_state);
> +
> continue;
> + }
>
> if (!new_crtc_state->uapi.enable) {
> + intel_crtc_copy_uapi_to_hw_state(new_crtc_state);
> +
> any_ms = true;
> continue;
> }
>
> + ret = intel_crtc_prepare_cleared_state(new_crtc_state);
> + if (ret)
> + goto fail;
> +
> ret = intel_modeset_pipe_config(new_crtc_state);
> if (ret)
> goto fail;
> @@ -17291,6 +17342,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> to_intel_crtc_state(crtc->base.state);
>
> __drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi);
> + intel_crtc_free_hw_state(crtc_state);
> memset(crtc_state, 0, sizeof(*crtc_state));
> __drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
>
> @@ -17419,6 +17471,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> * set a flag to indicate that a full recalculation is
> * needed on the next commit.
> */
> + crtc_state->hw.mode = crtc->base.mode;
> crtc_state->hw.mode.private_flags = I915_MODE_FLAG_INHERITED;
>
> intel_crtc_compute_pixel_rate(crtc_state);
> @@ -17467,6 +17520,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> intel_bw_crtc_update(bw_state, crtc_state);
>
> + intel_crtc_copy_hw_to_uapi_state(crtc_state);
> intel_pipe_config_sanity_check(dev_priv, crtc_state);
> }
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 573ced9dc909..bcf5f7640307 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -757,8 +757,6 @@ enum intel_output_format {
> };
>
> struct intel_crtc_state {
> - union {
> - struct drm_crtc_state base;
> /*
> * uapi (drm) state. This is the software state shown to userspace.
> * In particular, the following members are used for bookkeeping:
> @@ -781,8 +779,11 @@ struct intel_crtc_state {
> *
> * During initial hw readout, they need to be copied to uapi.
> */
> - struct drm_crtc_state hw;
> - };
> + struct {
> + bool active, enable;
> + struct drm_property_blob *degamma_lut, *gamma_lut, *ctm;
> + struct drm_display_mode mode, adjusted_mode;
> + } hw;
>
> /**
> * quirks - bitfield with hw state readout quirks
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [CI 07/12] drm/i915: Complete crtc hw/uapi split, v4.
Date: Tue, 29 Oct 2019 16:33:15 +0200 [thread overview]
Message-ID: <20191029143315.GW1208@intel.com> (raw)
Message-ID: <20191029143315.dohDhv9n3Y0XQx5sO2sqUdlidfjehWRoah1yJBWDTdo@z> (raw)
In-Reply-To: <20191029072229.27092-7-maarten.lankhorst@linux.intel.com>
On Tue, Oct 29, 2019 at 08:22:24AM +0100, Maarten Lankhorst wrote:
> Now that we separated everything into uapi and hw, it's
> time to make the split definitive. Remove the union and
> make a copy of the hw state on modeset and fastset.
>
> Color blobs are copied in crtc atomic_check(), right
> before color management is checked.
>
> Changes since v1:
> - Copy all blobs immediately after drm_atomic_helper_check_modeset().
> - Clear crtc_state->hw on disable, instead of using clear_intel_crtc_state().
> Changes since v2:
> - Use intel_crtc_free_hw_state + clear in intel_crtc_disable_noatomic().
> - Make a intel_crtc_prepare_state() function that clears the crtc_state
> and copies hw members.
> - Remove setting uapi.adjusted_mode, we now have a direct call to
> drm_calc_timestamping_constants().
> Changes since v3:
> - Rename prefix copy_hw_to_uapi_state() with intel_crtc.
> - Copy color blobs to uapi as well.
> - Add a intel_crtc_copy_uapi_to_hw_state_nomodeset() function for clarity.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_atomic.c | 44 ++++++++++
> drivers/gpu/drm/i915/display/intel_atomic.h | 2 +
> drivers/gpu/drm/i915/display/intel_display.c | 80 ++++++++++++++++---
> .../drm/i915/display/intel_display_types.h | 9 ++-
> 4 files changed, 118 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
> index 366275dc113d..557178906ccf 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -195,6 +195,14 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>
> __drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->uapi);
>
> + /* copy color blobs */
> + if (crtc_state->hw.degamma_lut)
> + drm_property_blob_get(crtc_state->hw.degamma_lut);
> + if (crtc_state->hw.ctm)
> + drm_property_blob_get(crtc_state->hw.ctm);
> + if (crtc_state->hw.gamma_lut)
> + drm_property_blob_get(crtc_state->hw.gamma_lut);
> +
> crtc_state->update_pipe = false;
> crtc_state->disable_lp_wm = false;
> crtc_state->disable_cxsr = false;
> @@ -208,6 +216,41 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
> return &crtc_state->uapi;
> }
>
> +static void intel_crtc_put_color_blobs(struct intel_crtc_state *crtc_state)
> +{
> + drm_property_blob_put(crtc_state->hw.degamma_lut);
> + drm_property_blob_put(crtc_state->hw.gamma_lut);
> + drm_property_blob_put(crtc_state->hw.ctm);
> +}
> +
> +void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state)
> +{
> + intel_crtc_put_color_blobs(crtc_state);
> +}
> +
> +void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state)
> +{
> + intel_crtc_put_color_blobs(crtc_state);
> +
> + if (crtc_state->uapi.degamma_lut)
> + crtc_state->hw.degamma_lut =
> + drm_property_blob_get(crtc_state->uapi.degamma_lut);
> + else
> + crtc_state->hw.degamma_lut = NULL;
> +
> + if (crtc_state->uapi.gamma_lut)
> + crtc_state->hw.gamma_lut =
> + drm_property_blob_get(crtc_state->uapi.gamma_lut);
> + else
> + crtc_state->hw.gamma_lut = NULL;
> +
> + if (crtc_state->uapi.ctm)
> + crtc_state->hw.ctm =
> + drm_property_blob_get(crtc_state->uapi.ctm);
> + else
> + crtc_state->hw.ctm = NULL;
> +}
> +
> /**
> * intel_crtc_destroy_state - destroy crtc state
> * @crtc: drm crtc
> @@ -223,6 +266,7 @@ intel_crtc_destroy_state(struct drm_crtc *crtc,
> struct intel_crtc_state *crtc_state = to_intel_crtc_state(state);
>
> __drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi);
> + intel_crtc_free_hw_state(crtc_state);
> kfree(crtc_state);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.h b/drivers/gpu/drm/i915/display/intel_atomic.h
> index 49d5cb1b9e0a..7b49623419ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.h
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.h
> @@ -36,6 +36,8 @@ intel_digital_connector_duplicate_state(struct drm_connector *connector);
> struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
> void intel_crtc_destroy_state(struct drm_crtc *crtc,
> struct drm_crtc_state *state);
> +void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state);
> +void intel_crtc_copy_color_blobs(struct intel_crtc_state *crtc_state);
> struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
> void intel_atomic_state_clear(struct drm_atomic_state *state);
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 1bc008c094e4..a765794597ed 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7185,6 +7185,8 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
> crtc->enabled = false;
> crtc->state->connector_mask = 0;
> crtc->state->encoder_mask = 0;
> + intel_crtc_free_hw_state(crtc_state);
> + memset(&crtc_state->hw, 0, sizeof(crtc_state->hw));
>
> for_each_encoder_on_crtc(crtc->dev, crtc, encoder)
> encoder->base.crtc = NULL;
> @@ -12560,8 +12562,48 @@ static bool check_digital_port_conflicts(struct intel_atomic_state *state)
> return ret;
> }
>
> +static void
> +intel_crtc_copy_uapi_to_hw_state_nomodeset(struct intel_crtc_state *crtc_state)
> +{
> + intel_crtc_copy_color_blobs(crtc_state);
> +}
> +
> +static void
> +intel_crtc_copy_uapi_to_hw_state(struct intel_crtc_state *crtc_state)
> +{
> + crtc_state->hw.enable = crtc_state->uapi.enable;
> + crtc_state->hw.active = crtc_state->uapi.active;
> + crtc_state->hw.mode = crtc_state->uapi.mode;
> + crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
> + intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
> +}
> +
> +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state)
> +{
> + crtc_state->uapi.enable = crtc_state->hw.enable;
> + crtc_state->uapi.active = crtc_state->hw.active;
> + crtc_state->uapi.mode = crtc_state->hw.mode;
> + crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
> +
> + /* copy color blobs to uapi */
> + drm_property_blob_put(crtc_state->hw.degamma_lut);
uapi.degamma_lut
These look like inlined versions of drm_property_replace_blob()
actually.
Otherwise lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> + crtc_state->uapi.degamma_lut = crtc_state->hw.degamma_lut;
> + if (crtc_state->uapi.degamma_lut)
> + drm_property_blob_get(crtc_state->uapi.degamma_lut);
> +
> + drm_property_blob_put(crtc_state->uapi.ctm);
> + crtc_state->uapi.ctm = crtc_state->hw.ctm;
> + if (crtc_state->uapi.ctm)
> + drm_property_blob_get(crtc_state->uapi.ctm);
> +
> + drm_property_blob_put(crtc_state->uapi.gamma_lut);
> + crtc_state->uapi.gamma_lut = crtc_state->hw.gamma_lut;
> + if (crtc_state->uapi.gamma_lut)
> + drm_property_blob_get(crtc_state->uapi.gamma_lut);
> +}
> +
> static int
> -clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> +intel_crtc_prepare_cleared_state(struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *dev_priv =
> to_i915(crtc_state->uapi.crtc->dev);
> @@ -12571,11 +12613,15 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> if (!saved_state)
> return -ENOMEM;
>
> + /* free the old crtc_state->hw members */
> + intel_crtc_free_hw_state(crtc_state);
> +
> /* FIXME: before the switch to atomic started, a new pipe_config was
> * kzalloc'd. Code that depends on any field being zero should be
> * fixed, so that the crtc_state can be safely duplicated. For now,
> * only fields that are know to not cause problems are preserved. */
>
> + saved_state->uapi = crtc_state->uapi;
> saved_state->scaler_state = crtc_state->scaler_state;
> saved_state->shared_dpll = crtc_state->shared_dpll;
> saved_state->dpll_hw_state = crtc_state->dpll_hw_state;
> @@ -12593,14 +12639,11 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> saved_state->sync_mode_slaves_mask =
> crtc_state->sync_mode_slaves_mask;
>
> - /* Keep base drm_crtc_state intact, only clear our extended struct */
> - BUILD_BUG_ON(offsetof(struct intel_crtc_state, base));
> - BUILD_BUG_ON(offsetof(struct intel_crtc_state, uapi));
> - BUILD_BUG_ON(offsetof(struct intel_crtc_state, hw));
> - memcpy(&crtc_state->uapi + 1, &saved_state->uapi + 1,
> - sizeof(*crtc_state) - sizeof(crtc_state->uapi));
> -
> + memcpy(crtc_state, saved_state, sizeof(*crtc_state));
> kfree(saved_state);
> +
> + intel_crtc_copy_uapi_to_hw_state(crtc_state);
> +
> return 0;
> }
>
> @@ -12616,10 +12659,6 @@ intel_modeset_pipe_config(struct intel_crtc_state *pipe_config)
> int i;
> bool retry = true;
>
> - ret = clear_intel_crtc_state(pipe_config);
> - if (ret)
> - return ret;
> -
> pipe_config->cpu_transcoder =
> (enum transcoder) to_intel_crtc(crtc)->pipe;
>
> @@ -13476,6 +13515,8 @@ verify_crtc_state(struct intel_crtc *crtc,
>
> state = old_crtc_state->uapi.state;
> __drm_atomic_helper_crtc_destroy_state(&old_crtc_state->uapi);
> + intel_crtc_free_hw_state(old_crtc_state);
> +
> pipe_config = old_crtc_state;
> memset(pipe_config, 0, sizeof(*pipe_config));
> pipe_config->uapi.crtc = &crtc->base;
> @@ -14010,14 +14051,24 @@ static int intel_atomic_check(struct drm_device *dev,
>
> for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> new_crtc_state, i) {
> - if (!needs_modeset(new_crtc_state))
> + if (!needs_modeset(new_crtc_state)) {
> + /* Light copy */
> + intel_crtc_copy_uapi_to_hw_state_nomodeset(new_crtc_state);
> +
> continue;
> + }
>
> if (!new_crtc_state->uapi.enable) {
> + intel_crtc_copy_uapi_to_hw_state(new_crtc_state);
> +
> any_ms = true;
> continue;
> }
>
> + ret = intel_crtc_prepare_cleared_state(new_crtc_state);
> + if (ret)
> + goto fail;
> +
> ret = intel_modeset_pipe_config(new_crtc_state);
> if (ret)
> goto fail;
> @@ -17291,6 +17342,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> to_intel_crtc_state(crtc->base.state);
>
> __drm_atomic_helper_crtc_destroy_state(&crtc_state->uapi);
> + intel_crtc_free_hw_state(crtc_state);
> memset(crtc_state, 0, sizeof(*crtc_state));
> __drm_atomic_helper_crtc_reset(&crtc->base, &crtc_state->uapi);
>
> @@ -17419,6 +17471,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> * set a flag to indicate that a full recalculation is
> * needed on the next commit.
> */
> + crtc_state->hw.mode = crtc->base.mode;
> crtc_state->hw.mode.private_flags = I915_MODE_FLAG_INHERITED;
>
> intel_crtc_compute_pixel_rate(crtc_state);
> @@ -17467,6 +17520,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>
> intel_bw_crtc_update(bw_state, crtc_state);
>
> + intel_crtc_copy_hw_to_uapi_state(crtc_state);
> intel_pipe_config_sanity_check(dev_priv, crtc_state);
> }
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 573ced9dc909..bcf5f7640307 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -757,8 +757,6 @@ enum intel_output_format {
> };
>
> struct intel_crtc_state {
> - union {
> - struct drm_crtc_state base;
> /*
> * uapi (drm) state. This is the software state shown to userspace.
> * In particular, the following members are used for bookkeeping:
> @@ -781,8 +779,11 @@ struct intel_crtc_state {
> *
> * During initial hw readout, they need to be copied to uapi.
> */
> - struct drm_crtc_state hw;
> - };
> + struct {
> + bool active, enable;
> + struct drm_property_blob *degamma_lut, *gamma_lut, *ctm;
> + struct drm_display_mode mode, adjusted_mode;
> + } hw;
>
> /**
> * quirks - bitfield with hw state readout quirks
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-10-29 14:33 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-29 7:22 [CI 01/12] drm/i915: Introduce intel_atomic_get_plane_state_after_check(), v2 Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 7:22 ` [CI 02/12] drm/i915: Handle a few more cases for crtc hw/uapi split, v3 Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 7:22 ` [CI 03/12] drm/i915: Add aliases for uapi and hw to crtc_state Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 7:22 ` [CI 04/12] drm/i915: Perform manual conversions for crtc uapi/hw split, v2 Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 13:23 ` Ville Syrjälä
2019-10-29 13:23 ` [Intel-gfx] " Ville Syrjälä
2019-10-30 10:12 ` Maarten Lankhorst
2019-10-30 10:12 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 7:22 ` [CI 05/12] drm/i915: Perform automated conversions for crtc uapi/hw split, base -> hw Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 7:22 ` [CI 06/12] drm/i915: Perform automated conversions for crtc uapi/hw split, base -> uapi Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 7:22 ` [CI 07/12] drm/i915: Complete crtc hw/uapi split, v4 Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 14:33 ` Ville Syrjälä [this message]
2019-10-29 14:33 ` Ville Syrjälä
2019-10-29 14:37 ` Ville Syrjälä
2019-10-29 14:37 ` [Intel-gfx] " Ville Syrjälä
2019-10-29 7:22 ` [CI 08/12] drm/i915: Add aliases for uapi and hw to plane_state Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 7:22 ` [CI 09/12] drm/i915: Perform manual conversions for plane uapi/hw split Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 7:22 ` [CI 10/12] drm/i915: Perform automated conversions for plane uapi/hw split, base -> hw Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 15:35 ` Ville Syrjälä
2019-10-29 15:35 ` [Intel-gfx] " Ville Syrjälä
2019-10-29 7:22 ` [CI 11/12] drm/i915: Perform automated conversions for plane uapi/hw split, base -> uapi Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 15:43 ` Ville Syrjälä
2019-10-29 15:43 ` [Intel-gfx] " Ville Syrjälä
2019-10-30 13:10 ` Maarten Lankhorst
2019-10-30 13:10 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 7:22 ` [CI 12/12] drm/i915: Complete plane hw and uapi split, v2 Maarten Lankhorst
2019-10-29 7:22 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 18:34 ` Ville Syrjälä
2019-10-29 18:34 ` [Intel-gfx] " Ville Syrjälä
2019-10-30 9:51 ` Maarten Lankhorst
2019-10-30 9:51 ` [Intel-gfx] " Maarten Lankhorst
2019-10-29 8:45 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [CI,01/12] drm/i915: Introduce intel_atomic_get_plane_state_after_check(), v2 Patchwork
2019-10-29 8:45 ` [Intel-gfx] " Patchwork
2019-10-29 9:05 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-10-29 9:05 ` [Intel-gfx] " Patchwork
2019-10-29 18:35 ` [CI 01/12] " Ville Syrjälä
2019-10-29 18:35 ` [Intel-gfx] " Ville Syrjälä
2019-10-30 9:17 ` Maarten Lankhorst
2019-10-30 9:17 ` [Intel-gfx] " Maarten Lankhorst
2019-10-30 13:37 ` Ville Syrjälä
2019-10-30 13:37 ` [Intel-gfx] " Ville Syrjälä
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191029143315.GW1208@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.