Re: [RFC PATCH 07/12] drm/i915/dsi: switch to drm_panel interface

[Shobhit Kumar <shobhit.kumar@linux.intel.com>]

On 01/23/2015 09:01 PM, Daniel Vetter wrote:
> On Fri, Jan 23, 2015 at 04:27:46PM +0530, Shobhit Kumar wrote:
>> On 01/16/2015 05:57 PM, Jani Nikula wrote:
>>> @@ -881,13 +889,23 @@ void intel_dsi_init(struct drm_device *dev)
>>>
>>>   	drm_connector_register(connector);
>>>
>>> -	fixed_mode = dsi->dev_ops->get_modes(&intel_dsi->dev);
>>> +	drm_panel_attach(intel_dsi->panel, connector);
>>> +	drm_panel_get_modes(intel_dsi->panel);
>>
>> Should be inside the config mutex_lock below.
>
> Why?

Because usually the drm_mode_probed_add gets called in the drm flow with 
correct locking, but in this case calling get_modes here results in 
drm_mode_probed_add being called which expects the lock to be taken. 
Else we will see a big WARN dump. I agree with your comment below but 
this was mainly to avoid the warning dump.

>
>>> +
>>> +	mutex_lock(&dev->mode_config.mutex);
>>> +	list_for_each_entry(scan, &connector->probed_modes, head) {
>>> +		if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
>>> +			fixed_mode = drm_mode_duplicate(dev, scan);
>>> +			break;
>>> +		}
>>> +	}
>>> +	mutex_unlock(&dev->mode_config.mutex);
>
> Generally this is single-threaded init code, no one else can touch it.
> Which means you also never ever need locks. We tend to occasionally
> sprinkle them around to satisfy general locking checks which are correct
> at runtime. But at least in tricky cases that means we also stick a
> comment next to them (see e.g. the various places in i915_irq.c).
>
> Merged up to the previous patch, thanks a lot.
> -Daniel
>

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