linux-kernel - Re: [PATCH 05/36] drm/atomic: Add drm_atomic_plane_get_crtc_state()

Open Source and information security mailing list archives

Date:	Tue, 20 Jan 2015 12:10:11 +0100
From:	Daniel Vetter <daniel@...ll.ch>
To:	Thierry Reding <thierry.reding@...il.com>
Cc:	dri-devel@...ts.freedesktop.org, linux-tegra@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/36] drm/atomic: Add drm_atomic_plane_get_crtc_state()

On Tue, Jan 20, 2015 at 11:48:24AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@...dia.com>
> 
> This function is useful to obtain the state of the CRTC that a plane is
> being attached to.
> 
> Signed-off-by: Thierry Reding <treding@...dia.com>
> ---
>  include/drm/drm_atomic.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 51168a8b723a..ddca107e5cce 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -75,4 +75,25 @@ int __must_check drm_atomic_check_only(struct drm_atomic_state *state);
>  int __must_check drm_atomic_commit(struct drm_atomic_state *state);
>  int __must_check drm_atomic_async_commit(struct drm_atomic_state *state);
>  
> +/**
> + * drm_atomic_plane_get_crtc_state - obtain atomic state of the CRTC that a
> + *     plane is being attached to
> + * @state: atomic state of plane
> + *
> + * Getting the atomic state of the CRTC that a given plane being attached to
> + * is non-trivial. We need to look at the global atomic state that the plane
> + * state is part of and extract the CRTC state using the CRTC's index. Using
> + * the drm_atomic_get_crtc_state() helper we make sure to acquire the proper
> + * locks, too.
> + *
> + * RETURNS:
> + * The atomic state of the CRTC that the plane is being attached to, or an
> + * ERR_PTR()-encoded negative error code on failure.
> + */
> +static inline struct drm_crtc_state *
> +drm_atomic_plane_get_crtc_state(struct drm_plane_state *state)
> +{
> +	return drm_atomic_get_crtc_state(state->state, state->crtc);

Hm, imo open-coding this is totally ok. Actually better since state->crtc
might be NULL, and hiding that isn't a good idea.

If you have a atomic_plane_disable hook then ->crtc is guaranteed to be
non-NULL in atomic_plane_update, but being explicit here is imo a feature.
So nacked from my side.
-Daniel

> +}
> +
>  #endif /* DRM_ATOMIC_H_ */
> -- 
> 2.1.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives