All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/crt: make intel_crt_reset() static again
@ 2018-06-21 13:03 Jani Nikula
  2018-06-21 13:46 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Jani Nikula @ 2018-06-21 13:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, Lyude

Commit 9504a8924759 ("drm/i915/vlv: Reset the ADPA in
vlv_display_power_well_init()") started calling intel_crt_reset()
directly, while we could just as well use the hooks and keep the
function static.

Cc: Lyude <cpaul@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c        | 2 +-
 drivers/gpu/drm/i915/intel_drv.h        | 1 -
 drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 0c6bf82bb059..c2cb3b7a255b 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -881,7 +881,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
 	return ret;
 }
 
-void intel_crt_reset(struct drm_encoder *encoder)
+static void intel_crt_reset(struct drm_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
 	struct intel_crt *crt = intel_encoder_to_crt(to_intel_encoder(encoder));
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0c3ac0eafde0..b2002fee1b58 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1373,7 +1373,6 @@ void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
 bool intel_crt_port_enabled(struct drm_i915_private *dev_priv,
 			    i915_reg_t adpa_reg, enum pipe *pipe);
 void intel_crt_init(struct drm_i915_private *dev_priv);
-void intel_crt_reset(struct drm_encoder *encoder);
 
 /* intel_ddi.c */
 void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index de3a81034f77..0b3da5818383 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -972,7 +972,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
 	/* Re-enable the ADPA, if we have one */
 	for_each_intel_encoder(&dev_priv->drm, encoder) {
 		if (encoder->type == INTEL_OUTPUT_ANALOG)
-			intel_crt_reset(&encoder->base);
+			encoder->base.funcs->reset(&encoder->base);
 	}
 
 	i915_redisable_vga_power_on(dev_priv);
-- 
2.11.0

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

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

* ✗ Fi.CI.BAT: failure for drm/i915/crt: make intel_crt_reset() static again
  2018-06-21 13:03 [PATCH] drm/i915/crt: make intel_crt_reset() static again Jani Nikula
@ 2018-06-21 13:46 ` Patchwork
  2018-06-21 14:00 ` [PATCH] " Ville Syrjälä
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-06-21 13:46 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/crt: make intel_crt_reset() static again
URL   : https://patchwork.freedesktop.org/series/45167/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4359 -> Patchwork_9382 =

== Summary - FAILURE ==

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-kbl-7560u:       PASS -> INCOMPLETE

    
== Known issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_ctx_create@basic-files:
      fi-skl-gvtdvm:      INCOMPLETE (fdo#106988, fdo#105600) -> DMESG-WARN (fdo#106954)

    
  fdo#105600 https://bugs.freedesktop.org/show_bug.cgi?id=105600
  fdo#106954 https://bugs.freedesktop.org/show_bug.cgi?id=106954
  fdo#106988 https://bugs.freedesktop.org/show_bug.cgi?id=106988


== Participating hosts (42 -> 37) ==

  Missing    (5): fi-byt-squawks fi-kbl-x1275 fi-ilk-m540 fi-glk-dsi fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4359 -> Patchwork_9382

  CI_DRM_4359: fe0300c16bff0f9c82050e56cdbc3880f87e39bd @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4527: 04afec3ccfcb35e994f2e78254ff499f6b94f097 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9382: caeefc8eeaf8ad18927a9bd623d3bfe35b0570f2 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

caeefc8eeaf8 drm/i915/crt: make intel_crt_reset() static again

== Logs ==

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

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

* Re: [PATCH] drm/i915/crt: make intel_crt_reset() static again
  2018-06-21 13:03 [PATCH] drm/i915/crt: make intel_crt_reset() static again Jani Nikula
  2018-06-21 13:46 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-06-21 14:00 ` Ville Syrjälä
  2018-06-27 11:49   ` Jani Nikula
  2018-06-25  7:35 ` ✓ Fi.CI.BAT: success for " Patchwork
  2018-06-25 11:25 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 1 reply; 8+ messages in thread
From: Ville Syrjälä @ 2018-06-21 14:00 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Lyude

On Thu, Jun 21, 2018 at 04:03:30PM +0300, Jani Nikula wrote:
> Commit 9504a8924759 ("drm/i915/vlv: Reset the ADPA in
> vlv_display_power_well_init()") started calling intel_crt_reset()
> directly, while we could just as well use the hooks and keep the
> function static.
> 
> Cc: Lyude <cpaul@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_crt.c        | 2 +-
>  drivers/gpu/drm/i915/intel_drv.h        | 1 -
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>  3 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 0c6bf82bb059..c2cb3b7a255b 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -881,7 +881,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
>  	return ret;
>  }
>  
> -void intel_crt_reset(struct drm_encoder *encoder)
> +static void intel_crt_reset(struct drm_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>  	struct intel_crt *crt = intel_encoder_to_crt(to_intel_encoder(encoder));
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 0c3ac0eafde0..b2002fee1b58 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1373,7 +1373,6 @@ void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
>  bool intel_crt_port_enabled(struct drm_i915_private *dev_priv,
>  			    i915_reg_t adpa_reg, enum pipe *pipe);
>  void intel_crt_init(struct drm_i915_private *dev_priv);
> -void intel_crt_reset(struct drm_encoder *encoder);
>  
>  /* intel_ddi.c */
>  void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index de3a81034f77..0b3da5818383 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -972,7 +972,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>  	/* Re-enable the ADPA, if we have one */
>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
>  		if (encoder->type == INTEL_OUTPUT_ANALOG)
> -			intel_crt_reset(&encoder->base);
> +			encoder->base.funcs->reset(&encoder->base);

I have a feeling I requested the direct call to make it less annoying to
figure out what it's actually calling. But if people prefer the function
pointer version I can live with that too.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  	}
>  
>  	i915_redisable_vga_power_on(dev_priv);
> -- 
> 2.11.0

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

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

* ✓ Fi.CI.BAT: success for drm/i915/crt: make intel_crt_reset() static again
  2018-06-21 13:03 [PATCH] drm/i915/crt: make intel_crt_reset() static again Jani Nikula
  2018-06-21 13:46 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2018-06-21 14:00 ` [PATCH] " Ville Syrjälä
@ 2018-06-25  7:35 ` Patchwork
  2018-06-25 11:25 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-06-25  7:35 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/crt: make intel_crt_reset() static again
URL   : https://patchwork.freedesktop.org/series/45167/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4372 -> Patchwork_9408 =

== Summary - SUCCESS ==

  No regressions found.

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


== Changes ==

  No changes found


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

  Additional (2): fi-bxt-dsi fi-snb-2520m 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-glk-dsi fi-bsw-cyan fi-ctg-p8600 fi-kbl-x1275 


== Build changes ==

    * Linux: CI_DRM_4372 -> Patchwork_9408

  CI_DRM_4372: d4ce215cacf172e6b8f1424d3750ea116c3c4abe @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4529: 23d50a49413aff619d00ec50fc2e051e9b45baa5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9408: 72abddeed6c5b783387862b60ef83854d44d2690 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

72abddeed6c5 drm/i915/crt: make intel_crt_reset() static again

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for drm/i915/crt: make intel_crt_reset() static again
  2018-06-21 13:03 [PATCH] drm/i915/crt: make intel_crt_reset() static again Jani Nikula
                   ` (2 preceding siblings ...)
  2018-06-25  7:35 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-06-25 11:25 ` Patchwork
  3 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-06-25 11:25 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/crt: make intel_crt_reset() static again
URL   : https://patchwork.freedesktop.org/series/45167/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4372_full -> Patchwork_9408_full =

== Summary - WARNING ==

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

  === IGT changes ===

    ==== Warnings ====

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

    igt@gem_mocs_settings@mocs-rc6-bsd1:
      shard-kbl:          PASS -> SKIP +1

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-mmap-cpu:
      shard-hsw:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

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

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

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

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

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724) +1

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

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

    igt@perf_pmu@idle-no-semaphores-rcs0:
      shard-snb:          NOTRUN -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      shard-kbl:          DMESG-FAIL (fdo#106947, fdo#106560) -> PASS

    igt@gem_exec_params@rs-invalid-on-bsd-ring:
      shard-snb:          INCOMPLETE (fdo#105411) -> SKIP

    igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#106509, fdo#105454) -> PASS

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

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

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

    
  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#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  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#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  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_4372 -> Patchwork_9408

  CI_DRM_4372: d4ce215cacf172e6b8f1424d3750ea116c3c4abe @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4529: 23d50a49413aff619d00ec50fc2e051e9b45baa5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9408: 72abddeed6c5b783387862b60ef83854d44d2690 @ 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_9408/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/crt: make intel_crt_reset() static again
  2018-06-21 14:00 ` [PATCH] " Ville Syrjälä
@ 2018-06-27 11:49   ` Jani Nikula
  2018-06-28  6:45     ` Daniel Vetter
  0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2018-06-27 11:49 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Lyude

On Thu, 21 Jun 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Jun 21, 2018 at 04:03:30PM +0300, Jani Nikula wrote:
>> Commit 9504a8924759 ("drm/i915/vlv: Reset the ADPA in
>> vlv_display_power_well_init()") started calling intel_crt_reset()
>> directly, while we could just as well use the hooks and keep the
>> function static.
>> 
>> Cc: Lyude <cpaul@redhat.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_crt.c        | 2 +-
>>  drivers/gpu/drm/i915/intel_drv.h        | 1 -
>>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>>  3 files changed, 2 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>> index 0c6bf82bb059..c2cb3b7a255b 100644
>> --- a/drivers/gpu/drm/i915/intel_crt.c
>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>> @@ -881,7 +881,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
>>  	return ret;
>>  }
>>  
>> -void intel_crt_reset(struct drm_encoder *encoder)
>> +static void intel_crt_reset(struct drm_encoder *encoder)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>>  	struct intel_crt *crt = intel_encoder_to_crt(to_intel_encoder(encoder));
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 0c3ac0eafde0..b2002fee1b58 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1373,7 +1373,6 @@ void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
>>  bool intel_crt_port_enabled(struct drm_i915_private *dev_priv,
>>  			    i915_reg_t adpa_reg, enum pipe *pipe);
>>  void intel_crt_init(struct drm_i915_private *dev_priv);
>> -void intel_crt_reset(struct drm_encoder *encoder);
>>  
>>  /* intel_ddi.c */
>>  void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index de3a81034f77..0b3da5818383 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -972,7 +972,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>>  	/* Re-enable the ADPA, if we have one */
>>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
>>  		if (encoder->type == INTEL_OUTPUT_ANALOG)
>> -			intel_crt_reset(&encoder->base);
>> +			encoder->base.funcs->reset(&encoder->base);
>
> I have a feeling I requested the direct call to make it less annoying to
> figure out what it's actually calling. But if people prefer the function
> pointer version I can live with that too.

True that, but I thought the direct call was a layering violation.

To push, or not to push, that is the question.

BR,
Jani.

>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>>  	}
>>  
>>  	i915_redisable_vga_power_on(dev_priv);
>> -- 
>> 2.11.0

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/crt: make intel_crt_reset() static again
  2018-06-27 11:49   ` Jani Nikula
@ 2018-06-28  6:45     ` Daniel Vetter
  2018-06-28  9:21       ` Jani Nikula
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2018-06-28  6:45 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Lyude

On Wed, Jun 27, 2018 at 02:49:40PM +0300, Jani Nikula wrote:
> On Thu, 21 Jun 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Jun 21, 2018 at 04:03:30PM +0300, Jani Nikula wrote:
> >> Commit 9504a8924759 ("drm/i915/vlv: Reset the ADPA in
> >> vlv_display_power_well_init()") started calling intel_crt_reset()
> >> directly, while we could just as well use the hooks and keep the
> >> function static.
> >> 
> >> Cc: Lyude <cpaul@redhat.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_crt.c        | 2 +-
> >>  drivers/gpu/drm/i915/intel_drv.h        | 1 -
> >>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
> >>  3 files changed, 2 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> >> index 0c6bf82bb059..c2cb3b7a255b 100644
> >> --- a/drivers/gpu/drm/i915/intel_crt.c
> >> +++ b/drivers/gpu/drm/i915/intel_crt.c
> >> @@ -881,7 +881,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
> >>  	return ret;
> >>  }
> >>  
> >> -void intel_crt_reset(struct drm_encoder *encoder)
> >> +static void intel_crt_reset(struct drm_encoder *encoder)
> >>  {
> >>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >>  	struct intel_crt *crt = intel_encoder_to_crt(to_intel_encoder(encoder));
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index 0c3ac0eafde0..b2002fee1b58 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -1373,7 +1373,6 @@ void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
> >>  bool intel_crt_port_enabled(struct drm_i915_private *dev_priv,
> >>  			    i915_reg_t adpa_reg, enum pipe *pipe);
> >>  void intel_crt_init(struct drm_i915_private *dev_priv);
> >> -void intel_crt_reset(struct drm_encoder *encoder);
> >>  
> >>  /* intel_ddi.c */
> >>  void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
> >> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> index de3a81034f77..0b3da5818383 100644
> >> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >> @@ -972,7 +972,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
> >>  	/* Re-enable the ADPA, if we have one */
> >>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
> >>  		if (encoder->type == INTEL_OUTPUT_ANALOG)
> >> -			intel_crt_reset(&encoder->base);
> >> +			encoder->base.funcs->reset(&encoder->base);
> >
> > I have a feeling I requested the direct call to make it less annoying to
> > figure out what it's actually calling. But if people prefer the function
> > pointer version I can live with that too.
> 
> True that, but I thought the direct call was a layering violation.
> 
> To push, or not to push, that is the question.

Lemma pile my bikeshed on top: Given that we specifically filter for the
CRT encoder I think a direct call is better here, since a bit more clear
what's going on. hw occasionally encouragse layering violations ...
-Daniel

> 
> BR,
> Jani.
> 
> >
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >>  	}
> >>  
> >>  	i915_redisable_vga_power_on(dev_priv);
> >> -- 
> >> 2.11.0
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/crt: make intel_crt_reset() static again
  2018-06-28  6:45     ` Daniel Vetter
@ 2018-06-28  9:21       ` Jani Nikula
  0 siblings, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2018-06-28  9:21 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, Lyude

On Thu, 28 Jun 2018, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Jun 27, 2018 at 02:49:40PM +0300, Jani Nikula wrote:
>> On Thu, 21 Jun 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Thu, Jun 21, 2018 at 04:03:30PM +0300, Jani Nikula wrote:
>> >> Commit 9504a8924759 ("drm/i915/vlv: Reset the ADPA in
>> >> vlv_display_power_well_init()") started calling intel_crt_reset()
>> >> directly, while we could just as well use the hooks and keep the
>> >> function static.
>> >> 
>> >> Cc: Lyude <cpaul@redhat.com>
>> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/i915/intel_crt.c        | 2 +-
>> >>  drivers/gpu/drm/i915/intel_drv.h        | 1 -
>> >>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>> >>  3 files changed, 2 insertions(+), 3 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>> >> index 0c6bf82bb059..c2cb3b7a255b 100644
>> >> --- a/drivers/gpu/drm/i915/intel_crt.c
>> >> +++ b/drivers/gpu/drm/i915/intel_crt.c
>> >> @@ -881,7 +881,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
>> >>  	return ret;
>> >>  }
>> >>  
>> >> -void intel_crt_reset(struct drm_encoder *encoder)
>> >> +static void intel_crt_reset(struct drm_encoder *encoder)
>> >>  {
>> >>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>> >>  	struct intel_crt *crt = intel_encoder_to_crt(to_intel_encoder(encoder));
>> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> >> index 0c3ac0eafde0..b2002fee1b58 100644
>> >> --- a/drivers/gpu/drm/i915/intel_drv.h
>> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> >> @@ -1373,7 +1373,6 @@ void gen9_disable_guc_interrupts(struct drm_i915_private *dev_priv);
>> >>  bool intel_crt_port_enabled(struct drm_i915_private *dev_priv,
>> >>  			    i915_reg_t adpa_reg, enum pipe *pipe);
>> >>  void intel_crt_init(struct drm_i915_private *dev_priv);
>> >> -void intel_crt_reset(struct drm_encoder *encoder);
>> >>  
>> >>  /* intel_ddi.c */
>> >>  void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
>> >> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> >> index de3a81034f77..0b3da5818383 100644
>> >> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> >> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> >> @@ -972,7 +972,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>> >>  	/* Re-enable the ADPA, if we have one */
>> >>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
>> >>  		if (encoder->type == INTEL_OUTPUT_ANALOG)
>> >> -			intel_crt_reset(&encoder->base);
>> >> +			encoder->base.funcs->reset(&encoder->base);
>> >
>> > I have a feeling I requested the direct call to make it less annoying to
>> > figure out what it's actually calling. But if people prefer the function
>> > pointer version I can live with that too.
>> 
>> True that, but I thought the direct call was a layering violation.
>> 
>> To push, or not to push, that is the question.
>
> Lemma pile my bikeshed on top: Given that we specifically filter for the
> CRT encoder I think a direct call is better here, since a bit more clear
> what's going on. hw occasionally encouragse layering violations ...

Okay, I'll drop the patch for now.

BR,
Jani.



> -Daniel
>
>> 
>> BR,
>> Jani.
>> 
>> >
>> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> >>  	}
>> >>  
>> >>  	i915_redisable_vga_power_on(dev_priv);
>> >> -- 
>> >> 2.11.0
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-06-28  9:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-21 13:03 [PATCH] drm/i915/crt: make intel_crt_reset() static again Jani Nikula
2018-06-21 13:46 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-06-21 14:00 ` [PATCH] " Ville Syrjälä
2018-06-27 11:49   ` Jani Nikula
2018-06-28  6:45     ` Daniel Vetter
2018-06-28  9:21       ` Jani Nikula
2018-06-25  7:35 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-06-25 11:25 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.