[PATCH v1 1/1] drm: drop unused drm_display_mode.private

[PATCH v1 1/1] drm: drop unused drm_display_mode.private

[Sam Ravnborg]

drm_display_mode.private was only referenced in one place where
is was copied but never assigned.

Drop the copy and drop the field in drm_display_mode.
Adjust the comment of private_flags as is referred to the comment for
private.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---

Again, something I just stumbled upon.
I also checked private_flags - it is used in a few modules.
And it looked legit.

Build tested with allmodconfig, allyesconfig, allnoconfig for relevant
architectures.

	Sam


 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  1 -
 include/drm/drm_modes.h                     | 11 ++---------
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index f96e142c4361..6197261e22c1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -516,7 +516,6 @@ static void _dpu_encoder_adjust_mode(struct drm_connector *connector,
 		if (cur_mode->vdisplay == adj_mode->vdisplay &&
 		    cur_mode->hdisplay == adj_mode->hdisplay &&
 		    drm_mode_vrefresh(cur_mode) == drm_mode_vrefresh(adj_mode)) {
-			adj_mode->private = cur_mode->private;
 			adj_mode->private_flags |= cur_mode->private_flags;
 		}
 	}
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index e946e20c61d8..99134d4f35eb 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -371,20 +371,13 @@ struct drm_display_mode {
 	int crtc_vtotal;
 
 	/**
-	 * @private:
+	 * @private_flags:
 	 *
-	 * Pointer for driver private data. This can only be used for mode
+	 * Driver private flags. private_flags can only be used for mode
 	 * objects passed to drivers in modeset operations. It shouldn't be used
 	 * by atomic drivers since they can store any additional data by
 	 * subclassing state structures.
 	 */
-	int *private;
-
-	/**
-	 * @private_flags:
-	 *
-	 * Similar to @private, but just an integer.
-	 */
 	int private_flags;
 
 	/**
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/1] drm: drop unused drm_display_mode.private

[Daniel Vetter]

On Sat, Feb 15, 2020 at 7:35 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> drm_display_mode.private was only referenced in one place where
> is was copied but never assigned.
>
> Drop the copy and drop the field in drm_display_mode.
> Adjust the comment of private_flags as is referred to the comment for
> private.
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>
> Again, something I just stumbled upon.
> I also checked private_flags - it is used in a few modules.
> And it looked legit.
>
> Build tested with allmodconfig, allyesconfig, allnoconfig for relevant
> architectures.

Iirc i915 used this, before we went full overdrive with entire atomic
state structure subclassing :-)

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>
>         Sam
>
>
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  1 -
>  include/drm/drm_modes.h                     | 11 ++---------
>  2 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index f96e142c4361..6197261e22c1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -516,7 +516,6 @@ static void _dpu_encoder_adjust_mode(struct drm_connector *connector,
>                 if (cur_mode->vdisplay == adj_mode->vdisplay &&
>                     cur_mode->hdisplay == adj_mode->hdisplay &&
>                     drm_mode_vrefresh(cur_mode) == drm_mode_vrefresh(adj_mode)) {
> -                       adj_mode->private = cur_mode->private;
>                         adj_mode->private_flags |= cur_mode->private_flags;
>                 }
>         }
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index e946e20c61d8..99134d4f35eb 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -371,20 +371,13 @@ struct drm_display_mode {
>         int crtc_vtotal;
>
>         /**
> -        * @private:
> +        * @private_flags:
>          *
> -        * Pointer for driver private data. This can only be used for mode
> +        * Driver private flags. private_flags can only be used for mode
>          * objects passed to drivers in modeset operations. It shouldn't be used
>          * by atomic drivers since they can store any additional data by
>          * subclassing state structures.
>          */
> -       int *private;
> -
> -       /**
> -        * @private_flags:
> -        *
> -        * Similar to @private, but just an integer.
> -        */
>         int private_flags;
>
>         /**
> --
> 2.20.1
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/1] drm: drop unused drm_display_mode.private

[Sam Ravnborg]

Hi Daniel.

> > I also checked private_flags - it is used in a few modules.
> > And it looked legit.
> >
> Iirc i915 used this, before we went full overdrive with entire atomic
> state structure subclassing :-)

$ git grep -l private_flags
gma500/psb_intel_drv.h
i915/display/intel_display.c
i915/display/intel_display_types.h
i915/display/intel_tv.c
i915/display/vlv_dsi.c
i915/i915_irq.c
msm/disp/dpu1/dpu_encoder.c
msm/disp/dpu1/dpu_trace.h <= false hit

i915 still has a few uses of private_flags.
Likewise msm and gma500

Looks doable to address this, but not on my TODO list.

	Sam

> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks - applied.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 1/1] drm: drop unused drm_display_mode.private

[Daniel Vetter]

On Sat, Feb 15, 2020 at 9:08 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Daniel.
>
> > > I also checked private_flags - it is used in a few modules.
> > > And it looked legit.
> > >
> > Iirc i915 used this, before we went full overdrive with entire atomic
> > state structure subclassing :-)
>
> $ git grep -l private_flags
> gma500/psb_intel_drv.h
> i915/display/intel_display.c
> i915/display/intel_display_types.h
> i915/display/intel_tv.c
> i915/display/vlv_dsi.c
> i915/i915_irq.c
> msm/disp/dpu1/dpu_encoder.c
> msm/disp/dpu1/dpu_trace.h <= false hit
>
> i915 still has a few uses of private_flags.
> Likewise msm and gma500
>
> Looks doable to address this, but not on my TODO list.

Oh I meant the private pointer, not the flags stuff. And yeah maybe we
could add this as a todo, at least for i915 and msm. For gma500 first
step would be converting that driver over to atomic, which I don't
think will happen.
-Daniel

>
>         Sam
>
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Thanks - applied.
>
>         Sam



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel