* [PATCH] drm/ssd130x: Use drm_atomic_get_new_plane_state()
@ 2022-09-23 8:34 Javier Martinez Canillas
2022-09-23 9:05 ` Ville Syrjälä
2022-09-26 7:24 ` Thomas Zimmermann
0 siblings, 2 replies; 6+ messages in thread
From: Javier Martinez Canillas @ 2022-09-23 8:34 UTC (permalink / raw)
To: linux-kernel
Cc: Thomas Zimmermann, David Airlie, Javier Martinez Canillas, dri-devel
The struct drm_plane .state shouldn't be accessed directly but instead the
drm_atomic_get_new_plane_state() helper function should be used.
This is based on a similar patch from Thomas Zimmermann for the simpledrm
driver. No functional changes.
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
drivers/gpu/drm/solomon/ssd130x.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 7fae9480aa11..a537692100d1 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -566,10 +566,10 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m
}
static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane,
- struct drm_atomic_state *old_state)
+ struct drm_atomic_state *state)
{
- struct drm_plane_state *plane_state = plane->state;
- struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(old_state, plane);
+ struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
+ struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
struct drm_device *drm = plane->dev;
struct drm_rect src_clip, dst_clip;
@@ -591,7 +591,7 @@ static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane,
}
static void ssd130x_primary_plane_helper_atomic_disable(struct drm_plane *plane,
- struct drm_atomic_state *old_state)
+ struct drm_atomic_state *state)
{
struct drm_device *drm = plane->dev;
struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
--
2.37.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/ssd130x: Use drm_atomic_get_new_plane_state()
2022-09-23 8:34 [PATCH] drm/ssd130x: Use drm_atomic_get_new_plane_state() Javier Martinez Canillas
@ 2022-09-23 9:05 ` Ville Syrjälä
2022-09-23 9:16 ` Javier Martinez Canillas
2022-09-26 7:24 ` Thomas Zimmermann
1 sibling, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2022-09-23 9:05 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: David Airlie, dri-devel, linux-kernel, Thomas Zimmermann
On Fri, Sep 23, 2022 at 10:34:47AM +0200, Javier Martinez Canillas wrote:
> The struct drm_plane .state shouldn't be accessed directly but instead the
> drm_atomic_get_new_plane_state() helper function should be used.
>
> This is based on a similar patch from Thomas Zimmermann for the simpledrm
> driver. No functional changes.
>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I wonder how many naked obj->state dereferences are still
left in places where they should be using the get_{new,old}()
stuff. Might have to write a bit of cocci to find out...
Btw on a somewhat related note, I've been thinking about bringing
for_each_crtc_in_state() & co. back (got removed in commit
77ac3b00b131 ("drm/atomic: Remove deprecated accessor macros"))
but this time without any object state iterator variable. Now that
we're more often just plumbing the full atomic state through I
think there are bunch of places that don't need the object state(s)
within the loop at all, so having to have those variables around
makes the whole thing a bit noisy. Also IIRC we had to add some
(void) casts into the current macros to hide some compiler warnings
about unused variables. Could get rid of at least some of those extra
casts again.
I don't suppose there's anyone interested in doing that so I don't
have to? ;)
>---
>
> drivers/gpu/drm/solomon/ssd130x.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> index 7fae9480aa11..a537692100d1 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -566,10 +566,10 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m
> }
>
> static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane,
> - struct drm_atomic_state *old_state)
> + struct drm_atomic_state *state)
> {
> - struct drm_plane_state *plane_state = plane->state;
> - struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(old_state, plane);
> + struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
> + struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
> struct drm_device *drm = plane->dev;
> struct drm_rect src_clip, dst_clip;
> @@ -591,7 +591,7 @@ static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane,
> }
>
> static void ssd130x_primary_plane_helper_atomic_disable(struct drm_plane *plane,
> - struct drm_atomic_state *old_state)
> + struct drm_atomic_state *state)
> {
> struct drm_device *drm = plane->dev;
> struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
> --
> 2.37.3
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/ssd130x: Use drm_atomic_get_new_plane_state()
2022-09-23 9:05 ` Ville Syrjälä
@ 2022-09-23 9:16 ` Javier Martinez Canillas
2022-09-23 9:43 ` Ville Syrjälä
0 siblings, 1 reply; 6+ messages in thread
From: Javier Martinez Canillas @ 2022-09-23 9:16 UTC (permalink / raw)
To: Ville Syrjälä
Cc: David Airlie, dri-devel, linux-kernel, Thomas Zimmermann
Hello Ville,
On 9/23/22 11:05, Ville Syrjälä wrote:
> On Fri, Sep 23, 2022 at 10:34:47AM +0200, Javier Martinez Canillas wrote:
>> The struct drm_plane .state shouldn't be accessed directly but instead the
>> drm_atomic_get_new_plane_state() helper function should be used.
>>
>> This is based on a similar patch from Thomas Zimmermann for the simpledrm
>> driver. No functional changes.
>>
>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
Thanks.
> I wonder how many naked obj->state dereferences are still
> left in places where they should be using the get_{new,old}()
> stuff. Might have to write a bit of cocci to find out...
>
>
> Btw on a somewhat related note, I've been thinking about bringing
> for_each_crtc_in_state() & co. back (got removed in commit
> 77ac3b00b131 ("drm/atomic: Remove deprecated accessor macros"))
> but this time without any object state iterator variable. Now that
> we're more often just plumbing the full atomic state through I
> think there are bunch of places that don't need the object state(s)
> within the loop at all, so having to have those variables around
> makes the whole thing a bit noisy. Also IIRC we had to add some
> (void) casts into the current macros to hide some compiler warnings
> about unused variables. Could get rid of at least some of those extra
> casts again.
>
> I don't suppose there's anyone interested in doing that so I don't
> have to? ;)
>
Maybe you can add an entry in Documentation/gpu/todo.rst, explaining
this and putting yourself as a contact?
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/ssd130x: Use drm_atomic_get_new_plane_state()
2022-09-23 9:16 ` Javier Martinez Canillas
@ 2022-09-23 9:43 ` Ville Syrjälä
0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2022-09-23 9:43 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: David Airlie, dri-devel, linux-kernel, Thomas Zimmermann
On Fri, Sep 23, 2022 at 11:16:00AM +0200, Javier Martinez Canillas wrote:
> Hello Ville,
>
> On 9/23/22 11:05, Ville Syrjälä wrote:
> > On Fri, Sep 23, 2022 at 10:34:47AM +0200, Javier Martinez Canillas wrote:
> >> The struct drm_plane .state shouldn't be accessed directly but instead the
> >> drm_atomic_get_new_plane_state() helper function should be used.
> >>
> >> This is based on a similar patch from Thomas Zimmermann for the simpledrm
> >> driver. No functional changes.
> >>
> >> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> >
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
>
> Thanks.
>
> > I wonder how many naked obj->state dereferences are still
> > left in places where they should be using the get_{new,old}()
> > stuff. Might have to write a bit of cocci to find out...
> >
> >
> > Btw on a somewhat related note, I've been thinking about bringing
> > for_each_crtc_in_state() & co. back (got removed in commit
> > 77ac3b00b131 ("drm/atomic: Remove deprecated accessor macros"))
> > but this time without any object state iterator variable. Now that
> > we're more often just plumbing the full atomic state through I
> > think there are bunch of places that don't need the object state(s)
> > within the loop at all, so having to have those variables around
> > makes the whole thing a bit noisy. Also IIRC we had to add some
> > (void) casts into the current macros to hide some compiler warnings
> > about unused variables. Could get rid of at least some of those extra
> > casts again.
> >
> > I don't suppose there's anyone interested in doing that so I don't
> > have to? ;)
> >
>
> Maybe you can add an entry in Documentation/gpu/todo.rst, explaining
> this and putting yourself as a contact?
Seems about as much work as just doing it.
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/ssd130x: Use drm_atomic_get_new_plane_state()
2022-09-23 8:34 [PATCH] drm/ssd130x: Use drm_atomic_get_new_plane_state() Javier Martinez Canillas
2022-09-23 9:05 ` Ville Syrjälä
@ 2022-09-26 7:24 ` Thomas Zimmermann
2022-09-27 11:36 ` Javier Martinez Canillas
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Zimmermann @ 2022-09-26 7:24 UTC (permalink / raw)
To: Javier Martinez Canillas, linux-kernel; +Cc: David Airlie, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 2270 bytes --]
Am 23.09.22 um 10:34 schrieb Javier Martinez Canillas:
> The struct drm_plane .state shouldn't be accessed directly but instead the
> drm_atomic_get_new_plane_state() helper function should be used.
>
> This is based on a similar patch from Thomas Zimmermann for the simpledrm
> driver. No functional changes.
>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>
> drivers/gpu/drm/solomon/ssd130x.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> index 7fae9480aa11..a537692100d1 100644
> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -566,10 +566,10 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m
> }
>
> static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane,
> - struct drm_atomic_state *old_state)
> + struct drm_atomic_state *state)
> {
> - struct drm_plane_state *plane_state = plane->state;
> - struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(old_state, plane);
> + struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
> + struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
> struct drm_device *drm = plane->dev;
> struct drm_rect src_clip, dst_clip;
> @@ -591,7 +591,7 @@ static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane,
> }
>
> static void ssd130x_primary_plane_helper_atomic_disable(struct drm_plane *plane,
> - struct drm_atomic_state *old_state)
> + struct drm_atomic_state *state)
> {
> struct drm_device *drm = plane->dev;
> struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/ssd130x: Use drm_atomic_get_new_plane_state()
2022-09-26 7:24 ` Thomas Zimmermann
@ 2022-09-27 11:36 ` Javier Martinez Canillas
0 siblings, 0 replies; 6+ messages in thread
From: Javier Martinez Canillas @ 2022-09-27 11:36 UTC (permalink / raw)
To: Thomas Zimmermann, linux-kernel; +Cc: David Airlie, dri-devel
On 9/26/22 09:24, Thomas Zimmermann wrote:
>
>
> Am 23.09.22 um 10:34 schrieb Javier Martinez Canillas:
>> The struct drm_plane .state shouldn't be accessed directly but instead the
>> drm_atomic_get_new_plane_state() helper function should be used.
>>
>> This is based on a similar patch from Thomas Zimmermann for the simpledrm
>> driver. No functional changes.
>>
>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
Pushed to drm-misc (drm-misc-next). Thanks!
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-27 11:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23 8:34 [PATCH] drm/ssd130x: Use drm_atomic_get_new_plane_state() Javier Martinez Canillas
2022-09-23 9:05 ` Ville Syrjälä
2022-09-23 9:16 ` Javier Martinez Canillas
2022-09-23 9:43 ` Ville Syrjälä
2022-09-26 7:24 ` Thomas Zimmermann
2022-09-27 11:36 ` Javier Martinez Canillas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).