[PATCH v3 12/13] drm/msm/dpu: add atomic private object to dpu kms
Sean Paul
sean at poorly.run
Wed Aug 15 15:06:08 UTC 2018
On Tue, Aug 14, 2018 at 05:38:31PM -0700, Jeykumar Sankaran wrote:
> On 2018-08-14 13:26, Sean Paul wrote:
> > 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 <jsanka at 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.
> >
> Now that resource manager is the only obj being tracked by the private
> state, only
> CRTC and Encoder are using this macro to retrieve the dpu_private_state
> objects.
> But going forward, when DPU starts supporting other value-added hw blocks
> (e.g. Post
> processing), we need to track more states. So I thought it would come in
> handy.
Ehh, let's not make it too easy to proliferate private state subclasses. Let's
use strongly typed static inlines instead.
Sean
>
> Jeykumar S.
>
> > > +
> > > +/**
> > > + * 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
> > >
>
> --
> Jeykumar S
--
Sean Paul, Software Engineer, Google / Chromium OS
More information about the dri-devel
mailing list