[PATCH] drm/i915: Enable atomic support by default on supported platforms.

[PATCH] drm/i915: Enable atomic support by default on supported platforms.

[Maarten Lankhorst]

i915 is pretty much feature complete. Support for atomic i915-specific
connector properties is still missing; those properties can (for now)
only be set through the legacy ioctl.

ILK style watermarks and gen9+ watermarks are handled atomically,
and nonblocking modesets work. FBC has also been made to work with
atomic.

gen4x- and vlv/chv watermarks still need to be fixed, so disable atomic
by default there for now.

Flip the switch!!

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Lyude <cpaul@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
I probably missed a lot of people who also worked on atomic support in i915,
feel free to ack the patch if you've worked on it. :)

 drivers/gpu/drm/i915/i915_drv.c    | 10 +++++++---
 drivers/gpu/drm/i915/i915_params.c |  4 ++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0e8eb4b6db00..d4d1abf6d9d2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1206,11 +1206,15 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
  */
 int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
+	const struct intel_device_info *match_info =
+		(struct intel_device_info *) ent->driver_data;
 	struct drm_i915_private *dev_priv;
 	int ret;
 
-	if (i915.nuclear_pageflip)
-		driver.driver_features |= DRIVER_ATOMIC;
+	/* Enable nuclear pageflip on ILK+, except vlv/chv */
+	if (!i915.nuclear_pageflip &&
+	    (match_info->gen < 5 || match_info->has_gmch_display))
+		driver.driver_features &= ~DRIVER_ATOMIC;
 
 	ret = -ENOMEM;
 	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
@@ -2614,7 +2618,7 @@ static struct drm_driver driver = {
 	 */
 	.driver_features =
 	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
-	    DRIVER_RENDER | DRIVER_MODESET,
+	    DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC,
 	.open = i915_driver_open,
 	.lastclose = i915_driver_lastclose,
 	.preclose = i915_driver_preclose,
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 0e280fbd52f1..bb3feabec238 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -205,9 +205,9 @@ module_param_named(verbose_state_checks, i915.verbose_state_checks, bool, 0600);
 MODULE_PARM_DESC(verbose_state_checks,
 	"Enable verbose logs (ie. WARN_ON()) in case of unexpected hw state conditions.");
 
-module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, 0600);
+module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip, bool, 0400);
 MODULE_PARM_DESC(nuclear_pageflip,
-		 "Force atomic modeset functionality; asynchronous mode is not yet supported. (default: false).");
+		 "Force enable atomic functionality on platforms that don't have full support yet.");
 
 /* WA to get away with the default setting in VBT for early platforms.Will be removed */
 module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, 0400);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Enable atomic support by default on supported platforms.

[Patchwork]

== Series Details ==

Series: drm/i915: Enable atomic support by default on supported platforms.
URL   : https://patchwork.freedesktop.org/series/18970/
State : success

== Summary ==

Series 18970v1 drm/i915: Enable atomic support by default on supported platforms.
https://patchwork.freedesktop.org/api/1.0/series/18970/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                skip       -> PASS       (fi-bxt-j4205)
                skip       -> PASS       (fi-skl-6260u)
                skip       -> PASS       (fi-skl-6700hq)
                skip       -> PASS       (fi-skl-6770hq)
                skip       -> PASS       (fi-snb-2520m)
                skip       -> PASS       (fi-ivb-3520m)
                skip       -> PASS       (fi-ivb-3770)
                skip       -> PASS       (fi-snb-2600)
                skip       -> PASS       (fi-skl-6700k)
                skip       -> PASS       (fi-kbl-7500u)
                skip       -> PASS       (fi-bdw-5557u)
                skip       -> PASS       (fi-hsw-4770)
                skip       -> PASS       (fi-hsw-4770r)
        Subgroup basic-flip-after-cursor-atomic:
                skip       -> PASS       (fi-bxt-j4205)
                skip       -> PASS       (fi-skl-6260u)
                skip       -> PASS       (fi-skl-6700hq)
                skip       -> PASS       (fi-skl-6770hq)
                skip       -> PASS       (fi-snb-2520m)
                skip       -> PASS       (fi-ivb-3520m)
                skip       -> PASS       (fi-ivb-3770)
                skip       -> PASS       (fi-snb-2600)
                skip       -> PASS       (fi-skl-6700k)
                skip       -> PASS       (fi-kbl-7500u)
                skip       -> PASS       (fi-bdw-5557u)
                skip       -> PASS       (fi-hsw-4770)
                skip       -> PASS       (fi-hsw-4770r)
        Subgroup basic-flip-before-cursor-atomic:
                skip       -> PASS       (fi-bxt-j4205)
                skip       -> PASS       (fi-skl-6260u)
                skip       -> PASS       (fi-skl-6700hq)
                skip       -> PASS       (fi-skl-6770hq)
                skip       -> PASS       (fi-snb-2520m)
                skip       -> PASS       (fi-ivb-3520m)
                skip       -> PASS       (fi-ivb-3770)
                skip       -> PASS       (fi-snb-2600)
                skip       -> PASS       (fi-skl-6700k)
                skip       -> PASS       (fi-kbl-7500u)
                skip       -> PASS       (fi-bdw-5557u)
                skip       -> PASS       (fi-hsw-4770)
                skip       -> PASS       (fi-hsw-4770r)

fi-bdw-5557u     total:247  pass:236  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:247  pass:208  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:247  pass:228  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:78   pass:65   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:247  pass:220  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:247  pass:216  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:247  pass:231  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:247  pass:231  dwarn:0   dfail:0   fail:0   skip:16 
fi-ivb-3520m     total:247  pass:229  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:247  pass:229  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:247  pass:227  dwarn:0   dfail:0   fail:2   skip:18 
fi-skl-6260u     total:247  pass:237  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:247  pass:230  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:247  pass:225  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:247  pass:237  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:247  pass:219  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:247  pass:218  dwarn:0   dfail:0   fail:0   skip:29 

a0cc425b0034c42eb3830f7dd612ac2a132a874c drm-tip: 2017y-02m-01d-17h-17m-34s UTC integration manifest
b519f7f drm/i915: Enable atomic support by default on supported platforms.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3670/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Enable atomic support by default on supported platforms.

[Lyude Paul]

Nice! I actually was already thinking of bringing up the fact we should
just be turning this on by default now, especially so we can see atomic
start getting some real use.

So, I'm more then happy to say I support flipping the switch :)
Reviewed-by: Lyude <lyude@redhat.com>

On Thu, 2017-02-02 at 08:41 +0100, Maarten Lankhorst wrote:
> i915 is pretty much feature complete. Support for atomic i915-
> specific
> connector properties is still missing; those properties can (for now)
> only be set through the legacy ioctl.
> 
> ILK style watermarks and gen9+ watermarks are handled atomically,
> and nonblocking modesets work. FBC has also been made to work with
> atomic.
> 
> gen4x- and vlv/chv watermarks still need to be fixed, so disable
> atomic
> by default there for now.
> 
> Flip the switch!!
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.co
> m>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Lyude <cpaul@redhat.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> I probably missed a lot of people who also worked on atomic support
> in i915,
> feel free to ack the patch if you've worked on it. :)
> 
>  drivers/gpu/drm/i915/i915_drv.c    | 10 +++++++---
>  drivers/gpu/drm/i915/i915_params.c |  4 ++--
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index 0e8eb4b6db00..d4d1abf6d9d2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1206,11 +1206,15 @@ static void i915_driver_unregister(struct
> drm_i915_private *dev_priv)
>   */
>  int i915_driver_load(struct pci_dev *pdev, const struct
> pci_device_id *ent)
>  {
> +	const struct intel_device_info *match_info =
> +		(struct intel_device_info *) ent->driver_data;
>  	struct drm_i915_private *dev_priv;
>  	int ret;
>  
> -	if (i915.nuclear_pageflip)
> -		driver.driver_features |= DRIVER_ATOMIC;
> +	/* Enable nuclear pageflip on ILK+, except vlv/chv */
> +	if (!i915.nuclear_pageflip &&
> +	    (match_info->gen < 5 || match_info->has_gmch_display))
> +		driver.driver_features &= ~DRIVER_ATOMIC;
>  
>  	ret = -ENOMEM;
>  	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
> @@ -2614,7 +2618,7 @@ static struct drm_driver driver = {
>  	 */
>  	.driver_features =
>  	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
> DRIVER_PRIME |
> -	    DRIVER_RENDER | DRIVER_MODESET,
> +	    DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC,
>  	.open = i915_driver_open,
>  	.lastclose = i915_driver_lastclose,
>  	.preclose = i915_driver_preclose,
> diff --git a/drivers/gpu/drm/i915/i915_params.c
> b/drivers/gpu/drm/i915/i915_params.c
> index 0e280fbd52f1..bb3feabec238 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -205,9 +205,9 @@ module_param_named(verbose_state_checks,
> i915.verbose_state_checks, bool, 0600);
>  MODULE_PARM_DESC(verbose_state_checks,
>  	"Enable verbose logs (ie. WARN_ON()) in case of unexpected
> hw state conditions.");
>  
> -module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip,
> bool, 0600);
> +module_param_named_unsafe(nuclear_pageflip, i915.nuclear_pageflip,
> bool, 0400);
>  MODULE_PARM_DESC(nuclear_pageflip,
> -		 "Force atomic modeset functionality; asynchronous
> mode is not yet supported. (default: false).");
> +		 "Force enable atomic functionality on platforms
> that don't have full support yet.");
>  
>  /* WA to get away with the default setting in VBT for early
> platforms.Will be removed */
>  module_param_named_unsafe(edp_vswing, i915.edp_vswing, int, 0400);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Enable atomic support by default on supported platforms.

[Daniel Stone]

Hi,

On 2 February 2017 at 07:41, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
> i915 is pretty much feature complete. Support for atomic i915-specific
> connector properties is still missing; those properties can (for now)
> only be set through the legacy ioctl.
>
> ILK style watermarks and gen9+ watermarks are handled atomically,
> and nonblocking modesets work. FBC has also been made to work with
> atomic.
>
> gen4x- and vlv/chv watermarks still need to be fixed, so disable atomic
> by default there for now.
>
> Flip the switch!!

Not until we have the multi-CRTC event support please. :\ I don't want
to have divergent event paths for atomic-but-useless-events.

I've been frantically typing up support for this in Weston (actual
proper atomic modesetting, which is difficult when you have fiercely
independent per-output repaint loops, but seems ~mostly done but for
typing), which I'd hoped to have done a week or two ago but got
derailed due to being sick. It's coming just as quickly as I can type
it tho.

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Enable atomic support by default on supported platforms.

[Maarten Lankhorst]

Hey,

Op 02-02-17 om 17:26 schreef Daniel Stone:
> Hi,
>
> On 2 February 2017 at 07:41, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> i915 is pretty much feature complete. Support for atomic i915-specific
>> connector properties is still missing; those properties can (for now)
>> only be set through the legacy ioctl.
>>
>> ILK style watermarks and gen9+ watermarks are handled atomically,
>> and nonblocking modesets work. FBC has also been made to work with
>> atomic.
>>
>> gen4x- and vlv/chv watermarks still need to be fixed, so disable atomic
>> by default there for now.
>>
>> Flip the switch!!
> Not until we have the multi-CRTC event support please. :\ I don't want
> to have divergent event paths for atomic-but-useless-events.
>
> I've been frantically typing up support for this in Weston (actual
> proper atomic modesetting, which is difficult when you have fiercely
> independent per-output repaint loops, but seems ~mostly done but for
> typing), which I'd hoped to have done a week or two ago but got
> derailed due to being sick. It's coming just as quickly as I can type
> it tho.
>
> Cheers,
> Daniel

Do you mean like ander's https://lists.freedesktop.org/archives/dri-devel/2015-August/088621.html reserved->crtc_id?

I've recreated the same patch locally for exactly that issue before I was aware of its existance. It's easy to hit, and it would be nice if it was merged.

~Maarten

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Enable atomic support by default on supported platforms.

[Maarten Lankhorst]

Hey,

Op 02-02-17 om 17:26 schreef Daniel Stone:
> Hi,
>
> On 2 February 2017 at 07:41, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> i915 is pretty much feature complete. Support for atomic i915-specific
>> connector properties is still missing; those properties can (for now)
>> only be set through the legacy ioctl.
>>
>> ILK style watermarks and gen9+ watermarks are handled atomically,
>> and nonblocking modesets work. FBC has also been made to work with
>> atomic.
>>
>> gen4x- and vlv/chv watermarks still need to be fixed, so disable atomic
>> by default there for now.
>>
>> Flip the switch!!
> Not until we have the multi-CRTC event support please. :\ I don't want
> to have divergent event paths for atomic-but-useless-events.
>
> I've been frantically typing up support for this in Weston (actual
> proper atomic modesetting, which is difficult when you have fiercely
> independent per-output repaint loops, but seems ~mostly done but for
> typing), which I'd hoped to have done a week or two ago but got
> derailed due to being sick. It's coming just as quickly as I can type
> it tho.
The patch for this has been shot down before, due to lack of userspace,
but here's a slightly earlier version.

https://patchwork.kernel.org/patch/7025111/

I guess it's less important now that we support out-fences, which provides
a nicer way of waiting for completion.

But since this is a problem with atomic core, not i915, do you have any
objections specifically against this patch?

~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Enable atomic support by default on supported platforms.

[Daniel Stone]

Hey,

On 9 February 2017 at 12:49, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
> Op 02-02-17 om 17:26 schreef Daniel Stone:
>> On 2 February 2017 at 07:41, Maarten Lankhorst
>> <maarten.lankhorst@linux.intel.com> wrote:
>>> Flip the switch!!
>> Not until we have the multi-CRTC event support please. :\ I don't want
>> to have divergent event paths for atomic-but-useless-events.
>>
>> I've been frantically typing up support for this in Weston (actual
>> proper atomic modesetting, which is difficult when you have fiercely
>> independent per-output repaint loops, but seems ~mostly done but for
>> typing), which I'd hoped to have done a week or two ago but got
>> derailed due to being sick. It's coming just as quickly as I can type
>> it tho.
> The patch for this has been shot down before, due to lack of userspace,
> but here's a slightly earlier version.
>
> https://patchwork.kernel.org/patch/7025111/
>
> I guess it's less important now that we support out-fences, which provides
> a nicer way of waiting for completion.
>
> But since this is a problem with atomic core, not i915, do you have any
> objections specifically against this patch?

Yeah, it's a pretty good point; we can merge this with a cap so
userspace can probe whether or not things are usable. There are a few
other drivers which already expose atomic by default regardless; it
was less of a problem for them since they're far less likely to be
used with multiple outputs, but that's not really a fair reason to
penalise you guys ...

Go for it.

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Enable atomic support by default on supported platforms.

[Maarten Lankhorst]

Op 09-02-17 om 14:54 schreef Daniel Stone:
> Hey,
>
> On 9 February 2017 at 12:49, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> Op 02-02-17 om 17:26 schreef Daniel Stone:
>>> On 2 February 2017 at 07:41, Maarten Lankhorst
>>> <maarten.lankhorst@linux.intel.com> wrote:
>>>> Flip the switch!!
>>> Not until we have the multi-CRTC event support please. :\ I don't want
>>> to have divergent event paths for atomic-but-useless-events.
>>>
>>> I've been frantically typing up support for this in Weston (actual
>>> proper atomic modesetting, which is difficult when you have fiercely
>>> independent per-output repaint loops, but seems ~mostly done but for
>>> typing), which I'd hoped to have done a week or two ago but got
>>> derailed due to being sick. It's coming just as quickly as I can type
>>> it tho.
>> The patch for this has been shot down before, due to lack of userspace,
>> but here's a slightly earlier version.
>>
>> https://patchwork.kernel.org/patch/7025111/
>>
>> I guess it's less important now that we support out-fences, which provides
>> a nicer way of waiting for completion.
>>
>> But since this is a problem with atomic core, not i915, do you have any
>> objections specifically against this patch?
> Yeah, it's a pretty good point; we can merge this with a cap so
> userspace can probe whether or not things are usable. There are a few
> other drivers which already expose atomic by default regardless; it
> was less of a problem for them since they're far less likely to be
> used with multiple outputs, but that's not really a fair reason to
> penalise you guys ...
>
> Go for it.
>
> Cheers,
> Daniel

Pushed, thanks!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx