Re: [PATCH RFC 0/4] drm/core: restore suspend/resume calbacks in KMS drm drivers
On Fri, Oct 03, 2014 at 10:24:09AM +0200, Andrzej Hajda wrote: The main intent of this patchset is to allow use of suspend/resume drm driver callbacks in KMS drivers, as these callbacks seems to me the best place to implement suspend/resume functionality in drm driver. Implementing this functionality in master component driver PM ops is problematic as those callbacks can be called asynchronously regardless of state/existence of drm device, thus it would require additional synchronization mechanism. Callbacks re-enabling requires small changes in i915 and exynos driver. The patchset contains also fix of exynos resume callback. Nack. Like completely and totally. The drm core has really no business doing hardware stuff, which includes runtime pm, system suspend and all that nonsense. It' an interface between userspace and drivers, with a big library to back it all up. Everything else just repeats the old midlayer mistake. If you driver needs this, do it there. Also, the component framework is probably the solution you're looking for. And if there are synchronization issues with that then we need to fix those instead of reinventing yet another half-assed broken wheel. Aside: With David Herrmann's latest patches to de-midlayer the drm init/teardown sequence the driver is in full control of when the drm data structures get allocate, initialized and registered. If you convert to that plus the component framework I'm pretty sure your problem is solved. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 0/4] drm/core: restore suspend/resume calbacks in KMS drm drivers
On 10/03/2014 10:31 AM, Daniel Vetter wrote: On Fri, Oct 03, 2014 at 10:24:09AM +0200, Andrzej Hajda wrote: The main intent of this patchset is to allow use of suspend/resume drm driver callbacks in KMS drivers, as these callbacks seems to me the best place to implement suspend/resume functionality in drm driver. Implementing this functionality in master component driver PM ops is problematic as those callbacks can be called asynchronously regardless of state/existence of drm device, thus it would require additional synchronization mechanism. Callbacks re-enabling requires small changes in i915 and exynos driver. The patchset contains also fix of exynos resume callback. Nack. Like completely and totally. The drm core has really no business doing hardware stuff, which includes runtime pm, system suspend and all that nonsense. It' an interface between userspace and drivers, with a big library to back it all up. Everything else just repeats the old midlayer mistake. Hmm, I have just tried to reuse the existing infrastructure, I did not see any sign do not touch, this is a mistake. Now I see it, thanks :) If you driver needs this, do it there. Also, the component framework is probably the solution you're looking for. And if there are synchronization issues with that then we need to fix those instead of reinventing yet another half-assed broken wheel. But this is an issue closely connected with component framework. Component framework separates master component probe and drm device initialization. As a result PM ops which are synchronized with probe (via device_lock) are no more synchronized with drm initialization which is usually called from .bind callback. Aside: With David Herrmann's latest patches to de-midlayer the drm init/teardown sequence the driver is in full control of when the drm data structures get allocate, initialized and registered. If you convert to that plus the component framework I'm pretty sure your problem is solved. I will look closer at it but as I described above it is rather matter of separation of master component and drm device initialization. My idea was to avoid creation of new synchronization mechanism and to reuse the existing ones which seems to fit perfectly to the scenario, but if there is big NO for it another solution should be found. Anyway I guess the problem exists for all drivers having component framework and suspend: exynos, msm and incoming rockchip. Regards Andrzej Thanks, Daniel -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 0/4] drm/core: restore suspend/resume calbacks in KMS drm drivers
On Fri, Oct 3, 2014 at 11:42 AM, Andrzej Hajda a.ha...@samsung.com wrote: On 10/03/2014 10:31 AM, Daniel Vetter wrote: On Fri, Oct 03, 2014 at 10:24:09AM +0200, Andrzej Hajda wrote: The main intent of this patchset is to allow use of suspend/resume drm driver callbacks in KMS drivers, as these callbacks seems to me the best place to implement suspend/resume functionality in drm driver. Implementing this functionality in master component driver PM ops is problematic as those callbacks can be called asynchronously regardless of state/existence of drm device, thus it would require additional synchronization mechanism. Callbacks re-enabling requires small changes in i915 and exynos driver. The patchset contains also fix of exynos resume callback. Nack. Like completely and totally. The drm core has really no business doing hardware stuff, which includes runtime pm, system suspend and all that nonsense. It' an interface between userspace and drivers, with a big library to back it all up. Everything else just repeats the old midlayer mistake. Hmm, I have just tried to reuse the existing infrastructure, I did not see any sign do not touch, this is a mistake. Now I see it, thanks :) As a rule of thumb, if you see a !DRIVER_MODESET check anywhere, then that's a clear sign that you're wandering off the light and into the dangerous parts of what drm was like age dark ages ;-) If you driver needs this, do it there. Also, the component framework is probably the solution you're looking for. And if there are synchronization issues with that then we need to fix those instead of reinventing yet another half-assed broken wheel. But this is an issue closely connected with component framework. Component framework separates master component probe and drm device initialization. As a result PM ops which are synchronized with probe (via device_lock) are no more synchronized with drm initialization which is usually called from .bind callback. Now I'm confused. component-bind and drm initialization is about driver load, but all this code here is about system suspend/resume really. So I'm rather confused what problem you actually want to fix .. Aside: With David Herrmann's latest patches to de-midlayer the drm init/teardown sequence the driver is in full control of when the drm data structures get allocate, initialized and registered. If you convert to that plus the component framework I'm pretty sure your problem is solved. I will look closer at it but as I described above it is rather matter of separation of master component and drm device initialization. My idea was to avoid creation of new synchronization mechanism and to reuse the existing ones which seems to fit perfectly to the scenario, but if there is big NO for it another solution should be found. Anyway I guess the problem exists for all drivers having component framework and suspend: exynos, msm and incoming rockchip. It sounds like the component framework needs to be teached to work with system suspend/resume. I guess either we need to have clever abuse of deferred probing to make sure that the master device is probed first and so in the correct place in the system/resume sequence. Or the master framework needs to grow pm_ops itself so that the state associated with the logical componentized framework can be saved/restored. So master pm_ops would save/restore kms state, and all the components would just save/restore any additional (register, clock, whatever) state that each componnent driver would need. But I'm not really an expert here, so adding more people. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 0/4] drm/core: restore suspend/resume calbacks in KMS drm drivers
On Fri, Oct 03, 2014 at 01:39:21PM +0200, Daniel Vetter wrote: On Fri, Oct 3, 2014 at 11:42 AM, Andrzej Hajda a.ha...@samsung.com wrote: But this is an issue closely connected with component framework. Component framework separates master component probe and drm device initialization. As a result PM ops which are synchronized with probe (via device_lock) are no more synchronized with drm initialization which is usually called from .bind callback. Now I'm confused. component-bind and drm initialization is about driver load, but all this code here is about system suspend/resume really. So I'm rather confused what problem you actually want to fix .. The component *helper* will disconnect the bind of the master device from its probe if the components aren't already known. If all the components are known at the point the master device probes, the master device lock will be held (since we'll be operating within the master device's probe function.) The issue here is that while the master device is being probed, the per-device lock is held. Beneath that lock there are locks in the component helper which will be taken, and thus will end up depending upon the per-device lock. This means that we pretty much can not guarantee synchronisation between PM on the master device and the component helper calling the bind callback - if we tried to take the per-device lock here, we would either deadlock, or we would end up with AB-BA lock dependencies. What we /could/ do is expose lock/unlock functions from the component helper so that PM implementations can synchronise with binds - or I could look at whether we could integrate the component helper into the device model. I suspect the latter would not be met with enthusiasm as it would mean adding bloat to the driver core structures, which would not be needed in 99% of cases. -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
