[Freedreno] [DPU PATCH 1/4] drm/msm/dpu: add atomic private object to dpu kms
Jeykumar Sankaran
jsanka at codeaurora.org
Wed Jun 13 18:25:42 UTC 2018
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 <jsanka at 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.
>
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,
>> + &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.
>
Will remove
>> + 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
>> --
--
Jeykumar S
More information about the dri-devel
mailing list