On Tue, Oct 20, 2026 at 12:11:56PM -0400, Leo Li wrote: > Thanks for cleaning up all the drivers, are these all part of the > DRM_MODE_ATOMIC_RESET effort?
Somewhat, yes. It actually originated from the state readout work I've
been doing, but it's now a dependency to enable DRM_MODE_ATOMIC_RESET
indeed.
> > On Mon, Oct 19, 2026 at 12:38:47PM -0400, Leo Li wrote:
> >>> @@ -736,11 +736,11 @@ dm_dp_add_mst_connector(struct
> >>> drm_dp_mst_topology_mgr *mgr,
> >>>
> >>> /*
> >>> * Initialize connector state before adding the connectror to drm and
> >>> * framebuffer lists
> >>> */
> >>> - amdgpu_dm_connector_funcs_reset(connector);
> >>
> >> I'm actually not sure why there was a state reset here.
> >> amdgpu_dm_connector_init_helper() is called earlier in this func and
> >> that already did a reset.
> >
> > Yeah, it's something sashiko flagged too and I removed that call (and
> > updated the comment).
> >
> >>> + connector->state = amdgpu_dm_connector_atomic_create_state(connector);
> >>
> >> That asside, looks like this can overwrite the alloc'd state from
> >> init_helper() without freeing. So to keep the previous behavior and
> >> avoid regressing, perhaps we can do this:
> >>
> >> conn_state = amdgpu_dm_connector_atomic_create_state(connector);
> >> if (!IS_ERR(conn_state)) {
> >> if (connector->state)
> >> connector->funcs->atomic_destroy_state(connector,
> >> connector->state);
> >> connector->state = conn_state;
> >> }
> >
> > If we are to handle errors properly and remain equivalent, then
> > something like the following would be more accurate I guess?
> >
> > if (connector->state) {
> > connector->funcs->atomic_destroy_state(connector, connector->state);
> > connector->state = NULL;
> > }
> >
> > conn_state = amdgpu_dm_connector_atomic_create_state(connector);
> > if (IS_ERR(conn_state))
> > return NULL;
> >
> > connector->state = conn_state;
> >
> > Would you prefer that, or should I remove the call to create_state
> > entirely because it's redundant?
>
> My preference would be to copy existing behavior, and keep the scope of this
> series to just dropping the reset hook. Dropping the redundant create_state
> and
> handling associated testing/regressions can be a followup.
>
> In my parsing of the old reset behavior, we don't return a NULL connector on
> error. Rather, we keep the existing connector->state and return the connector
> as
> is. The rest of reset() only runs if state was alloc'd successfully. My
> previous
> snippet should copy this.
Ack. I'll use your earlier suggestion then
Maxime
signature.asc
Description: PGP signature
