[PATCH v2 0/4] reserve RM resources in private obj state

[PATCH v2 0/4] reserve RM resources in private obj state

[Jeykumar Sankaran]

v2 patches for [1]. 

changes in v2:
	- Reserve and track hw resources allocated per
	  display in subclassed drm private object states. 
	- Private objects are created per CRTC.

[1] https://patchwork.freedesktop.org/series/50722/

Follow up series will be submitted to clean up RM iterator
API's.

Thanks and Regards,
Jeykumar S.

Jeykumar Sankaran (4):
  drm/msm/dpu: add atomic private object to dpu crtc
  drm/msm/dpu: track HW resources using private object state
  drm/msm/dpu: remove reserve in encoder mode_set
  drm/msm/dpu: remove mode_set_complete

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |  10 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 163 ++++++++++------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |  73 ++++++++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     |  32 ++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      | 131 +++++++++++++++-------
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h      |   8 +-
 6 files changed, 257 insertions(+), 160 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 1/4] drm/msm/dpu: add atomic private object to dpu crtc

[Jeykumar Sankaran]

Subclass drm private object state for DPU for handling driver
specific data. Adds atomic private object to dpu crtc to
track hw resources per display. Provide helper function to
retrieve DPU private data from current atomic state before
atomic swap.

changes in v2:
	- private objects are maintained in dpu_crtc as
	  the resources are tracked per display

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |  3 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 64 +++++++++++++++++++++++++++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h  | 15 ++++++++
 3 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index e59d62b..be07554 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -141,6 +141,7 @@ struct dpu_crtc_frame_event {
  * @frame_pending : Whether or not an update is pending
  * @frame_events  : static allocation of in-flight frame events
  * @frame_event_list : available frame event list
+ * @priv_obj	  : private state object to track hw resources
  * @spin_lock     : spin lock for frame event, transaction status, etc...
  * @frame_done_comp    : for frame_event_done synchronization
  * @event_thread  : Pointer to event handler thread
@@ -176,6 +177,8 @@ struct dpu_crtc {
 	spinlock_t spin_lock;
 	struct completion frame_done_comp;
 
+	struct drm_private_obj priv_obj;
+
 	/* for handling internal event thread */
 	spinlock_t event_lock;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 885bf88..1677862 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -459,6 +459,49 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
 	return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
 }
 
+struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state *state,
+						struct dpu_crtc *crtc)
+{
+	struct drm_private_state *priv_state;
+
+	priv_state = drm_atomic_get_private_obj_state(state, &crtc->priv_obj);
+	if (IS_ERR(priv_state))
+		return ERR_PTR(-ENOMEM);
+
+	return container_of(priv_state, struct dpu_private_state, base);
+}
+
+static struct drm_private_state *
+dpu_private_obj_duplicate_state(struct drm_private_obj *obj)
+{
+	struct dpu_private_state *dpu_priv_state;
+
+	dpu_priv_state = kmemdup(obj->state, sizeof(*dpu_priv_state),
+				 GFP_KERNEL);
+	if (!dpu_priv_state)
+		return NULL;
+
+	__drm_atomic_helper_private_obj_duplicate_state(obj,
+							&dpu_priv_state->base);
+
+	return &dpu_priv_state->base;
+}
+
+static void dpu_private_obj_destroy_state(struct drm_private_obj *obj,
+				      struct drm_private_state *state)
+{
+	struct dpu_private_state *dpu_priv_state = container_of(state,
+						      struct dpu_private_state,
+						      base);
+
+	kfree(dpu_priv_state);
+}
+
+static const struct drm_private_state_funcs priv_obj_funcs = {
+	.atomic_duplicate_state = dpu_private_obj_duplicate_state,
+	.atomic_destroy_state = dpu_private_obj_destroy_state,
+};
+
 static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
 {
 	struct msm_drm_private *priv;
@@ -476,8 +519,12 @@ static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
 	}
 	priv = dpu_kms->dev->dev_private;
 
-	for (i = 0; i < priv->num_crtcs; i++)
+	for (i = 0; i < priv->num_crtcs; i++) {
+		struct dpu_crtc *dpu_crtc = to_dpu_crtc(priv->crtcs[i]);
+
+		drm_atomic_private_obj_fini(&dpu_crtc->priv_obj);
 		priv->crtcs[i]->funcs->destroy(priv->crtcs[i]);
+	}
 	priv->num_crtcs = 0;
 
 	for (i = 0; i < priv->num_planes; i++)
@@ -499,6 +546,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 	struct drm_plane *primary_planes[MAX_PLANES], *plane;
 	struct drm_plane *cursor_planes[MAX_PLANES] = { NULL };
 	struct drm_crtc *crtc;
+	struct dpu_private_state *dpu_priv_state;
 
 	struct msm_drm_private *priv;
 	struct dpu_mdss_cfg *catalog;
@@ -560,11 +608,25 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 
 	/* Create one CRTC per encoder */
 	for (i = 0; i < max_crtc_count; i++) {
+		struct dpu_crtc *dpu_crtc;
+
 		crtc = dpu_crtc_init(dev, primary_planes[i], cursor_planes[i]);
 		if (IS_ERR(crtc)) {
 			ret = PTR_ERR(crtc);
 			goto fail;
 		}
+
+		dpu_crtc = to_dpu_crtc(crtc);
+
+		/* Initialize private obj's */
+		dpu_priv_state = kzalloc(sizeof(*dpu_priv_state), GFP_KERNEL);
+		if (!dpu_priv_state)
+			return -ENOMEM;
+
+		drm_atomic_private_obj_init(&dpu_crtc->priv_obj,
+					    &dpu_priv_state->base,
+					    &priv_obj_funcs);
+
 		priv->crtcs[priv->num_crtcs++] = crtc;
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index ac75cfc..3deedfb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -32,6 +32,8 @@
 #include "dpu_rm.h"
 #include "dpu_core_perf.h"
 
+struct dpu_crtc;
+
 #define DRMID(x) ((x) ? (x)->base.id : -1)
 
 /**
@@ -136,6 +138,10 @@ struct dpu_kms {
 	struct dss_module_power mp;
 };
 
+struct dpu_private_state {
+	struct drm_private_state base;
+};
+
 struct vsync_info {
 	u32 frame_count;
 	u32 line_count;
@@ -143,6 +149,15 @@ struct vsync_info {
 
 #define to_dpu_kms(x) container_of(x, struct dpu_kms, base)
 
+/**
+ * dpu_get_private_state - get dpu private state from atomic state
+ * @state: drm atomic state
+ * @crtc: pointer to crtc obj
+ * Return: pointer to dpu private state object
+ */
+struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state *state,
+						struct dpu_crtc *crtc);
+
 /* get struct msm_kms * from drm_device * */
 #define ddev_to_msm_kms(D) ((D) && (D)->dev_private ? \
 		((struct msm_drm_private *)((D)->dev_private))->kms : NULL)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/4] drm/msm/dpu: add atomic private object to dpu crtc

[Sean Paul]

On Wed, Feb 13, 2019 at 05:52:19PM -0800, Jeykumar Sankaran wrote:
> Subclass drm private object state for DPU for handling driver
> specific data. Adds atomic private object to dpu crtc to
> track hw resources per display. Provide helper function to
> retrieve DPU private data from current atomic state before
> atomic swap.
> 
> changes in v2:
> 	- private objects are maintained in dpu_crtc as
> 	  the resources are tracked per display

Seems like you could just store it in crtc_state if it's per-crtc?

Sean

> 
> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |  3 ++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 64 +++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h  | 15 ++++++++
>  3 files changed, 81 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> index e59d62b..be07554 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> @@ -141,6 +141,7 @@ struct dpu_crtc_frame_event {
>   * @frame_pending : Whether or not an update is pending
>   * @frame_events  : static allocation of in-flight frame events
>   * @frame_event_list : available frame event list
> + * @priv_obj	  : private state object to track hw resources
>   * @spin_lock     : spin lock for frame event, transaction status, etc...
>   * @frame_done_comp    : for frame_event_done synchronization
>   * @event_thread  : Pointer to event handler thread
> @@ -176,6 +177,8 @@ struct dpu_crtc {
>  	spinlock_t spin_lock;
>  	struct completion frame_done_comp;
>  
> +	struct drm_private_obj priv_obj;
> +
>  	/* for handling internal event thread */
>  	spinlock_t event_lock;
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 885bf88..1677862 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -459,6 +459,49 @@ static int _dpu_kms_setup_displays(struct drm_device *dev,
>  	return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
>  }
>  
> +struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state *state,
> +						struct dpu_crtc *crtc)
> +{
> +	struct drm_private_state *priv_state;
> +
> +	priv_state = drm_atomic_get_private_obj_state(state, &crtc->priv_obj);
> +	if (IS_ERR(priv_state))
> +		return ERR_PTR(-ENOMEM);
> +
> +	return container_of(priv_state, struct dpu_private_state, base);
> +}
> +
> +static struct drm_private_state *
> +dpu_private_obj_duplicate_state(struct drm_private_obj *obj)
> +{
> +	struct dpu_private_state *dpu_priv_state;
> +
> +	dpu_priv_state = kmemdup(obj->state, sizeof(*dpu_priv_state),
> +				 GFP_KERNEL);
> +	if (!dpu_priv_state)
> +		return NULL;
> +
> +	__drm_atomic_helper_private_obj_duplicate_state(obj,
> +							&dpu_priv_state->base);
> +
> +	return &dpu_priv_state->base;
> +}
> +
> +static void dpu_private_obj_destroy_state(struct drm_private_obj *obj,
> +				      struct drm_private_state *state)
> +{
> +	struct dpu_private_state *dpu_priv_state = container_of(state,
> +						      struct dpu_private_state,
> +						      base);
> +
> +	kfree(dpu_priv_state);
> +}
> +
> +static const struct drm_private_state_funcs priv_obj_funcs = {
> +	.atomic_duplicate_state = dpu_private_obj_duplicate_state,
> +	.atomic_destroy_state = dpu_private_obj_destroy_state,
> +};
> +
>  static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
>  {
>  	struct msm_drm_private *priv;
> @@ -476,8 +519,12 @@ static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
>  	}
>  	priv = dpu_kms->dev->dev_private;
>  
> -	for (i = 0; i < priv->num_crtcs; i++)
> +	for (i = 0; i < priv->num_crtcs; i++) {
> +		struct dpu_crtc *dpu_crtc = to_dpu_crtc(priv->crtcs[i]);
> +
> +		drm_atomic_private_obj_fini(&dpu_crtc->priv_obj);
>  		priv->crtcs[i]->funcs->destroy(priv->crtcs[i]);
> +	}
>  	priv->num_crtcs = 0;
>  
>  	for (i = 0; i < priv->num_planes; i++)
> @@ -499,6 +546,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
>  	struct drm_plane *primary_planes[MAX_PLANES], *plane;
>  	struct drm_plane *cursor_planes[MAX_PLANES] = { NULL };
>  	struct drm_crtc *crtc;
> +	struct dpu_private_state *dpu_priv_state;
>  
>  	struct msm_drm_private *priv;
>  	struct dpu_mdss_cfg *catalog;
> @@ -560,11 +608,25 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
>  
>  	/* Create one CRTC per encoder */
>  	for (i = 0; i < max_crtc_count; i++) {
> +		struct dpu_crtc *dpu_crtc;
> +
>  		crtc = dpu_crtc_init(dev, primary_planes[i], cursor_planes[i]);
>  		if (IS_ERR(crtc)) {
>  			ret = PTR_ERR(crtc);
>  			goto fail;
>  		}
> +
> +		dpu_crtc = to_dpu_crtc(crtc);
> +
> +		/* Initialize private obj's */
> +		dpu_priv_state = kzalloc(sizeof(*dpu_priv_state), GFP_KERNEL);
> +		if (!dpu_priv_state)
> +			return -ENOMEM;
> +
> +		drm_atomic_private_obj_init(&dpu_crtc->priv_obj,
> +					    &dpu_priv_state->base,
> +					    &priv_obj_funcs);
> +
>  		priv->crtcs[priv->num_crtcs++] = crtc;
>  	}
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index ac75cfc..3deedfb 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -32,6 +32,8 @@
>  #include "dpu_rm.h"
>  #include "dpu_core_perf.h"
>  
> +struct dpu_crtc;
> +
>  #define DRMID(x) ((x) ? (x)->base.id : -1)
>  
>  /**
> @@ -136,6 +138,10 @@ struct dpu_kms {
>  	struct dss_module_power mp;
>  };
>  
> +struct dpu_private_state {
> +	struct drm_private_state base;
> +};
> +
>  struct vsync_info {
>  	u32 frame_count;
>  	u32 line_count;
> @@ -143,6 +149,15 @@ struct vsync_info {
>  
>  #define to_dpu_kms(x) container_of(x, struct dpu_kms, base)
>  
> +/**
> + * dpu_get_private_state - get dpu private state from atomic state
> + * @state: drm atomic state
> + * @crtc: pointer to crtc obj
> + * Return: pointer to dpu private state object
> + */
> +struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state *state,
> +						struct dpu_crtc *crtc);
> +
>  /* get struct msm_kms * from drm_device * */
>  #define ddev_to_msm_kms(D) ((D) && (D)->dev_private ? \
>  		((struct msm_drm_private *)((D)->dev_private))->kms : NULL)
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/4] drm/msm/dpu: add atomic private object to dpu crtc

[Jeykumar Sankaran]

On 2019-03-04 13:32, Sean Paul wrote:
> On Wed, Feb 13, 2019 at 05:52:19PM -0800, Jeykumar Sankaran wrote:
>> Subclass drm private object state for DPU for handling driver
>> specific data. Adds atomic private object to dpu crtc to
>> track hw resources per display. Provide helper function to
>> retrieve DPU private data from current atomic state before
>> atomic swap.
>> 
>> changes in v2:
>> 	- private objects are maintained in dpu_crtc as
>> 	  the resources are tracked per display
> 
> Seems like you could just store it in crtc_state if it's per-crtc?
> 
If you mean - "no need for priv object, maintain in crtc state", that
was the original v1 implementation. But you proposed to have them
tracked in a private object since I had to use crtc_state for tracking
interfaces too which were mapped in encoders.

It made sense as the private objects not bounded to any DRM object
domain and is the best candidate to track per display assignments.
So v2 implementation moved all the RM assignment to private objects 
state
and provided an interface for CRTC and Encoder to query its domain
specific hw blocks.

v1: https://patchwork.freedesktop.org/patch/255452/

Thanks and Regards,
Jeykumar S.


> Sean
> 
>> 
>> Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
>> ---
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |  3 ++
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 64
> +++++++++++++++++++++++++++++++-
>>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h  | 15 ++++++++
>>  3 files changed, 81 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
>> index e59d62b..be07554 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
>> @@ -141,6 +141,7 @@ struct dpu_crtc_frame_event {
>>   * @frame_pending : Whether or not an update is pending
>>   * @frame_events  : static allocation of in-flight frame events
>>   * @frame_event_list : available frame event list
>> + * @priv_obj	  : private state object to track hw resources
>>   * @spin_lock     : spin lock for frame event, transaction status,
> etc...
>>   * @frame_done_comp    : for frame_event_done synchronization
>>   * @event_thread  : Pointer to event handler thread
>> @@ -176,6 +177,8 @@ struct dpu_crtc {
>>  	spinlock_t spin_lock;
>>  	struct completion frame_done_comp;
>> 
>> +	struct drm_private_obj priv_obj;
>> +
>>  	/* for handling internal event thread */
>>  	spinlock_t event_lock;
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> index 885bf88..1677862 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
>> @@ -459,6 +459,49 @@ static int _dpu_kms_setup_displays(struct
> drm_device *dev,
>>  	return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
>>  }
>> 
>> +struct dpu_private_state *dpu_get_private_state(struct 
>> drm_atomic_state
> *state,
>> +						struct dpu_crtc *crtc)
>> +{
>> +	struct drm_private_state *priv_state;
>> +
>> +	priv_state = drm_atomic_get_private_obj_state(state,
> &crtc->priv_obj);
>> +	if (IS_ERR(priv_state))
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	return container_of(priv_state, struct dpu_private_state, base);
>> +}
>> +
>> +static struct drm_private_state *
>> +dpu_private_obj_duplicate_state(struct drm_private_obj *obj)
>> +{
>> +	struct dpu_private_state *dpu_priv_state;
>> +
>> +	dpu_priv_state = kmemdup(obj->state, sizeof(*dpu_priv_state),
>> +				 GFP_KERNEL);
>> +	if (!dpu_priv_state)
>> +		return NULL;
>> +
>> +	__drm_atomic_helper_private_obj_duplicate_state(obj,
>> +
> &dpu_priv_state->base);
>> +
>> +	return &dpu_priv_state->base;
>> +}
>> +
>> +static void dpu_private_obj_destroy_state(struct drm_private_obj 
>> *obj,
>> +				      struct drm_private_state *state)
>> +{
>> +	struct dpu_private_state *dpu_priv_state = container_of(state,
>> +						      struct
> dpu_private_state,
>> +						      base);
>> +
>> +	kfree(dpu_priv_state);
>> +}
>> +
>> +static const struct drm_private_state_funcs priv_obj_funcs = {
>> +	.atomic_duplicate_state = dpu_private_obj_duplicate_state,
>> +	.atomic_destroy_state = dpu_private_obj_destroy_state,
>> +};
>> +
>>  static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
>>  {
>>  	struct msm_drm_private *priv;
>> @@ -476,8 +519,12 @@ static void _dpu_kms_drm_obj_destroy(struct 
>> dpu_kms
> *dpu_kms)
>>  	}
>>  	priv = dpu_kms->dev->dev_private;
>> 
>> -	for (i = 0; i < priv->num_crtcs; i++)
>> +	for (i = 0; i < priv->num_crtcs; i++) {
>> +		struct dpu_crtc *dpu_crtc = to_dpu_crtc(priv->crtcs[i]);
>> +
>> +		drm_atomic_private_obj_fini(&dpu_crtc->priv_obj);
>>  		priv->crtcs[i]->funcs->destroy(priv->crtcs[i]);
>> +	}
>>  	priv->num_crtcs = 0;
>> 
>>  	for (i = 0; i < priv->num_planes; i++)
>> @@ -499,6 +546,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
> *dpu_kms)
>>  	struct drm_plane *primary_planes[MAX_PLANES], *plane;
>>  	struct drm_plane *cursor_planes[MAX_PLANES] = { NULL };
>>  	struct drm_crtc *crtc;
>> +	struct dpu_private_state *dpu_priv_state;
>> 
>>  	struct msm_drm_private *priv;
>>  	struct dpu_mdss_cfg *catalog;
>> @@ -560,11 +608,25 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
> *dpu_kms)
>> 
>>  	/* Create one CRTC per encoder */
>>  	for (i = 0; i < max_crtc_count; i++) {
>> +		struct dpu_crtc *dpu_crtc;
>> +
>>  		crtc = dpu_crtc_init(dev, primary_planes[i],
> cursor_planes[i]);
>>  		if (IS_ERR(crtc)) {
>>  			ret = PTR_ERR(crtc);
>>  			goto fail;
>>  		}
>> +
>> +		dpu_crtc = to_dpu_crtc(crtc);
>> +
>> +		/* Initialize private obj's */
>> +		dpu_priv_state = kzalloc(sizeof(*dpu_priv_state),
> GFP_KERNEL);
>> +		if (!dpu_priv_state)
>> +			return -ENOMEM;
>> +
>> +		drm_atomic_private_obj_init(&dpu_crtc->priv_obj,
>> +					    &dpu_priv_state->base,
>> +					    &priv_obj_funcs);
>> +
>>  		priv->crtcs[priv->num_crtcs++] = crtc;
>>  	}
>> 
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> index ac75cfc..3deedfb 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
>> @@ -32,6 +32,8 @@
>>  #include "dpu_rm.h"
>>  #include "dpu_core_perf.h"
>> 
>> +struct dpu_crtc;
>> +
>>  #define DRMID(x) ((x) ? (x)->base.id : -1)
>> 
>>  /**
>> @@ -136,6 +138,10 @@ struct dpu_kms {
>>  	struct dss_module_power mp;
>>  };
>> 
>> +struct dpu_private_state {
>> +	struct drm_private_state base;
>> +};
>> +
>>  struct vsync_info {
>>  	u32 frame_count;
>>  	u32 line_count;
>> @@ -143,6 +149,15 @@ struct vsync_info {
>> 
>>  #define to_dpu_kms(x) container_of(x, struct dpu_kms, base)
>> 
>> +/**
>> + * dpu_get_private_state - get dpu private state from atomic state
>> + * @state: drm atomic state
>> + * @crtc: pointer to crtc obj
>> + * Return: pointer to dpu private state object
>> + */
>> +struct dpu_private_state *dpu_get_private_state(struct 
>> drm_atomic_state
> *state,
>> +						struct dpu_crtc *crtc);
>> +
>>  /* get struct msm_kms * from drm_device * */
>>  #define ddev_to_msm_kms(D) ((D) && (D)->dev_private ? \
>>  		((struct msm_drm_private *)((D)->dev_private))->kms :
> NULL)
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
>> a Linux Foundation Collaborative Project
>> 
>> _______________________________________________
>> Freedreno mailing list
>> Freedreno@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [PATCH v2 1/4] drm/msm/dpu: add atomic private object to dpu crtc

[Sean Paul]

On Tue, Mar 05, 2019 at 11:34:48AM -0800, Jeykumar Sankaran wrote:
> On 2019-03-04 13:32, Sean Paul wrote:
> > On Wed, Feb 13, 2019 at 05:52:19PM -0800, Jeykumar Sankaran wrote:
> > > Subclass drm private object state for DPU for handling driver
> > > specific data. Adds atomic private object to dpu crtc to
> > > track hw resources per display. Provide helper function to
> > > retrieve DPU private data from current atomic state before
> > > atomic swap.
> > > 
> > > changes in v2:
> > > 	- private objects are maintained in dpu_crtc as
> > > 	  the resources are tracked per display
> > 
> > Seems like you could just store it in crtc_state if it's per-crtc?
> > 
> If you mean - "no need for priv object, maintain in crtc state", that
> was the original v1 implementation. But you proposed to have them
> tracked in a private object since I had to use crtc_state for tracking
> interfaces too which were mapped in encoders.
> 
> It made sense as the private objects not bounded to any DRM object
> domain and is the best candidate to track per display assignments.
> So v2 implementation moved all the RM assignment to private objects state
> and provided an interface for CRTC and Encoder to query its domain
> specific hw blocks.
> 

Apologies for the churn, I'm trying my best to grok all of this, but obviously
failing :(

I think it makes sense to use private objects if you're tracking resources
across the entire driver. So if that's what you're trying to do, I'd
prefer storing it all in one private object at the top-level. If you want to
store it per-crtc, then use the crtc state since it's already managed by the
core/helpers.

Sean

> v1: https://patchwork.freedesktop.org/patch/255452/
> 
> Thanks and Regards,
> Jeykumar S.
> 
> 
> > Sean
> > 
> > > 
> > > Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> > > ---
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |  3 ++
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 64
> > +++++++++++++++++++++++++++++++-
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h  | 15 ++++++++
> > >  3 files changed, 81 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > index e59d62b..be07554 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > @@ -141,6 +141,7 @@ struct dpu_crtc_frame_event {
> > >   * @frame_pending : Whether or not an update is pending
> > >   * @frame_events  : static allocation of in-flight frame events
> > >   * @frame_event_list : available frame event list
> > > + * @priv_obj	  : private state object to track hw resources
> > >   * @spin_lock     : spin lock for frame event, transaction status,
> > etc...
> > >   * @frame_done_comp    : for frame_event_done synchronization
> > >   * @event_thread  : Pointer to event handler thread
> > > @@ -176,6 +177,8 @@ struct dpu_crtc {
> > >  	spinlock_t spin_lock;
> > >  	struct completion frame_done_comp;
> > > 
> > > +	struct drm_private_obj priv_obj;
> > > +
> > >  	/* for handling internal event thread */
> > >  	spinlock_t event_lock;
> > > 
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > index 885bf88..1677862 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > @@ -459,6 +459,49 @@ static int _dpu_kms_setup_displays(struct
> > drm_device *dev,
> > >  	return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
> > >  }
> > > 
> > > +struct dpu_private_state *dpu_get_private_state(struct
> > > drm_atomic_state
> > *state,
> > > +						struct dpu_crtc *crtc)
> > > +{
> > > +	struct drm_private_state *priv_state;
> > > +
> > > +	priv_state = drm_atomic_get_private_obj_state(state,
> > &crtc->priv_obj);
> > > +	if (IS_ERR(priv_state))
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	return container_of(priv_state, struct dpu_private_state, base);
> > > +}
> > > +
> > > +static struct drm_private_state *
> > > +dpu_private_obj_duplicate_state(struct drm_private_obj *obj)
> > > +{
> > > +	struct dpu_private_state *dpu_priv_state;
> > > +
> > > +	dpu_priv_state = kmemdup(obj->state, sizeof(*dpu_priv_state),
> > > +				 GFP_KERNEL);
> > > +	if (!dpu_priv_state)
> > > +		return NULL;
> > > +
> > > +	__drm_atomic_helper_private_obj_duplicate_state(obj,
> > > +
> > &dpu_priv_state->base);
> > > +
> > > +	return &dpu_priv_state->base;
> > > +}
> > > +
> > > +static void dpu_private_obj_destroy_state(struct drm_private_obj
> > > *obj,
> > > +				      struct drm_private_state *state)
> > > +{
> > > +	struct dpu_private_state *dpu_priv_state = container_of(state,
> > > +						      struct
> > dpu_private_state,
> > > +						      base);
> > > +
> > > +	kfree(dpu_priv_state);
> > > +}
> > > +
> > > +static const struct drm_private_state_funcs priv_obj_funcs = {
> > > +	.atomic_duplicate_state = dpu_private_obj_duplicate_state,
> > > +	.atomic_destroy_state = dpu_private_obj_destroy_state,
> > > +};
> > > +
> > >  static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
> > >  {
> > >  	struct msm_drm_private *priv;
> > > @@ -476,8 +519,12 @@ static void _dpu_kms_drm_obj_destroy(struct
> > > dpu_kms
> > *dpu_kms)
> > >  	}
> > >  	priv = dpu_kms->dev->dev_private;
> > > 
> > > -	for (i = 0; i < priv->num_crtcs; i++)
> > > +	for (i = 0; i < priv->num_crtcs; i++) {
> > > +		struct dpu_crtc *dpu_crtc = to_dpu_crtc(priv->crtcs[i]);
> > > +
> > > +		drm_atomic_private_obj_fini(&dpu_crtc->priv_obj);
> > >  		priv->crtcs[i]->funcs->destroy(priv->crtcs[i]);
> > > +	}
> > >  	priv->num_crtcs = 0;
> > > 
> > >  	for (i = 0; i < priv->num_planes; i++)
> > > @@ -499,6 +546,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
> > *dpu_kms)
> > >  	struct drm_plane *primary_planes[MAX_PLANES], *plane;
> > >  	struct drm_plane *cursor_planes[MAX_PLANES] = { NULL };
> > >  	struct drm_crtc *crtc;
> > > +	struct dpu_private_state *dpu_priv_state;
> > > 
> > >  	struct msm_drm_private *priv;
> > >  	struct dpu_mdss_cfg *catalog;
> > > @@ -560,11 +608,25 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
> > *dpu_kms)
> > > 
> > >  	/* Create one CRTC per encoder */
> > >  	for (i = 0; i < max_crtc_count; i++) {
> > > +		struct dpu_crtc *dpu_crtc;
> > > +
> > >  		crtc = dpu_crtc_init(dev, primary_planes[i],
> > cursor_planes[i]);
> > >  		if (IS_ERR(crtc)) {
> > >  			ret = PTR_ERR(crtc);
> > >  			goto fail;
> > >  		}
> > > +
> > > +		dpu_crtc = to_dpu_crtc(crtc);
> > > +
> > > +		/* Initialize private obj's */
> > > +		dpu_priv_state = kzalloc(sizeof(*dpu_priv_state),
> > GFP_KERNEL);
> > > +		if (!dpu_priv_state)
> > > +			return -ENOMEM;
> > > +
> > > +		drm_atomic_private_obj_init(&dpu_crtc->priv_obj,
> > > +					    &dpu_priv_state->base,
> > > +					    &priv_obj_funcs);
> > > +
> > >  		priv->crtcs[priv->num_crtcs++] = crtc;
> > >  	}
> > > 
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > > index ac75cfc..3deedfb 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > > @@ -32,6 +32,8 @@
> > >  #include "dpu_rm.h"
> > >  #include "dpu_core_perf.h"
> > > 
> > > +struct dpu_crtc;
> > > +
> > >  #define DRMID(x) ((x) ? (x)->base.id : -1)
> > > 
> > >  /**
> > > @@ -136,6 +138,10 @@ struct dpu_kms {
> > >  	struct dss_module_power mp;
> > >  };
> > > 
> > > +struct dpu_private_state {
> > > +	struct drm_private_state base;
> > > +};
> > > +
> > >  struct vsync_info {
> > >  	u32 frame_count;
> > >  	u32 line_count;
> > > @@ -143,6 +149,15 @@ struct vsync_info {
> > > 
> > >  #define to_dpu_kms(x) container_of(x, struct dpu_kms, base)
> > > 
> > > +/**
> > > + * dpu_get_private_state - get dpu private state from atomic state
> > > + * @state: drm atomic state
> > > + * @crtc: pointer to crtc obj
> > > + * Return: pointer to dpu private state object
> > > + */
> > > +struct dpu_private_state *dpu_get_private_state(struct
> > > drm_atomic_state
> > *state,
> > > +						struct dpu_crtc *crtc);
> > > +
> > >  /* get struct msm_kms * from drm_device * */
> > >  #define ddev_to_msm_kms(D) ((D) && (D)->dev_private ? \
> > >  		((struct msm_drm_private *)((D)->dev_private))->kms :
> > NULL)
> > > --
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > Forum,
> > > a Linux Foundation Collaborative Project
> > > 
> > > _______________________________________________
> > > Freedreno mailing list
> > > Freedreno@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/freedreno
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/4] drm/msm/dpu: track HW resources using private object state

[Jeykumar Sankaran]

DPU maintained reservation lists to cache assigned
HW blocks for the display and a retrieval mechanism for
the individual DRM components to query their respective
HW blocks.

This patch uses the sub-classed private object state to store
and track HW blocks assigned for different components
of the display pipeline. It helps the driver:
- to get rid of unwanted store and retrieval RM API's
- to preserve HW resources assigned in atomic_check
  through atomic swap/duplicate.

Resources are reserved only when drm_atomic_crtc_needs_modeset
is set to TRUE and are released in atomic disable path.

With TEST_ONLY atomic commit path, reserved resources
are released back to RM pool in private_obj_destroy_state
call.

changes in v2 (comments from Sean Paul):
	- Track resources in private object state instead
	  of crtc state.
	- Avoid duplicate tracking of hw_ctls in crtc
	- No explicit count for hw_ctl as they match
	  with hw_intf count

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h    |   7 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 157 ++++++++++++----------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c     |   9 ++
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h     |  17 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      | 131 +++++++++++++++--------
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h      |   8 +-
 6 files changed, 185 insertions(+), 144 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index be07554..cec0674 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -202,8 +202,6 @@ struct dpu_crtc {
  * @new_perf: new performance state being requested
  * @num_mixers    : Number of mixers in use
  * @mixers        : List of active mixers
- * @num_ctls      : Number of ctl paths in use
- * @hw_ctls       : List of active ctl paths
  */
 struct dpu_crtc_state {
 	struct drm_crtc_state base;
@@ -217,11 +215,8 @@ struct dpu_crtc_state {
 	struct dpu_core_perf_params new_perf;
 
 	/* HW Resources reserved for the crtc */
-	u32 num_mixers;
+	int num_mixers;
 	struct dpu_crtc_mixer mixers[CRTC_DUAL_MIXERS];
-
-	u32 num_ctls;
-	struct dpu_hw_ctl *hw_ctls[CRTC_DUAL_MIXERS];
 };
 
 #define to_dpu_crtc_state(x) \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 98ea478..bd646c5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -559,7 +559,6 @@ static int dpu_encoder_virt_atomic_check(
 	struct dpu_kms *dpu_kms;
 	const struct drm_display_mode *mode;
 	struct drm_display_mode *adj_mode;
-	struct msm_display_topology topology;
 	int i = 0;
 	int ret = 0;
 
@@ -605,20 +604,24 @@ static int dpu_encoder_virt_atomic_check(
 		}
 	}
 
-	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
+	/*
+	 * Reserve dynamic resources now. Indicating AtomicTest phase
+	 *
+	 * Avoid reserving resources when mode set is pending. Topology
+	 * info may not be available to complete reservation.
+	 */
+	if (!ret && drm_atomic_crtc_needs_modeset(crtc_state)
+			&& dpu_enc->mode_set_complete) {
+		struct msm_display_topology topology;
+		struct dpu_private_state *dpu_priv_state;
 
-	/* Reserve dynamic resources now. Indicating AtomicTest phase */
-	if (!ret) {
-		/*
-		 * Avoid reserving resources when mode set is pending. Topology
-		 * info may not be available to complete reservation.
-		 */
-		if (drm_atomic_crtc_needs_modeset(crtc_state)
-				&& dpu_enc->mode_set_complete) {
-			ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, crtc_state,
-					     topology, true);
-			dpu_enc->mode_set_complete = false;
-		}
+		topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
+		dpu_priv_state = dpu_get_private_state(crtc_state->state,
+					       to_dpu_crtc(crtc_state->crtc));
+
+		ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc,
+				     &dpu_priv_state->base, topology, true);
+		dpu_enc->mode_set_complete = false;
 	}
 
 	if (!ret)
@@ -962,12 +965,10 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 	struct list_head *connector_list;
 	struct drm_connector *conn = NULL, *conn_iter;
 	struct drm_crtc *drm_crtc;
-	struct dpu_crtc_state *cstate;
-	struct dpu_rm_hw_iter hw_iter;
+	struct dpu_crtc_state *dpu_cstate;
 	struct msm_display_topology topology;
-	struct dpu_hw_ctl *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
-	struct dpu_hw_mixer *hw_lm[MAX_CHANNELS_PER_ENC] = { NULL };
-	int num_lm = 0, num_ctl = 0;
+	struct dpu_crtc *dpu_crtc;
+	struct dpu_private_state *dpu_private_state;
 	int i, j, ret;
 
 	if (!drm_enc) {
@@ -1001,100 +1002,59 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 			break;
 
 	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
+	dpu_crtc = to_dpu_crtc(drm_crtc);
 
 	/* Reserve dynamic resources now. Indicating non-AtomicTest phase */
-	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, drm_crtc->state,
+	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, dpu_crtc->priv_obj.state,
 			     topology, false);
 	if (ret) {
 		DPU_ERROR_ENC(dpu_enc,
-				"failed to reserve hw resources, %d\n", ret);
+			     "failed to reserve hw resources, %d\n", ret);
 		return;
 	}
 
-	dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id, DPU_HW_BLK_PINGPONG);
-	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
-		dpu_enc->hw_pp[i] = NULL;
-		if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
-			break;
-		dpu_enc->hw_pp[i] = (struct dpu_hw_pingpong *) hw_iter.hw;
-	}
-
-	dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id, DPU_HW_BLK_CTL);
-	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
-		if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
-			break;
-		hw_ctl[i] = (struct dpu_hw_ctl *)hw_iter.hw;
-		num_ctl++;
-	}
-
-	dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id, DPU_HW_BLK_LM);
-	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
-		if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
-			break;
-		hw_lm[i] = (struct dpu_hw_mixer *)hw_iter.hw;
-		num_lm++;
-	}
+	dpu_cstate = to_dpu_crtc_state(drm_crtc->state);
+	dpu_private_state = container_of(dpu_crtc->priv_obj.state,
+					 struct dpu_private_state, base);
 
-	cstate = to_dpu_crtc_state(drm_crtc->state);
+	for (i = 0; i < dpu_private_state->num_mixers; i++) {
+		int ctl_idx;
 
-	for (i = 0; i < num_lm; i++) {
-		int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
+		dpu_cstate->mixers[i].hw_lm = dpu_private_state->hw_lms[i];
 
-		cstate->mixers[i].hw_lm = hw_lm[i];
-		cstate->mixers[i].lm_ctl = hw_ctl[ctl_idx];
+		/*
+		 * hw_ctl count may be <= hw_lm count, if less, multiple LMs are
+		 * controlled by 1 CTL
+		 */
+		ctl_idx = min(i, dpu_private_state->num_intfs - 1);
+		dpu_cstate->mixers[i].lm_ctl =
+			dpu_private_state->hw_ctls[ctl_idx];
 	}
-
-	cstate->num_mixers = num_lm;
+	dpu_cstate->num_mixers = dpu_private_state->num_mixers;
 
 	for (i = 0; i < dpu_enc->num_phys_encs; i++) {
 		struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
 
-		if (phys) {
-			if (!dpu_enc->hw_pp[i]) {
-				DPU_ERROR_ENC(dpu_enc, "no pp block assigned"
-					     "at idx: %d\n", i);
-				goto error;
-			}
-
-			if (!hw_ctl[i]) {
-				DPU_ERROR_ENC(dpu_enc, "no ctl block assigned"
-					     "at idx: %d\n", i);
-				goto error;
-			}
-
-			phys->hw_pp = dpu_enc->hw_pp[i];
-			phys->hw_ctl = hw_ctl[i];
-
-			dpu_rm_init_hw_iter(&hw_iter, drm_enc->base.id,
-					    DPU_HW_BLK_INTF);
-			for (j = 0; j < MAX_CHANNELS_PER_ENC; j++) {
-				struct dpu_hw_intf *hw_intf;
+		phys->hw_pp = dpu_private_state->hw_pps[i];
+		dpu_enc->hw_pp[i] = dpu_private_state->hw_pps[i];
 
-				if (!dpu_rm_get_hw(&dpu_kms->rm, &hw_iter))
-					break;
+		phys->hw_ctl = dpu_cstate->mixers[i].lm_ctl;
 
-				hw_intf = (struct dpu_hw_intf *)hw_iter.hw;
-				if (hw_intf->idx == phys->intf_idx)
-					phys->hw_intf = hw_intf;
+		for (j = 0; j < dpu_private_state->num_intfs; j++) {
+			struct dpu_hw_intf *hw_intf =
+				dpu_private_state->hw_intfs[j];
+			if (hw_intf->idx == phys->intf_idx) {
+				phys->hw_intf = hw_intf;
+				break;
 			}
-
-			if (!phys->hw_intf) {
-				DPU_ERROR_ENC(dpu_enc,
-					      "no intf block assigned at idx: %d\n",
-					      i);
-				goto error;
-			}
-
-			phys->connector = conn->state->connector;
-			if (phys->ops.mode_set)
-				phys->ops.mode_set(phys, mode, adj_mode);
 		}
+
+		phys->connector = conn->state->connector;
+		if (phys->ops.mode_set)
+			phys->ops.mode_set(phys, mode, adj_mode);
 	}
 
 	dpu_enc->mode_set_complete = true;
-
-error:
-	dpu_rm_release(&dpu_kms->rm, drm_enc);
 }
 
 static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
@@ -1196,6 +1156,9 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
 	struct msm_drm_private *priv;
 	struct dpu_kms *dpu_kms;
 	struct drm_display_mode *mode;
+	struct drm_crtc *drm_crtc;
+	struct dpu_crtc *dpu_crtc;
+	unsigned long lock_flags;
 	int i = 0;
 
 	if (!drm_enc) {
@@ -1212,10 +1175,20 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
 	dpu_enc = to_dpu_encoder_virt(drm_enc);
 	DPU_DEBUG_ENC(dpu_enc, "\n");
 
+	/*
+	 * client may have reset the crtc encoder_mask before encoder->disable.
+	 * Rely on the dpu_enc->crtc which will be reset only on crtc->disable.
+	 */
+	spin_lock_irqsave(&dpu_enc->enc_spinlock, lock_flags);
+	drm_crtc = dpu_enc->crtc;
+	spin_unlock_irqrestore(&dpu_enc->enc_spinlock, lock_flags);
+
+	dpu_crtc = to_dpu_crtc(drm_crtc);
+
 	mutex_lock(&dpu_enc->enc_lock);
 	dpu_enc->enabled = false;
 
-	mode = &drm_enc->crtc->state->adjusted_mode;
+	mode = &drm_crtc->state->adjusted_mode;
 
 	priv = drm_enc->dev->dev_private;
 	dpu_kms = to_dpu_kms(priv->kms);
@@ -1249,7 +1222,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
 
 	DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");
 
-	dpu_rm_release(&dpu_kms->rm, drm_enc);
+	dpu_rm_release(&dpu_kms->rm, dpu_crtc->priv_obj.state);
 
 	mutex_unlock(&dpu_enc->enc_lock);
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1677862..43e3211 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -490,10 +490,19 @@ struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state *state,
 static void dpu_private_obj_destroy_state(struct drm_private_obj *obj,
 				      struct drm_private_state *state)
 {
+	struct msm_drm_private *priv = state->state->dev->dev_private;
+	struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
 	struct dpu_private_state *dpu_priv_state = container_of(state,
 						      struct dpu_private_state,
 						      base);
 
+	/*
+	 * Destroy state will be triggering RM release only when resources
+	 * are allocated during TEST_ONLY commits where resources need
+	 * to be freed back to the RM pool
+	 */
+	dpu_rm_release(&dpu_kms->rm, state);
+
 	kfree(dpu_priv_state);
 }
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 3deedfb..790c4d7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -26,6 +26,7 @@
 #include "dpu_hw_catalog.h"
 #include "dpu_hw_ctl.h"
 #include "dpu_hw_lm.h"
+#include "dpu_hw_intf.h"
 #include "dpu_hw_interrupts.h"
 #include "dpu_hw_top.h"
 #include "dpu_io_util.h"
@@ -140,6 +141,22 @@ struct dpu_kms {
 
 struct dpu_private_state {
 	struct drm_private_state base;
+
+	/*
+	 * layer mixers and ping pong blocks
+	 * are hard chained
+	 */
+	int num_mixers;
+	struct dpu_hw_mixer *hw_lms[CRTC_DUAL_MIXERS];
+	struct dpu_hw_pingpong *hw_pps[CRTC_DUAL_MIXERS];
+
+	/*
+	 * until SDM845 each interface is controlled
+	 * by its own hw_ctl
+	 */
+	int num_intfs;
+	struct dpu_hw_ctl *hw_ctls[CRTC_DUAL_MIXERS];
+	struct dpu_hw_intf *hw_intfs[CRTC_DUAL_MIXERS];
 };
 
 struct vsync_info {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 037d9f4..4884683 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -368,6 +368,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(
 }
 
 static int _dpu_rm_reserve_lms(struct dpu_rm *rm, uint32_t enc_id,
+			       struct dpu_private_state *dpu_priv_state,
 			       struct dpu_rm_requirements *reqs)
 
 {
@@ -429,8 +430,13 @@ static int _dpu_rm_reserve_lms(struct dpu_rm *rm, uint32_t enc_id,
 		lm[i]->enc_id = enc_id;
 		pp[i]->enc_id = enc_id;
 
+		dpu_priv_state->hw_lms[i] = to_dpu_hw_mixer(lm[i]->hw);
+		dpu_priv_state->hw_pps[i] = container_of(pp[i]->hw,
+							 struct dpu_hw_pingpong,
+							 base);
 		trace_dpu_rm_reserve_lms(lm[i]->id, enc_id, pp[i]->id);
 	}
+	dpu_priv_state->num_mixers = lm_count;
 
 	return rc;
 }
@@ -438,6 +444,7 @@ static int _dpu_rm_reserve_lms(struct dpu_rm *rm, uint32_t enc_id,
 static int _dpu_rm_reserve_ctls(
 		struct dpu_rm *rm,
 		uint32_t enc_id,
+		struct dpu_private_state *dpu_priv_state,
 		const struct msm_display_topology *top)
 {
 	struct dpu_rm_hw_blk *ctls[MAX_BLOCKS];
@@ -480,20 +487,20 @@ static int _dpu_rm_reserve_ctls(
 
 	for (i = 0; i < ARRAY_SIZE(ctls) && i < num_ctls; i++) {
 		ctls[i]->enc_id = enc_id;
+		dpu_priv_state->hw_ctls[i] = to_dpu_hw_ctl(ctls[i]->hw);
 		trace_dpu_rm_reserve_ctls(ctls[i]->id, enc_id);
 	}
 
 	return 0;
 }
 
-static int _dpu_rm_reserve_intf(
+static struct dpu_rm_hw_blk *_dpu_rm_reserve_intf(
 		struct dpu_rm *rm,
 		uint32_t enc_id,
 		uint32_t id,
 		enum dpu_hw_blk_type type)
 {
 	struct dpu_rm_hw_iter iter;
-	int ret = 0;
 
 	/* Find the block entry in the rm, and note the reservation */
 	dpu_rm_init_hw_iter(&iter, 0, type);
@@ -503,7 +510,7 @@ static int _dpu_rm_reserve_intf(
 
 		if (RESERVED_BY_OTHER(iter.blk, enc_id)) {
 			DPU_ERROR("type %d id %d already reserved\n", type, id);
-			return -ENAVAIL;
+			return NULL;
 		}
 
 		iter.blk->enc_id = enc_id;
@@ -512,56 +519,63 @@ static int _dpu_rm_reserve_intf(
 	}
 
 	/* Shouldn't happen since intfs are fixed at probe */
-	if (!iter.hw) {
+	if (!iter.blk) {
 		DPU_ERROR("couldn't find type %d id %d\n", type, id);
-		return -EINVAL;
+		return NULL;
 	}
 
-	return ret;
+	return iter.blk;
 }
 
 static int _dpu_rm_reserve_intf_related_hw(
 		struct dpu_rm *rm,
 		uint32_t enc_id,
+		struct dpu_private_state *dpu_priv_state,
 		struct dpu_encoder_hw_resources *hw_res)
 {
-	int i, ret = 0;
-	u32 id;
+	struct dpu_rm_hw_blk *blk;
+	int i, num_intfs = 0;
 
 	for (i = 0; i < ARRAY_SIZE(hw_res->intfs); i++) {
 		if (hw_res->intfs[i] == INTF_MODE_NONE)
 			continue;
-		id = i + INTF_0;
-		ret = _dpu_rm_reserve_intf(rm, enc_id, id,
+
+		blk = _dpu_rm_reserve_intf(rm, enc_id, i + INTF_0,
 				DPU_HW_BLK_INTF);
-		if (ret)
-			return ret;
+		if (!blk)
+			return -ENAVAIL;
+
+		dpu_priv_state->hw_intfs[num_intfs++] =
+			container_of(blk->hw, struct dpu_hw_intf, base);
 	}
+	dpu_priv_state->num_intfs = num_intfs;
 
-	return ret;
+	return 0;
 }
 
 static int _dpu_rm_make_reservation(
 		struct dpu_rm *rm,
 		struct drm_encoder *enc,
-		struct drm_crtc_state *crtc_state,
+		struct dpu_private_state *dpu_priv_state,
 		struct dpu_rm_requirements *reqs)
 {
 	int ret;
 
-	ret = _dpu_rm_reserve_lms(rm, enc->base.id, reqs);
+	ret = _dpu_rm_reserve_lms(rm, enc->base.id, dpu_priv_state, reqs);
 	if (ret) {
 		DPU_ERROR("unable to find appropriate mixers\n");
 		return ret;
 	}
 
-	ret = _dpu_rm_reserve_ctls(rm, enc->base.id, &reqs->topology);
+	ret = _dpu_rm_reserve_ctls(rm, enc->base.id, dpu_priv_state,
+				   &reqs->topology);
 	if (ret) {
 		DPU_ERROR("unable to find appropriate CTL\n");
 		return ret;
 	}
 
-	ret = _dpu_rm_reserve_intf_related_hw(rm, enc->base.id, &reqs->hw_res);
+	ret = _dpu_rm_reserve_intf_related_hw(rm, enc->base.id, dpu_priv_state,
+					      &reqs->hw_res);
 	if (ret)
 		return ret;
 
@@ -571,7 +585,6 @@ static int _dpu_rm_make_reservation(
 static int _dpu_rm_populate_requirements(
 		struct dpu_rm *rm,
 		struct drm_encoder *enc,
-		struct drm_crtc_state *crtc_state,
 		struct dpu_rm_requirements *reqs,
 		struct msm_display_topology req_topology)
 {
@@ -586,27 +599,64 @@ static int _dpu_rm_populate_requirements(
 	return 0;
 }
 
-static void _dpu_rm_release_reservation(struct dpu_rm *rm, uint32_t enc_id)
+static int _dpu_rm_release_hw(struct dpu_rm *rm, enum dpu_hw_blk_type type,
+			      int id)
 {
 	struct dpu_rm_hw_blk *blk;
-	enum dpu_hw_blk_type type;
-
-	for (type = 0; type < DPU_HW_BLK_MAX; type++) {
-		list_for_each_entry(blk, &rm->hw_blks[type], list) {
-			if (blk->enc_id == enc_id) {
-				blk->enc_id = 0;
-				DPU_DEBUG("rel enc %d %d %d\n", enc_id,
-					  type, blk->id);
-			}
+	list_for_each_entry(blk, &rm->hw_blks[type], list) {
+		if (blk->hw->id == id) {
+			blk->enc_id = 0;
+			return 0;
 		}
 	}
+
+	DRM_DEBUG_KMS("failed to find hw id(%d) of type(%d) for releasing\n",
+		      id, type);
+
+	return -EINVAL;
+}
+
+static void _dpu_rm_release_reservation(struct dpu_rm *rm,
+					struct drm_private_state *priv_state)
+{
+	struct dpu_private_state *dpu_priv_state =
+		container_of(priv_state, struct dpu_private_state, base);
+	int i;
+
+	for (i = 0; i < dpu_priv_state->num_mixers; i++) {
+		if (!dpu_priv_state->hw_lms[i])
+			continue;
+
+		if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_LM,
+					dpu_priv_state->hw_lms[i]->base.id))
+			dpu_priv_state->hw_lms[i] = NULL;
+
+		if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_PINGPONG,
+					dpu_priv_state->hw_pps[i]->base.id))
+			dpu_priv_state->hw_pps[i] = NULL;
+	}
+	dpu_priv_state->num_mixers = 0;
+
+	for (i = 0; i < dpu_priv_state->num_intfs; i++) {
+		if (!dpu_priv_state->hw_ctls[i])
+			continue;
+
+		if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_CTL,
+					dpu_priv_state->hw_ctls[i]->base.id))
+			dpu_priv_state->hw_ctls[i] = NULL;
+
+		if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_INTF,
+					dpu_priv_state->hw_intfs[i]->base.id))
+			dpu_priv_state->hw_intfs[i] = NULL;
+	}
+	dpu_priv_state->num_intfs = 0;
 }
 
-void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
+void dpu_rm_release(struct dpu_rm *rm, struct drm_private_state *priv_state)
 {
 	mutex_lock(&rm->rm_lock);
 
-	_dpu_rm_release_reservation(rm, enc->base.id);
+	_dpu_rm_release_reservation(rm, priv_state);
 
 	mutex_unlock(&rm->rm_lock);
 }
@@ -614,38 +664,35 @@ void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
 int dpu_rm_reserve(
 		struct dpu_rm *rm,
 		struct drm_encoder *enc,
-		struct drm_crtc_state *crtc_state,
+		struct drm_private_state *priv_state,
 		struct msm_display_topology topology,
 		bool test_only)
 {
 	struct dpu_rm_requirements reqs;
+	struct dpu_private_state *dpu_priv_state =
+		container_of(priv_state, struct dpu_private_state, base);
 	int ret;
 
-	/* Check if this is just a page-flip */
-	if (!drm_atomic_crtc_needs_modeset(crtc_state))
-		return 0;
-
-	DRM_DEBUG_KMS("reserving hw for enc %d crtc %d test_only %d\n",
-		      enc->base.id, crtc_state->crtc->base.id, test_only);
+	DRM_DEBUG_KMS("reserving hw for enc %d test_only %d\n",
+		      enc->base.id, test_only);
 
 	mutex_lock(&rm->rm_lock);
 
-	ret = _dpu_rm_populate_requirements(rm, enc, crtc_state, &reqs,
-					    topology);
+	ret = _dpu_rm_populate_requirements(rm, enc, &reqs, topology);
 	if (ret) {
 		DPU_ERROR("failed to populate hw requirements\n");
 		goto end;
 	}
 
-	ret = _dpu_rm_make_reservation(rm, enc, crtc_state, &reqs);
+	ret = _dpu_rm_make_reservation(rm, enc, dpu_priv_state, &reqs);
 	if (ret) {
 		DPU_ERROR("failed to reserve hw resources: %d\n", ret);
-		_dpu_rm_release_reservation(rm, enc->base.id);
+		_dpu_rm_release_reservation(rm, priv_state);
 	} else if (test_only) {
 		 /* test_only: test the reservation and then undo */
 		DPU_DEBUG("test_only: discard test [enc: %d]\n",
 				enc->base.id);
-		_dpu_rm_release_reservation(rm, enc->base.id);
+		_dpu_rm_release_reservation(rm, priv_state);
 	}
 
 end:
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 381611f..252e173 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -81,14 +81,14 @@ int dpu_rm_init(struct dpu_rm *rm,
  *	HW Reservations should be released via dpu_rm_release_hw.
  * @rm: DPU Resource Manager handle
  * @drm_enc: DRM Encoder handle
- * @crtc_state: Proposed Atomic DRM CRTC State handle
+ * @priv_state: Pointer to drm private obj state
  * @topology: Pointer to topology info for the display
  * @test_only: Atomic-Test phase, discard results (unless property overrides)
  * @Return: 0 on Success otherwise -ERROR
  */
 int dpu_rm_reserve(struct dpu_rm *rm,
 		struct drm_encoder *drm_enc,
-		struct drm_crtc_state *crtc_state,
+		struct drm_private_state *priv_state,
 		struct msm_display_topology topology,
 		bool test_only);
 
@@ -96,10 +96,10 @@ int dpu_rm_reserve(struct dpu_rm *rm,
  * dpu_rm_reserve - Given the encoder for the display chain, release any
  *	HW blocks previously reserved for that use case.
  * @rm: DPU Resource Manager handle
- * @enc: DRM Encoder handle
+ * @priv_state: Pointer to drm private obj state
  * @Return: 0 on Success otherwise -ERROR
  */
-void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc);
+void dpu_rm_release(struct dpu_rm *rm, struct drm_private_state *priv_state);
 
 /**
  * dpu_rm_init_hw_iter - setup given iterator for new iteration over hw list
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 3/4] drm/msm/dpu: remove reserve in encoder mode_set

[Jeykumar Sankaran]

Now that we have dpu private state tracking the reserved
HW resources, we have access to them after atomic swap.
So avoid reserving the resources in mode_set.

changes in v2:
	- removal applied on private object based reservation

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index bd646c5..86c025b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -605,8 +605,6 @@ static int dpu_encoder_virt_atomic_check(
 	}
 
 	/*
-	 * Reserve dynamic resources now. Indicating AtomicTest phase
-	 *
 	 * Avoid reserving resources when mode set is pending. Topology
 	 * info may not be available to complete reservation.
 	 */
@@ -620,7 +618,7 @@ static int dpu_encoder_virt_atomic_check(
 					       to_dpu_crtc(crtc_state->crtc));
 
 		ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc,
-				     &dpu_priv_state->base, topology, true);
+				     &dpu_priv_state->base, topology, false);
 		dpu_enc->mode_set_complete = false;
 	}
 
@@ -966,10 +964,9 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 	struct drm_connector *conn = NULL, *conn_iter;
 	struct drm_crtc *drm_crtc;
 	struct dpu_crtc_state *dpu_cstate;
-	struct msm_display_topology topology;
 	struct dpu_crtc *dpu_crtc;
 	struct dpu_private_state *dpu_private_state;
-	int i, j, ret;
+	int i, j;
 
 	if (!drm_enc) {
 		DPU_ERROR("invalid encoder\n");
@@ -1001,18 +998,7 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 		if (drm_crtc->state->encoder_mask & drm_encoder_mask(drm_enc))
 			break;
 
-	topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
 	dpu_crtc = to_dpu_crtc(drm_crtc);
-
-	/* Reserve dynamic resources now. Indicating non-AtomicTest phase */
-	ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc, dpu_crtc->priv_obj.state,
-			     topology, false);
-	if (ret) {
-		DPU_ERROR_ENC(dpu_enc,
-			     "failed to reserve hw resources, %d\n", ret);
-		return;
-	}
-
 	dpu_cstate = to_dpu_crtc_state(drm_crtc->state);
 	dpu_private_state = container_of(dpu_crtc->priv_obj.state,
 					 struct dpu_private_state, base);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 4/4] drm/msm/dpu: remove mode_set_complete

[Jeykumar Sankaran]

This flag was introduced as a fix to notify modeset complete
when hw reservations were happening in both atomic_check
and atomic_commit paths. Now that we are reserving only in
atomic_check, we can get rid of this flag.

changes in v2:
	- none

Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 86c025b..beb9f86 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -170,7 +170,6 @@ enum dpu_enc_rc_states {
  *				clks and resources after IDLE_TIMEOUT time.
  * @vsync_event_work:		worker to handle vsync event for autorefresh
  * @topology:                   topology of the display
- * @mode_set_complete:          flag to indicate modeset completion
  * @idle_timeout:		idle timeout duration in milliseconds
  */
 struct dpu_encoder_virt {
@@ -208,7 +207,6 @@ struct dpu_encoder_virt {
 	struct delayed_work delayed_off_work;
 	struct kthread_work vsync_event_work;
 	struct msm_display_topology topology;
-	bool mode_set_complete;
 
 	u32 idle_timeout;
 };
@@ -604,12 +602,7 @@ static int dpu_encoder_virt_atomic_check(
 		}
 	}
 
-	/*
-	 * Avoid reserving resources when mode set is pending. Topology
-	 * info may not be available to complete reservation.
-	 */
-	if (!ret && drm_atomic_crtc_needs_modeset(crtc_state)
-			&& dpu_enc->mode_set_complete) {
+	if (!ret && drm_atomic_crtc_needs_modeset(crtc_state)) {
 		struct msm_display_topology topology;
 		struct dpu_private_state *dpu_priv_state;
 
@@ -619,7 +612,6 @@ static int dpu_encoder_virt_atomic_check(
 
 		ret = dpu_rm_reserve(&dpu_kms->rm, drm_enc,
 				     &dpu_priv_state->base, topology, false);
-		dpu_enc->mode_set_complete = false;
 	}
 
 	if (!ret)
@@ -1039,8 +1031,6 @@ static void dpu_encoder_virt_mode_set(struct drm_encoder *drm_enc,
 		if (phys->ops.mode_set)
 			phys->ops.mode_set(phys, mode, adj_mode);
 	}
-
-	dpu_enc->mode_set_complete = true;
 }
 
 static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno