[PATCH 1/5] drm/i915/vlv: Move czclk to intel_pm

[PATCH 1/5] drm/i915/vlv: Move czclk to intel_pm

[José Roberto de Souza]

Moving VLV/CHV/BYT czclk to intel_pm as it is a core clock used as
base by several other GPU blocks including GT.

BSpec: 14370

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ------------
 drivers/gpu/drm/i915/intel_pm.c      | 10 ++++++++++
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7c5e84ef5171..91a8ee611b12 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -180,17 +180,6 @@ int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
 				 dev_priv->hpll_freq);
 }
 
-static void intel_update_czclk(struct drm_i915_private *dev_priv)
-{
-	if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
-		return;
-
-	dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
-						      CCK_CZ_CLOCK_CONTROL);
-
-	DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
-}
-
 static inline u32 /* units of 100MHz */
 intel_fdi_link_freq(struct drm_i915_private *dev_priv,
 		    const struct intel_crtc_state *pipe_config)
@@ -15533,7 +15522,6 @@ int intel_modeset_init(struct drm_device *dev)
 	intel_shared_dpll_init(dev);
 	intel_update_fdi_pll_freq(dev_priv);
 
-	intel_update_czclk(dev_priv);
 	intel_modeset_init_hw(dev);
 
 	intel_hdcp_component_init(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9c97a95c1816..cd363fa47cbc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7622,6 +7622,14 @@ static void vlv_init_gpll_ref_freq(struct drm_i915_private *dev_priv)
 			 dev_priv->gt_pm.rps.gpll_ref_freq);
 }
 
+static void vlv_update_czclk(struct drm_i915_private *dev_priv)
+{
+	dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
+						      CCK_CZ_CLOCK_CONTROL);
+
+	DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
+}
+
 static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
 {
 	struct intel_rps *rps = &dev_priv->gt_pm.rps;
@@ -7629,6 +7637,7 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
 
 	valleyview_setup_pctx(dev_priv);
 
+	vlv_update_czclk(dev_priv);
 	vlv_init_gpll_ref_freq(dev_priv);
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
@@ -7675,6 +7684,7 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
 
 	cherryview_setup_pctx(dev_priv);
 
+	vlv_update_czclk(dev_priv);
 	vlv_init_gpll_ref_freq(dev_priv);
 
 	mutex_lock(&dev_priv->sb_lock);
-- 
2.21.0

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

[PATCH 2/5] drm/i915: Rename i915_load_modeset_init() to i915_modeset_load()

[José Roberto de Souza]

i915_load_modeset_init() sounds horrible also lets rename it so
the future cleanup function of it can be easially recognized.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index c08abdef5eb6..90c77fab3d70 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -639,7 +639,7 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
 	.can_switch = i915_switcheroo_can_switch,
 };
 
-static int i915_load_modeset_init(struct drm_device *dev)
+static int i915_modeset_load(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct pci_dev *pdev = dev_priv->drm.pdev;
@@ -1748,7 +1748,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret < 0)
 		goto out_cleanup_mmio;
 
-	ret = i915_load_modeset_init(&dev_priv->drm);
+	ret = i915_modeset_load(&dev_priv->drm);
 	if (ret < 0)
 		goto out_cleanup_hw;
 
-- 
2.21.0

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

Re: [PATCH 2/5] drm/i915: Rename i915_load_modeset_init() to i915_modeset_load()

[Jani Nikula]

On Fri, 01 Mar 2019, José Roberto de Souza <jose.souza@intel.com> wrote:
> i915_load_modeset_init() sounds horrible also lets rename it so
> the future cleanup function of it can be easially recognized.

We load the driver, but init modeset. The name implies modeset init at
driver load.

The modeset load/unload sounds a bit odd to me. Even more so with
begin/finish load and begin unload. It's not obvious to me what those
should do. They're not a pattern we have.

I'm not all that averse to renaming stuff in general, but any rename
increases the cognitive burden for a while, and should make things
*easier* to understand. I don't think that's the case in this series.

I can understand i915_load_modeset_init sounding funny. If you insist on
renaming, I'd go with i915_driver_modeset_init. It's in line with the
rest of the functions.

Here are my suggestions for the names:

i915_modeset_load/i915_modeset_begin_load
-> i915_driver_modeset_init

i915_modeset_unload/i915_modeset_begin_unload
-> i915_driver_modeset_cleanup or _fini

i915_modeset_finish_load
-> i915_driver_modeset_init_late


BR,
Jani.


>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index c08abdef5eb6..90c77fab3d70 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -639,7 +639,7 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
>  	.can_switch = i915_switcheroo_can_switch,
>  };
>  
> -static int i915_load_modeset_init(struct drm_device *dev)
> +static int i915_modeset_load(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct pci_dev *pdev = dev_priv->drm.pdev;
> @@ -1748,7 +1748,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (ret < 0)
>  		goto out_cleanup_mmio;
>  
> -	ret = i915_load_modeset_init(&dev_priv->drm);
> +	ret = i915_modeset_load(&dev_priv->drm);
>  	if (ret < 0)
>  		goto out_cleanup_hw;

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

Re: [PATCH 2/5] drm/i915: Rename i915_load_modeset_init() to i915_modeset_load()

[Lucas De Marchi]

On Mon, Mar 04, 2019 at 01:00:40PM +0200, Jani Nikula wrote:
>On Fri, 01 Mar 2019, José Roberto de Souza <jose.souza@intel.com> wrote:
>> i915_load_modeset_init() sounds horrible also lets rename it so
>> the future cleanup function of it can be easially recognized.
>
>We load the driver, but init modeset. The name implies modeset init at
>driver load.
>
>The modeset load/unload sounds a bit odd to me. Even more so with
>begin/finish load and begin unload. It's not obvious to me what those
>should do. They're not a pattern we have.
>
>I'm not all that averse to renaming stuff in general, but any rename
>increases the cognitive burden for a while, and should make things
>*easier* to understand. I don't think that's the case in this series.
>
>I can understand i915_load_modeset_init sounding funny. If you insist on
>renaming, I'd go with i915_driver_modeset_init. It's in line with the
>rest of the functions.
>
>Here are my suggestions for the names:
>
>i915_modeset_load/i915_modeset_begin_load
>-> i915_driver_modeset_init
>
>i915_modeset_unload/i915_modeset_begin_unload
>-> i915_driver_modeset_cleanup or _fini
>
>i915_modeset_finish_load
>-> i915_driver_modeset_init_late

+1 for the name suggestions

Lucas De Marchi

>
>
>BR,
>Jani.
>
>
>>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index c08abdef5eb6..90c77fab3d70 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -639,7 +639,7 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
>>  	.can_switch = i915_switcheroo_can_switch,
>>  };
>>
>> -static int i915_load_modeset_init(struct drm_device *dev)
>> +static int i915_modeset_load(struct drm_device *dev)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>  	struct pci_dev *pdev = dev_priv->drm.pdev;
>> @@ -1748,7 +1748,7 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	if (ret < 0)
>>  		goto out_cleanup_mmio;
>>
>> -	ret = i915_load_modeset_init(&dev_priv->drm);
>> +	ret = i915_modeset_load(&dev_priv->drm);
>>  	if (ret < 0)
>>  		goto out_cleanup_hw;
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/5] drm/i915: Add a cleanup function for i915_modeset_load()

[José Roberto de Souza]

Lets make i915_driver_unload() easier to read by starting to move
components initialized by i915_modeset_load() to
i915_modeset_unload().

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 27 ++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_display.c |  2 --
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 90c77fab3d70..cc07259ec946 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -639,6 +639,23 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
 	.can_switch = i915_switcheroo_can_switch,
 };
 
+static void i915_modeset_unload(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+
+	intel_modeset_cleanup(dev);
+
+	intel_teardown_gmbus(dev_priv);
+
+	intel_bios_cleanup(dev_priv);
+
+	vga_switcheroo_unregister_client(pdev);
+	vga_client_register(pdev, NULL, NULL, NULL);
+
+	intel_csr_ucode_fini(dev_priv);
+}
+
 static int i915_modeset_load(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -1778,7 +1795,6 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 void i915_driver_unload(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct pci_dev *pdev = dev_priv->drm.pdev;
 
 	disable_rpm_wakeref_asserts(dev_priv);
 
@@ -1794,14 +1810,7 @@ void i915_driver_unload(struct drm_device *dev)
 
 	intel_gvt_cleanup(dev_priv);
 
-	intel_modeset_cleanup(dev);
-
-	intel_bios_cleanup(dev_priv);
-
-	vga_switcheroo_unregister_client(pdev);
-	vga_client_register(pdev, NULL, NULL, NULL);
-
-	intel_csr_ucode_fini(dev_priv);
+	i915_modeset_unload(dev);
 
 	/* Free error state after interrupts are fully disabled. */
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 91a8ee611b12..7963348f1c64 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16393,8 +16393,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	intel_overlay_cleanup(dev_priv);
 
-	intel_teardown_gmbus(dev_priv);
-
 	destroy_workqueue(dev_priv->modeset_wq);
 
 	intel_fbc_cleanup_cfb(dev_priv);
-- 
2.21.0

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

[PATCH 4/5] drm/i915: Move rawclck, power_domain and irq un/initialization from modeset functions

[José Roberto de Souza]

The initialization of those componentes is required by the GEM/GT not
only display so lets move then to a more the appropriate place.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      | 39 ++++++++++++++++------------
 drivers/gpu/drm/i915/intel_display.c |  7 -----
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index cc07259ec946..2b5ce764e694 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -691,24 +691,15 @@ static int i915_modeset_load(struct drm_device *dev)
 	if (ret)
 		goto cleanup_vga_client;
 
-	/* must happen before intel_power_domains_init_hw() on VLV/CHV */
-	intel_update_rawclk(dev_priv);
-
-	intel_power_domains_init_hw(dev_priv, false);
-
 	intel_csr_ucode_init(dev_priv);
 
-	ret = intel_irq_install(dev_priv);
-	if (ret)
-		goto cleanup_csr;
-
 	intel_setup_gmbus(dev_priv);
 
 	/* Important: The output setup functions called by modeset_init need
 	 * working irqs for e.g. gmbus and dp aux transfers. */
 	ret = intel_modeset_init(dev);
 	if (ret)
-		goto cleanup_irq;
+		goto cleanup_gmbus;
 
 	ret = i915_gem_init(dev_priv);
 	if (ret)
@@ -736,12 +727,9 @@ static int i915_modeset_load(struct drm_device *dev)
 	i915_gem_fini(dev_priv);
 cleanup_modeset:
 	intel_modeset_cleanup(dev);
-cleanup_irq:
-	drm_irq_uninstall(dev);
+cleanup_gmbus:
 	intel_teardown_gmbus(dev_priv);
-cleanup_csr:
 	intel_csr_ucode_fini(dev_priv);
-	intel_power_domains_fini_hw(dev_priv);
 	vga_switcheroo_unregister_client(pdev);
 cleanup_vga_client:
 	vga_client_register(pdev, NULL, NULL, NULL);
@@ -1765,9 +1753,18 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret < 0)
 		goto out_cleanup_mmio;
 
+	/* must happen before intel_power_domains_init_hw() on VLV/CHV */
+	intel_update_rawclk(dev_priv);
+
+	intel_power_domains_init_hw(dev_priv, false);
+
+	ret = intel_irq_install(dev_priv);
+	if (ret)
+		goto out_cleanup_power;
+
 	ret = i915_modeset_load(&dev_priv->drm);
 	if (ret < 0)
-		goto out_cleanup_hw;
+		goto out_cleanup_irq;
 
 	i915_driver_register(dev_priv);
 
@@ -1777,7 +1774,10 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	return 0;
 
-out_cleanup_hw:
+out_cleanup_irq:
+	drm_irq_uninstall(&dev_priv->drm);
+out_cleanup_power:
+	intel_power_domains_fini_hw(dev_priv);
 	i915_driver_cleanup_hw(dev_priv);
 out_cleanup_mmio:
 	i915_driver_cleanup_mmio(dev_priv);
@@ -1810,6 +1810,13 @@ void i915_driver_unload(struct drm_device *dev)
 
 	intel_gvt_cleanup(dev_priv);
 
+	/*
+	 * Interrupts and polling as the first thing to avoid creating havoc.
+	 * Too much stuff here (turning of connectors, ...) would
+	 * experience fancy races otherwise.
+	 */
+	intel_irq_uninstall(dev_priv);
+
 	i915_modeset_unload(dev);
 
 	/* Free error state after interrupts are fully disabled. */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7963348f1c64..5158e8ecb9ed 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16364,13 +16364,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	flush_work(&dev_priv->atomic_helper.free_work);
 	WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
 
-	/*
-	 * Interrupts and polling as the first thing to avoid creating havoc.
-	 * Too much stuff here (turning of connectors, ...) would
-	 * experience fancy races otherwise.
-	 */
-	intel_irq_uninstall(dev_priv);
-
 	/*
 	 * Due to the hpd irq storm handling the hotplug work can re-arm the
 	 * poll handlers. Hence disable polling after hpd handling is shut down.
-- 
2.21.0

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

Re: [PATCH 4/5] drm/i915: Move rawclck, power_domain and irq un/initialization from modeset functions

[Ville Syrjälä]

On Fri, Mar 01, 2019 at 04:49:34PM -0800, José Roberto de Souza wrote:
> The initialization of those componentes is required by the GEM/GT not
> only display so lets move then to a more the appropriate place.
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      | 39 ++++++++++++++++------------
>  drivers/gpu/drm/i915/intel_display.c |  7 -----
>  2 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index cc07259ec946..2b5ce764e694 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -691,24 +691,15 @@ static int i915_modeset_load(struct drm_device *dev)
>  	if (ret)
>  		goto cleanup_vga_client;
>  
> -	/* must happen before intel_power_domains_init_hw() on VLV/CHV */
> -	intel_update_rawclk(dev_priv);
> -
> -	intel_power_domains_init_hw(dev_priv, false);
> -
>  	intel_csr_ucode_init(dev_priv);
>  
> -	ret = intel_irq_install(dev_priv);
> -	if (ret)
> -		goto cleanup_csr;
> -
>  	intel_setup_gmbus(dev_priv);
>  
>  	/* Important: The output setup functions called by modeset_init need
>  	 * working irqs for e.g. gmbus and dp aux transfers. */
>  	ret = intel_modeset_init(dev);
>  	if (ret)
> -		goto cleanup_irq;
> +		goto cleanup_gmbus;
>  
>  	ret = i915_gem_init(dev_priv);
>  	if (ret)
> @@ -736,12 +727,9 @@ static int i915_modeset_load(struct drm_device *dev)
>  	i915_gem_fini(dev_priv);
>  cleanup_modeset:
>  	intel_modeset_cleanup(dev);
> -cleanup_irq:
> -	drm_irq_uninstall(dev);
> +cleanup_gmbus:
>  	intel_teardown_gmbus(dev_priv);
> -cleanup_csr:
>  	intel_csr_ucode_fini(dev_priv);
> -	intel_power_domains_fini_hw(dev_priv);
>  	vga_switcheroo_unregister_client(pdev);
>  cleanup_vga_client:
>  	vga_client_register(pdev, NULL, NULL, NULL);
> @@ -1765,9 +1753,18 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	if (ret < 0)
>  		goto out_cleanup_mmio;
>  
> +	/* must happen before intel_power_domains_init_hw() on VLV/CHV */
> +	intel_update_rawclk(dev_priv);
> +
> +	intel_power_domains_init_hw(dev_priv, false);
> +
> +	ret = intel_irq_install(dev_priv);
> +	if (ret)
> +		goto out_cleanup_power;

What are the steps we're reordering wrt. irq enable and
why is it OK to reorder them?

> +
>  	ret = i915_modeset_load(&dev_priv->drm);
>  	if (ret < 0)
> -		goto out_cleanup_hw;
> +		goto out_cleanup_irq;
>  
>  	i915_driver_register(dev_priv);
>  
> @@ -1777,7 +1774,10 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	return 0;
>  
> -out_cleanup_hw:
> +out_cleanup_irq:
> +	drm_irq_uninstall(&dev_priv->drm);
> +out_cleanup_power:
> +	intel_power_domains_fini_hw(dev_priv);
>  	i915_driver_cleanup_hw(dev_priv);
>  out_cleanup_mmio:
>  	i915_driver_cleanup_mmio(dev_priv);
> @@ -1810,6 +1810,13 @@ void i915_driver_unload(struct drm_device *dev)
>  
>  	intel_gvt_cleanup(dev_priv);
>  
> +	/*
> +	 * Interrupts and polling as the first thing to avoid creating havoc.
> +	 * Too much stuff here (turning of connectors, ...) would
> +	 * experience fancy races otherwise.
> +	 */
> +	intel_irq_uninstall(dev_priv);

This too seems slightly questionable considering the flush_workqueue()
etc. in intel_modeset_cleanup(). Can we be sure all modeset activity has
in fact finished sufficiently to no require interrupts?

> +
>  	i915_modeset_unload(dev);
>  
>  	/* Free error state after interrupts are fully disabled. */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7963348f1c64..5158e8ecb9ed 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -16364,13 +16364,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
>  	flush_work(&dev_priv->atomic_helper.free_work);
>  	WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
>  
> -	/*
> -	 * Interrupts and polling as the first thing to avoid creating havoc.
> -	 * Too much stuff here (turning of connectors, ...) would
> -	 * experience fancy races otherwise.
> -	 */
> -	intel_irq_uninstall(dev_priv);
> -
>  	/*
>  	 * Due to the hpd irq storm handling the hotplug work can re-arm the
>  	 * poll handlers. Hence disable polling after hpd handling is shut down.
> -- 
> 2.21.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 4/5] drm/i915: Move rawclck, power_domain and irq un/initialization from modeset functions

[Jani Nikula]

On Mon, 04 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Mar 01, 2019 at 04:49:34PM -0800, José Roberto de Souza wrote:
>> The initialization of those componentes is required by the GEM/GT not
>> only display so lets move then to a more the appropriate place.
>> 
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c      | 39 ++++++++++++++++------------
>>  drivers/gpu/drm/i915/intel_display.c |  7 -----
>>  2 files changed, 23 insertions(+), 23 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index cc07259ec946..2b5ce764e694 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -691,24 +691,15 @@ static int i915_modeset_load(struct drm_device *dev)
>>  	if (ret)
>>  		goto cleanup_vga_client;
>>  
>> -	/* must happen before intel_power_domains_init_hw() on VLV/CHV */
>> -	intel_update_rawclk(dev_priv);
>> -
>> -	intel_power_domains_init_hw(dev_priv, false);
>> -
>>  	intel_csr_ucode_init(dev_priv);
>>  
>> -	ret = intel_irq_install(dev_priv);
>> -	if (ret)
>> -		goto cleanup_csr;
>> -
>>  	intel_setup_gmbus(dev_priv);
>>  
>>  	/* Important: The output setup functions called by modeset_init need
>>  	 * working irqs for e.g. gmbus and dp aux transfers. */
>>  	ret = intel_modeset_init(dev);
>>  	if (ret)
>> -		goto cleanup_irq;
>> +		goto cleanup_gmbus;
>>  
>>  	ret = i915_gem_init(dev_priv);
>>  	if (ret)
>> @@ -736,12 +727,9 @@ static int i915_modeset_load(struct drm_device *dev)
>>  	i915_gem_fini(dev_priv);
>>  cleanup_modeset:
>>  	intel_modeset_cleanup(dev);
>> -cleanup_irq:
>> -	drm_irq_uninstall(dev);
>> +cleanup_gmbus:
>>  	intel_teardown_gmbus(dev_priv);
>> -cleanup_csr:
>>  	intel_csr_ucode_fini(dev_priv);
>> -	intel_power_domains_fini_hw(dev_priv);
>>  	vga_switcheroo_unregister_client(pdev);
>>  cleanup_vga_client:
>>  	vga_client_register(pdev, NULL, NULL, NULL);
>> @@ -1765,9 +1753,18 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	if (ret < 0)
>>  		goto out_cleanup_mmio;
>>  
>> +	/* must happen before intel_power_domains_init_hw() on VLV/CHV */
>> +	intel_update_rawclk(dev_priv);
>> +
>> +	intel_power_domains_init_hw(dev_priv, false);
>> +
>> +	ret = intel_irq_install(dev_priv);
>> +	if (ret)
>> +		goto out_cleanup_power;
>
> What are the steps we're reordering wrt. irq enable and
> why is it OK to reorder them?

I keep thinking we should have preconditions within the functions to
ensure we don't inadvertently break anything. We have some comments,
like above, but nowhere near enough, and they don't jump out at you if
something goes wrong.

BR,
Jani.


>
>> +
>>  	ret = i915_modeset_load(&dev_priv->drm);
>>  	if (ret < 0)
>> -		goto out_cleanup_hw;
>> +		goto out_cleanup_irq;
>>  
>>  	i915_driver_register(dev_priv);
>>  
>> @@ -1777,7 +1774,10 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  
>>  	return 0;
>>  
>> -out_cleanup_hw:
>> +out_cleanup_irq:
>> +	drm_irq_uninstall(&dev_priv->drm);
>> +out_cleanup_power:
>> +	intel_power_domains_fini_hw(dev_priv);
>>  	i915_driver_cleanup_hw(dev_priv);
>>  out_cleanup_mmio:
>>  	i915_driver_cleanup_mmio(dev_priv);
>> @@ -1810,6 +1810,13 @@ void i915_driver_unload(struct drm_device *dev)
>>  
>>  	intel_gvt_cleanup(dev_priv);
>>  
>> +	/*
>> +	 * Interrupts and polling as the first thing to avoid creating havoc.
>> +	 * Too much stuff here (turning of connectors, ...) would
>> +	 * experience fancy races otherwise.
>> +	 */
>> +	intel_irq_uninstall(dev_priv);
>
> This too seems slightly questionable considering the flush_workqueue()
> etc. in intel_modeset_cleanup(). Can we be sure all modeset activity has
> in fact finished sufficiently to no require interrupts?
>
>> +
>>  	i915_modeset_unload(dev);
>>  
>>  	/* Free error state after interrupts are fully disabled. */
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 7963348f1c64..5158e8ecb9ed 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -16364,13 +16364,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
>>  	flush_work(&dev_priv->atomic_helper.free_work);
>>  	WARN_ON(!llist_empty(&dev_priv->atomic_helper.free_list));
>>  
>> -	/*
>> -	 * Interrupts and polling as the first thing to avoid creating havoc.
>> -	 * Too much stuff here (turning of connectors, ...) would
>> -	 * experience fancy races otherwise.
>> -	 */
>> -	intel_irq_uninstall(dev_priv);
>> -
>>  	/*
>>  	 * Due to the hpd irq storm handling the hotplug work can re-arm the
>>  	 * poll handlers. Hence disable polling after hpd handling is shut down.
>> -- 
>> 2.21.0
>> 
>> _______________________________________________
>> 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

[PATCH 5/5] drm/i915: Extract gem_init() from modeset_load()

[José Roberto de Souza]

modeset_load() is definally not the right place to initialize gem but
it is there because it should only be called after intel_mode_init()
as one of the tasks of this function is check if BIOS have already
allocated a framebuffer and if so reuse it to accomplish a smooth
boot transition.

So here it is spliting the loading into two functions and
initializing gem between those functions.

Also renaming i915_modeset_unload() to i915_modeset_begin_unload()
as so far it is only doing this job and this way it can be used in
the errors paths of i915_driver_load().

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 58 ++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 2b5ce764e694..f4163a8bb244 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -639,7 +639,7 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
 	.can_switch = i915_switcheroo_can_switch,
 };
 
-static void i915_modeset_unload(struct drm_device *dev)
+static void i915_modeset_begin_unload(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct pci_dev *pdev = dev_priv->drm.pdev;
@@ -656,7 +656,7 @@ static void i915_modeset_unload(struct drm_device *dev)
 	intel_csr_ucode_fini(dev_priv);
 }
 
-static int i915_modeset_load(struct drm_device *dev)
+static int i915_modeset_begin_load(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct pci_dev *pdev = dev_priv->drm.pdev;
@@ -701,9 +701,22 @@ static int i915_modeset_load(struct drm_device *dev)
 	if (ret)
 		goto cleanup_gmbus;
 
-	ret = i915_gem_init(dev_priv);
-	if (ret)
-		goto cleanup_modeset;
+	return 0;
+
+cleanup_gmbus:
+	intel_teardown_gmbus(dev_priv);
+	intel_csr_ucode_fini(dev_priv);
+	vga_switcheroo_unregister_client(pdev);
+cleanup_vga_client:
+	vga_client_register(pdev, NULL, NULL, NULL);
+out:
+	return ret;
+}
+
+static int i915_modeset_finish_load(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	int ret;
 
 	intel_overlay_setup(dev_priv);
 
@@ -712,7 +725,7 @@ static int i915_modeset_load(struct drm_device *dev)
 
 	ret = intel_fbdev_init(dev);
 	if (ret)
-		goto cleanup_gem;
+		return ret;
 
 	/* Only enable hotplug handling once the fbdev is fully set up. */
 	intel_hpd_init(dev_priv);
@@ -720,21 +733,6 @@ static int i915_modeset_load(struct drm_device *dev)
 	intel_init_ipc(dev_priv);
 
 	return 0;
-
-cleanup_gem:
-	if (i915_gem_suspend(dev_priv))
-		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
-	i915_gem_fini(dev_priv);
-cleanup_modeset:
-	intel_modeset_cleanup(dev);
-cleanup_gmbus:
-	intel_teardown_gmbus(dev_priv);
-	intel_csr_ucode_fini(dev_priv);
-	vga_switcheroo_unregister_client(pdev);
-cleanup_vga_client:
-	vga_client_register(pdev, NULL, NULL, NULL);
-out:
-	return ret;
 }
 
 static int i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
@@ -1762,10 +1760,18 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (ret)
 		goto out_cleanup_power;
 
-	ret = i915_modeset_load(&dev_priv->drm);
+	ret = i915_modeset_begin_load(&dev_priv->drm);
 	if (ret < 0)
 		goto out_cleanup_irq;
 
+	ret = i915_gem_init(dev_priv);
+	if (ret)
+		goto out_cleanup_modeset_begin_load;
+
+	ret = i915_modeset_finish_load(&dev_priv->drm);
+	if (ret < 0)
+		goto out_cleanup_gem;
+
 	i915_driver_register(dev_priv);
 
 	enable_rpm_wakeref_asserts(dev_priv);
@@ -1774,6 +1780,12 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	return 0;
 
+out_cleanup_gem:
+	if (i915_gem_suspend(dev_priv))
+		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
+	i915_gem_fini(dev_priv);
+out_cleanup_modeset_begin_load:
+	i915_modeset_begin_unload(&dev_priv->drm);
 out_cleanup_irq:
 	drm_irq_uninstall(&dev_priv->drm);
 out_cleanup_power:
@@ -1817,7 +1829,7 @@ void i915_driver_unload(struct drm_device *dev)
 	 */
 	intel_irq_uninstall(dev_priv);
 
-	i915_modeset_unload(dev);
+	i915_modeset_begin_unload(dev);
 
 	/* Free error state after interrupts are fully disabled. */
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
-- 
2.21.0

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

✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/vlv: Move czclk to intel_pm

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/i915/vlv: Move czclk to intel_pm
URL   : https://patchwork.freedesktop.org/series/57453/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5683 -> Patchwork_12351
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@kms_busy@basic-flip-c:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] +20

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       NOTRUN -> INCOMPLETE [fdo#107718]

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720] / [fdo#109799]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#109799]: https://bugs.freedesktop.org/show_bug.cgi?id=109799


Participating hosts (45 -> 39)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5683 -> Patchwork_12351

  CI_DRM_5683: 40251405bb454b06259738bcebf4529c888f7fe0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4866: 189956af183c245eb237b3be4fa22953ec93bbe0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12351: 06211158028af0eb8e8ce1ef8b71889253e139ba @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

06211158028a drm/i915: Extract gem_init() from modeset_load()
ada5cd59f87a drm/i915: Move rawclck, power_domain and irq un/initialization from modeset functions
fd3e84069434 drm/i915: Add a cleanup function for i915_modeset_load()
1a0f043cb3e3 drm/i915: Rename i915_load_modeset_init() to i915_modeset_load()
7fd936317d66 drm/i915/vlv: Move czclk to intel_pm

== Logs ==

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

✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/vlv: Move czclk to intel_pm

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/i915/vlv: Move czclk to intel_pm
URL   : https://patchwork.freedesktop.org/series/57453/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5683_full -> Patchwork_12351_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12351_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12351_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_12351_full:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@modeset-stress-extra-wait:
    - shard-iclb:         PASS -> INCOMPLETE

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_create@stolen-invalid-flag:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277]

  * igt@gem_ctx_isolation@rcs0-dirty-create:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109281] +1

  * igt@gem_exec_schedule@preempt-other-chain-bsd2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +2

  * igt@gem_mocs_settings@mocs-settings-vebox:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109287] +1

  * igt@gem_userptr_blits@process-exit-gtt:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +20

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#107807] +1

  * igt@i915_pm_rpm@gem-mmap-gtt:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724]

  * igt@i915_pm_rpm@universal-planes:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#108654] / [fdo#108756]

  * igt@kms_atomic_transition@2x-modeset-transitions:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +4

  * igt@kms_atomic_transition@4x-modeset-transitions-nonblocking-fencing:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +5

  * igt@kms_busy@extended-modeset-hang-newfb-render-f:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-oldfb-render-f:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278]

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-e:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-e:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-512x512-dpms:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109279]

  * igt@kms_fbcon_fbt@fbc:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#109593]

  * igt@kms_flip@2x-nonexisting-fb:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +3

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-iclb:         PASS -> FAIL [fdo#105363]

  * igt@kms_flip@modeset-vs-vblank-race-interruptible:
    - shard-apl:          PASS -> FAIL [fdo#103060]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-plflip-blt:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-shrfb-plflip-blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +7

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167]

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - shard-apl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815]

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@perf_pmu@busy-accuracy-50-vcs1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +47

  * igt@prime_busy@wait-after-bsd:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +30

  * igt@prime_nv_api@i915_nv_double_export:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +1

  * igt@prime_nv_test@i915_nv_sharing:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291]

  * igt@tools_test@sysfs_l3_parity:
    - shard-hsw:          PASS -> SKIP [fdo#109271]

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  * igt@gem_mmap_wc@set-cache-level:
    - shard-snb:          SKIP [fdo#109271] -> PASS +3

  * igt@i915_pm_rpm@fences:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +4

  * igt@i915_pm_rpm@legacy-planes:
    - shard-iclb:         DMESG-WARN [fdo#108654] -> PASS

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-hsw:          FAIL [fdo#102887] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_flip@flip-vs-panning-vs-hang:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_flip@plain-flip-fb-recreate:
    - shard-skl:          FAIL [fdo#100368] -> PASS

  * igt@kms_flip_tiling@flip-changes-tiling:
    - shard-skl:          FAIL [fdo#108303] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +2

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
    - shard-skl:          FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-none:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2
    - shard-iclb:         FAIL [fdo#103166] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          DMESG-FAIL [fdo#105763] -> PASS

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  
#### Warnings ####

  * igt@i915_pm_rpm@pc8-residency:
    - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP [fdo#109271] +1

  * igt@runner@aborted:
    - shard-iclb:         FAIL [fdo#108654] -> ( 2 FAIL ) [fdo#108654] / [fdo#109593]

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

  [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#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108756]: https://bugs.freedesktop.org/show_bug.cgi?id=108756
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109281]: https://bugs.freedesktop.org/show_bug.cgi?id=109281
  [fdo#109287]: https://bugs.freedesktop.org/show_bug.cgi?id=109287
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5683 -> Patchwork_12351

  CI_DRM_5683: 40251405bb454b06259738bcebf4529c888f7fe0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4866: 189956af183c245eb237b3be4fa22953ec93bbe0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12351: 06211158028af0eb8e8ce1ef8b71889253e139ba @ 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_12351/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/5] drm/i915/vlv: Move czclk to intel_pm

[Ville Syrjälä]

On Fri, Mar 01, 2019 at 04:49:31PM -0800, José Roberto de Souza wrote:
> Moving VLV/CHV/BYT czclk to intel_pm as it is a core clock used as
> base by several other GPU blocks including GT.
> 
> BSpec: 14370
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 ------------
>  drivers/gpu/drm/i915/intel_pm.c      | 10 ++++++++++
>  2 files changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7c5e84ef5171..91a8ee611b12 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -180,17 +180,6 @@ int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
>  				 dev_priv->hpll_freq);
>  }
>  
> -static void intel_update_czclk(struct drm_i915_private *dev_priv)
> -{
> -	if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
> -		return;
> -
> -	dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
> -						      CCK_CZ_CLOCK_CONTROL);
> -
> -	DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> -}
> -
>  static inline u32 /* units of 100MHz */
>  intel_fdi_link_freq(struct drm_i915_private *dev_priv,
>  		    const struct intel_crtc_state *pipe_config)
> @@ -15533,7 +15522,6 @@ int intel_modeset_init(struct drm_device *dev)
>  	intel_shared_dpll_init(dev);
>  	intel_update_fdi_pll_freq(dev_priv);
>  
> -	intel_update_czclk(dev_priv);
>  	intel_modeset_init_hw(dev);

Is is here because intel_modeset_init_hw() needs it.

>  
>  	intel_hdcp_component_init(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 9c97a95c1816..cd363fa47cbc 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -7622,6 +7622,14 @@ static void vlv_init_gpll_ref_freq(struct drm_i915_private *dev_priv)
>  			 dev_priv->gt_pm.rps.gpll_ref_freq);
>  }
>  
> +static void vlv_update_czclk(struct drm_i915_private *dev_priv)
> +{
> +	dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
> +						      CCK_CZ_CLOCK_CONTROL);
> +
> +	DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> +}
> +
>  static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_rps *rps = &dev_priv->gt_pm.rps;
> @@ -7629,6 +7637,7 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  
>  	valleyview_setup_pctx(dev_priv);
>  
> +	vlv_update_czclk(dev_priv);

This is far too late AFAICS. Also putting it into gt/gem specific code
is equally wrong as having it in the modeset code.

>  	vlv_init_gpll_ref_freq(dev_priv);
>  
>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> @@ -7675,6 +7684,7 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  
>  	cherryview_setup_pctx(dev_priv);
>  
> +	vlv_update_czclk(dev_priv);
>  	vlv_init_gpll_ref_freq(dev_priv);
>  
>  	mutex_lock(&dev_priv->sb_lock);
> -- 
> 2.21.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 1/5] drm/i915/vlv: Move czclk to intel_pm

[Ville Syrjälä]

On Mon, Mar 04, 2019 at 06:57:15PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 01, 2019 at 04:49:31PM -0800, José Roberto de Souza wrote:
> > Moving VLV/CHV/BYT czclk to intel_pm as it is a core clock used as
> > base by several other GPU blocks including GT.
> > 
> > BSpec: 14370
> > 
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 12 ------------
> >  drivers/gpu/drm/i915/intel_pm.c      | 10 ++++++++++
> >  2 files changed, 10 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7c5e84ef5171..91a8ee611b12 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -180,17 +180,6 @@ int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> >  				 dev_priv->hpll_freq);
> >  }
> >  
> > -static void intel_update_czclk(struct drm_i915_private *dev_priv)
> > -{
> > -	if (!(IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)))
> > -		return;
> > -
> > -	dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
> > -						      CCK_CZ_CLOCK_CONTROL);
> > -
> > -	DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> > -}
> > -
> >  static inline u32 /* units of 100MHz */
> >  intel_fdi_link_freq(struct drm_i915_private *dev_priv,
> >  		    const struct intel_crtc_state *pipe_config)
> > @@ -15533,7 +15522,6 @@ int intel_modeset_init(struct drm_device *dev)
> >  	intel_shared_dpll_init(dev);
> >  	intel_update_fdi_pll_freq(dev_priv);
> >  
> > -	intel_update_czclk(dev_priv);
> >  	intel_modeset_init_hw(dev);
> 
> Is is here because intel_modeset_init_hw() needs it.

No, actually it doesn't. Dang, I'm starting to forget the details. It
is needed by modeset code when it comes time to actually reprogam
cdclk.

> 
> >  
> >  	intel_hdcp_component_init(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 9c97a95c1816..cd363fa47cbc 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -7622,6 +7622,14 @@ static void vlv_init_gpll_ref_freq(struct drm_i915_private *dev_priv)
> >  			 dev_priv->gt_pm.rps.gpll_ref_freq);
> >  }
> >  
> > +static void vlv_update_czclk(struct drm_i915_private *dev_priv)
> > +{
> > +	dev_priv->czclk_freq = vlv_get_cck_clock_hpll(dev_priv, "czclk",
> > +						      CCK_CZ_CLOCK_CONTROL);
> > +
> > +	DRM_DEBUG_DRIVER("CZ clock rate: %d kHz\n", dev_priv->czclk_freq);
> > +}
> > +
> >  static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
> >  {
> >  	struct intel_rps *rps = &dev_priv->gt_pm.rps;
> > @@ -7629,6 +7637,7 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
> >  
> >  	valleyview_setup_pctx(dev_priv);
> >  
> > +	vlv_update_czclk(dev_priv);
> 
> This is far too late AFAICS. Also putting it into gt/gem specific code
> is equally wrong as having it in the modeset code.

So this last argument still holds. We should put it into some common
code. Maybe we need some kind of clock_init() thing.

> 
> >  	vlv_init_gpll_ref_freq(dev_priv);
> >  
> >  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
> > @@ -7675,6 +7684,7 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
> >  
> >  	cherryview_setup_pctx(dev_priv);
> >  
> > +	vlv_update_czclk(dev_priv);
> >  	vlv_init_gpll_ref_freq(dev_priv);
> >  
> >  	mutex_lock(&dev_priv->sb_lock);
> > -- 
> > 2.21.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel

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