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(-)
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)
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
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
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
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
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);
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)
dri-devel@lists.freedesktop.org