intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller
@ 2018-06-07 23:07 Paulo Zanoni
  2018-06-07 23:43 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Paulo Zanoni @ 2018-06-07 23:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

While things may have been different before, right now the function is
very simple and has a single caller. IMHO any possible benefits from
an abstraction here are gone and not worth the price of the current
indirection while reading the code.

Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 53aaaa3e6886..018aae9f5769 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5141,17 +5141,6 @@ skl_compute_ddb(struct drm_atomic_state *state)
 	return 0;
 }
 
-static void
-skl_copy_ddb_for_pipe(struct skl_ddb_values *dst,
-		      struct skl_ddb_values *src,
-		      enum pipe pipe)
-{
-	memcpy(dst->ddb.uv_plane[pipe], src->ddb.uv_plane[pipe],
-	       sizeof(dst->ddb.uv_plane[pipe]));
-	memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
-	       sizeof(dst->ddb.plane[pipe]));
-}
-
 static void
 skl_print_wm_changes(const struct drm_atomic_state *state)
 {
@@ -5381,7 +5370,10 @@ static void skl_initial_wm(struct intel_atomic_state *state,
 	if (cstate->base.active_changed)
 		skl_atomic_update_crtc_wm(state, cstate);
 
-	skl_copy_ddb_for_pipe(hw_vals, results, pipe);
+	memcpy(hw_vals->ddb.uv_plane[pipe], results->ddb.uv_plane[pipe],
+	       sizeof(hw_vals->ddb.uv_plane[pipe]));
+	memcpy(hw_vals->ddb.plane[pipe], results->ddb.plane[pipe],
+	       sizeof(hw_vals->ddb.plane[pipe]));
 
 	mutex_unlock(&dev_priv->wm.wm_mutex);
 }
-- 
2.14.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: inline skl_copy_ddb_for_pipe() to its only caller
  2018-06-07 23:07 [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller Paulo Zanoni
@ 2018-06-07 23:43 ` Patchwork
  2018-06-07 23:49 ` [PATCH] " Chris Wilson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-06-07 23:43 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: inline skl_copy_ddb_for_pipe() to its only caller
URL   : https://patchwork.freedesktop.org/series/44445/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4291 -> Patchwork_9236 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9236 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9236, 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/44445/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@core_auth@basic-auth:
      fi-elk-e7500:       PASS -> DMESG-WARN (fdo#105225)

    igt@core_prop_blob@basic:
      fi-elk-e7500:       PASS -> INCOMPLETE (fdo#103989)

    igt@gem_sync@basic-many-each:
      fi-cnl-y3:          PASS -> INCOMPLETE (fdo#105086)

    
    ==== Possible fixes ====

    igt@core_auth@basic-auth:
      fi-bdw-gvtdvm:      DMESG-WARN (fdo#105600) -> PASS +2

    igt@gem_ctx_switch@basic-default-heavy:
      fi-glk-j4005:       DMESG-WARN (fdo#106000) -> PASS +2

    igt@gem_exec_flush@basic-uc-set-default:
      fi-glk-j4005:       DMESG-WARN (fdo#105719) -> PASS

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128) -> PASS

    
    ==== Warnings ====

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       FAIL (fdo#106724) -> DMESG-WARN (fdo#105719, fdo#106745)

    
  fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
  fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#105225 https://bugs.freedesktop.org/show_bug.cgi?id=105225
  fdo#105600 https://bugs.freedesktop.org/show_bug.cgi?id=105600
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106724 https://bugs.freedesktop.org/show_bug.cgi?id=106724
  fdo#106745 https://bugs.freedesktop.org/show_bug.cgi?id=106745


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

  Missing    (3): fi-ilk-m540 fi-byt-squawks fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4291 -> Patchwork_9236

  CI_DRM_4291: 1d2b97e15aaf8a354a58904dc6c4ca7366f78ed1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4510: d1a93aa7e1507de76c6c71be15931cc4b90111bb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9236: 331503d813d07605c9447a57afc63bac621c3c2e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

331503d813d0 drm/i915: inline skl_copy_ddb_for_pipe() to its only caller

== Logs ==

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller
  2018-06-07 23:07 [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller Paulo Zanoni
  2018-06-07 23:43 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-06-07 23:49 ` Chris Wilson
  2018-07-12 21:33   ` Paulo Zanoni
  2018-06-08  2:00 ` ✓ Fi.CI.IGT: success for " Patchwork
  2018-07-26  6:05 ` [PATCH] " Kumar, Mahesh
  3 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2018-06-07 23:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

Quoting Paulo Zanoni (2018-06-08 00:07:00)
>  static void
>  skl_print_wm_changes(const struct drm_atomic_state *state)
>  {
> @@ -5381,7 +5370,10 @@ static void skl_initial_wm(struct intel_atomic_state *state,
>         if (cstate->base.active_changed)
>                 skl_atomic_update_crtc_wm(state, cstate);
>  
> -       skl_copy_ddb_for_pipe(hw_vals, results, pipe);
> +       memcpy(hw_vals->ddb.uv_plane[pipe], results->ddb.uv_plane[pipe],
> +              sizeof(hw_vals->ddb.uv_plane[pipe]));

I must be seeing things.

ddb.uv_plane[pipe] must be a pointer, right?
Therefore sizeof(ddb.uv_plane[pipe]) must the size of that pointer and
not of the struct. Do you not mean sizeof(*ddb.uv_plane[pipe]) ?

Definitely time to stop reading code...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: inline skl_copy_ddb_for_pipe() to its only caller
  2018-06-07 23:07 [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller Paulo Zanoni
  2018-06-07 23:43 ` ✓ Fi.CI.BAT: success for " Patchwork
  2018-06-07 23:49 ` [PATCH] " Chris Wilson
@ 2018-06-08  2:00 ` Patchwork
  2018-07-26  6:05 ` [PATCH] " Kumar, Mahesh
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-06-08  2:00 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: inline skl_copy_ddb_for_pipe() to its only caller
URL   : https://patchwork.freedesktop.org/series/44445/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4291_full -> Patchwork_9236_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9236_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9236_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_9236_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-blt:
      shard-kbl:          PASS -> SKIP +1

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

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-apl:          PASS -> FAIL (fdo#105347)

    igt@gem_eio@wait-1us:
      shard-snb:          NOTRUN -> INCOMPLETE (fdo#105411)

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

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#102887, fdo#105363)

    igt@kms_flip@plain-flip-fb-recreate:
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724, fdo#103822)

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

    
    ==== Possible fixes ====

    igt@gem_eio@hibernate:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@gem_wait@write-busy-bsd2:
      shard-snb:          INCOMPLETE (fdo#105411) -> SKIP

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

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-apl:          FAIL (fdo#102887, fdo#105363) -> PASS

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

    igt@kms_flip@plain-flip-fb-recreate:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_vblank@pipe-c-ts-continuation-modeset:
      shard-glk:          DMESG-WARN (fdo#106247) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_gtt:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359) -> FAIL (fdo#105347)

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
  fdo#106247 https://bugs.freedesktop.org/show_bug.cgi?id=106247
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4291 -> Patchwork_9236

  CI_DRM_4291: 1d2b97e15aaf8a354a58904dc6c4ca7366f78ed1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4510: d1a93aa7e1507de76c6c71be15931cc4b90111bb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9236: 331503d813d07605c9447a57afc63bac621c3c2e @ 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_9236/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller
  2018-06-07 23:49 ` [PATCH] " Chris Wilson
@ 2018-07-12 21:33   ` Paulo Zanoni
  0 siblings, 0 replies; 6+ messages in thread
From: Paulo Zanoni @ 2018-07-12 21:33 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Em Sex, 2018-06-08 às 00:49 +0100, Chris Wilson escreveu:
> Quoting Paulo Zanoni (2018-06-08 00:07:00)
> >  static void
> >  skl_print_wm_changes(const struct drm_atomic_state *state)
> >  {
> > @@ -5381,7 +5370,10 @@ static void skl_initial_wm(struct
> > intel_atomic_state *state,
> >         if (cstate->base.active_changed)
> >                 skl_atomic_update_crtc_wm(state, cstate);
> >  
> > -       skl_copy_ddb_for_pipe(hw_vals, results, pipe);
> > +       memcpy(hw_vals->ddb.uv_plane[pipe], results-
> > >ddb.uv_plane[pipe],
> > +              sizeof(hw_vals->ddb.uv_plane[pipe]));
> 
> I must be seeing things.
> 
> ddb.uv_plane[pipe] must be a pointer, right?

No, it's an array of structs.


> Therefore sizeof(ddb.uv_plane[pipe]) must the size of that pointer
> and
> not of the struct.

It is the size of the array, which is 20 (both before and after the
patch). Each member has size 4.


>  Do you not mean sizeof(*ddb.uv_plane[pipe]) ?

What's written above means "size of the first element of
ddb.uv_plane[pipe]". This sizeof operation returns 4 instead of 20. We
don't want to copy just the first element of uv_plane[pipe].

> 
> Definitely time to stop reading code...
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller
  2018-06-07 23:07 [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller Paulo Zanoni
                   ` (2 preceding siblings ...)
  2018-06-08  2:00 ` ✓ Fi.CI.IGT: success for " Patchwork
@ 2018-07-26  6:05 ` Kumar, Mahesh
  3 siblings, 0 replies; 6+ messages in thread
From: Kumar, Mahesh @ 2018-07-26  6:05 UTC (permalink / raw)
  To: Paulo Zanoni, intel-gfx

Hi,

Patch LGTM.

Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com>

thanks,

-Mahesh
On 6/8/2018 4:37 AM, Paulo Zanoni wrote:
> While things may have been different before, right now the function is
> very simple and has a single caller. IMHO any possible benefits from
> an abstraction here are gone and not worth the price of the current
> indirection while reading the code.
>
> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 16 ++++------------
>   1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 53aaaa3e6886..018aae9f5769 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5141,17 +5141,6 @@ skl_compute_ddb(struct drm_atomic_state *state)
>   	return 0;
>   }
>   
> -static void
> -skl_copy_ddb_for_pipe(struct skl_ddb_values *dst,
> -		      struct skl_ddb_values *src,
> -		      enum pipe pipe)
> -{
> -	memcpy(dst->ddb.uv_plane[pipe], src->ddb.uv_plane[pipe],
> -	       sizeof(dst->ddb.uv_plane[pipe]));
> -	memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
> -	       sizeof(dst->ddb.plane[pipe]));
> -}
> -
>   static void
>   skl_print_wm_changes(const struct drm_atomic_state *state)
>   {
> @@ -5381,7 +5370,10 @@ static void skl_initial_wm(struct intel_atomic_state *state,
>   	if (cstate->base.active_changed)
>   		skl_atomic_update_crtc_wm(state, cstate);
>   
> -	skl_copy_ddb_for_pipe(hw_vals, results, pipe);
> +	memcpy(hw_vals->ddb.uv_plane[pipe], results->ddb.uv_plane[pipe],
> +	       sizeof(hw_vals->ddb.uv_plane[pipe]));
> +	memcpy(hw_vals->ddb.plane[pipe], results->ddb.plane[pipe],
> +	       sizeof(hw_vals->ddb.plane[pipe]));
>   
>   	mutex_unlock(&dev_priv->wm.wm_mutex);
>   }

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-07-26  6:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-07 23:07 [PATCH] drm/i915: inline skl_copy_ddb_for_pipe() to its only caller Paulo Zanoni
2018-06-07 23:43 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-06-07 23:49 ` [PATCH] " Chris Wilson
2018-07-12 21:33   ` Paulo Zanoni
2018-06-08  2:00 ` ✓ Fi.CI.IGT: success for " Patchwork
2018-07-26  6:05 ` [PATCH] " Kumar, Mahesh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).