On Fri, Nov 30, 2012 at 04:44:04PM +0100, Daniel Vetter wrote: > On Fri, Nov 30, 2012 at 3:01 PM, Ville Syrj?l? > <ville.syrjala at linux.intel.com> wrote: > > I had another look at this, and I think you're aiming for something other > > than what this patch is trying to do. > > Oh, I know that I'm trying to volunteer you to do a bit more ... it's > why I'm the bastard maintainer ;-) > > > The thing is that we are holding struct_mutex while waiting for pending > > flips here, so we just need to get out asap to allow the reset work do > > its thing. > > Hm, I didn't notice that we're holding the struct_mutex there, so I > guess we need to indeed bail out. Or rework the finsh_fb vs. gpu hang > logic to not require the struct_mutex in finish_fb ... > > > Completing pending page flips when a reset occurs is separate issue. > > This code never did any of that, and I don't see why it should. We > > would need to complete them anyway regardless of whether anyone is > > currently waiting for them. > > Yeah, that's my idea, but I haven't though through the details (see my > ignorance with locking details above ...). > > > Perhaps we can just call intel_finish_page_flip() from the reset > > code and call it a day. I suppose doing that could end up unpinning > > the current scanout buffer in case the display registers were never > > written with the new values. One solution for that would be to always > > do a set_base() after a reset. That would make sure we end up showing > > the latest buffer at least, although the contents could be total > > garbage. > > Hm, I think first we should aim to no longer hang either the kernel or > leave stuck userspace (since the drm_event never shows up) behind. Atm > a gpu hang is allowed to corrupt pretty much everything. Later on, > when we successfully avoid the hung kernel/userspace problems we can > worry about making it look decent. Judging by how long it took to get > basic reset working somewhat okish, it'll take a while. And we need > some form of testcase to exercise the code. At least the current patch would avoid having the process stuck in D state. So I think it's better than nothing. I do have other things on my plate, so I don't want to spend any more time on this currently. -- Ville Syrj?l? Intel OTC
