Op 18-05-15 om 17:45 schreef Daniel Vetter:
> On Wed, May 13, 2015 at 10:23:41PM +0200, Maarten Lankhorst wrote:
>> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@xxxxxxxxx>
>>
>> Now that we can subclass drm_atomic_state we can also use it to keep
>> track of all the pll settings. atomic_state is a better place to hold
>> all shared state than keeping pll->new_config everywhere.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 1 -
>> drivers/gpu/drm/i915/intel_atomic.c | 49 ++++++++++++++++
>> drivers/gpu/drm/i915/intel_display.c | 111 +++++++++++------------------------
>> drivers/gpu/drm/i915/intel_drv.h | 13 ++++
>> 4 files changed, 95 insertions(+), 79 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index a0e4868653f2..0e200018c9aa 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -319,7 +319,6 @@ struct intel_shared_dpll_config {
>>
>> struct intel_shared_dpll {
>> struct intel_shared_dpll_config config;
>> - struct intel_shared_dpll_config *new_config;
>>
>> int active; /* count of number of active CRTCs (i.e. DPMS on) */
>> bool on; /* is the PLL actually active? Disabled during modeset */
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>> index 7ed8033aae60..aff87054406c 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -421,3 +421,52 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
>>
>> return 0;
>> }
>> +
>> +static void intel_atomic_duplicate_dpll_state(struct drm_device *dev,
>> + struct intel_atomic_state *state)
>> +{
>> + struct drm_i915_private *dev_priv = to_i915(dev);
>> + struct intel_shared_dpll *pll;
>> + enum intel_dpll_id i;
>> +
>> + state->dpll_set = true;
> The ww mutex locking dance here is missing. For simplicity I think we
> could just repurpose the dev->mode_config.connection_mutex. We need that
> anyway full modesets, and dpll changes should only be required in those.
> Which means this wont introduce any unecessary parallelism.
>
> It means though that we need to wire up a proper error code to all callers
> of duplicate/get_dpll_state, like with all the other atomic states. Might
> be best to follow the design for connector/crtc/planes an pass around
> pointers with error codes explicitly (instead of returning
> state->shared_dpll below since that can only cope with NULL and not with
> -EDEADLK).
>
> Sorry that I didn't spot this earlier.
>
The dpll state should only ever be done during a modeset in which case the connection_mutex is guaranteed to be held.
Instead of making this return a value can we add a lockdep_assert_held ?
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
