| Message ID | 20191029072229.27092-7-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [CI,01/12] drm/i915: Introduce intel_atomic_get_plane_state_after_check(), v2. | expand |
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
On Tue, Oct 29, 2019 at 04:33:15PM +0200, Ville Syrjälä wrote: > 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. And seems to me that you should be able to use that for the other direction as well. > > 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
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); + 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
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(-)