On 2018-07-05 12:09 PM, Andrey Grodzovsky wrote: > On 07/05/2018 12:06 PM, Greg KH wrote: >> On Thu, Jul 05, 2018 at 12:05:09PM -0400, Andrey Grodzovsky wrote: >>> This patch is wrong as noted by MIchel a while ago - quote from his review >>> of the patch. >>> >>> "Actually, pflip_status should really only be set to AMDGPU_FLIP_SUBMITTED >>> after the flip has been programmed to the hardware, at least as far as the >>> lock holder is concerned. That's why the code was previously holding the >>> lock until after the dc_commit_updates_for_stream call. Otherwise, it's at >>> least theoretically possible that either: >>> >>> * dm_pflip_high_irq is called before dc_commit_updates_for_stream, but sees >>> flip_status == AMDGPU_FLIP_SUBMITTED and sends the event to userspace >>> prematurely >>> >>> * dm_pflip_high_irq is called after dc_commit_updates_for_stream, but sees >>> flip_status != AMDGPU_FLIP_SUBMITTED, so it incorrectly doesn't send the >>> event to userspace " >>> >>> It shouldn't go in. >> Is there a fix for this in Linus's tree for these problems? >> If not, why not? If so, what is that git commit id? > > AFAIK there is still no fix for the original issue which this patch was intended to fix. > > Harry, can you please comment on this ? > That can be better answered by Shirish. In my opinion the issue described by Michel is a lesser evil than attempting to allocate memory inside a spinlocked area. Shirish, we need to understand why dc_commit_updates_for_stream() is trying to do a UPDATE_TYPE_FULL in your scenario. This should never really happen through the amdgpu_dm_do_flip code path and we need to understand why. If we never do UPDATE_TYPE_FULL here we should not need to allocate memory and should be find with leaving the spinlock around dc_commit_updates_for_stream(). Harry > Andrey > >> >> thanks, >> >> greg k-h >
