[PATCH 05/12] drm/msm/dpu: Stop copying around mode->private_flags

[PATCH 05/12] drm/msm/dpu: Stop copying around mode->private_flags

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The driver never sets mode->private_flags so copying
it back and forth is entirely pointless. Stop doing it.

Also drop private_flags from the tracepoint.

Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 29 +--------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h   | 10 +++----
 2 files changed, 5 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 58d3400668f5..4511e2ba3680 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -500,23 +500,6 @@ void dpu_encoder_helper_split_config(
 	}
 }
 
-static void _dpu_encoder_adjust_mode(struct drm_connector *connector,
-		struct drm_display_mode *adj_mode)
-{
-	struct drm_display_mode *cur_mode;
-
-	if (!connector || !adj_mode)
-		return;
-
-	list_for_each_entry(cur_mode, &connector->modes, head) {
-		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_flags |= cur_mode->private_flags;
-		}
-	}
-}
-
 static struct msm_display_topology dpu_encoder_get_topology(
 			struct dpu_encoder_virt *dpu_enc,
 			struct dpu_kms *dpu_kms,
@@ -580,15 +563,6 @@ static int dpu_encoder_virt_atomic_check(
 	adj_mode = &crtc_state->adjusted_mode;
 	trace_dpu_enc_atomic_check(DRMID(drm_enc));
 
-	/*
-	 * display drivers may populate private fields of the drm display mode
-	 * structure while registering possible modes of a connector with DRM.
-	 * These private fields are not populated back while DRM invokes
-	 * the mode_set callbacks. This module retrieves and populates the
-	 * private fields of the given mode.
-	 */
-	_dpu_encoder_adjust_mode(conn_state->connector, adj_mode);
-
 	/* perform atomic check on the first physical encoder (master) */
 	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
 		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
@@ -623,8 +597,7 @@ static int dpu_encoder_virt_atomic_check(
 		}
 	}
 
-	trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags,
-			adj_mode->private_flags);
+	trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index eecfe9b3199e..6714b088970f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -327,20 +327,18 @@ DEFINE_EVENT(dpu_enc_keyval_template, dpu_enc_trigger_start,
 );
 
 TRACE_EVENT(dpu_enc_atomic_check_flags,
-	TP_PROTO(uint32_t drm_id, unsigned int flags, int private_flags),
-	TP_ARGS(drm_id, flags, private_flags),
+	TP_PROTO(uint32_t drm_id, unsigned int flags),
+	TP_ARGS(drm_id, flags),
 	TP_STRUCT__entry(
 		__field(	uint32_t,		drm_id		)
 		__field(	unsigned int,		flags		)
-		__field(	int,			private_flags	)
 	),
 	TP_fast_assign(
 		__entry->drm_id = drm_id;
 		__entry->flags = flags;
-		__entry->private_flags = private_flags;
 	),
-	TP_printk("id=%u, flags=%u, private_flags=%d",
-		  __entry->drm_id, __entry->flags, __entry->private_flags)
+	TP_printk("id=%u, flags=%u",
+		  __entry->drm_id, __entry->flags)
 );
 
 DECLARE_EVENT_CLASS(dpu_enc_id_enable_template,
-- 
2.24.1

Re: [PATCH 05/12] drm/msm/dpu: Stop copying around mode->private_flags

[Emil Velikov]

On Wed, 19 Feb 2020 at 20:36, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The driver never sets mode->private_flags so copying
> it back and forth is entirely pointless. Stop doing it.
>
> Also drop private_flags from the tracepoint.
>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Perhaps the msm team has a WIP which makes use of it ?

Otherwise:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil

Re: [PATCH 05/12] drm/msm/dpu: Stop copying around mode->private_flags

[Ville Syrjälä]

On Thu, Feb 20, 2020 at 11:24:20AM +0000, Emil Velikov wrote:
> On Wed, 19 Feb 2020 at 20:36, Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The driver never sets mode->private_flags so copying
> > it back and forth is entirely pointless. Stop doing it.
> >
> > Also drop private_flags from the tracepoint.
> >
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: linux-arm-msm@vger.kernel.org
> > Cc: freedreno@lists.freedesktop.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Perhaps the msm team has a WIP which makes use of it ?

Maybe if it's one of them five year projects. But anyways, 
with an atomic driver there are certainly better ways to
handle this.

-- 
Ville Syrjälä
Intel

Re: [PATCH 05/12] drm/msm/dpu: Stop copying around mode->private_flags

[Daniel Vetter]

On Thu, Feb 20, 2020 at 05:33:09PM +0200, Ville Syrjälä wrote:
> On Thu, Feb 20, 2020 at 11:24:20AM +0000, Emil Velikov wrote:
> > On Wed, 19 Feb 2020 at 20:36, Ville Syrjala
> > <ville.syrjala@linux.intel.com> wrote:
> > >
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > The driver never sets mode->private_flags so copying
> > > it back and forth is entirely pointless. Stop doing it.
> > >
> > > Also drop private_flags from the tracepoint.
> > >
> > > Cc: Rob Clark <robdclark@gmail.com>
> > > Cc: Sean Paul <sean@poorly.run>
> > > Cc: linux-arm-msm@vger.kernel.org
> > > Cc: freedreno@lists.freedesktop.org
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Perhaps the msm team has a WIP which makes use of it ?
> 
> Maybe if it's one of them five year projects. But anyways, 
> with an atomic driver there are certainly better ways to
> handle this.

Yeah with atomic you have your display mode in drm_crtc_state, which
you're subposed to subclass so that you can have terabytes of private
state. At least in theory :-)

->private_flags was really only useful in pre-atomic drivers.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch