Re: [PATCH 4/6] drm/i915: Make intel_get_pipe_from_connector atomic

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Op 20-12-16 om 14:49 schreef Daniel Vetter:
> On Tue, Dec 20, 2016 at 02:35:40PM +0100, Maarten Lankhorst wrote:
>> Op 19-12-16 om 09:24 schreef Daniel Vetter:
>>> Drive-by fixup while looking at all the connector_list walkers -
>>> holding connection_mutex does actually _not_ give you locking to look
>>> at the legacy drm_connector->encoder->crtc pointer chain. That one is
>>> solely owned by the atomic commit workers. Instead we must inspect the
>>> atomic state.
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_display.c | 5 ++---
>>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 0b0d7e8be630..438d27f93aca 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -15427,15 +15427,14 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>>>  
>>>  enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
>>>  {
>>> -	struct drm_encoder *encoder = connector->base.encoder;
>>>  	struct drm_device *dev = connector->base.dev;
>>>  
>>>  	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>>>  
>>> -	if (!encoder || WARN_ON(!encoder->crtc))
>>> +	if (!connector->base.state->crtc)
>>>  		return INVALID_PIPE;
>>>  
>>> -	return to_intel_crtc(encoder->crtc)->pipe;
>>> +	return to_intel_crtc(connector->base.state->crtc)->pipe;
>>>  }
>>>  
>>>  int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
>> This patch clashes with the previous patch. intel_panel_set_backlight_acpi is called
>> from asle_set_backlight. You should probably keep connection_mutex alive for it.
> Oh, I've forgotten to update the commit message for the previous patch - I
> ended up _not_ nuking the locking because it did blow up here ;-)
>
> r-b on both if I fix up the previous commit message that we can't nuke the
> locking because the asle_set_backlight thing needs it for the backlight
> callbacks?
> -Daniel

Yeah ok. I'd rather we pass the state, but this will do for now.


Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx