Re: [Intel-gfx] [PATCH 18/22] drm/i915: Use drm_mode_init() for on-stack modes
On Mon, 21 Mar 2022, Ville Syrjälä wrote: > On Wed, Mar 16, 2022 at 10:00:06AM +0200, Jani Nikula wrote: > > On Fri, 18 Feb 2022, Ville Syrjala wrote: > > > From: Ville Syrjälä > > > > > > Initialize on-stack modes with drm_mode_init() to guarantee > > > no stack garbage in the list head, or that we aren't copying > > > over another mode's list head. > > > > > > Based on the following cocci script, with manual fixups: > > > @decl@ > > > identifier M; > > > expression E; > > > @@ > > > - struct drm_display_mode M = E; > > > + struct drm_display_mode M; > > > > > > @@ > > > identifier decl.M; > > > expression decl.E; > > > statement S, S1; > > > @@ > > > struct drm_display_mode M; > > > ... when != S > > > + drm_mode_init(, ); > > > + > > > S1 > > > > > > @@ > > > expression decl.E; > > > @@ > > > - &*E > > > + E > > > > > > Signed-off-by: Ville Syrjälä > > > > I wonder if that cocci could be added to scripts/coccinelle or something > > to detect anyone adding new ones? > > Maybe. > > Julia & co, would you be open to having drm subsystem specific > coccinelle scripts? If so where should we put the? > scripts/coccinelle/drm perhaps? That would be fine. It is possible to make a script only apply to a specific directory, but I think that that is not necessary in this case, since you mention types that are only relevant to drm code. julia
Re: [Intel-gfx] [PATCH 18/22] drm/i915: Use drm_mode_init() for on-stack modes
On Wed, Mar 16, 2022 at 10:00:06AM +0200, Jani Nikula wrote: > On Fri, 18 Feb 2022, Ville Syrjala wrote: > > From: Ville Syrjälä > > > > Initialize on-stack modes with drm_mode_init() to guarantee > > no stack garbage in the list head, or that we aren't copying > > over another mode's list head. > > > > Based on the following cocci script, with manual fixups: > > @decl@ > > identifier M; > > expression E; > > @@ > > - struct drm_display_mode M = E; > > + struct drm_display_mode M; > > > > @@ > > identifier decl.M; > > expression decl.E; > > statement S, S1; > > @@ > > struct drm_display_mode M; > > ... when != S > > + drm_mode_init(, ); > > + > > S1 > > > > @@ > > expression decl.E; > > @@ > > - &*E > > + E > > > > Signed-off-by: Ville Syrjälä > > I wonder if that cocci could be added to scripts/coccinelle or something > to detect anyone adding new ones? Maybe. Julia & co, would you be open to having drm subsystem specific coccinelle scripts? If so where should we put the? scripts/coccinelle/drm perhaps? -- Ville Syrjälä Intel
Re: [Intel-gfx] [PATCH 18/22] drm/i915: Use drm_mode_init() for on-stack modes
On Fri, 18 Feb 2022, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Initialize on-stack modes with drm_mode_init() to guarantee
> no stack garbage in the list head, or that we aren't copying
> over another mode's list head.
>
> Based on the following cocci script, with manual fixups:
> @decl@
> identifier M;
> expression E;
> @@
> - struct drm_display_mode M = E;
> + struct drm_display_mode M;
>
> @@
> identifier decl.M;
> expression decl.E;
> statement S, S1;
> @@
> struct drm_display_mode M;
> ... when != S
> + drm_mode_init(, );
> +
> S1
>
> @@
> expression decl.E;
> @@
> - &*E
> + E
>
> Signed-off-by: Ville Syrjälä
I wonder if that cocci could be added to scripts/coccinelle or something
to detect anyone adding new ones?
Anyway,
Reviewed-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 740620ef07ad..74c5a99ab276 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6898,8 +6898,9 @@ intel_crtc_update_active_timings(const struct
> intel_crtc_state *crtc_state)
> {
> struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - struct drm_display_mode adjusted_mode =
> - crtc_state->hw.adjusted_mode;
> + struct drm_display_mode adjusted_mode;
> +
> + drm_mode_init(_mode, _state->hw.adjusted_mode);
>
> if (crtc_state->vrr.enable) {
> adjusted_mode.crtc_vtotal = crtc_state->vrr.vmax;
--
Jani Nikula, Intel Open Source Graphics Center
