Bridges implement their state using a drm_private_obj and an
hand-crafted reset implementation.

Since drm_private_obj doesn't have a set of reset helper like the other
states, __drm_atomic_helper_bridge_reset() was initializing both the
drm_private_state and the drm_bridge_state structures.

This initialization however was missing the drm_private_state.obj
pointer to the drm_private_obj the state was allocated for, creating a
NULL pointer dereference when trying to access it.

Fixes: 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/drm_atomic_state_helper.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..b962c342b16aabf4e3bea52a914e5deb1c2080ce 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -707,10 +707,17 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
 	__drm_atomic_helper_connector_destroy_state(state);
 	kfree(state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
 
+static void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
+						  struct drm_private_state *state)
+{
+	memset(state, 0, sizeof(*state));
+	state->obj = obj;
+}
+
 /**
  * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
  * @obj: CRTC object
  * @state: new private object state
  *
@@ -796,10 +803,11 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
  */
 void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge,
 				      struct drm_bridge_state *state)
 {
 	memset(state, 0, sizeof(*state));
+	__drm_atomic_helper_private_obj_reset(&bridge->base, &state->base);
 	state->bridge = bridge;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_bridge_reset);
 
 /**

-- 
2.50.1

Hi Maxime,

On Tue, Sep 02, 2025 at 10:32:33AM +0200, Maxime Ripard wrote:
> Bridges implement their state using a drm_private_obj and an

s/and an/and a/

> hand-crafted reset implementation.
> 
> Since drm_private_obj doesn't have a set of reset helper like the other
> states, __drm_atomic_helper_bridge_reset() was initializing both the

s/was initializing/initializes/

> drm_private_state and the drm_bridge_state structures.
> 
> This initialization however was missing the drm_private_state.obj

s/was missing/is missing/

Or do I incorrectly think that the commit message should describe the
current situation in the present tense ?

> pointer to the drm_private_obj the state was allocated for, creating a
> NULL pointer dereference when trying to access it.
> 
> Fixes: 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/drm_atomic_state_helper.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..b962c342b16aabf4e3bea52a914e5deb1c2080ce 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -707,10 +707,17 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
>  	__drm_atomic_helper_connector_destroy_state(state);
>  	kfree(state);
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>  
> +static void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
> +						  struct drm_private_state *state)
> +{
> +	memset(state, 0, sizeof(*state));

As Thomas mentioned, the memset is likely not needed.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +	state->obj = obj;
> +}
> +
>  /**
>   * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
>   * @obj: CRTC object
>   * @state: new private object state
>   *
> @@ -796,10 +803,11 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
>   */
>  void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge,
>  				      struct drm_bridge_state *state)
>  {
>  	memset(state, 0, sizeof(*state));
> +	__drm_atomic_helper_private_obj_reset(&bridge->base, &state->base);
>  	state->bridge = bridge;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_bridge_reset);
>  
>  /**
> 

-- 
Regards,

Laurent Pinchart

Hi

Am 02.09.25 um 10:32 schrieb Maxime Ripard:
> Bridges implement their state using a drm_private_obj and an
> hand-crafted reset implementation.
>
> Since drm_private_obj doesn't have a set of reset helper like the other
> states, __drm_atomic_helper_bridge_reset() was initializing both the
> drm_private_state and the drm_bridge_state structures.
>
> This initialization however was missing the drm_private_state.obj
> pointer to the drm_private_obj the state was allocated for, creating a
> NULL pointer dereference when trying to access it.
>
> Fixes: 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>   drivers/gpu/drm/drm_atomic_state_helper.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..b962c342b16aabf4e3bea52a914e5deb1c2080ce 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -707,10 +707,17 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
>   	__drm_atomic_helper_connector_destroy_state(state);
>   	kfree(state);
>   }
>   EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>   
> +static void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
> +						  struct drm_private_state *state)
> +{
> +	memset(state, 0, sizeof(*state));

This argument is guaranteed to be zero'd, I think. No need for a memset.

> +	state->obj = obj;
> +}
> +
>   /**
>    * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
>    * @obj: CRTC object
>    * @state: new private object state
>    *
> @@ -796,10 +803,11 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
>    */
>   void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge,
>   				      struct drm_bridge_state *state)
>   {
>   	memset(state, 0, sizeof(*state));

Another unnecessary memset?

Best regards
Thomas

> +	__drm_atomic_helper_private_obj_reset(&bridge->base, &state->base);
>   	state->bridge = bridge;
>   }
>   EXPORT_SYMBOL(__drm_atomic_helper_bridge_reset);
>   
>   /**
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

On Tue, Sep 02, 2025 at 03:18:17PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 02.09.25 um 10:32 schrieb Maxime Ripard:
> > Bridges implement their state using a drm_private_obj and an
> > hand-crafted reset implementation.
> > 
> > Since drm_private_obj doesn't have a set of reset helper like the other
> > states, __drm_atomic_helper_bridge_reset() was initializing both the
> > drm_private_state and the drm_bridge_state structures.
> > 
> > This initialization however was missing the drm_private_state.obj
> > pointer to the drm_private_obj the state was allocated for, creating a
> > NULL pointer dereference when trying to access it.
> > 
> > Fixes: 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >   drivers/gpu/drm/drm_atomic_state_helper.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..b962c342b16aabf4e3bea52a914e5deb1c2080ce 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -707,10 +707,17 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
> >   	__drm_atomic_helper_connector_destroy_state(state);
> >   	kfree(state);
> >   }
> >   EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
> > +static void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
> > +						  struct drm_private_state *state)

Which should probably be used for other private objects. Do we have a
good place to add a warning on state->obj being NULL for all private
objects? It looks like we have only drm_atomic_helper_swap_state(), but
it feels weird.

> > +{
> > +	memset(state, 0, sizeof(*state));
> 
> This argument is guaranteed to be zero'd, I think. No need for a memset.

In this case, but not in case of a generic object.

> 
> > +	state->obj = obj;
> > +}
> > +
> >   /**
> >    * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
> >    * @obj: CRTC object
> >    * @state: new private object state
> >    *
> > @@ -796,10 +803,11 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
> >    */
> >   void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge,
> >   				      struct drm_bridge_state *state)
> >   {
> >   	memset(state, 0, sizeof(*state));
> 
> Another unnecessary memset?
> 
> Best regards
> Thomas
> 
> > +	__drm_atomic_helper_private_obj_reset(&bridge->base, &state->base);
> >   	state->bridge = bridge;
> >   }
> >   EXPORT_SYMBOL(__drm_atomic_helper_bridge_reset);
> >   /**
> > 
> 
> -- 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
> 
> 

-- 
With best wishes
Dmitry

On Tue, Sep 02, 2025 at 03:18:17PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 02.09.25 um 10:32 schrieb Maxime Ripard:
> > Bridges implement their state using a drm_private_obj and an
> > hand-crafted reset implementation.
> > 
> > Since drm_private_obj doesn't have a set of reset helper like the other
> > states, __drm_atomic_helper_bridge_reset() was initializing both the
> > drm_private_state and the drm_bridge_state structures.
> > 
> > This initialization however was missing the drm_private_state.obj
> > pointer to the drm_private_obj the state was allocated for, creating a
> > NULL pointer dereference when trying to access it.
> > 
> > Fixes: 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >   drivers/gpu/drm/drm_atomic_state_helper.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..b962c342b16aabf4e3bea52a914e5deb1c2080ce 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -707,10 +707,17 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
> >   	__drm_atomic_helper_connector_destroy_state(state);
> >   	kfree(state);
> >   }
> >   EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
> > +static void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
> > +						  struct drm_private_state *state)
> > +{
> > +	memset(state, 0, sizeof(*state));
> 
> This argument is guaranteed to be zero'd, I think. No need for a memset.
>
> > +	state->obj = obj;
> > +}
> > +
> >   /**
> >    * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
> >    * @obj: CRTC object
> >    * @state: new private object state
> >    *
> > @@ -796,10 +803,11 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
> >    */
> >   void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge,
> >   				      struct drm_bridge_state *state)
> >   {
> >   	memset(state, 0, sizeof(*state));
> 
> Another unnecessary memset?

I guess the two can be seen as redundant, but I'd argue the one in
__drm_atomic_helper_private_obj_reset should still be there.

What guarantees that the pointer points to a zero'd structure?

Maxime

Hi

Am 15.09.25 um 13:27 schrieb Maxime Ripard:
> On Tue, Sep 02, 2025 at 03:18:17PM +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 02.09.25 um 10:32 schrieb Maxime Ripard:
>>> Bridges implement their state using a drm_private_obj and an
>>> hand-crafted reset implementation.
>>>
>>> Since drm_private_obj doesn't have a set of reset helper like the other
>>> states, __drm_atomic_helper_bridge_reset() was initializing both the
>>> drm_private_state and the drm_bridge_state structures.
>>>
>>> This initialization however was missing the drm_private_state.obj
>>> pointer to the drm_private_obj the state was allocated for, creating a
>>> NULL pointer dereference when trying to access it.
>>>
>>> Fixes: 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
>>> Signed-off-by: Maxime Ripard <mripard@kernel.org>
>>> ---
>>>    drivers/gpu/drm/drm_atomic_state_helper.c | 8 ++++++++
>>>    1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
>>> index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..b962c342b16aabf4e3bea52a914e5deb1c2080ce 100644
>>> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
>>> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
>>> @@ -707,10 +707,17 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
>>>    	__drm_atomic_helper_connector_destroy_state(state);
>>>    	kfree(state);
>>>    }
>>>    EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>>> +static void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
>>> +						  struct drm_private_state *state)
>>> +{
>>> +	memset(state, 0, sizeof(*state));
>> This argument is guaranteed to be zero'd, I think. No need for a memset.
>>
>>> +	state->obj = obj;
>>> +}
>>> +
>>>    /**
>>>     * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
>>>     * @obj: CRTC object
>>>     * @state: new private object state
>>>     *
>>> @@ -796,10 +803,11 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
>>>     */
>>>    void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge,
>>>    				      struct drm_bridge_state *state)
>>>    {
>>>    	memset(state, 0, sizeof(*state));
>> Another unnecessary memset?
> I guess the two can be seen as redundant, but I'd argue the one in
> __drm_atomic_helper_private_obj_reset should still be there.
>
> What guarantees that the pointer points to a zero'd structure?

We only call this helper after allocation AFAICT. And the DRM APIs 
already assume that allocation clears to zero.

Best regards
Thomas


>
> Maxime

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

On Mon, Sep 15, 2025 at 03:12:11PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 15.09.25 um 13:27 schrieb Maxime Ripard:
> > On Tue, Sep 02, 2025 at 03:18:17PM +0200, Thomas Zimmermann wrote:
> > > Hi
> > > 
> > > Am 02.09.25 um 10:32 schrieb Maxime Ripard:
> > > > Bridges implement their state using a drm_private_obj and an
> > > > hand-crafted reset implementation.
> > > > 
> > > > Since drm_private_obj doesn't have a set of reset helper like the other
> > > > states, __drm_atomic_helper_bridge_reset() was initializing both the
> > > > drm_private_state and the drm_bridge_state structures.
> > > > 
> > > > This initialization however was missing the drm_private_state.obj
> > > > pointer to the drm_private_obj the state was allocated for, creating a
> > > > NULL pointer dereference when trying to access it.
> > > > 
> > > > Fixes: 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
> > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > ---
> > > >    drivers/gpu/drm/drm_atomic_state_helper.c | 8 ++++++++
> > > >    1 file changed, 8 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
> > > > index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..b962c342b16aabf4e3bea52a914e5deb1c2080ce 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > > > @@ -707,10 +707,17 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
> > > >    	__drm_atomic_helper_connector_destroy_state(state);
> > > >    	kfree(state);
> > > >    }
> > > >    EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
> > > > +static void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
> > > > +						  struct drm_private_state *state)
> > > > +{
> > > > +	memset(state, 0, sizeof(*state));
> > > This argument is guaranteed to be zero'd, I think. No need for a memset.
> > > 
> > > > +	state->obj = obj;
> > > > +}
> > > > +
> > > >    /**
> > > >     * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
> > > >     * @obj: CRTC object
> > > >     * @state: new private object state
> > > >     *
> > > > @@ -796,10 +803,11 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
> > > >     */
> > > >    void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge,
> > > >    				      struct drm_bridge_state *state)
> > > >    {
> > > >    	memset(state, 0, sizeof(*state));
> > > Another unnecessary memset?
> > I guess the two can be seen as redundant, but I'd argue the one in
> > __drm_atomic_helper_private_obj_reset should still be there.
> > 
> > What guarantees that the pointer points to a zero'd structure?
> 
> We only call this helper after allocation AFAICT. And the DRM APIs already
> assume that allocation clears to zero.

Really? Do we have that documented anywhere?

And even then, there's nothing that requires that helper to be called
straight-away after allocation.

More importantly, do we really care about skipping them? Like, all the
other reset helpers are doing it, it's cheap, safe, why should we remove
it?

Maxime

Hi

Am 23.09.25 um 11:33 schrieb Maxime Ripard:
> On Mon, Sep 15, 2025 at 03:12:11PM +0200, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 15.09.25 um 13:27 schrieb Maxime Ripard:
>>> On Tue, Sep 02, 2025 at 03:18:17PM +0200, Thomas Zimmermann wrote:
>>>> Hi
>>>>
>>>> Am 02.09.25 um 10:32 schrieb Maxime Ripard:
>>>>> Bridges implement their state using a drm_private_obj and an
>>>>> hand-crafted reset implementation.
>>>>>
>>>>> Since drm_private_obj doesn't have a set of reset helper like the other
>>>>> states, __drm_atomic_helper_bridge_reset() was initializing both the
>>>>> drm_private_state and the drm_bridge_state structures.
>>>>>
>>>>> This initialization however was missing the drm_private_state.obj
>>>>> pointer to the drm_private_obj the state was allocated for, creating a
>>>>> NULL pointer dereference when trying to access it.
>>>>>
>>>>> Fixes: 751465913f04 ("drm/bridge: Add a drm_bridge_state object")
>>>>> Signed-off-by: Maxime Ripard <mripard@kernel.org>
>>>>> ---
>>>>>     drivers/gpu/drm/drm_atomic_state_helper.c | 8 ++++++++
>>>>>     1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
>>>>> index 7142e163e618ea0d7d9d828e1bd9ff2a6ec0dfeb..b962c342b16aabf4e3bea52a914e5deb1c2080ce 100644
>>>>> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
>>>>> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
>>>>> @@ -707,10 +707,17 @@ void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
>>>>>     	__drm_atomic_helper_connector_destroy_state(state);
>>>>>     	kfree(state);
>>>>>     }
>>>>>     EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>>>>> +static void __drm_atomic_helper_private_obj_reset(struct drm_private_obj *obj,
>>>>> +						  struct drm_private_state *state)
>>>>> +{
>>>>> +	memset(state, 0, sizeof(*state));
>>>> This argument is guaranteed to be zero'd, I think. No need for a memset.
>>>>
>>>>> +	state->obj = obj;
>>>>> +}
>>>>> +
>>>>>     /**
>>>>>      * __drm_atomic_helper_private_obj_duplicate_state - copy atomic private state
>>>>>      * @obj: CRTC object
>>>>>      * @state: new private object state
>>>>>      *
>>>>> @@ -796,10 +803,11 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
>>>>>      */
>>>>>     void __drm_atomic_helper_bridge_reset(struct drm_bridge *bridge,
>>>>>     				      struct drm_bridge_state *state)
>>>>>     {
>>>>>     	memset(state, 0, sizeof(*state));
>>>> Another unnecessary memset?
>>> I guess the two can be seen as redundant, but I'd argue the one in
>>> __drm_atomic_helper_private_obj_reset should still be there.
>>>
>>> What guarantees that the pointer points to a zero'd structure?
>> We only call this helper after allocation AFAICT. And the DRM APIs already
>> assume that allocation clears to zero.
> Really? Do we have that documented anywhere?

I don't think it's documented anywhere, but that's what Sima told me 
when I tried to write similar code. The rule was to alloc zero'd memory 
and than have init functions only set what is necessary, but keep others 
to zero. I think it can be seen in certain places, where drivers set a 
variety of flags or fields before calling an init helper. Creating a GEM 
object from shmem is an example. [1]

[1] 
https://elixir.bootlin.com/linux/v6.17/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L52

Best regards
Thomas

>
> And even then, there's nothing that requires that helper to be called
> straight-away after allocation.
>
> More importantly, do we really care about skipping them? Like, all the
> other reset helpers are doing it, it's cheap, safe, why should we remove
> it?
>
> Maxime

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)