On Fri, Oct 3, 2014 at 11:42 AM, Andrzej Hajda <a.hajda@xxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |