[PATCH] drm/i915: Remove fb pitch limit for no display

[PATCH] drm/i915: Remove fb pitch limit for no display

[Chris Wilson]

If there is not a display (and so no CRTCs) then there is no upper limit
to the framebuffer pitch imposed by the CRTC.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3be5fa0acee8..7db14086fb02 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14403,9 +14403,9 @@ static const struct drm_framebuffer_funcs intel_fb_funcs = {
 	.dirty = intel_user_framebuffer_dirty,
 };
 
-static
-u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
-			 uint64_t fb_modifier, uint32_t pixel_format)
+static u32
+intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
+		     uint64_t fb_modifier, uint32_t pixel_format)
 {
 	struct intel_crtc *crtc;
 	struct intel_plane *plane;
@@ -14415,6 +14415,9 @@ u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
 	 * the highest stride limits of them all.
 	 */
 	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
+	if (!crtc)
+		return U32_MAX;
+
 	plane = to_intel_plane(crtc->base.primary);
 
 	return plane->max_stride(plane, pixel_format, fb_modifier,
-- 
2.19.0

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

Re: [PATCH] drm/i915: Remove fb pitch limit for no display

[Ville Syrjälä]

On Thu, Sep 13, 2018 at 11:39:23AM +0100, Chris Wilson wrote:
> If there is not a display (and so no CRTCs) then there is no upper limit
> to the framebuffer pitch imposed by the CRTC.

Should we still allow you to create framebuffers in that case?

If yes then my plan to also query the planes which pixel formats/modifiers
to accept in addfb is going to hit hard times.

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3be5fa0acee8..7db14086fb02 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14403,9 +14403,9 @@ static const struct drm_framebuffer_funcs intel_fb_funcs = {
>  	.dirty = intel_user_framebuffer_dirty,
>  };
>  
> -static
> -u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
> -			 uint64_t fb_modifier, uint32_t pixel_format)
> +static u32
> +intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
> +		     uint64_t fb_modifier, uint32_t pixel_format)
>  {
>  	struct intel_crtc *crtc;
>  	struct intel_plane *plane;
> @@ -14415,6 +14415,9 @@ u32 intel_fb_pitch_limit(struct drm_i915_private *dev_priv,
>  	 * the highest stride limits of them all.
>  	 */
>  	crtc = intel_get_crtc_for_pipe(dev_priv, PIPE_A);
> +	if (!crtc)
> +		return U32_MAX;
> +
>  	plane = to_intel_plane(crtc->base.primary);
>  
>  	return plane->max_stride(plane, pixel_format, fb_modifier,
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915: Remove fb pitch limit for no display

[Chris Wilson]

Quoting Ville Syrjälä (2018-09-13 11:56:56)
> On Thu, Sep 13, 2018 at 11:39:23AM +0100, Chris Wilson wrote:
> > If there is not a display (and so no CRTCs) then there is no upper limit
> > to the framebuffer pitch imposed by the CRTC.
> 
> Should we still allow you to create framebuffers in that case?

Up to you, imho is that fb are just bo with a bit of description. I
didn't see much harm in creating an fb even if it was never going to be
attached to any pipe. I don't have a solid usecase, just feels like it
reduces the impact on the API.

Hmm, however if we
	if (num_pipes == 0) driver_features &= ~DRIVER_MODESET;
we will kill the unusable API at the ioctl boundary.

> If yes then my plan to also query the planes which pixel formats/modifiers
> to accept in addfb is going to hit hard times.

Spreading an ugly if !plane around :(
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Remove fb pitch limit for no display

[Patchwork]

== Series Details ==

Series: drm/i915: Remove fb pitch limit for no display
URL   : https://patchwork.freedesktop.org/series/49625/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4816 -> Patchwork_10166 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49625/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10166 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_psr@primary_mmap_gtt:
      {fi-cnl-u}:         NOTRUN -> FAIL (fdo#107383) +3

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383


== Participating hosts (49 -> 46) ==

  Additional (2): fi-cnl-u fi-icl-u 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4816 -> Patchwork_10166

  CI_DRM_4816: 5bebc54ac552e3716bfe0f1f7eb0babfbda49f09 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10166: 1b805e4c8215a284269e9cfcf3b0bf7921f37539 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1b805e4c8215 drm/i915: Remove fb pitch limit for no display

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10166/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Remove fb pitch limit for no display

[Ville Syrjälä]

On Thu, Sep 13, 2018 at 12:05:49PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-09-13 11:56:56)
> > On Thu, Sep 13, 2018 at 11:39:23AM +0100, Chris Wilson wrote:
> > > If there is not a display (and so no CRTCs) then there is no upper limit
> > > to the framebuffer pitch imposed by the CRTC.
> > 
> > Should we still allow you to create framebuffers in that case?
> 
> Up to you, imho is that fb are just bo with a bit of description. I
> didn't see much harm in creating an fb even if it was never going to be
> attached to any pipe. I don't have a solid usecase, just feels like it
> reduces the impact on the API.

To me it feels a bit like giving userspace false hope that they
can actually do something with the fb later.

> 
> Hmm, however if we
> 	if (num_pipes == 0) driver_features &= ~DRIVER_MODESET;
> we will kill the unusable API at the ioctl boundary.

That seems a bit wrong. We'd really want to device_features for
something like that. Not sure how many things we have in the driver
struct that really ought to be under the device.

> 
> > If yes then my plan to also query the planes which pixel formats/modifiers
> > to accept in addfb is going to hit hard times.
> 
> Spreading an ugly if !plane around :(

Should just be for_each_plane() and I guess if none are there the
addfb would get rejected as the format wouldn't match anything.
But I haven't actually figured out how to do this in the best way.

Another option would be to cache the union of all format/modifier
combos of all planes somewhere and check against that (should be
slightly more efficient as we wouldn't check the same thing many
times). And in that case I guess we could always add some kind
of fallback of say just XRGB8888 for the num_pipes==0 case,
should we think there is some benefit to allowing it.

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

Re: [PATCH] drm/i915: Remove fb pitch limit for no display

[Chris Wilson]

Quoting Ville Syrjälä (2018-09-13 13:26:48)
> On Thu, Sep 13, 2018 at 12:05:49PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjälä (2018-09-13 11:56:56)
> > > On Thu, Sep 13, 2018 at 11:39:23AM +0100, Chris Wilson wrote:
> > > > If there is not a display (and so no CRTCs) then there is no upper limit
> > > > to the framebuffer pitch imposed by the CRTC.
> > > 
> > > Should we still allow you to create framebuffers in that case?
> > 
> > Up to you, imho is that fb are just bo with a bit of description. I
> > didn't see much harm in creating an fb even if it was never going to be
> > attached to any pipe. I don't have a solid usecase, just feels like it
> > reduces the impact on the API.
> 
> To me it feels a bit like giving userspace false hope that they
> can actually do something with the fb later.
> 
> > 
> > Hmm, however if we
> >       if (num_pipes == 0) driver_features &= ~DRIVER_MODESET;
> > we will kill the unusable API at the ioctl boundary.
> 
> That seems a bit wrong. We'd really want to device_features for
> something like that. Not sure how many things we have in the driver
> struct that really ought to be under the device.

Hah, yes it is one level too deep. I think the current set of
driver_features is more or less device_features? That seems like an easy
job though -- though some may point out that this smells of midlayer ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Remove fb pitch limit for no display

[Patchwork]

== Series Details ==

Series: drm/i915: Remove fb pitch limit for no display
URL   : https://patchwork.freedesktop.org/series/49625/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4816_full -> Patchwork_10166_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10166_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10166_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10166_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-xtiled:
      shard-snb:          PASS -> SKIP +5

    
== Known issues ==

  Here are the changes found in Patchwork_10166_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
      shard-hsw:          PASS -> FAIL (fdo#105767)

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          PASS -> FAIL (fdo#103355)

    igt@kms_flip@dpms-vs-vblank-race-interruptible:
      shard-kbl:          PASS -> FAIL (fdo#103060)

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt:
      shard-glk:          PASS -> DMESG-FAIL (fdo#106538)

    igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
      shard-glk:          PASS -> DMESG-WARN (fdo#105763, fdo#106538) +1

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@gem_exec_await@wide-contexts:
      shard-glk:          FAIL (fdo#106680) -> PASS

    igt@kms_color@pipe-c-ctm-max:
      shard-kbl:          DMESG-WARN (fdo#103558, fdo#105602) -> PASS +34

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
      shard-hsw:          FAIL (fdo#105767) -> PASS

    igt@kms_setmode@basic:
      shard-hsw:          FAIL (fdo#99912) -> PASS

    
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4816 -> Patchwork_10166

  CI_DRM_4816: 5bebc54ac552e3716bfe0f1f7eb0babfbda49f09 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10166: 1b805e4c8215a284269e9cfcf3b0bf7921f37539 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10166/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx