On Tue, Aug 07, 2018 at 08:20:10PM -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.
>
> changes in v2:
> - none
> changes in v3:
> - rebase on [1]
>
> [1] https://gitlab.freedesktop.org/seanpaul/dpu-staging/commits/for-next
>
> Change-Id: Iaab32badff224ffed024e6ef6576efc8b3af3aec
> Signed-off-by: Jeykumar Sankaran <jsa...@codeaurora.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 61
> +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 15 ++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 7dd6bd2..5e87b9d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -1168,10 +1168,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(&dpu_kms->priv_obj_lock, state->acquire_ctx);
> + if (rc)
> + return ERR_PTR(rc);
> +
> + priv_state = drm_atomic_get_private_obj_state(state,
> + &dpu_kms->priv_obj);
> + if (IS_ERR(priv_state))
> + return ERR_PTR(-ENOMEM);
> +
> + 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,
> + &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 = 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,
> +};
> +
All of this copypasta between mdp5 and dpu is pretty icky. Can we do a better
job of sharing code? Perhaps some helpers in msm_atomic to help manage the
priv_obj?
> 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) {
> @@ -1189,6 +1238,18 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev)
> }
> dpu_kms->base.irq = irq;
>
> + /* Initialize private obj's */
> + drm_modeset_lock_init(&dpu_kms->priv_obj_lock);
> +
> + dpu_priv_state = kzalloc(sizeof(*dpu_priv_state), GFP_KERNEL);
> + if (!dpu_priv_state)
> + return ERR_PTR(-ENOMEM);
> +
> +
> + drm_atomic_private_obj_init(&dpu_kms->priv_obj,
> + &dpu_priv_state->base,
> + &priv_obj_funcs);
> +
> return &dpu_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 66d4666..2579c983 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> @@ -145,6 +145,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;
> @@ -152,12 +155,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)
Do we really need this? It seems like we shouldn't have _that_ many structs
containing dpu_private_state that we need the generic macro.
> +
> +/**
> + * 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);
>
> /* get struct msm_kms * from drm_device * */
> #define ddev_to_msm_kms(D) ((D) && (D)->dev_private ? \
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
