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 <jsa...@codeaurora.org>
> ---
>  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(&dpu_kms->priv_obj);
> +     drm_modeset_lock_fini(&dpu_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(&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 (!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,
> +                     &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,
> +};
> +
>  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(&dpu_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(&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 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 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

Reply via email to