On Tue, 18 Apr 2023, Rodrigo Vivi <[email protected]> wrote: > On Mon, Apr 17, 2023 at 06:37:41PM +0300, Jani Nikula wrote: >> An error-valued pointer can handle all in one without the wrapper >> struct. >> >> Signed-off-by: Jani Nikula <[email protected]> >> --- >> drivers/gpu/drm/i915/display/intel_crt.c | 18 ++++++++--------- >> .../gpu/drm/i915/display/intel_load_detect.c | 20 ++++++++----------- >> .../gpu/drm/i915/display/intel_load_detect.h | 12 ++++------- >> drivers/gpu/drm/i915/display/intel_tv.c | 16 +++++++-------- >> 4 files changed, 29 insertions(+), 37 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c >> b/drivers/gpu/drm/i915/display/intel_crt.c >> index 96acdf98a0c0..13519f78cf9f 100644 >> --- a/drivers/gpu/drm/i915/display/intel_crt.c >> +++ b/drivers/gpu/drm/i915/display/intel_crt.c >> @@ -822,9 +822,9 @@ intel_crt_detect(struct drm_connector *connector, >> struct drm_i915_private *dev_priv = to_i915(connector->dev); >> struct intel_crt *crt = >> intel_attached_crt(to_intel_connector(connector)); >> struct intel_encoder *intel_encoder = &crt->base; >> + struct drm_atomic_state *state; >> intel_wakeref_t wakeref; >> - int status, ret; >> - struct intel_load_detect_pipe tmp; >> + int status; >> >> drm_dbg_kms(&dev_priv->drm, "[CONNECTOR:%d:%s] force=%d\n", >> connector->base.id, connector->name, >> @@ -882,8 +882,12 @@ intel_crt_detect(struct drm_connector *connector, >> } >> >> /* for pre-945g platforms use load detect */ >> - ret = intel_load_detect_get_pipe(connector, &tmp, ctx); >> - if (ret > 0) { >> + state = intel_load_detect_get_pipe(connector, ctx); >> + if (IS_ERR(state)) { >> + status = PTR_ERR(state); >> + } else if (!state) { >> + status = connector_status_unknown; >> + } else { >> if (intel_crt_detect_ddc(connector)) >> status = connector_status_connected; >> else if (DISPLAY_VER(dev_priv) < 4) >> @@ -893,11 +897,7 @@ intel_crt_detect(struct drm_connector *connector, >> status = connector_status_disconnected; >> else >> status = connector_status_unknown; >> - intel_load_detect_release_pipe(connector, &tmp, ctx); >> - } else if (ret == 0) { >> - status = connector_status_unknown; >> - } else { >> - status = ret; >> + intel_load_detect_release_pipe(connector, state, ctx); > > I confess it took me a while to see that we have the same logic in place. > I think I need more coffee.
It also took me a while when writing it! :) > Reviewed-by: Rodrigo Vivi <[email protected]> Thanks, both pushed to din. BR, Jani. > >> } >> >> out: >> diff --git a/drivers/gpu/drm/i915/display/intel_load_detect.c >> b/drivers/gpu/drm/i915/display/intel_load_detect.c >> index 5d6bb6d712bc..d5a0aecf3e8f 100644 >> --- a/drivers/gpu/drm/i915/display/intel_load_detect.c >> +++ b/drivers/gpu/drm/i915/display/intel_load_detect.c >> @@ -44,9 +44,9 @@ static int intel_modeset_disable_planes(struct >> drm_atomic_state *state, >> return 0; >> } >> >> -int intel_load_detect_get_pipe(struct drm_connector *connector, >> - struct intel_load_detect_pipe *old, >> - struct drm_modeset_acquire_ctx *ctx) >> +struct drm_atomic_state * >> +intel_load_detect_get_pipe(struct drm_connector *connector, >> + struct drm_modeset_acquire_ctx *ctx) >> { >> struct intel_encoder *encoder = >> intel_attached_encoder(to_intel_connector(connector)); >> @@ -64,8 +64,6 @@ int intel_load_detect_get_pipe(struct drm_connector >> *connector, >> connector->base.id, connector->name, >> encoder->base.base.id, encoder->base.name); >> >> - old->restore_state = NULL; >> - >> drm_WARN_ON(dev, !drm_modeset_is_locked(&config->connection_mutex)); >> >> /* >> @@ -179,13 +177,12 @@ int intel_load_detect_get_pipe(struct drm_connector >> *connector, >> goto fail; >> } >> >> - old->restore_state = restore_state; >> drm_atomic_state_put(state); >> >> /* let the connector get through one full cycle before testing */ >> intel_crtc_wait_for_next_vblank(crtc); >> >> - return true; >> + return restore_state; >> >> fail: >> if (state) { >> @@ -198,27 +195,26 @@ int intel_load_detect_get_pipe(struct drm_connector >> *connector, >> } >> >> if (ret == -EDEADLK) >> - return ret; >> + return ERR_PTR(ret); >> >> - return false; >> + return NULL; >> } >> >> void intel_load_detect_release_pipe(struct drm_connector *connector, >> - struct intel_load_detect_pipe *old, >> + struct drm_atomic_state *state, >> struct drm_modeset_acquire_ctx *ctx) >> { >> struct intel_encoder *intel_encoder = >> intel_attached_encoder(to_intel_connector(connector)); >> struct drm_i915_private *i915 = to_i915(intel_encoder->base.dev); >> struct drm_encoder *encoder = &intel_encoder->base; >> - struct drm_atomic_state *state = old->restore_state; >> int ret; >> >> drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", >> connector->base.id, connector->name, >> encoder->base.id, encoder->name); >> >> - if (!state) >> + if (IS_ERR_OR_NULL(state)) >> return; >> >> ret = drm_atomic_helper_commit_duplicated_state(state, ctx); >> diff --git a/drivers/gpu/drm/i915/display/intel_load_detect.h >> b/drivers/gpu/drm/i915/display/intel_load_detect.h >> index 9b69da1867a5..aed51901b9ba 100644 >> --- a/drivers/gpu/drm/i915/display/intel_load_detect.h >> +++ b/drivers/gpu/drm/i915/display/intel_load_detect.h >> @@ -10,15 +10,11 @@ struct drm_atomic_state; >> struct drm_connector; >> struct drm_modeset_acquire_ctx; >> >> -struct intel_load_detect_pipe { >> - struct drm_atomic_state *restore_state; >> -}; >> - >> -int intel_load_detect_get_pipe(struct drm_connector *connector, >> - struct intel_load_detect_pipe *old, >> - struct drm_modeset_acquire_ctx *ctx); >> +struct drm_atomic_state * >> +intel_load_detect_get_pipe(struct drm_connector *connector, >> + struct drm_modeset_acquire_ctx *ctx); >> void intel_load_detect_release_pipe(struct drm_connector *connector, >> - struct intel_load_detect_pipe *old, >> + struct drm_atomic_state *old, >> struct drm_modeset_acquire_ctx *ctx); >> >> #endif /* __INTEL_LOAD_DETECT_H__ */ >> diff --git a/drivers/gpu/drm/i915/display/intel_tv.c >> b/drivers/gpu/drm/i915/display/intel_tv.c >> index 07e7f7cdd961..e3ccface0c9d 100644 >> --- a/drivers/gpu/drm/i915/display/intel_tv.c >> +++ b/drivers/gpu/drm/i915/display/intel_tv.c >> @@ -1723,21 +1723,21 @@ intel_tv_detect(struct drm_connector *connector, >> return connector_status_disconnected; >> >> if (force) { >> - struct intel_load_detect_pipe tmp; >> - int ret; >> + struct drm_atomic_state *state; >> >> - ret = intel_load_detect_get_pipe(connector, &tmp, ctx); >> - if (ret < 0) >> - return ret; >> + state = intel_load_detect_get_pipe(connector, ctx); >> + if (IS_ERR(state)) >> + return PTR_ERR(state); >> >> - if (ret > 0) { >> + if (state) { >> type = intel_tv_detect_type(intel_tv, connector); >> - intel_load_detect_release_pipe(connector, &tmp, ctx); >> + intel_load_detect_release_pipe(connector, state, ctx); >> status = type < 0 ? >> connector_status_disconnected : >> connector_status_connected; >> - } else >> + } else { >> status = connector_status_unknown; >> + } >> >> if (status == connector_status_connected) { >> intel_tv->type = type; >> -- >> 2.39.2 >> -- Jani Nikula, Intel Open Source Graphics Center
