[Intel-gfx] [PATCH v5 09/35] drm/i915: Force MMIO flips when scheduler enabled

[John Harrison]

So is the summary that currently MMIO flips do not work on some platforms?

That could be a problem because the scheduler is intended to be enabled 
for everything and thus will be forcing MMIO flips on everything. So 
should I reverse the logic here - only enable the scheduler if MMIO 
flips are already enabled?

Thanks,
John.

On 22/02/2016 09:41, Lankhorst, Maarten wrote:
> Hey,
>
>
> Jesse Barnes schreef op vr 19-02-2016 om 12:01 [-0800]:
>> On 02/19/2016 11:53 AM, Ville Syrjälä wrote:
>>> On Fri, Feb 19, 2016 at 11:28:05AM -0800, Jesse Barnes wrote:
>>>> On 02/18/2016 06:26 AM, John.C.Harrison at Intel.com wrote:
>>>>> From: John Harrison <John.C.Harrison at Intel.com>
>>>>>
>>>>> MMIO flips are the preferred mechanism now but more importantly, pipe
>>>>> based flips cause issues for the scheduler. Specifically, submitting
>>>>> work to the rings around the side of the scheduler could cause that
>>>>> work to be lost if the scheduler generates a pre-emption event on that
>>>>> ring.
>>>>>
>>>>> For: VIZ-1587
>>>>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/intel_display.c | 3 +++
>>>>>   1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>>> index 6e12ed7..731d20a 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>>> @@ -46,6 +46,7 @@
>>>>>   #include <linux/dma_remapping.h>
>>>>>   #include <linux/reservation.h>
>>>>>   #include <linux/dma-buf.h>
>>>>> +#include "i915_scheduler.h"
>>>>>   
>>>>>   /* Primary plane formats for gen <= 3 */
>>>>>   static const uint32_t i8xx_primary_formats[] = {
>>>>> @@ -11330,6 +11331,8 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
>>>>>   		return true;
>>>>>   	else if (i915.enable_execlists)
>>>>>   		return true;
>>>>> +	else if (i915_scheduler_is_enabled(ring->dev))
>>>>> +		return true;
>>>>>   	else if (obj->base.dma_buf &&
>>>>>   		 !reservation_object_test_signaled_rcu(obj->base.dma_buf->resv,
>>>>>   						       false))
>>>>>
>>>> Why can't we use mmio flips unconditionally?  Maarten or Ville?
>>> We do when execlists are used, which is always on gen9+. So I guess I'm
>>> missing the point of this patch. For gen5+ we could also do it trivially.
>> didn't check if the scheduler is also enabled for gen8 (I guess it would be nice, that would cover BDW and BSW).
> I have a patch that would enable support for mmio flips on all
> platforms, but because of Chris Wilson's objections I still didn't force
> mmio flips by default.
>
>>> For older platforms it'd require a bit of work since we'd need to
>>> complete the flips from the vblank interrupt. Well, we actually do
>>> that already even with CS flips on those platforms, but we do look
>>> at the flip pending interrupt to figure out if CS already issued
>>> the flip or not. So that part would need changing.
> It was easy to fix in a way similar to that.
>
>>> I also think we should switch to using the vblank interrupt for this
>>> stuff on all platforms, mainly since the flip done interrupt is somewhat
>>> broken on at least BDW (no idea if it got fixed in SKL or later), and
>>> doing things in more than one way certainly won't decrease our bug count.
>> Yeah that's probably the way to go; I haven't checked the behavior on SKL either.