[56/75] drm/atomic-state-helper: Rename connector_tv_reset to state_init

Recent commits renamed other reset helpers to use the state_init
suffix when they only initialize properties rather than allocate or
update the object state itself.

drm_atomic_helper_connector_tv_reset() falls in the same category:
it initializes the analog TV properties from the kernel command line
defaults without allocating any state. It also reads the connector
state through connector->state, coupling it to the reset path.

Rename it to drm_atomic_helper_connector_tv_state_init() and pass
the connector state explicitly as a parameter. This decouples it
from the reset hook and prepares for the conversion to
atomic_create_state where connector->state is not yet assigned.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
Cc: "Maíra Canal" <mcanal@igalia.com>
Cc: Chen-Yu Tsai <wens@kernel.org>
Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
Cc: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
Cc: Samuel Holland <samuel@sholland.org>
Cc: linux-sunxi@lists.linux.dev
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 11 ++++++-----
 drivers/gpu/drm/sun4i/sun4i_tv.c          |  2 +-
 drivers/gpu/drm/vc4/vc4_vec.c             |  2 +-
 include/drm/drm_atomic_state_helper.h     |  3 ++-
 4 files changed, 10 insertions(+), 8 deletions(-)
  

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm: Incomplete state decoupling causes NULL deref in tv_margins_state_init
--

commit 44e808ff1921e4945725ba2420217e2015fc2db5
Author: Maxime Ripard <mripard@kernel.org>

drm/atomic-state-helper: Rename connector_tv_reset to state_init

This commit renames drm_atomic_helper_connector_tv_reset to
drm_atomic_helper_connector_tv_state_init. It adds a connector state
parameter to decouple the initialization from the reset hook, preparing it
for contexts like atomic_create_state where the connector state isn't
assigned yet.

> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 6238066fd5d60..1424c4873251c 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
[ ... ]
> @@ -501,16 +501,17 @@ void drm_atomic_helper_connector_tv_margins_state_init(struct drm_connector_stat
>  EXPORT_SYMBOL(drm_atomic_helper_connector_tv_margins_state_init);
>  
>  /**
> - * drm_atomic_helper_connector_tv_reset - Resets Analog TV connector properties
> + * drm_atomic_helper_connector_tv_state_init - Initializes Analog TV connector properties
> + * @state: the DRM State object
>   * @connector: DRM connector
>   *
> - * Resets the analog TV properties attached to a connector
> + * Initializes the analog TV properties attached to a connector
>   */
> -void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector)
> +void drm_atomic_helper_connector_tv_state_init(struct drm_connector_state *state,
> +					       struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
>  	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
> -	struct drm_connector_state *state = connector->state;
>  	struct drm_property *prop;
>  	uint64_t val;
>  
[ ... ]
> @@ -574,7 +575,7 @@ void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector)
>  	drm_atomic_helper_connector_tv_margins_state_init(connector->state,
>  							  connector);

[Severity: High]
Since the new state parameter is added to decouple this function from
the reset hook, should this call pass the newly provided state instead of
connector->state?

When a driver like vc4_vec calls this via its atomic_create_state hook,
connector->state is not yet assigned and is NULL.

Passing connector->state here forwards a NULL pointer to
drm_atomic_helper_connector_tv_margins_state_init(), which then writes
directly to state->tv.margins.left, leading to a NULL pointer dereference
during device probe:

void drm_atomic_helper_connector_tv_margins_state_init(struct drm_connector_state *state,
						       struct drm_connector *connector)
{
	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;

	state->tv.margins.left = cmdline->tv_margins.left;

>  }
> -EXPORT_SYMBOL(drm_atomic_helper_connector_tv_reset);
> +EXPORT_SYMBOL(drm_atomic_helper_connector_tv_state_init);
  

Hi Maxime,

On 14/10/26 14:27, Maxime Ripard wrote:
> Recent commits renamed other reset helpers to use the state_init
> suffix when they only initialize properties rather than allocate or
> update the object state itself.
> 
> drm_atomic_helper_connector_tv_reset() falls in the same category:
> it initializes the analog TV properties from the kernel command line
> defaults without allocating any state. It also reads the connector
> state through connector->state, coupling it to the reset path.
> 
> Rename it to drm_atomic_helper_connector_tv_state_init() and pass
> the connector state explicitly as a parameter. This decouples it
> from the reset hook and prepares for the conversion to
> atomic_create_state where connector->state is not yet assigned.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
> Cc: "Maíra Canal" <mcanal@igalia.com>
> Cc: Chen-Yu Tsai <wens@kernel.org>
> Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Cc: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> Cc: Samuel Holland <samuel@sholland.org>
> Cc: linux-sunxi@lists.linux.dev
> ---
>   drivers/gpu/drm/drm_atomic_state_helper.c | 11 ++++++-----
>   drivers/gpu/drm/sun4i/sun4i_tv.c          |  2 +-
>   drivers/gpu/drm/vc4/vc4_vec.c             |  2 +-
>   include/drm/drm_atomic_state_helper.h     |  3 ++-
>   4 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 6238066fd5d6..1424c4873251 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -499,20 +499,21 @@ void drm_atomic_helper_connector_tv_margins_state_init(struct drm_connector_stat
>   	state->tv.margins.bottom = cmdline->tv_margins.bottom;
>   }
>   EXPORT_SYMBOL(drm_atomic_helper_connector_tv_margins_state_init);
>   
>   /**
> - * drm_atomic_helper_connector_tv_reset - Resets Analog TV connector properties
> + * drm_atomic_helper_connector_tv_state_init - Initializes Analog TV connector properties
> + * @state: the DRM State object
>    * @connector: DRM connector
>    *
> - * Resets the analog TV properties attached to a connector
> + * Initializes the analog TV properties attached to a connector
>    */
> -void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector)
> +void drm_atomic_helper_connector_tv_state_init(struct drm_connector_state *state,
> +					       struct drm_connector *connector)
>   {
>   	struct drm_device *dev = connector->dev;
>   	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
> -	struct drm_connector_state *state = connector->state;
>   	struct drm_property *prop;
>   	uint64_t val;
>   
>   	prop = dev->mode_config.tv_mode_property;
>   	if (prop)
> @@ -572,11 +573,11 @@ void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector)
>   			state->tv.hue = val;
>   
>   	drm_atomic_helper_connector_tv_margins_state_init(connector->state,

Shouldn't it be s/connector->state/state here?

Best regards,
- Maíra

>   							  connector);
>   }
> -EXPORT_SYMBOL(drm_atomic_helper_connector_tv_reset);
> +EXPORT_SYMBOL(drm_atomic_helper_connector_tv_state_init);
>   
>   /**
>    * drm_atomic_helper_connector_tv_check - Validate an analog TV connector state
>    * @connector: DRM Connector
>    * @state: the DRM State object
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
> index ce22f1662c0c..ea86f8a6a35e 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
> @@ -406,11 +406,11 @@ static const struct drm_connector_helper_funcs sun4i_tv_comp_connector_helper_fu
>   };
>   
>   static void sun4i_tv_connector_reset(struct drm_connector *connector)
>   {
>   	drm_atomic_helper_connector_reset(connector);
> -	drm_atomic_helper_connector_tv_reset(connector);
> +	drm_atomic_helper_connector_tv_state_init(connector->state, connector);
>   }
>   
>   static const struct drm_connector_funcs sun4i_tv_comp_connector_funcs = {
>   	.fill_modes		= drm_helper_probe_single_connector_modes,
>   	.destroy		= drm_connector_cleanup,
> diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
> index 792b7b39be59..98cece1e48d8 100644
> --- a/drivers/gpu/drm/vc4/vc4_vec.c
> +++ b/drivers/gpu/drm/vc4/vc4_vec.c
> @@ -379,11 +379,11 @@ vc4_vec_connector_detect(struct drm_connector *connector, bool force)
>   }
>   
>   static void vc4_vec_connector_reset(struct drm_connector *connector)
>   {
>   	drm_atomic_helper_connector_reset(connector);
> -	drm_atomic_helper_connector_tv_reset(connector);
> +	drm_atomic_helper_connector_tv_state_init(connector->state, connector);
>   }
>   
>   static int
>   vc4_vec_connector_set_property(struct drm_connector *connector,
>   			       struct drm_connector_state *state,
> diff --git a/include/drm/drm_atomic_state_helper.h b/include/drm/drm_atomic_state_helper.h
> index 4dac741af22c..5e8619aeef21 100644
> --- a/include/drm/drm_atomic_state_helper.h
> +++ b/include/drm/drm_atomic_state_helper.h
> @@ -69,11 +69,12 @@ void __drm_atomic_helper_connector_state_init(struct drm_connector_state *conn_s
>   void __drm_atomic_helper_connector_reset(struct drm_connector *connector,
>   					 struct drm_connector_state *conn_state);
>   void drm_atomic_helper_connector_reset(struct drm_connector *connector);
>   struct drm_connector_state *
>   drm_atomic_helper_connector_create_state(struct drm_connector *connector);
> -void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector);
> +void drm_atomic_helper_connector_tv_state_init(struct drm_connector_state *state,
> +					       struct drm_connector *connector);
>   int drm_atomic_helper_connector_tv_check(struct drm_connector *connector,
>   					 struct drm_atomic_commit *state);
>   void drm_atomic_helper_connector_tv_margins_state_init(struct drm_connector_state *state,
>   						       struct drm_connector *connector);
>   void
>
  

Hi Maíra,

On Thu, Oct 15, 2026 at 07:56:05AM -0300, Maíra Canal wrote:
> Hi Maxime,
> 
> On 14/10/26 14:27, Maxime Ripard wrote:
> > Recent commits renamed other reset helpers to use the state_init
> > suffix when they only initialize properties rather than allocate or
> > update the object state itself.
> > 
> > drm_atomic_helper_connector_tv_reset() falls in the same category:
> > it initializes the analog TV properties from the kernel command line
> > defaults without allocating any state. It also reads the connector
> > state through connector->state, coupling it to the reset path.
> > 
> > Rename it to drm_atomic_helper_connector_tv_state_init() and pass
> > the connector state explicitly as a parameter. This decouples it
> > from the reset hook and prepares for the conversion to
> > atomic_create_state where connector->state is not yet assigned.
> > 
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> > Cc: "Maíra Canal" <mcanal@igalia.com>
> > Cc: Chen-Yu Tsai <wens@kernel.org>
> > Cc: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > Cc: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> > Cc: Samuel Holland <samuel@sholland.org>
> > Cc: linux-sunxi@lists.linux.dev
> > ---
> >   drivers/gpu/drm/drm_atomic_state_helper.c | 11 ++++++-----
> >   drivers/gpu/drm/sun4i/sun4i_tv.c          |  2 +-
> >   drivers/gpu/drm/vc4/vc4_vec.c             |  2 +-
> >   include/drm/drm_atomic_state_helper.h     |  3 ++-
> >   4 files changed, 10 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index 6238066fd5d6..1424c4873251 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -499,20 +499,21 @@ void drm_atomic_helper_connector_tv_margins_state_init(struct drm_connector_stat
> >   	state->tv.margins.bottom = cmdline->tv_margins.bottom;
> >   }
> >   EXPORT_SYMBOL(drm_atomic_helper_connector_tv_margins_state_init);
> >   /**
> > - * drm_atomic_helper_connector_tv_reset - Resets Analog TV connector properties
> > + * drm_atomic_helper_connector_tv_state_init - Initializes Analog TV connector properties
> > + * @state: the DRM State object
> >    * @connector: DRM connector
> >    *
> > - * Resets the analog TV properties attached to a connector
> > + * Initializes the analog TV properties attached to a connector
> >    */
> > -void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector)
> > +void drm_atomic_helper_connector_tv_state_init(struct drm_connector_state *state,
> > +					       struct drm_connector *connector)
> >   {
> >   	struct drm_device *dev = connector->dev;
> >   	struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
> > -	struct drm_connector_state *state = connector->state;
> >   	struct drm_property *prop;
> >   	uint64_t val;
> >   	prop = dev->mode_config.tv_mode_property;
> >   	if (prop)
> > @@ -572,11 +573,11 @@ void drm_atomic_helper_connector_tv_reset(struct drm_connector *connector)
> >   			state->tv.hue = val;
> >   	drm_atomic_helper_connector_tv_margins_state_init(connector->state,
> 
> Shouldn't it be s/connector->state/state here?

Yes, it should be. It's going to be fixed in the next version, thanks!
Maxime