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

Sean Paul
2019-03-04T21:32:41Z

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