Re: [Freedreno] [DPU PATCH 1/4] drm/msm/dpu: add atomic private object to dpu kms
On 2018-06-13 09:29, Jordan Crouse wrote:
On Tue, Jun 12, 2018 at 06:17:44PM -0700, Jeykumar Sankaran wrote:
Subclass drm private state for DPU for handling driver
specific data. Adds atomic private object and private object
lock to dpu kms. Provides helper function to retrieve DPU
private data from current atomic state.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66
+
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15
2 files changed, 81 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index fe614c0..a4ab783 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1076,6 +1076,10 @@ static void dpu_kms_destroy(struct msm_kms
*kms)
dpu_kms = to_dpu_kms(kms);
_dpu_kms_hw_destroy(dpu_kms);
+
+ drm_atomic_private_obj_fini(_kms->priv_obj);
+ drm_modeset_lock_fini(_kms->priv_obj_lock);
+
}
static void dpu_kms_preclose(struct msm_kms *kms, struct drm_file
*file)
@@ -1618,10 +1622,59 @@ static int dpu_kms_hw_init(struct msm_kms
*kms)
return rc;
}
+struct dpu_private_state *dpu_get_private_state(struct
drm_atomic_state
*state)
+{
+ struct msm_drm_private *priv = state->dev->dev_private;
+ struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
+ struct drm_private_state *priv_state;
+ int rc = 0;
+
+ rc = drm_modeset_lock(_kms->priv_obj_lock,
state->acquire_ctx);
+ if (rc)
+ return ERR_PTR(rc);
+
+ priv_state = drm_atomic_get_private_obj_state(state,
+ _kms->priv_obj);
+ if (!priv_state)
+ return NULL;
I'll have to see later when this function is used, but I generally
don't
like it
when functions return both ERR_PTR and NULL on error but I'm leaving
open
the
possibility that this could NULL for legitimate reasons. If not, please
convert
to a ERR_PTR.
I see drm_atomic_get_private_obj_state returns ERR_PTR(-ENOMEM) on error
cases. So I should
be good with IS_ERR/ERR_PTR.
+ return to_dpu_private_state(priv_state);
+}
+
+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,
+ _priv_state->base);
+
+ return _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 =
to_dpu_private_state(state);
+
+ 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,
+};
+
struct msm_kms *dpu_kms_init(struct drm_device *dev)
{
struct msm_drm_private *priv;
struct dpu_kms *dpu_kms;
+ struct dpu_private_state *dpu_priv_state;
int irq;
if (!dev || !dev->dev_private) {
@@ -1639,6 +1692,19 @@ struct msm_kms *dpu_kms_init(struct drm_device
*dev)
}
dpu_kms->base.irq = irq;
+ /* Initialize private obj's */
+ drm_modeset_lock_init(_kms->priv_obj_lock);
+
+ dpu_priv_state = kzalloc(sizeof(*dpu_priv_state), GFP_KERNEL);
+ if (!dpu_priv_state) {
+ DPU_ERROR("failed to allocate dpu priv obj\n");
We don't need an error message on memory failure - you will have no
problem
identifying when this went boom if it goes boom.
Will remove
+ return ERR_PTR(-ENOMEM);
+ }
+
+ drm_atomic_private_obj_init(_kms->priv_obj,
+ _priv_state->base,
+ _obj_funcs);
+
return _kms->base;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 046e6f7..924d8967 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -190,6 +190,9 @@ struct dpu_kms {
struct dpu_hw_vbif *hw_vbif[VBIF_MAX];
struct dpu_hw_mdp *hw_mdp;
+ struct drm_modeset_lock priv_obj_lock;
+ struct drm_private_obj priv_obj;
+
bool has_danger_ctrl;
struct platform_device *pdev;
@@ -197,12 +200,24 @@ 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;
};
#define to_dpu_kms(x) container_of(x, struct dpu_kms, base)
+#define to_dpu_private_state(x) container_of(x, struct
dpu_private_state, base)
+
Re: [Freedreno] [DPU PATCH 1/4] drm/msm/dpu: add atomic private object to dpu kms
On Tue, Jun 12, 2018 at 06:17:44PM -0700, Jeykumar Sankaran wrote:
> Subclass drm private state for DPU for handling driver
> specific data. Adds atomic private object and private object
> lock to dpu kms. Provides helper function to retrieve DPU
> private data from current atomic state.
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66
> +
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15
> 2 files changed, 81 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index fe614c0..a4ab783 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1076,6 +1076,10 @@ static void dpu_kms_destroy(struct msm_kms *kms)
>
> dpu_kms = to_dpu_kms(kms);
> _dpu_kms_hw_destroy(dpu_kms);
> +
> + drm_atomic_private_obj_fini(_kms->priv_obj);
> + drm_modeset_lock_fini(_kms->priv_obj_lock);
> +
> }
>
> static void dpu_kms_preclose(struct msm_kms *kms, struct drm_file *file)
> @@ -1618,10 +1622,59 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
> return rc;
> }
>
> +struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state
> *state)
> +{
> + struct msm_drm_private *priv = state->dev->dev_private;
> + struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
> + struct drm_private_state *priv_state;
> + int rc = 0;
> +
> + rc = drm_modeset_lock(_kms->priv_obj_lock, state->acquire_ctx);
> + if (rc)
> + return ERR_PTR(rc);
> +
> + priv_state = drm_atomic_get_private_obj_state(state,
> + _kms->priv_obj);
> + if (!priv_state)
> + return NULL;
I'll have to see later when this function is used, but I generally don't like it
when functions return both ERR_PTR and NULL on error but I'm leaving open the
possibility that this could NULL for legitimate reasons. If not, please convert
to a ERR_PTR.
> + return to_dpu_private_state(priv_state);
> +}
> +
> +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,
> + _priv_state->base);
> +
> + return _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 = to_dpu_private_state(state);
> +
> + 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,
> +};
> +
> struct msm_kms *dpu_kms_init(struct drm_device *dev)
> {
> struct msm_drm_private *priv;
> struct dpu_kms *dpu_kms;
> + struct dpu_private_state *dpu_priv_state;
> int irq;
>
> if (!dev || !dev->dev_private) {
> @@ -1639,6 +1692,19 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev)
> }
> dpu_kms->base.irq = irq;
>
> + /* Initialize private obj's */
> + drm_modeset_lock_init(_kms->priv_obj_lock);
> +
> + dpu_priv_state = kzalloc(sizeof(*dpu_priv_state), GFP_KERNEL);
> + if (!dpu_priv_state) {
> + DPU_ERROR("failed to allocate dpu priv obj\n");
We don't need an error message on memory failure - you will have no problem
identifying when this went boom if it goes boom.
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + drm_atomic_private_obj_init(_kms->priv_obj,
> + _priv_state->base,
> + _obj_funcs);
> +
> return _kms->base;
> }
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> index 046e6f7..924d8967 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -190,6 +190,9 @@ struct dpu_kms {
> struct dpu_hw_vbif *hw_vbif[VBIF_MAX];
> struct dpu_hw_mdp *hw_mdp;
>
> + struct drm_modeset_lock priv_obj_lock;
> + struct drm_private_obj priv_obj;
> +
> bool has_danger_ctrl;
>
> struct platform_device *pdev;
> @@ -197,12 +200,24 @@ 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;
> };
>
> #define to_dpu_kms(x) container_of(x, struct dpu_kms, base)
> +#define to_dpu_private_state(x) container_of(x, struct dpu_private_state,
> base)
> +
> +/**
> + * dpu_get_private_state -
[DPU PATCH 1/4] drm/msm/dpu: add atomic private object to dpu kms
Subclass drm private state for DPU for handling driver
specific data. Adds atomic private object and private object
lock to dpu kms. Provides helper function to retrieve DPU
private data from current atomic state.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 66 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15
2 files changed, 81 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index fe614c0..a4ab783 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1076,6 +1076,10 @@ static void dpu_kms_destroy(struct msm_kms *kms)
dpu_kms = to_dpu_kms(kms);
_dpu_kms_hw_destroy(dpu_kms);
+
+ drm_atomic_private_obj_fini(_kms->priv_obj);
+ drm_modeset_lock_fini(_kms->priv_obj_lock);
+
}
static void dpu_kms_preclose(struct msm_kms *kms, struct drm_file *file)
@@ -1618,10 +1622,59 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
return rc;
}
+struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state *state)
+{
+ struct msm_drm_private *priv = state->dev->dev_private;
+ struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms);
+ struct drm_private_state *priv_state;
+ int rc = 0;
+
+ rc = drm_modeset_lock(_kms->priv_obj_lock, state->acquire_ctx);
+ if (rc)
+ return ERR_PTR(rc);
+
+ priv_state = drm_atomic_get_private_obj_state(state,
+ _kms->priv_obj);
+ if (!priv_state)
+ return NULL;
+
+ return to_dpu_private_state(priv_state);
+}
+
+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,
+ _priv_state->base);
+
+ return _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 = to_dpu_private_state(state);
+
+ 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,
+};
+
struct msm_kms *dpu_kms_init(struct drm_device *dev)
{
struct msm_drm_private *priv;
struct dpu_kms *dpu_kms;
+ struct dpu_private_state *dpu_priv_state;
int irq;
if (!dev || !dev->dev_private) {
@@ -1639,6 +1692,19 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev)
}
dpu_kms->base.irq = irq;
+ /* Initialize private obj's */
+ drm_modeset_lock_init(_kms->priv_obj_lock);
+
+ dpu_priv_state = kzalloc(sizeof(*dpu_priv_state), GFP_KERNEL);
+ if (!dpu_priv_state) {
+ DPU_ERROR("failed to allocate dpu priv obj\n");
+ return ERR_PTR(-ENOMEM);
+ }
+
+ drm_atomic_private_obj_init(_kms->priv_obj,
+ _priv_state->base,
+ _obj_funcs);
+
return _kms->base;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 046e6f7..924d8967 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -190,6 +190,9 @@ struct dpu_kms {
struct dpu_hw_vbif *hw_vbif[VBIF_MAX];
struct dpu_hw_mdp *hw_mdp;
+ struct drm_modeset_lock priv_obj_lock;
+ struct drm_private_obj priv_obj;
+
bool has_danger_ctrl;
struct platform_device *pdev;
@@ -197,12 +200,24 @@ 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;
};
#define to_dpu_kms(x) container_of(x, struct dpu_kms, base)
+#define to_dpu_private_state(x) container_of(x, struct dpu_private_state, base)
+
+/**
+ * dpu_get_private_state - get dpu private state from atomic state
+ * @state: drm atomic state
+ * Return: pointer to dpu private state object
+ */
+struct dpu_private_state *dpu_get_private_state(struct drm_atomic_state
*state);
/**
* dpu_is_custom_client - whether or not to enable non-standard customizations
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
