[PATCH] drm/i915: Remove src adjustment in intel_check_sprite_plane.

[PATCH] drm/i915: Remove src adjustment in intel_check_sprite_plane.

[Maarten Lankhorst]

The rounding can cause a perfectly normal 16x16 src to full-screen
dst to be rounded down, even without clipping involved. Because of
this we should just remove the adjustment, as no other driver or plane
does it.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a1d048af0261..203ca8b362a5 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -996,11 +996,6 @@ intel_check_sprite_plane(struct intel_plane *plane,
 			return vscale;
 		}
 
-		/* Make the source viewport size an exact multiple of the scaling factors. */
-		drm_rect_adjust_size(src,
-				     drm_rect_width(dst) * hscale - drm_rect_width(src),
-				     drm_rect_height(dst) * vscale - drm_rect_height(src));
-
 		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
 				    state->base.rotation);
 
-- 
2.17.0

_______________________________________________
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 src adjustment in intel_check_sprite_plane.

[Patchwork]

== Series Details ==

Series: drm/i915: Remove src adjustment in intel_check_sprite_plane.
URL   : https://patchwork.freedesktop.org/series/42113/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4078 -> Patchwork_8773 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS

    
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084


== Participating hosts (36 -> 33) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4078 -> Patchwork_8773

  CI_DRM_4078: 9391010824b34ec58217f816ba5e314e7312191d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4444: dcc44347494231feabc588c2a76998cbc9afdf8c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8773: dedb75c52059a931ae14dca63f4b70880f678cf4 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4444: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit


== Linux commits ==

dedb75c52059 drm/i915: Remove src adjustment in intel_check_sprite_plane.

== Logs ==

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

Re: [PATCH] drm/i915: Remove src adjustment in intel_check_sprite_plane.

[Ville Syrjälä]

On Mon, Apr 23, 2018 at 12:49:22PM +0200, Maarten Lankhorst wrote:
> The rounding can cause a perfectly normal 16x16 src to full-screen
> dst to be rounded down, even without clipping involved. Because of
> this we should just remove the adjustment, as no other driver or plane
> does it.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index a1d048af0261..203ca8b362a5 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -996,11 +996,6 @@ intel_check_sprite_plane(struct intel_plane *plane,
>  			return vscale;
>  		}
>  
> -		/* Make the source viewport size an exact multiple of the scaling factors. */
> -		drm_rect_adjust_size(src,
> -				     drm_rect_width(dst) * hscale - drm_rect_width(src),
> -				     drm_rect_height(dst) * vscale - drm_rect_height(src));
> -

This makes the scaling factor checks a slightly incorrect. Ie. we might
exceed the max h/vscale a bit without realizing it. It's a shame the
hardware doesn't let us actually program the scaling factors/increments
and starting phases anymore :( Also we don't actually know how the
hardware calculates that stuff (assuming it has such things internally),
so I'm not actually sure how we should be checking the max limit so
that we actually check what the hardware will use.

Without a more detailed study of the hardware behaviour I'm thinking
we should perhaps just check the final src vs. dst coordinates like so:
if (src > dst*max)
	fail;
as that would avoid the precision issues with the .16 scaling factors.

Another option could be to round h/vscale up. That should guarantee
that we never exceed the max.

>  		drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
>  				    state->base.rotation);
>  
> -- 
> 2.17.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 src adjustment in intel_check_sprite_plane.

[Maarten Lankhorst]

Op 23-04-18 om 16:30 schreef Ville Syrjälä:
> On Mon, Apr 23, 2018 at 12:49:22PM +0200, Maarten Lankhorst wrote:
>> The rounding can cause a perfectly normal 16x16 src to full-screen
>> dst to be rounded down, even without clipping involved. Because of
>> this we should just remove the adjustment, as no other driver or plane
>> does it.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_sprite.c | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
>> index a1d048af0261..203ca8b362a5 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -996,11 +996,6 @@ intel_check_sprite_plane(struct intel_plane *plane,
>>  			return vscale;
>>  		}
>>  
>> -		/* Make the source viewport size an exact multiple of the scaling factors. */
>> -		drm_rect_adjust_size(src,
>> -				     drm_rect_width(dst) * hscale - drm_rect_width(src),
>> -				     drm_rect_height(dst) * vscale - drm_rect_height(src));
>> -
> This makes the scaling factor checks a slightly incorrect. Ie. we might
> exceed the max h/vscale a bit without realizing it. It's a shame the
> hardware doesn't let us actually program the scaling factors/increments
> and starting phases anymore :( Also we don't actually know how the
> hardware calculates that stuff (assuming it has such things internally),
> so I'm not actually sure how we should be checking the max limit so
> that we actually check what the hardware will use.
>
> Without a more detailed study of the hardware behaviour I'm thinking
> we should perhaps just check the final src vs. dst coordinates like so:
> if (src > dst*max)
> 	fail;
> as that would avoid the precision issues with the .16 scaling factors.
>
> Another option could be to round h/vscale up. That should guarantee
> that we never exceed the max.
Could we take a pessimistic view for both sides? Round up if scaling > 1, round down when scaling < 1?
That way we should never be afraid of any limits..

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

Re: [PATCH] drm/i915: Remove src adjustment in intel_check_sprite_plane.

[Ville Syrjälä]

On Mon, Apr 23, 2018 at 04:39:54PM +0200, Maarten Lankhorst wrote:
> Op 23-04-18 om 16:30 schreef Ville Syrjälä:
> > On Mon, Apr 23, 2018 at 12:49:22PM +0200, Maarten Lankhorst wrote:
> >> The rounding can cause a perfectly normal 16x16 src to full-screen
> >> dst to be rounded down, even without clipping involved. Because of
> >> this we should just remove the adjustment, as no other driver or plane
> >> does it.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_sprite.c | 5 -----
> >>  1 file changed, 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >> index a1d048af0261..203ca8b362a5 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -996,11 +996,6 @@ intel_check_sprite_plane(struct intel_plane *plane,
> >>  			return vscale;
> >>  		}
> >>  
> >> -		/* Make the source viewport size an exact multiple of the scaling factors. */
> >> -		drm_rect_adjust_size(src,
> >> -				     drm_rect_width(dst) * hscale - drm_rect_width(src),
> >> -				     drm_rect_height(dst) * vscale - drm_rect_height(src));
> >> -
> > This makes the scaling factor checks a slightly incorrect. Ie. we might
> > exceed the max h/vscale a bit without realizing it. It's a shame the
> > hardware doesn't let us actually program the scaling factors/increments
> > and starting phases anymore :( Also we don't actually know how the
> > hardware calculates that stuff (assuming it has such things internally),
> > so I'm not actually sure how we should be checking the max limit so
> > that we actually check what the hardware will use.
> >
> > Without a more detailed study of the hardware behaviour I'm thinking
> > we should perhaps just check the final src vs. dst coordinates like so:
> > if (src > dst*max)
> > 	fail;
> > as that would avoid the precision issues with the .16 scaling factors.
> >
> > Another option could be to round h/vscale up. That should guarantee
> > that we never exceed the max.
> Could we take a pessimistic view for both sides? Round up if scaling > 1, round down when scaling < 1?
> That way we should never be afraid of any limits..

That does sound like a decent idea to me. At least I can't immediately
think why it wouldn't work out.

-- 
Ville Syrjälä
Intel
_______________________________________________
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 src adjustment in intel_check_sprite_plane.

[Patchwork]

== Series Details ==

Series: drm/i915: Remove src adjustment in intel_check_sprite_plane.
URL   : https://patchwork.freedesktop.org/series/42113/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4078_full -> Patchwork_8773_full =

== Summary - WARNING ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          SKIP -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
      shard-glk:          PASS -> SKIP +67

    igt@kms_vblank@pipe-b-wait-forked-busy-hang:
      shard-glk:          SKIP -> PASS +46

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@flip-vs-wf_vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

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

    igt@kms_flip@plain-flip-ts-check-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103928)

    igt@kms_hdmi_inject@inject-audio:
      shard-glk:          PASS -> FAIL (fdo#102370)

    igt@kms_plane@plane-position-hole-pipe-a-planes:
      shard-apl:          PASS -> FAIL (fdo#103166)

    
    ==== Possible fixes ====

    igt@gem_eio@suspend:
      shard-kbl:          DMESG-WARN -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
      shard-apl:          FAIL (fdo#103207) -> PASS

    igt@kms_flip@2x-flip-vs-absolute-wf_vblank-interruptible:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible:
      shard-glk:          FAIL (fdo#105312) -> PASS

    igt@kms_flip@modeset-vs-vblank-race:
      shard-hsw:          FAIL (fdo#103060) -> PASS

    igt@kms_sysfs_edid_timing:
      shard-apl:          WARN (fdo#100047) -> PASS

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102370 https://bugs.freedesktop.org/show_bug.cgi?id=102370
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103207 https://bugs.freedesktop.org/show_bug.cgi?id=103207
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105312 https://bugs.freedesktop.org/show_bug.cgi?id=105312


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

  Missing    (1): shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4078 -> Patchwork_8773

  CI_DRM_4078: 9391010824b34ec58217f816ba5e314e7312191d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4444: dcc44347494231feabc588c2a76998cbc9afdf8c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8773: dedb75c52059a931ae14dca63f4b70880f678cf4 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4444: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit

== Logs ==

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