[Intel-gfx] [PATCH v3 4/8] drm: Add driver-private objects to atomic state
Archit Taneja
architt at codeaurora.org
Fri Feb 17 10:07:48 UTC 2017
On 02/16/2017 05:43 AM, Pandiyan, Dhinakaran wrote:
> On Wed, 2017-02-15 at 16:53 +0530, Archit Taneja wrote:
>> Hi,
>>
>> On 02/09/2017 12:08 PM, Dhinakaran Pandiyan wrote:
>>> It is necessary to track states for objects other than connector, crtc
>>> and plane for atomic modesets. But adding objects like DP MST link
>>> bandwidth to drm_atomic_state would mean that a non-core object will be
>>> modified by the core helper functions for swapping and clearing
>>> it's state. So, lets add void * objects and helper functions that operate
>>> on void * types to keep these objects and states private to the core.
>>> Drivers can then implement specific functions to swap and clear states.
>>> The other advantage having just void * for these objects in
>>> drm_atomic_state is that objects of different types can be managed in the
>>> same state array.
>>>
>>> v2: Added docs and new iterator to filter private objects (Daniel)
>>>
>>> Suggested-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
>>> ---
>>> drivers/gpu/drm/drm_atomic.c | 68 +++++++++++++++++++++++++++
>>> drivers/gpu/drm/drm_atomic_helper.c | 5 ++
>>> include/drm/drm_atomic.h | 91 +++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 164 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>> index a567310..1a9ffe8 100644
>>> --- a/drivers/gpu/drm/drm_atomic.c
>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>> @@ -57,6 +57,7 @@ void drm_atomic_state_default_release(struct drm_atomic_state *state)
>>> kfree(state->connectors);
>>> kfree(state->crtcs);
>>> kfree(state->planes);
>>> + kfree(state->private_objs);
>>> }
>>> EXPORT_SYMBOL(drm_atomic_state_default_release);
>>>
>>> @@ -184,6 +185,20 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
>>> state->planes[i].ptr = NULL;
>>> state->planes[i].state = NULL;
>>> }
>>> +
>>> + for (i = 0; i < state->num_private_objs; i++) {
>>> + void *private_obj = state->private_objs[i].obj;
>>> + void *obj_state = state->private_objs[i].obj_state;
>>> +
>>> + if (!private_obj)
>>> + continue;
>>> +
>>> + state->private_objs[i].funcs->destroy_state(obj_state);
>>> + state->private_objs[i].obj = NULL;
>>> + state->private_objs[i].obj_state = NULL;
>>> + state->private_objs[i].funcs = NULL;
>>> + }
>>> +
>>> }
>>> EXPORT_SYMBOL(drm_atomic_state_default_clear);
>>>
>>> @@ -974,6 +989,59 @@ static void drm_atomic_plane_print_state(struct drm_printer *p,
>>> }
>>>
>>> /**
>>> + * drm_atomic_get_private_obj_state - get private object state
>>> + * @state: global atomic state
>>> + * @obj: private object to get the state for
>>> + * @funcs: pointer to the struct of function pointers that identify the object
>>> + * type
>>> + *
>>> + * This function returns the private object state for the given private object,
>>> + * allocating the state if needed. It does not grab any locks as the caller is
>>> + * expected to care of any required locking.
>>> + *
>>> + * RETURNS:
>>> + *
>>> + * Either the allocated state or the error code encoded into a pointer.
>>> + */
>>> +void *
>>> +drm_atomic_get_private_obj_state(struct drm_atomic_state *state, void *obj,
>>> + const struct drm_private_state_funcs *funcs)
>>> +{
>>> + int index, num_objs, i;
>>> + size_t size;
>>> + struct __drm_private_objs_state *arr;
>>> +
>>> + for (i = 0; i < state->num_private_objs; i++)
>>> + if (obj == state->private_objs[i].obj &&
>>> + state->private_objs[i].obj_state)
>>> + return state->private_objs[i].obj_state;
>>
>> Comparing this func to drm_atomic_get_plane_state/drm_atomic_get_crtc_state, it
>> doesn't seem to call drm_modeset_lock if the obj_state doesn't already exist. I
>> don't understand the locking stuff toowell, I just noticed this difference when
>> comparing this approach with what is done in the msm kms driver (where we
>> have subclassed drm_atomic_state to msm_kms_state).
>>
>> Thanks,
>> Archit
>>
>
>
> The caller is expected to take care of any required locking. The
> driver-private objects are opaque from core's pov, so the core is not
> aware of necessary locks for that object type.
I had a look at the rest of the series, and I couldn't easily understand
whether the caller code protects the MST related driver private state. Is
it expected to be protect via the drm_mode_config.connection_mutex lock?
Thanks,
Archit
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
More information about the Intel-gfx
mailing list