Re: [Intel-gfx] [PATCH] drm/i915/bxt: Don't save/restore eDP panel power during suspend (v3)
On Wed, 06 Jan 2016, Matt Roper wrote:
> Our attempts save/restore panel power state in i915_suspend.c are
> causing unclaimed register warnings on BXT since the registers for this
> platform differ from older platforms.
>
> The big hammer suspend/resume shouldn't be necessary for PP since the
> connector/encoder hooks should already handle this. In theory we could
> remove this for all platforms, but in practice it's likely that would
> cause some regressions since older platforms with LVDS may have
> incomplete PP handling. For now we'll leave the PCH save/restore alone
> and change the non-PCH branch to only operate on gen <= 4 so that BXT
> and future platforms aren't included.
>
> v2: Typo fix: s/||/&&/
>
> v3: Change non-PCH condition to a gen <= 4 test rather than listing
> VLV/CHV/BXT as specific platforms to exclude; should be more
> future-proof as we add new platforms. (Daniel)
>
> Cc: Vandana Kannan
> Cc: Jani Nikula
> Cc: Daniel Vetter
> Cc: drm-intel-fi...@lists.freedesktop.org
> Signed-off-by: Matt Roper
> ---
> Although it's the direction we want to move, I'm not brave enough to blow away
> the entire PP save/restore for all platforms since I don't have the HW
> necessary to deal with the regressions that might pop up. The consensus on
> IRC
> is that there probably will be a few regressions when we do that, so I'd
> rather
> have people with appropriate platform access make those changes.
This is the first step we want to take anyway.
Reviewed-by: Jani Nikula
>
> drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_suspend.c
> b/drivers/gpu/drm/i915/i915_suspend.c
> index a2aa09c..a8af594 100644
> --- a/drivers/gpu/drm/i915/i915_suspend.c
> +++ b/drivers/gpu/drm/i915/i915_suspend.c
> @@ -49,7 +49,7 @@ static void i915_save_display(struct drm_device *dev)
> dev_priv->regfile.savePP_ON_DELAYS =
> I915_READ(PCH_PP_ON_DELAYS);
> dev_priv->regfile.savePP_OFF_DELAYS =
> I915_READ(PCH_PP_OFF_DELAYS);
> dev_priv->regfile.savePP_DIVISOR = I915_READ(PCH_PP_DIVISOR);
> - } else if (!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
> + } else if (INTEL_INFO(dev)->gen <= 4) {
> dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
> dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
> dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PP_OFF_DELAYS);
> @@ -84,7 +84,7 @@ static void i915_restore_display(struct drm_device *dev)
> I915_WRITE(PCH_PP_OFF_DELAYS,
> dev_priv->regfile.savePP_OFF_DELAYS);
> I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
> I915_WRITE(PCH_PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
> - } else if (!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
> + } else if (INTEL_INFO(dev)->gen <= 4) {
> I915_WRITE(PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
> I915_WRITE(PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
> I915_WRITE(PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: shut up gen8+ SDE irq dmesg noise, again
We still keep getting
[4.249930] [drm:gen8_irq_handler [i915]] *ERROR* The master control
interrupt lied (SDE)!
This reverts
commit 820da7ae46332fa709b171eb7ba57cbd023fa6df
Author: Jani Nikula
Date: Wed Nov 25 16:47:23 2015 +0200
Revert "drm/i915: shut up gen8+ SDE irq dmesg noise"
which in itself is a revert, so this is just doing
commit 97e5eddcc5300a0f59a55248cd243937a8ab
Author: Daniel Vetter
Date: Fri Oct 23 10:56:12 2015 +0200
drm/i915: shut up gen8+ SDE irq dmesg noise
all over again. I'll stop pretending I understand what's going on like I
did when I thought I'd fixed this for good in
commit 6a39d7c986be4fd18eb019e9cdbf774ec36c9f77
Author: Jani Nikula
Date: Wed Nov 25 16:47:22 2015 +0200
drm/i915: fix the SDE irq dmesg warnings properly
Reported-by: Chris Wilson
Reference: http://mid.gmane.org/20151213124945.ga5...@nuc-i3427.alporthouse.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92084
Cc: drm-intel-fi...@lists.freedesktop.org
Fixes: 820da7ae4633 ("Revert "drm/i915: shut up gen8+ SDE irq dmesg noise"")
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_irq.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 3f8c753997ba..fa8afa7860ae 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2414,9 +2414,13 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
spt_irq_handler(dev, pch_iir);
else
cpt_irq_handler(dev, pch_iir);
- } else
- DRM_ERROR("The master control interrupt lied (SDE)!\n");
-
+ } else {
+ /*
+* Like on previous PCH there seems to be something
+* fishy going on with forwarding PCH interrupts.
+*/
+ DRM_DEBUG_DRIVER("The master control interrupt lied
(SDE)!\n");
+ }
}
I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix eDP panel power save/restore
On Wed, 06 Jan 2016, Daniel Vetter wrote: > On Sat, Jan 02, 2016 at 03:41:29PM +0200, Jani Nikula wrote: >> On Thu, 31 Dec 2015, Matt Roper wrote: >> > On Thu, Dec 31, 2015 at 08:31:45AM +0530, Kannan, Vandana wrote: >> >> When I submitted the PPS patch in April, I got an input from Jani to >> >> not make changes in i915_suspend.c as it was to become obsolete. >> >> Below mail for your reference. >> >> >> >> Jani, >> >> Does your initial comment still hold good? >> > >> > I don't think not touching i915_suspend.c at all is an option since that >> > means that the current code will try to read/write registers that simply >> > don't exist on the platform. Based on Jani's comment, maybe we should >> > just be adding BXT to the list of platforms we don't do any PP >> > save/restore on? >> >> Not touching the registers in i915_suspend.c for BXT was what I meant, >> but probably failed at expressing it. The encoder/connector hooks should >> cover this, and if not, should be updated to cover this. The big hammer >> suspend/resume in i915_suspend.c is oblivious to most modeset state and >> is unable to do anything clever. > > Hm, I thought it died already? What's keeping the pp code in > i915_suspend.c on life-support? We just haven't added stuff for new platforms, but the old cruft is still there for old platforms. Haven't been bold enought to nuke it. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Add RPM references in the *_get_hw_state functions
On ma, 2016-01-04 at 17:05 +0200, Ville Syrjälä wrote:
> On Thu, Dec 31, 2015 at 05:52:07PM +0200, Gabriel Feceoru wrote
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2014,15 +2014,18 @@ bool intel_ddi_get_hw_state(struct
> > intel_encoder *encoder,
> > enum intel_display_power_domain power_domain;
> > u32 tmp;
> > int i;
> > + bool ret = false;
> >
> > power_domain = intel_display_port_power_domain(encoder);
> > if (!intel_display_power_is_enabled(dev_priv,
> > power_domain))
> > - return false;
> > + return ret;
>
> I think what we really need here is some kind of
> intel_display_power_get_unless_zero() thing. We need to make sure not
> only that the rpm reference is held when reading out the state, but
> also
> that the relevant power well(s) remain enabled while we're reading
> out
> the state.
>
Once this gets merged to our trees, should be rather easy to add:
PM / runtime: Add new helper for conditional usage count incrementation
https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit
/?h=bleeding-edge&id=a436b6a19f57656a6557439523923d89eb4a880d
Regards, Joonas
>
> > +
> > + intel_runtime_pm_get(dev_priv);
> >
> > tmp = I915_READ(DDI_BUF_CTL(port));
> >
> > if (!(tmp & DDI_BUF_CTL_ENABLE))
> > - return false;
> > + goto out;
> >
> > if (port == PORT_A) {
> > tmp =
> > I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> > @@ -2040,7 +2043,8 @@ bool intel_ddi_get_hw_state(struct
> > intel_encoder *encoder,
> > break;
> > }
> >
> > - return true;
> > + ret = true;
> > + goto out;
> > } else {
> > for (i = TRANSCODER_A; i <= TRANSCODER_C; i++) {
> > tmp = I915_READ(TRANS_DDI_FUNC_CTL(i));
> > @@ -2048,17 +2052,19 @@ bool intel_ddi_get_hw_state(struct
> > intel_encoder *encoder,
> > if ((tmp & TRANS_DDI_PORT_MASK)
> > == TRANS_DDI_SELECT_PORT(port)) {
> > if ((tmp &
> > TRANS_DDI_MODE_SELECT_MASK) == TRANS_DDI_MODE_SELECT_DP_MST)
> > - return false;
> > + goto out;
> >
> > *pipe = i;
> > - return true;
> > + ret = true;
> > + goto out;
> > }
> > }
> > }
> >
> > DRM_DEBUG_KMS("No pipe for ddi port %c found\n",
> > port_name(port));
> > -
> > - return false;
> > +out:
> > + intel_runtime_pm_put(dev_priv);
> > + return ret;
> > }
> >
> > void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> > @@ -2510,7 +2516,10 @@ static bool
> > hsw_ddi_wrpll_get_hw_state(struct drm_i915_private *dev_priv,
> > if (!intel_display_power_is_enabled(dev_priv,
> > POWER_DOMAIN_PLLS))
> > return false;
> >
> > + intel_runtime_pm_get(dev_priv);
> > val = I915_READ(WRPLL_CTL(pll->id));
> > + intel_runtime_pm_put(dev_priv);
> > +
> > hw_state->wrpll = val;
> >
> > return val & WRPLL_PLL_ENABLE;
> > @@ -2525,7 +2534,10 @@ static bool hsw_ddi_spll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> > if (!intel_display_power_is_enabled(dev_priv,
> > POWER_DOMAIN_PLLS))
> > return false;
> >
> > + intel_runtime_pm_get(dev_priv);
> > val = I915_READ(SPLL_CTL);
> > + intel_runtime_pm_put(dev_priv);
> > +
> > hw_state->spll = val;
> >
> > return val & SPLL_PLL_ENABLE;
> > @@ -2644,16 +2656,19 @@ static bool skl_ddi_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> > uint32_t val;
> > unsigned int dpll;
> > const struct skl_dpll_regs *regs = skl_dpll_regs;
> > + bool ret = false;
> >
> > if (!intel_display_power_is_enabled(dev_priv,
> > POWER_DOMAIN_PLLS))
> > - return false;
> > + return ret;
> >
> > /* DPLL0 is not part of the shared DPLLs, so pll->id is 0
> > for DPLL1 */
> > dpll = pll->id + 1;
> >
> > + intel_runtime_pm_get(dev_priv);
> > +
> > val = I915_READ(regs[pll->id].ctl);
> > if (!(val & LCPLL_PLL_ENABLE))
> > - return false;
> > + goto out;
> >
> > val = I915_READ(DPLL_CTRL1);
> > hw_state->ctrl1 = (val >> (dpll * 6)) & 0x3f;
> > @@ -2664,7 +2679,10 @@ static bool skl_ddi_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> > hw_state->cfgcr2 = I915_READ(regs[pll
> > ->id].cfgcr2);
> > }
> >
> > - return true;
> > + ret = true;
> > +out:
> > + intel_runtime_pm_put(dev_priv);
> > + return ret;
> > }
> >
> > static void skl_shared_dplls_init(struct drm_i915_private
> > *dev_priv)
> > @@ -2931,13 +2949,16 @@ static bool bxt_ddi_pll_get_hw_state(struct
> > drm_i915_private *dev_priv,
> > {
> > enum port port = (enum port)pll->id;/* 1:1 port
> > -
[Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
Since
commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä
Date: Fri Nov 27 18:55:26 2015 +0200
drm/i915: Introduce a gmbus power domain
gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.
Cc: Ville Syrjälä
Cc: Patrik Jakobsson
Cc: Imre Deak
Cc: Jani Nikula
Cc: Meelis Roos
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: sta...@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_dma.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b4741d121a74..405aba2ca736 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -396,8 +396,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv);
-
ret = intel_irq_install(dev_priv);
if (ret)
goto cleanup_gem_stolen;
@@ -1025,6 +1023,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_irq_init(dev_priv);
intel_uncore_sanitize(dev);
+ intel_power_domains_init(dev_priv);
+ intel_power_domains_init_hw(dev_priv);
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
@@ -1057,8 +1057,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
goto out_gem_unload;
}
- intel_power_domains_init(dev_priv);
-
ret = i915_load_modeset_init(dev);
if (ret < 0) {
DRM_ERROR("failed to init modeset\n");
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt] core/sighelper: Send SIGUSR1 to everyone in the process group
On Thu, Jan 07, 2016 at 09:07:03AM +, Chris Wilson wrote: > Some stress tests create both the signal helper and a lot of competing > processes. In these tests, the parent is just waiting upon the children, > and the intention is not to keep waking up the waiting parent, but to > keep interrupting the children (as we hope to trigger races in our > kernel code). raise(-pid) sends the signal to all members of the process s/raise/kill/ oops. > group, not just the target pid. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
On Thu, Jan 07, 2016 at 10:10:56AM +0100, Daniel Vetter wrote:
> Since
>
> commit ac9b8236551d1177fd07b56aef9b565d1864420d
> Author: Ville Syrjälä
> Date: Fri Nov 27 18:55:26 2015 +0200
>
> drm/i915: Introduce a gmbus power domain
>
> gmbus also needs the power domain infrastructure right from the start,
> since as soon as we register the i2c controllers someone can use them.
>
> Cc: Ville Syrjälä
> Cc: Patrik Jakobsson
> Cc: Imre Deak
> Cc: Jani Nikula
> Cc: Meelis Roos
> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
> Cc: sta...@vger.kernel.org
> References: http://www.spinics.net/lists/intel-gfx/msg83075.html
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_dma.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index b4741d121a74..405aba2ca736 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -396,8 +396,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
> if (ret)
> goto cleanup_vga_switcheroo;
>
> - intel_power_domains_init_hw(dev_priv);
> -
> ret = intel_irq_install(dev_priv);
> if (ret)
> goto cleanup_gem_stolen;
> @@ -1025,6 +1023,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long flags)
>
> intel_irq_init(dev_priv);
> intel_uncore_sanitize(dev);
> + intel_power_domains_init(dev_priv);
> + intel_power_domains_init_hw(dev_priv);
A long time ago you wished for static/runtime analysis to check the
ordering constraints, so do I :|
>
> /* Try to make sure MCHBAR is enabled before poking at it */
> intel_setup_mchbar(dev);
> @@ -1057,8 +1057,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long flags)
> goto out_gem_unload;
> }
>
> - intel_power_domains_init(dev_priv);
Wouldn't you also have to update the unwind-on-error paths?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: shut up gen8+ SDE irq dmesg noise, again
On Thu, Jan 07, 2016 at 10:29:10AM +0200, Jani Nikula wrote:
> We still keep getting
>
> [4.249930] [drm:gen8_irq_handler [i915]] *ERROR* The master control
> interrupt lied (SDE)!
>
> This reverts
>
> commit 820da7ae46332fa709b171eb7ba57cbd023fa6df
> Author: Jani Nikula
> Date: Wed Nov 25 16:47:23 2015 +0200
>
> Revert "drm/i915: shut up gen8+ SDE irq dmesg noise"
>
> which in itself is a revert, so this is just doing
>
> commit 97e5eddcc5300a0f59a55248cd243937a8ab
> Author: Daniel Vetter
> Date: Fri Oct 23 10:56:12 2015 +0200
>
> drm/i915: shut up gen8+ SDE irq dmesg noise
>
> all over again. I'll stop pretending I understand what's going on like I
> did when I thought I'd fixed this for good in
>
> commit 6a39d7c986be4fd18eb019e9cdbf774ec36c9f77
> Author: Jani Nikula
> Date: Wed Nov 25 16:47:22 2015 +0200
>
> drm/i915: fix the SDE irq dmesg warnings properly
>
> Reported-by: Chris Wilson
> Reference:
> http://mid.gmane.org/20151213124945.ga5...@nuc-i3427.alporthouse.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92084
> Cc: drm-intel-fi...@lists.freedesktop.org
> Fixes: 820da7ae4633 ("Revert "drm/i915: shut up gen8+ SDE irq dmesg noise"")
> Signed-off-by: Jani Nikula
Acked-by: Chris Wilson
(still not convinced that even a debug error message gives us any
insight over and above mmio tracing)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bxt: Don't save/restore eDP panel power during suspend (v3)
On Thu, Jan 07, 2016 at 10:15:00AM +0200, Jani Nikula wrote:
> On Wed, 06 Jan 2016, Matt Roper wrote:
> > Our attempts save/restore panel power state in i915_suspend.c are
> > causing unclaimed register warnings on BXT since the registers for this
> > platform differ from older platforms.
> >
> > The big hammer suspend/resume shouldn't be necessary for PP since the
> > connector/encoder hooks should already handle this. In theory we could
> > remove this for all platforms, but in practice it's likely that would
> > cause some regressions since older platforms with LVDS may have
> > incomplete PP handling. For now we'll leave the PCH save/restore alone
> > and change the non-PCH branch to only operate on gen <= 4 so that BXT
> > and future platforms aren't included.
> >
> > v2: Typo fix: s/||/&&/
> >
> > v3: Change non-PCH condition to a gen <= 4 test rather than listing
> > VLV/CHV/BXT as specific platforms to exclude; should be more
> > future-proof as we add new platforms. (Daniel)
> >
> > Cc: Vandana Kannan
> > Cc: Jani Nikula
> > Cc: Daniel Vetter
> > Cc: drm-intel-fi...@lists.freedesktop.org
> > Signed-off-by: Matt Roper
> > ---
> > Although it's the direction we want to move, I'm not brave enough to blow
> > away
> > the entire PP save/restore for all platforms since I don't have the HW
> > necessary to deal with the regressions that might pop up. The consensus on
> > IRC
> > is that there probably will be a few regressions when we do that, so I'd
> > rather
> > have people with appropriate platform access make those changes.
>
> This is the first step we want to take anyway.
>
> Reviewed-by: Jani Nikula
Just looked at this some more, and the code here is the only code that
restores PP registers. Except for vlv/chv, where we have a call to
intel_dp_init_panel_power_sequencer_registers in vlv_power_sequencer_pipe.
I think we first need to sprinkle more calls to restore PP registers
around before we can disable this here for bxt. This patch here just
papers over a legit bug, without fixing it really.
-Daniel
>
>
>
> >
> > drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c
> > b/drivers/gpu/drm/i915/i915_suspend.c
> > index a2aa09c..a8af594 100644
> > --- a/drivers/gpu/drm/i915/i915_suspend.c
> > +++ b/drivers/gpu/drm/i915/i915_suspend.c
> > @@ -49,7 +49,7 @@ static void i915_save_display(struct drm_device *dev)
> > dev_priv->regfile.savePP_ON_DELAYS =
> > I915_READ(PCH_PP_ON_DELAYS);
> > dev_priv->regfile.savePP_OFF_DELAYS =
> > I915_READ(PCH_PP_OFF_DELAYS);
> > dev_priv->regfile.savePP_DIVISOR = I915_READ(PCH_PP_DIVISOR);
> > - } else if (!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
> > + } else if (INTEL_INFO(dev)->gen <= 4) {
> > dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
> > dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
> > dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PP_OFF_DELAYS);
> > @@ -84,7 +84,7 @@ static void i915_restore_display(struct drm_device *dev)
> > I915_WRITE(PCH_PP_OFF_DELAYS,
> > dev_priv->regfile.savePP_OFF_DELAYS);
> > I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
> > I915_WRITE(PCH_PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
> > - } else if (!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
> > + } else if (INTEL_INFO(dev)->gen <= 4) {
> > I915_WRITE(PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
> > I915_WRITE(PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
> > I915_WRITE(PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915/kbl: Remove preliminary_hw_support protection from Kabylake.
On Wed, Jan 06, 2016 at 05:15:30PM -0800, Rodrigo Vivi wrote:
> The only missing gap compared to Skylake is the GuC
> because we don't have yet a GuC firmware image to publish.
>
> However with all other parts in place this is very similar to
> Skylake which is out of this procection.
>
> So I'm now confident that Kabylake is in a good stage and won't
> cause troubles like blank displays, hard hangs, etc.
>
> Signed-off-by: Rodrigo Vivi
Imo we should have at least 1 kbl in CI before doing this. Do we have
that?
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3ac616d..c8ddf3e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -346,14 +346,12 @@ static const struct intel_device_info
> intel_broxton_info = {
>
> static const struct intel_device_info intel_kabylake_info = {
> HSW_FEATURES,
> - .is_preliminary = 1,
> .is_kabylake = 1,
> .gen = 9,
> };
>
> static const struct intel_device_info intel_kabylake_gt3_info = {
> HSW_FEATURES,
> - .is_preliminary = 1,
> .is_kabylake = 1,
> .gen = 9,
> .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> --
> 2.4.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: shut up gen8+ SDE irq dmesg noise, again
On Thu, Jan 07, 2016 at 10:29:10AM +0200, Jani Nikula wrote:
> We still keep getting
>
> [4.249930] [drm:gen8_irq_handler [i915]] *ERROR* The master control
> interrupt lied (SDE)!
>
> This reverts
>
> commit 820da7ae46332fa709b171eb7ba57cbd023fa6df
> Author: Jani Nikula
> Date: Wed Nov 25 16:47:23 2015 +0200
>
> Revert "drm/i915: shut up gen8+ SDE irq dmesg noise"
>
> which in itself is a revert, so this is just doing
>
> commit 97e5eddcc5300a0f59a55248cd243937a8ab
> Author: Daniel Vetter
> Date: Fri Oct 23 10:56:12 2015 +0200
>
> drm/i915: shut up gen8+ SDE irq dmesg noise
>
> all over again. I'll stop pretending I understand what's going on like I
> did when I thought I'd fixed this for good in
>
> commit 6a39d7c986be4fd18eb019e9cdbf774ec36c9f77
> Author: Jani Nikula
> Date: Wed Nov 25 16:47:22 2015 +0200
>
> drm/i915: fix the SDE irq dmesg warnings properly
>
> Reported-by: Chris Wilson
> Reference:
> http://mid.gmane.org/20151213124945.ga5...@nuc-i3427.alporthouse.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92084
> Cc: drm-intel-fi...@lists.freedesktop.org
> Fixes: 820da7ae4633 ("Revert "drm/i915: shut up gen8+ SDE irq dmesg noise"")
> Signed-off-by: Jani Nikula
Oh well, agreed it's time to give up :(
Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_irq.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 3f8c753997ba..fa8afa7860ae 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2414,9 +2414,13 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
> spt_irq_handler(dev, pch_iir);
> else
> cpt_irq_handler(dev, pch_iir);
> - } else
> - DRM_ERROR("The master control interrupt lied (SDE)!\n");
> -
> + } else {
> + /*
> + * Like on previous PCH there seems to be something
> + * fishy going on with forwarding PCH interrupts.
> + */
> + DRM_DEBUG_DRIVER("The master control interrupt lied
> (SDE)!\n");
> + }
> }
>
> I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bxt: Don't save/restore eDP panel power during suspend (v3)
On Thu, 07 Jan 2016, Daniel Vetter wrote:
> On Thu, Jan 07, 2016 at 10:15:00AM +0200, Jani Nikula wrote:
>> On Wed, 06 Jan 2016, Matt Roper wrote:
>> > Our attempts save/restore panel power state in i915_suspend.c are
>> > causing unclaimed register warnings on BXT since the registers for this
>> > platform differ from older platforms.
>> >
>> > The big hammer suspend/resume shouldn't be necessary for PP since the
>> > connector/encoder hooks should already handle this. In theory we could
>> > remove this for all platforms, but in practice it's likely that would
>> > cause some regressions since older platforms with LVDS may have
>> > incomplete PP handling. For now we'll leave the PCH save/restore alone
>> > and change the non-PCH branch to only operate on gen <= 4 so that BXT
>> > and future platforms aren't included.
>> >
>> > v2: Typo fix: s/||/&&/
>> >
>> > v3: Change non-PCH condition to a gen <= 4 test rather than listing
>> > VLV/CHV/BXT as specific platforms to exclude; should be more
>> > future-proof as we add new platforms. (Daniel)
>> >
>> > Cc: Vandana Kannan
>> > Cc: Jani Nikula
>> > Cc: Daniel Vetter
>> > Cc: drm-intel-fi...@lists.freedesktop.org
>> > Signed-off-by: Matt Roper
>> > ---
>> > Although it's the direction we want to move, I'm not brave enough to blow
>> > away
>> > the entire PP save/restore for all platforms since I don't have the HW
>> > necessary to deal with the regressions that might pop up. The consensus
>> > on IRC
>> > is that there probably will be a few regressions when we do that, so I'd
>> > rather
>> > have people with appropriate platform access make those changes.
>>
>> This is the first step we want to take anyway.
>>
>> Reviewed-by: Jani Nikula
>
> Just looked at this some more, and the code here is the only code that
> restores PP registers. Except for vlv/chv, where we have a call to
> intel_dp_init_panel_power_sequencer_registers in vlv_power_sequencer_pipe.
>
> I think we first need to sprinkle more calls to restore PP registers
> around before we can disable this here for bxt. This patch here just
> papers over a legit bug, without fixing it really.
No, this patch doesn't paper over anything. The code that gets run for
Broxton before this patch doesn't save/restore any meaningful registers
that could possibly help with PP. This fixes unclaimed register
read/writes, and it's a valid fix as such.
The commit message should probably be fixed to say that we're not there
yet for Broxton with the hooks. But IMO this patch is a valid fix and
orthogonal to the issue of fixing the hooks.
BR,
Jani.
> -Daniel
>
>>
>>
>>
>> >
>> > drivers/gpu/drm/i915/i915_suspend.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c
>> > b/drivers/gpu/drm/i915/i915_suspend.c
>> > index a2aa09c..a8af594 100644
>> > --- a/drivers/gpu/drm/i915/i915_suspend.c
>> > +++ b/drivers/gpu/drm/i915/i915_suspend.c
>> > @@ -49,7 +49,7 @@ static void i915_save_display(struct drm_device *dev)
>> >dev_priv->regfile.savePP_ON_DELAYS =
>> > I915_READ(PCH_PP_ON_DELAYS);
>> >dev_priv->regfile.savePP_OFF_DELAYS =
>> > I915_READ(PCH_PP_OFF_DELAYS);
>> >dev_priv->regfile.savePP_DIVISOR = I915_READ(PCH_PP_DIVISOR);
>> > - } else if (!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
>> > + } else if (INTEL_INFO(dev)->gen <= 4) {
>> >dev_priv->regfile.savePP_CONTROL = I915_READ(PP_CONTROL);
>> >dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PP_ON_DELAYS);
>> >dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PP_OFF_DELAYS);
>> > @@ -84,7 +84,7 @@ static void i915_restore_display(struct drm_device *dev)
>> >I915_WRITE(PCH_PP_OFF_DELAYS,
>> > dev_priv->regfile.savePP_OFF_DELAYS);
>> >I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
>> >I915_WRITE(PCH_PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
>> > - } else if (!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)) {
>> > + } else if (INTEL_INFO(dev)->gen <= 4) {
>> >I915_WRITE(PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
>> >I915_WRITE(PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
>> >I915_WRITE(PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/core: Add drm_for_each_encoder_mask, v2.
This is similar to the other drm_for_each_*_mask functions.
Changes since v1:
- Use for_each_if
Signed-off-by: Maarten Lankhorst
---
include/drm/drm_crtc.h | 11 +++
1 file changed, 11 insertions(+)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fd2ace4a18de..c0226f945d62 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2153,6 +2153,17 @@ struct drm_mode_config {
list_for_each_entry((plane), &(dev)->mode_config.plane_list, head) \
for_each_if ((plane_mask) & (1 << drm_plane_index(plane)))
+/**
+ * drm_for_each_encoder_mask - iterate over encoders specified by bitmask
+ * @encoder: the loop cursor
+ * @dev: the DRM device
+ * @encoder_mask: bitmask of encoder indices
+ *
+ * Iterate over all encoders specified by bitmask.
+ */
+#define drm_for_each_encoder_mask(encoder, dev, encoder_mask) \
+ list_for_each_entry((encoder), &(dev)->mode_config.encoder_list, head) \
+ for_each_if ((encoder_mask) & (1 << drm_encoder_index(encoder)))
#define obj_to_crtc(x) container_of(x, struct drm_crtc, base)
#define obj_to_connector(x) container_of(x, struct drm_connector, base)
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/core: Add drm_encoder_index.
This is useful for adding encoder_mask in crtc_state.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/drm_crtc.c | 23 +++
include/drm/drm_crtc.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 62fa95fa5471..b7c990e0c8b4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1161,6 +1161,29 @@ out_unlock:
EXPORT_SYMBOL(drm_encoder_init);
/**
+ * drm_encoder_index - find the index of a registered encoder
+ * @encoder: encoder to find index for
+ *
+ * Given a registered encoder, return the index of that encoder within a DRM
+ * device's list of encoders.
+ */
+unsigned int drm_encoder_index(struct drm_encoder *encoder)
+{
+ unsigned int index = 0;
+ struct drm_encoder *tmp;
+
+ drm_for_each_encoder(tmp, encoder->dev) {
+ if (tmp == encoder)
+ return index;
+
+ index++;
+ }
+
+ BUG();
+}
+EXPORT_SYMBOL(drm_encoder_index);
+
+/**
* drm_encoder_cleanup - cleans up an initialised encoder
* @encoder: encoder to cleanup
*
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c65a212db77e..fd2ace4a18de 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2225,6 +2225,7 @@ int drm_encoder_init(struct drm_device *dev,
struct drm_encoder *encoder,
const struct drm_encoder_funcs *funcs,
int encoder_type, const char *name, ...);
+extern unsigned int drm_encoder_index(struct drm_encoder *encoder);
/**
* drm_encoder_crtc_ok - can a given crtc drive a given encoder?
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/5] Add encoder_mask to crtc_state, v2.
Another attempt at adding encoder_mask, with some behavioral fixes. Maarten Lankhorst (5): drm/core: Add drm_encoder_index. drm/core: Add drm_for_each_encoder_mask, v2. drm/i915: Do not touch best_encoder for load detect. drm/atomic: Do not unset crtc when an encoder is stolen drm/atomic: Add encoder_mask to crtc_state, v2. drivers/gpu/drm/drm_atomic_helper.c | 46 ++-- drivers/gpu/drm/drm_crtc.c | 23 ++ drivers/gpu/drm/i915/intel_display.c | 5 ++-- include/drm/drm_crtc.h | 14 +++ 4 files changed, 79 insertions(+), 9 deletions(-) -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/atomic: Do not unset crtc when an encoder is stolen
While we steal the encoder away from the connector the connector may be updated to use a different encoder. Without this change if 2 connectors swap encoders one of them will end up without a crtc. Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/drm_atomic_helper.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 57cccd68ca52..9c84b3b37631 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -134,7 +134,6 @@ steal_encoder(struct drm_atomic_state *state, struct drm_crtc_state *crtc_state; struct drm_connector *connector; struct drm_connector_state *connector_state; - int ret; /* * We can only steal an encoder coming from a connector, which means we @@ -165,9 +164,6 @@ steal_encoder(struct drm_atomic_state *state, if (IS_ERR(connector_state)) return PTR_ERR(connector_state); - ret = drm_atomic_set_crtc_for_connector(connector_state, NULL); - if (ret) - return ret; connector_state->best_encoder = NULL; } -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 0/3] improve handling of the driver's internal (default) context
During startup, the driver creates a unique "intel_context" that will provide a home for orphan requests (i.e. those generated by the driver internally, not associated with user batchbuffers). However, one of the infelicities of the current code is that the driver keeps a per-engine pointer to the default context for that engine (this is probably from a decision made early in execlists implementation, when it was thought that a context was engine-specific, and not revised when it was decided that an "intel_context" represents a multiplex of engine-level contexts). All these per-engine pointers actually end up pointing to the same unique object. To compound this, the refcounting is bogus; the driver holds just one reference to the default context, even though there are multiple pointers to it (RCS is considered to hold this "on behalf of" the other engines, but this can lead to problems during unload as it makes the code sensitive to the order of engine teardown -- RCS should be done last, but it isn't). Also, some of the execlists batch submission code treats the default context differently from any other; testing for it by comparing against the per-engine pointer is quite clumsy, especially in debugfs where contexts are enumerated from a global list and therefore not automatically known to be associated with a particular engine. Therefore, we should replace the per-engine pointers with a single driver-wide reference, which will make the refcounting sane and simplify the code that uses this context for non-user-related requests. THIS patchset does NOT address the execlists code's special handling of the default context, but it will simplify the planned future cleanup of that code. v3: don't flag the context created during startup (and fully instantiated on all engines) for driver-internal purposes as "is_global_default", 1448630935-27377-1-git-send-email-ch...@chris-wilson.co.uk notwithstanding. [Chris Wilson] (We now call it "the kernel context", and compare against the device-wide pointer). ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 2/3] drm/i915: abolish separate per-ring default_context pointers
Now that we've eliminated a lot of uses of ring->default_context,
we can eliminate the pointer itself.
All the engines share the same default intel_context, so we can just
keep a single reference to it in the dev_priv structure rather than one
in each of the engine[] elements. This make refcounting more sensible
too, as we now have a refcount of one for the one pointer, rather than
a refcount of one but multiple pointers.
From an idea by Chris Wilson.
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/i915_debugfs.c| 4 ++--
drivers/gpu/drm/i915/i915_drv.h| 2 ++
drivers/gpu/drm/i915/i915_gem.c| 6 +++---
drivers/gpu/drm/i915/i915_gem_context.c| 22 --
drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
drivers/gpu/drm/i915/i915_guc_submission.c | 6 +++---
drivers/gpu/drm/i915/intel_lrc.c | 24 +---
drivers/gpu/drm/i915/intel_ringbuffer.h| 1 -
8 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 0fc38bb..2613708 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1943,7 +1943,7 @@ static int i915_context_status(struct seq_file *m, void
*unused)
seq_puts(m, "HW context ");
describe_ctx(m, ctx);
for_each_ring(ring, dev_priv, i) {
- if (ring->default_context == ctx)
+ if (dev_priv->kernel_context == ctx)
seq_printf(m, "(default context %s) ",
ring->name);
}
@@ -2039,7 +2039,7 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
list_for_each_entry(ctx, &dev_priv->context_list, link) {
for_each_ring(ring, dev_priv, i) {
- if (ring->default_context != ctx)
+ if (dev_priv->kernel_context != ctx)
i915_dump_lrc_obj(m, ring,
ctx->engine[i].state);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c2b000a..aef86a8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1940,6 +1940,8 @@ struct drm_i915_private {
void (*stop_ring)(struct intel_engine_cs *ring);
} gt;
+ struct intel_context *kernel_context;
+
bool edp_low_vswing;
/* perform PHY state sanity checks? */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c908ed1..8f101121 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2678,7 +2678,7 @@ void i915_gem_request_free(struct kref *req_ref)
if (ctx) {
if (i915.enable_execlists) {
- if (ctx != req->ring->default_context)
+ if (ctx != req->i915->kernel_context)
intel_lr_context_unpin(req);
}
@@ -2774,7 +2774,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
int err;
if (ctx == NULL)
- ctx = engine->default_context;
+ ctx = to_i915(engine->dev)->kernel_context;
err = __i915_gem_request_alloc(engine, ctx, &req);
return err ? ERR_PTR(err) : req;
}
@@ -4862,7 +4862,7 @@ i915_gem_init_hw(struct drm_device *dev)
*/
init_unused_rings(dev);
- BUG_ON(!dev_priv->ring[RCS].default_context);
+ BUG_ON(!dev_priv->kernel_context);
ret = i915_ppgtt_init_hw(dev);
if (ret) {
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 900ffd0..e1d767e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -354,11 +354,10 @@ int i915_gem_context_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_context *ctx;
- int i;
/* Init should only be called once per module load. Eventually the
* restriction on the context_disabled check can be loosened. */
- if (WARN_ON(dev_priv->ring[RCS].default_context))
+ if (WARN_ON(dev_priv->kernel_context))
return 0;
if (intel_vgpu_active(dev) && HAS_LOGICAL_RING_CONTEXTS(dev)) {
@@ -388,12 +387,7 @@ int i915_gem_context_init(struct drm_device *dev)
return PTR_ERR(ctx);
}
- for (i = 0; i < I915_NUM_RINGS; i++) {
- struct intel_engine_cs *ring = &dev_priv->ring[i];
-
- /* NB: RCS will hold a ref for all rings */
- ring->default_context = ctx;
- }
+ dev_priv->kernel_context = ctx;
DRM_DEBUG_DRIVER("%s context support initialized\n",
i915.enable_execlists ? "LR"
Re: [Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
> commit ac9b8236551d1177fd07b56aef9b565d1864420d
> Author: Ville Syrjälä
> Date: Fri Nov 27 18:55:26 2015 +0200
>
> drm/i915: Introduce a gmbus power domain
>
> gmbus also needs the power domain infrastructure right from the start,
> since as soon as we register the i2c controllers someone can use them.
>
> Cc: Ville Syrjälä
> Cc: Patrik Jakobsson
> Cc: Imre Deak
> Cc: Jani Nikula
> Cc: Meelis Roos
> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
> Cc: sta...@vger.kernel.org
> References: http://www.spinics.net/lists/intel-gfx/msg83075.html
> Signed-off-by: Daniel Vetter
Tested-by: Meelis Roos
Worked fine on my SNB computer.
> ---
> drivers/gpu/drm/i915/i915_dma.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index b4741d121a74..405aba2ca736 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -396,8 +396,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
> if (ret)
> goto cleanup_vga_switcheroo;
>
> - intel_power_domains_init_hw(dev_priv);
> -
> ret = intel_irq_install(dev_priv);
> if (ret)
> goto cleanup_gem_stolen;
> @@ -1025,6 +1023,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long flags)
>
> intel_irq_init(dev_priv);
> intel_uncore_sanitize(dev);
> + intel_power_domains_init(dev_priv);
> + intel_power_domains_init_hw(dev_priv);
>
> /* Try to make sure MCHBAR is enabled before poking at it */
> intel_setup_mchbar(dev);
> @@ -1057,8 +1057,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long flags)
> goto out_gem_unload;
> }
>
> - intel_power_domains_init(dev_priv);
> -
> ret = i915_load_modeset_init(dev);
> if (ret < 0) {
> DRM_ERROR("failed to init modeset\n");
>
--
Meelis Roos (mr...@ut.ee) http://www.cs.ut.ee/~mroos/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/7] drm/i915: Remove some visibility checks from intel_crtc_update_cursor.
This is duplicated with intel_check_cursor_plane, and with all
non-atomic paths removed this should be dead code.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/intel_display.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 63286f16a9e6..35b881d156b0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -85,8 +85,6 @@ static const uint32_t intel_cursor_formats[] = {
DRM_FORMAT_ARGB,
};
-static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
-
static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
static void ironlake_pch_clock_get(struct intel_crtc *crtc,
@@ -10201,25 +10199,13 @@ static void intel_crtc_update_cursor(struct drm_crtc
*crtc,
base = intel_crtc->cursor_addr;
- if (x >= intel_crtc->config->pipe_src_w)
- on = false;
-
- if (y >= intel_crtc->config->pipe_src_h)
- on = false;
-
if (x < 0) {
- if (x + cursor_state->crtc_w <= 0)
- on = false;
-
pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
x = -x;
}
pos |= x << CURSOR_X_SHIFT;
if (y < 0) {
- if (y + cursor_state->crtc_h <= 0)
- on = false;
-
pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
y = -y;
}
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] bisected: i915 modeset broken in ac9b8236551d1177fd07b56aef9b565d1864420d
> > intel_setup_gmbus registers the i2c adapters, which does transfers on > > the i2c bus on probe, and this happens before intel_power_domains_init > > which initializes the power domain lock. > > > > The bisect and backtrace make sense and are not mysterious at all. > > > > Not sure of the fix though, are we better off changing the init order, > > or making sure the probes don't happen or don't screw us up. > > So I started wondering why we are not seeing this. To reproduce, looks > like you'll need to have an i2c driver with class I2C_CLASS_DDC for the > i2c detect (and the bug) to happen. In tree, the only ones seem to be > > drivers/misc/eeprom/eeprom.c Yes, I have that for DIMM SPD information access. > drivers/staging/olpc_dcon/olpc_dcon.c > > I presume you have one or the other. > > No matter what, userspace can access the adapter right away when we > register it, so this needs to be fixed along the lines of [1]. > > BR, > Jani. > > > [1] > http://patchwork.freedesktop.org/patch/msgid/1452157856-27360-1-git-send-email-daniel.vet...@ffwll.ch -- Meelis Roos (mr...@linux.ee) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
Since
commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä
Date: Fri Nov 27 18:55:26 2015 +0200
drm/i915: Introduce a gmbus power domain
gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.
v2: Adjust cleanup paths too (Chris).
Cc: Ville Syrjälä
Cc: Patrik Jakobsson
Cc: Imre Deak
Cc: Jani Nikula
Cc: Meelis Roos
Cc: Chris Wilson
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: sta...@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Tested-by: Meelis Roos
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_dma.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 988a3806512a..490d8b0d931e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv, false);
intel_csr_ucode_init(dev_priv);
@@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_irq_init(dev_priv);
intel_uncore_sanitize(dev);
+ intel_power_domains_init(dev_priv);
+ intel_power_domains_init_hw(dev_priv);
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
@@ -1057,12 +1058,10 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
goto out_gem_unload;
}
- intel_power_domains_init(dev_priv);
-
ret = i915_load_modeset_init(dev);
if (ret < 0) {
DRM_ERROR("failed to init modeset\n");
- goto out_power_well;
+ goto out_vblank;
}
/*
@@ -1091,8 +1090,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
return 0;
-out_power_well:
- intel_power_domains_fini(dev_priv);
+out_vblank:
drm_vblank_cleanup(dev);
out_gem_unload:
WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
@@ -1103,6 +1101,7 @@ out_gem_unload:
intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
+ intel_power_domains_fini(dev_priv);
pm_qos_remove_request(&dev_priv->pm_qos);
destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
out_freedpwq:
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
Hi Daniel, [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on next-20160106] [cannot apply to v4.4-rc8] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-i915-Init-power-domains-early-in-driver-load/20160107-194542 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-x007-01060743 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/gpu/drm/i915/i915_dma.c: In function 'i915_driver_load': >> drivers/gpu/drm/i915/i915_dma.c:1028:2: error: too few arguments to function >> 'intel_power_domains_init_hw' intel_power_domains_init_hw(dev_priv); ^ In file included from drivers/gpu/drm/i915/i915_dma.c:35:0: drivers/gpu/drm/i915/intel_drv.h:1417:6: note: declared here void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume); ^ vim +/intel_power_domains_init_hw +1028 drivers/gpu/drm/i915/i915_dma.c 1022 goto out_freedpwq; 1023 } 1024 1025 intel_irq_init(dev_priv); 1026 intel_uncore_sanitize(dev); 1027 intel_power_domains_init(dev_priv); > 1028 intel_power_domains_init_hw(dev_priv); 1029 1030 /* Try to make sure MCHBAR is enabled before poking at it */ 1031 intel_setup_mchbar(dev); --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: simplify allocation of driver-internal requests
On Thu, Jan 07, 2016 at 10:20:50AM +, Dave Gordon wrote: > There are a number of places where the driver needs a request, but isn't > working on behalf of any specific user or in a specific context. At > present, we associate them with the per-engine default context. A future > patch will abolish those per-engine context pointers; but we can already > eliminate a lot of the references to them, just by making the allocator > allow NULL as a shorthand for "an appropriate context for this ring", > which will mean that the callers don't need to know anything about how > the "appropriate context" is found (e.g. per-ring vs per-device, etc). > > So this patch renames the existing i915_gem_request_alloc(), and makes > it local (static inline), and replaces it with a wrapper that provides > a default if the context is NULL, and also has a nicer calling > convention (doesn't require a pointer to an output parameter). Then we > change all callers to use the new convention: > OLD: > err = i915_gem_request_alloc(ring, user_ctx, &req); > if (err) ... > NEW: > req = i915_gem_request_alloc(ring, user_ctx); > if (IS_ERR(req)) ... > OLD: > err = i915_gem_request_alloc(ring, ring->default_context, &req); > if (err) ... > NEW: > req = i915_gem_request_alloc(ring, NULL); > if (IS_ERR(req)) ... Nak. You haven't fixed i915_gem_request_alloc() at all. http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=breadcrumbs&id=82c72e1a2b4385f0ab07dccee45acef38303e96f is the patch I have been carrying ever since. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
On Thu, 07 Jan 2016, Daniel Vetter wrote:
> Since
>
> commit ac9b8236551d1177fd07b56aef9b565d1864420d
> Author: Ville Syrjälä
> Date: Fri Nov 27 18:55:26 2015 +0200
>
> drm/i915: Introduce a gmbus power domain
>
> gmbus also needs the power domain infrastructure right from the start,
> since as soon as we register the i2c controllers someone can use them.
>
> v2: Adjust cleanup paths too (Chris).
>
> Cc: Ville Syrjälä
> Cc: Patrik Jakobsson
> Cc: Imre Deak
> Cc: Jani Nikula
> Cc: Meelis Roos
> Cc: Chris Wilson
> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
> Cc: sta...@vger.kernel.org
> References: http://www.spinics.net/lists/intel-gfx/msg83075.html
> Tested-by: Meelis Roos
> Signed-off-by: Daniel Vetter
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93608
> ---
> drivers/gpu/drm/i915/i915_dma.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 988a3806512a..490d8b0d931e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
> if (ret)
> goto cleanup_vga_switcheroo;
>
> - intel_power_domains_init_hw(dev_priv, false);
>
> intel_csr_ucode_init(dev_priv);
>
> @@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long flags)
>
> intel_irq_init(dev_priv);
> intel_uncore_sanitize(dev);
> + intel_power_domains_init(dev_priv);
> + intel_power_domains_init_hw(dev_priv);
>
> /* Try to make sure MCHBAR is enabled before poking at it */
> intel_setup_mchbar(dev);
> @@ -1057,12 +1058,10 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long flags)
> goto out_gem_unload;
> }
>
> - intel_power_domains_init(dev_priv);
> -
> ret = i915_load_modeset_init(dev);
> if (ret < 0) {
> DRM_ERROR("failed to init modeset\n");
> - goto out_power_well;
> + goto out_vblank;
> }
>
> /*
> @@ -1091,8 +1090,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long flags)
>
> return 0;
>
> -out_power_well:
> - intel_power_domains_fini(dev_priv);
> +out_vblank:
> drm_vblank_cleanup(dev);
> out_gem_unload:
> WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
> @@ -1103,6 +1101,7 @@ out_gem_unload:
>
> intel_teardown_gmbus(dev);
> intel_teardown_mchbar(dev);
> + intel_power_domains_fini(dev_priv);
> pm_qos_remove_request(&dev_priv->pm_qos);
> destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
> out_freedpwq:
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915: Use passed plane state for sprite planes, v4.
On Thu, Jan 07, 2016 at 11:54:06AM +0100, Maarten Lankhorst wrote:
> Don't use plane->state directly, use the pointer from commit_plane.
>
> Changes since v1:
> - Fix uses of plane->state->rotation and color key to use the passed state
> too.
> - Only pass crtc_state and plane_state to update_plane.
> Changes since v2:
> - Rebased.
> Changes since v3:
> - Small whitespace changes and only assign 1 variable per line.
> - Constify plane_state and crtc_state. (vsyrjala)
>
> Signed-off-by: Maarten Lankhorst
> Cc: Ville Syrjälä
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_drv.h| 10 +--
> drivers/gpu/drm/i915/intel_sprite.c | 124
>
> 2 files changed, 71 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index a3b2025da563..068050884353 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -658,16 +658,12 @@ struct intel_plane {
> /*
>* NOTE: Do not place new plane state fields here (e.g., when adding
>* new plane properties). New runtime state should now be placed in
> - * the intel_plane_state structure and accessed via drm_plane->state.
> + * the intel_plane_state structure and accessed via plane_state.
>*/
>
> void (*update_plane)(struct drm_plane *plane,
> - struct drm_crtc *crtc,
> - struct drm_framebuffer *fb,
> - int crtc_x, int crtc_y,
> - unsigned int crtc_w, unsigned int crtc_h,
> - uint32_t x, uint32_t y,
> - uint32_t src_w, uint32_t src_h);
> + const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state);
> void (*disable_plane)(struct drm_plane *plane,
> struct drm_crtc *crtc);
> int (*check_plane)(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 4ff7a1f4183e..9f64289333e8 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -178,28 +178,33 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
> }
>
> static void
> -skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> - struct drm_framebuffer *fb,
> - int crtc_x, int crtc_y,
> - unsigned int crtc_w, unsigned int crtc_h,
> - uint32_t x, uint32_t y,
> - uint32_t src_w, uint32_t src_h)
> +skl_update_plane(struct drm_plane *drm_plane,
> + const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state *plane_state)
> {
> struct drm_device *dev = drm_plane->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_plane *intel_plane = to_intel_plane(drm_plane);
> + struct drm_framebuffer *fb = plane_state->base.fb;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> const int pipe = intel_plane->pipe;
> const int plane = intel_plane->plane + 1;
> u32 plane_ctl, stride_div, stride;
> - const struct drm_intel_sprite_colorkey *key =
> - &to_intel_plane_state(drm_plane->state)->ckey;
> + const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
> u32 surf_addr;
> u32 tile_height, plane_offset, plane_size;
> unsigned int rotation;
> int x_offset, y_offset;
> - struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config;
> - int scaler_id;
> + int crtc_x = plane_state->dst.x1;
> + int crtc_y = plane_state->dst.y1;
> + uint32_t crtc_w = drm_rect_width(&plane_state->dst);
> + uint32_t crtc_h = drm_rect_height(&plane_state->dst);
> + uint32_t x = plane_state->src.x1 >> 16;
> + uint32_t y = plane_state->src.y1 >> 16;
> + uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
> + uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
> + const struct intel_scaler *scaler =
> + &crtc_state->scaler_state.scalers[plane_state->scaler_id];
>
> plane_ctl = PLANE_CTL_ENABLE |
> PLANE_CTL_PIPE_GAMMA_ENABLE |
> @@ -208,14 +213,12 @@ skl_update_plane(struct drm_plane *drm_plane, struct
> drm_crtc *crtc,
> plane_ctl |= skl_plane_ctl_format(fb->pixel_format);
> plane_ctl |= skl_plane_ctl_tiling(fb->modifier[0]);
>
> - rotation = drm_plane->state->rotation;
> + rotation = plane_state->base.rotation;
> plane_ctl |= skl_plane_ctl_rotation(rotation);
>
> stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> fb->pixel_format);
>
> - scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
> -
> /* Sizes are 0 based */
> sr
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Remove some visibility checks from intel_crtc_update_cursor.
On Thu, Jan 07, 2016 at 11:54:08AM +0100, Maarten Lankhorst wrote:
> This is duplicated with intel_check_cursor_plane, and with all
> non-atomic paths removed this should be dead code.
>
> Signed-off-by: Maarten Lankhorst
I have some cursor stuff in a branch somewhere that does some of the
same, but I should be able to rebase that stuff rather easily.
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 14 --
> 1 file changed, 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 63286f16a9e6..35b881d156b0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -85,8 +85,6 @@ static const uint32_t intel_cursor_formats[] = {
> DRM_FORMAT_ARGB,
> };
>
> -static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
> -
> static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
> static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> @@ -10201,25 +10199,13 @@ static void intel_crtc_update_cursor(struct
> drm_crtc *crtc,
>
> base = intel_crtc->cursor_addr;
>
> - if (x >= intel_crtc->config->pipe_src_w)
> - on = false;
> -
> - if (y >= intel_crtc->config->pipe_src_h)
> - on = false;
> -
> if (x < 0) {
> - if (x + cursor_state->crtc_w <= 0)
> - on = false;
> -
> pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
> x = -x;
> }
> pos |= x << CURSOR_X_SHIFT;
>
> if (y < 0) {
> - if (y + cursor_state->crtc_h <= 0)
> - on = false;
> -
> pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
> y = -y;
> }
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: Make disable_cursor_plane similar to commit_cursor_plane.
On Thu, Jan 07, 2016 at 11:54:09AM +0100, Maarten Lankhorst wrote:
> Update cursor_bo and cursor_addr after being called.
cursor_bo no longer exists. My cursor stuff sitting in branch also gets
rid of cursor_addr, but until I land that we can go with this.
Reviewed-by: Ville Syrjälä
> This is required to make commit_cursor_plane take a
> crtc_state and a plane_state.
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/intel_display.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 35b881d156b0..7ea49d5e2ce0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14177,6 +14177,9 @@ static void
> intel_disable_cursor_plane(struct drm_plane *plane,
> struct drm_crtc *crtc)
> {
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> + intel_crtc->cursor_addr = 0;
> intel_crtc_update_cursor(crtc, false);
> }
>
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: Use plane state for primary plane updates.
On Thu, Jan 07, 2016 at 11:54:11AM +0100, Maarten Lankhorst wrote:
> Pass in the atomic states to allow for proper updates.
> This removes uses of intel_crtc->config and direct access
> to plane->state.
>
> This breaks the last bit of kgdboc, but that appears to be dead code.
>
> Signed-off-by: Maarten Lankhorst
lgtm.
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 -
> drivers/gpu/drm/i915/intel_display.c | 248
> +++
> 2 files changed, 103 insertions(+), 148 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bde9c764b415..75d8f89088aa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -648,9 +648,6 @@ struct drm_i915_display_funcs {
> struct drm_i915_gem_object *obj,
> struct drm_i915_gem_request *req,
> uint32_t flags);
> - void (*update_primary_plane)(struct drm_crtc *crtc,
> - struct drm_framebuffer *fb,
> - int x, int y);
> void (*hpd_irq_setup)(struct drm_device *dev);
> /* clock updates for mode set */
> /* cursor updates */
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f7c293638df4..f276caf65a68 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2679,36 +2679,23 @@ valid_fb:
> obj->frontbuffer_bits |= to_intel_plane(primary)->frontbuffer_bit;
> }
>
> -static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> - struct drm_framebuffer *fb,
> - int x, int y)
> +static void i9xx_update_primary_plane(struct drm_plane *primary,
> + const struct intel_crtc_state *crtc_state,
> + const struct intel_plane_state
> *plane_state)
> {
> - struct drm_device *dev = crtc->dev;
> + struct drm_device *dev = primary->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct drm_plane *primary = crtc->primary;
> - bool visible = to_intel_plane_state(primary->state)->visible;
> - struct drm_i915_gem_object *obj;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_framebuffer *fb = plane_state->base.fb;
> + struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> int plane = intel_crtc->plane;
> unsigned long linear_offset;
> + int x = plane_state->src.x1 >> 16;
> + int y = plane_state->src.y1 >> 16;
> u32 dspcntr;
> i915_reg_t reg = DSPCNTR(plane);
> int pixel_size;
>
> - if (!visible || !fb) {
> - I915_WRITE(reg, 0);
> - if (INTEL_INFO(dev)->gen >= 4)
> - I915_WRITE(DSPSURF(plane), 0);
> - else
> - I915_WRITE(DSPADDR(plane), 0);
> - POSTING_READ(reg);
> - return;
> - }
> -
> - obj = intel_fb_obj(fb);
> - if (WARN_ON(obj == NULL))
> - return;
> -
> pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
>
> dspcntr = DISPPLANE_GAMMA_ENABLE;
> @@ -2723,13 +2710,13 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
>* which should always be the user's requested size.
>*/
> I915_WRITE(DSPSIZE(plane),
> -((intel_crtc->config->pipe_src_h - 1) << 16) |
> -(intel_crtc->config->pipe_src_w - 1));
> +((crtc_state->pipe_src_h - 1) << 16) |
> +(crtc_state->pipe_src_w - 1));
> I915_WRITE(DSPPOS(plane), 0);
> } else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) {
> I915_WRITE(PRIMSIZE(plane),
> -((intel_crtc->config->pipe_src_h - 1) << 16) |
> -(intel_crtc->config->pipe_src_w - 1));
> +((crtc_state->pipe_src_h - 1) << 16) |
> +(crtc_state->pipe_src_w - 1));
> I915_WRITE(PRIMPOS(plane), 0);
> I915_WRITE(PRIMCNSTALPHA(plane), 0);
> }
> @@ -2780,17 +2767,17 @@ static void i9xx_update_primary_plane(struct drm_crtc
> *crtc,
> intel_crtc->dspaddr_offset = linear_offset;
> }
>
> - if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) {
> + if (plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> dspcntr |= DISPPLANE_ROTATE_180;
>
> - x += (intel_crtc->config->pipe_src_w - 1);
> - y += (intel_crtc->config->pipe_src_h - 1);
> + x += (crtc_state->pipe_src_w - 1);
> + y += (crtc_state->pipe_src_h - 1);
>
> /* Findin
Re: [Intel-gfx] [PATCH 7/7] drm/i915: Remove commit_plane function pointer.
On Thu, Jan 07, 2016 at 11:54:12AM +0100, Maarten Lankhorst wrote:
> With sprites, cursors and primary planes taking the atomic state
> this is now unused. It's removed in a separate commit to allow
> a revert.
>
> Signed-off-by: Maarten Lankhorst
One less weird difference between the planes. Yay.
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_atomic_plane.c | 4 +---
> drivers/gpu/drm/i915/intel_drv.h | 2 --
> 2 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 969aa410deaa..e0b851a0004a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -198,9 +198,7 @@ static void intel_plane_atomic_update(struct drm_plane
> *plane,
> struct drm_crtc_state *crtc_state =
> drm_atomic_get_existing_crtc_state(old_state->state, crtc);
>
> - if (intel_plane->commit_plane)
> - intel_plane->commit_plane(plane, intel_state);
> - else if (intel_state->visible)
> + if (intel_state->visible)
> intel_plane->update_plane(plane,
> to_intel_crtc_state(crtc_state),
> intel_state);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 068050884353..5c41d2030594 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -669,8 +669,6 @@ struct intel_plane {
> int (*check_plane)(struct drm_plane *plane,
> struct intel_crtc_state *crtc_state,
> struct intel_plane_state *state);
> - void (*commit_plane)(struct drm_plane *plane,
> - struct intel_plane_state *state);
> };
>
> struct intel_watermark_params {
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
Since
commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä
Date: Fri Nov 27 18:55:26 2015 +0200
drm/i915: Introduce a gmbus power domain
gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.
v2: Adjust cleanup paths too (Chris).
v3: Rebase onto -nightly (totally bogus tree I had lying around) and
also move dpio init head (Ville).
Cc: Ville Syrjälä
Cc: Patrik Jakobsson
Cc: Imre Deak
Cc: Jani Nikula
Cc: Meelis Roos
Cc: Chris Wilson
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: sta...@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Tested-by: Meelis Roos
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_dma.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 988a3806512a..fcefd3beef50 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_vga_switcheroo;
- intel_power_domains_init_hw(dev_priv, false);
intel_csr_ucode_init(dev_priv);
@@ -1025,6 +1024,9 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_irq_init(dev_priv);
intel_uncore_sanitize(dev);
+ intel_power_domains_init(dev_priv);
+ intel_init_dpio(dev_priv);
+ intel_power_domains_init_hw(dev_priv, false);
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
@@ -1049,20 +1051,16 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
intel_device_info_runtime_init(dev);
- intel_init_dpio(dev_priv);
-
if (INTEL_INFO(dev)->num_pipes) {
ret = drm_vblank_init(dev, INTEL_INFO(dev)->num_pipes);
if (ret)
goto out_gem_unload;
}
- intel_power_domains_init(dev_priv);
-
ret = i915_load_modeset_init(dev);
if (ret < 0) {
DRM_ERROR("failed to init modeset\n");
- goto out_power_well;
+ goto out_vblank;
}
/*
@@ -1091,8 +1089,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
return 0;
-out_power_well:
- intel_power_domains_fini(dev_priv);
+out_vblank:
drm_vblank_cleanup(dev);
out_gem_unload:
WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
@@ -1103,6 +1100,7 @@ out_gem_unload:
intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
+ intel_power_domains_fini(dev_priv);
pm_qos_remove_request(&dev_priv->pm_qos);
destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
out_freedpwq:
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
Since
commit ac9b8236551d1177fd07b56aef9b565d1864420d
Author: Ville Syrjälä
Date: Fri Nov 27 18:55:26 2015 +0200
drm/i915: Introduce a gmbus power domain
gmbus also needs the power domain infrastructure right from the start,
since as soon as we register the i2c controllers someone can use them.
v2: Adjust cleanup paths too (Chris).
v3: Rebase onto -nightly (totally bogus tree I had lying around) and
also move dpio init head (Ville).
v4: Ville instead suggested to move gmbus setup later in the sequence,
since it's only needed by the modeset code.
v5: Move even close to the actual user, right next to the comment that
states where we really need gmbus (and interrupts!).
Cc: Ville Syrjälä
Cc: Patrik Jakobsson
Cc: Imre Deak
Cc: Jani Nikula
Cc: Meelis Roos
Cc: Chris Wilson
Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
Cc: sta...@vger.kernel.org
References: http://www.spinics.net/lists/intel-gfx/msg83075.html
Signed-off-by: Daniel Vetter
---
Meelis, can you pls retest this one?
Thanks, Daniel
---
drivers/gpu/drm/i915/i915_dma.c | 6 +++---
drivers/gpu/drm/i915/intel_display.c | 2 ++
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 988a3806512a..d70d96fe553b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -406,6 +406,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_gem_stolen;
+ intel_setup_gmbus(dev);
+
/* Important: The output setup functions called by modeset_init need
* working irqs for e.g. gmbus and dp aux transfers. */
intel_modeset_init(dev);
@@ -455,6 +457,7 @@ cleanup_gem:
cleanup_irq:
intel_guc_ucode_fini(dev);
drm_irq_uninstall(dev);
+ intel_teardown_gmbus(dev);
cleanup_gem_stolen:
i915_gem_cleanup_stolen(dev);
cleanup_vga_switcheroo:
@@ -1028,7 +1031,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
/* Try to make sure MCHBAR is enabled before poking at it */
intel_setup_mchbar(dev);
- intel_setup_gmbus(dev);
intel_opregion_setup(dev);
i915_gem_load(dev);
@@ -1101,7 +1103,6 @@ out_gem_unload:
if (dev->pdev->msi_enabled)
pci_disable_msi(dev->pdev);
- intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
pm_qos_remove_request(&dev_priv->pm_qos);
destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
@@ -1203,7 +1204,6 @@ int i915_driver_unload(struct drm_device *dev)
intel_csr_ucode_fini(dev_priv);
- intel_teardown_gmbus(dev);
intel_teardown_mchbar(dev);
destroy_workqueue(dev_priv->hotplug.dp_wq);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 37945ddb4dad..ac0038bf4fbf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15971,6 +15971,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
mutex_lock(&dev->struct_mutex);
intel_cleanup_gt_powersave(dev);
mutex_unlock(&dev->struct_mutex);
+
+ intel_teardown_gmbus(dev);
}
/*
--
2.6.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/15] drm/i915/bios: interpret the i2c element
On Thu, Jan 07, 2016 at 11:31:25AM +0200, Jani Nikula wrote:
> On Tue, 05 Jan 2016, Ville Syrjälä wrote:
> > On Mon, Dec 21, 2015 at 03:11:00PM +0200, Jani Nikula wrote:
> >> Add parsing of the i2c element, defined in MIPI sequence block v2. Drop
> >> the status operation byte while at it, that does not exist.
> >>
> >> Signed-off-by: Jani Nikula
> >> ---
> >> drivers/gpu/drm/i915/intel_bios.c | 5 +
> >> drivers/gpu/drm/i915/intel_bios.h | 2 +-
> >> 2 files changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> >> b/drivers/gpu/drm/i915/intel_bios.c
> >> index d6eaf32f33e5..45a7a2bc96c6 100644
> >> --- a/drivers/gpu/drm/i915/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/intel_bios.c
> >> @@ -812,6 +812,11 @@ static int goto_next_sequence(const u8 *data, int
> >> index, int total)
> >>case MIPI_SEQ_ELEM_GPIO:
> >>len = 2;
> >
> > Somewhat off topic, but I wonder if this is correct. The "structure"
> > diagram shows it as 2 bytes for v1 and v2, but I'm not sure that section
> > isn't there just as an example. Later the describing the GPIO block it
> > seems to say it's 2 bytes for v1, and three bytes for v2.
>
> I've held on to some old spec versions (bdb version 177 has sequence v1,
> bdb version 185 has sequence v2). Both v1 and v2 have 2 bytes payload
> for the GPIO element.
>
> The *meaning* of the first of those bytes has changed from v1->v2
> though. Can't do much about that here, it's up to the generic vbt dsi
> "driver"...
>
> >
> >>break;
> >> + case MIPI_SEQ_ELEM_I2C:
> >> + if (index + 7 > total)
> >> + return 0;
> >> + len = *(data + index + 6) + 7;
> >> + break;
> >
> > This one isn't show in the structure diagrams at all, so I guess we get
> > to trust the other section. It says this was introduced in v2. Should be
> > add a paranoia check for that?
>
> The spec with bdb version 185 has this.
>
> I contemplated adding a check for v2, but then I thought it probably
> doesn't really matter all that much. If we get an i2c elem with v1 and
> reject it, we'll probably end up with a black screen. If we just assume
> it's an i2c element but it isn't, it'll trip over something else later.
>
> > Should we also check that the payload length is below the specified max
> > of 240 bytes?
>
> You'll love this. In v2 the max is actually the whole byte i.e. 255. In
> v3 they added a length field for these operations for forward
> compatibility (can now skip unknown new operations). And that's 8
> bits. So the header + payload for the i2c data can't exceed 255, so
> there's now an arbitrary 240 byte limit for i2c payload in v3.
I don't really see where the 240 comes from. The spec lists 240 byte
payload size limit for i2c, spi, and send packet operations, but the
size of the header is different for those so I can't see how all
would end up with the same payload length limitation. So to me it seems
like the max payload limit should be 255-7 for i2c, 255-6 for spi, and
255-4 for send packet (since the "size of operation" byte doesn't seem
to include itself or the "operation byte"). So to me the 240 seems like
a totally arbitrary limit.
>
> BR,
> Jani.
>
>
> >
> >>default:
> >>DRM_ERROR("Unknown operation byte\n");
> >>return 0;
> >> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> >> b/drivers/gpu/drm/i915/intel_bios.h
> >> index 4e87df16e7b3..411b33794536 100644
> >> --- a/drivers/gpu/drm/i915/intel_bios.h
> >> +++ b/drivers/gpu/drm/i915/intel_bios.h
> >> @@ -968,7 +968,7 @@ enum mipi_seq_element {
> >>MIPI_SEQ_ELEM_SEND_PKT,
> >>MIPI_SEQ_ELEM_DELAY,
> >>MIPI_SEQ_ELEM_GPIO,
> >> - MIPI_SEQ_ELEM_STATUS,
> >> + MIPI_SEQ_ELEM_I2C, /* sequence block v2+ */
> >>MIPI_SEQ_ELEM_MAX
> >> };
> >>
> >> --
> >> 2.1.4
> >>
> >> ___
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ failure: Fi.CI.BAT
On Thu, Jan 07, 2016 at 03:06:13PM +0200, Jani Nikula wrote: > On Thu, 07 Jan 2016, Dave Gordon wrote: > > On 06/01/16 10:20, Patchwork wrote: > >> == Summary == > >> > >> Built on 24b053acb16b4b3b021575e4ee30ffedd3ab2920 drm-intel-nightly: > >> 2016y-01m-06d-08h-16m-11s UTC integration manifest > >> > >> Test drv_getparams_basic: > >> Subgroup basic-eu-total: > >> pass -> DMESG-FAIL (skl-i5k-2) > >> pass -> DMESG-FAIL (skl-i7k-2) > > [snip] > > >> > >> bdw-nuci7total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9 > >> bdw-ultratotal:132 pass:126 dwarn:0 dfail:0 fail:0 skip:6 > >> bsw-nuc-2total:135 pass:115 dwarn:0 dfail:0 fail:0 skip:20 > >> byt-nuc total:135 pass:121 dwarn:1 dfail:0 fail:0 skip:13 > >> hsw-brixbox total:135 pass:128 dwarn:0 dfail:0 fail:0 skip:7 > >> hsw-gt2 total:135 pass:131 dwarn:0 dfail:0 fail:0 skip:4 > >> ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35 > >> ivb-t430stotal:135 pass:129 dwarn:0 dfail:0 fail:0 skip:6 > >> skl-i5k-2total:135 pass:2dwarn:0 dfail:132 fail:0 skip:1 > >> skl-i7k-2total:135 pass:2dwarn:0 dfail:132 fail:0 skip:1 > >> snb-dellxps total:135 pass:123 dwarn:0 dfail:0 fail:0 skip:12 > >> snb-x220ttotal:135 pass:123 dwarn:0 dfail:0 fail:1 skip:11 > >> > >> Results at /archive/results/CI_IGT_test/Patchwork_1094/ > >> > >> ___ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > Looks like the CI SKL doesn't have the GuC firmware installed? > > It's tons of: > > [ 38.169461] [drm:intel_lr_context_deferred_alloc [i915]] *ERROR* ring > create req: -5 > > If that gets fixed by installing the GuC firmware, the answer is *not* > to install the GuC firmware on the CI machines. The answer is to make > the driver handle missing firmware gracefully. I kinda don't want to support 2 different ways to run things on any given platform, because we can't even support one way properly. But since it took forever to get guc enabled people will indeed scream if guc isn't there, so either we enable this only for bxt and later or we indeed need to support both cases on skl :( Either way we do need to install guc firmware first on CI boxes, since otherwise coverage isn't there. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/15] drm/i915/bios: add sequences for MIPI sequence block v2
On Mon, Dec 21, 2015 at 03:11:01PM +0200, Jani Nikula wrote:
> Properly parse the new sequences added in MIPI sequence block v2.
>
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_bios.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> b/drivers/gpu/drm/i915/intel_bios.h
> index 411b33794536..6146f1b0cf48 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -960,6 +960,9 @@ enum mipi_seq {
> MIPI_SEQ_DISPLAY_ON,
> MIPI_SEQ_DISPLAY_OFF,
> MIPI_SEQ_DEASSERT_RESET,
> + MIPI_SEQ_BACKLIGHT_ON, /* sequence block v2+ */
> + MIPI_SEQ_BACKLIGHT_OFF, /* sequence block v2+ */
> + MIPI_SEQ_TEAR_ON, /* sequence block v2+ */
Can't comment on the v2+ part since the spec fails to mention it, but
otherwise looks sane.
It's a bit hard to review w/o an explicit assignments for each, but
assuming the MIPI_SEQ_DEASSERT_RESET value is correct these should be
correct too.
Not sure why we stopped at "tear on" though? The spec has "tear off",
"panel on" and "panel off" listed as well.
> MIPI_SEQ_MAX
> };
>
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bxt: Don't save/restore eDP panel power during suspend (v3)
On Thu, Jan 07, 2016 at 11:40:22AM +0200, Jani Nikula wrote: > On Thu, 07 Jan 2016, Daniel Vetter wrote: > > On Thu, Jan 07, 2016 at 10:15:00AM +0200, Jani Nikula wrote: > >> On Wed, 06 Jan 2016, Matt Roper wrote: > >> > Our attempts save/restore panel power state in i915_suspend.c are > >> > causing unclaimed register warnings on BXT since the registers for this > >> > platform differ from older platforms. > >> > > >> > The big hammer suspend/resume shouldn't be necessary for PP since the > >> > connector/encoder hooks should already handle this. In theory we could > >> > remove this for all platforms, but in practice it's likely that would > >> > cause some regressions since older platforms with LVDS may have > >> > incomplete PP handling. For now we'll leave the PCH save/restore alone > >> > and change the non-PCH branch to only operate on gen <= 4 so that BXT > >> > and future platforms aren't included. > >> > > >> > v2: Typo fix: s/||/&&/ > >> > > >> > v3: Change non-PCH condition to a gen <= 4 test rather than listing > >> > VLV/CHV/BXT as specific platforms to exclude; should be more > >> > future-proof as we add new platforms. (Daniel) > >> > > >> > Cc: Vandana Kannan > >> > Cc: Jani Nikula > >> > Cc: Daniel Vetter > >> > Cc: drm-intel-fi...@lists.freedesktop.org > >> > Signed-off-by: Matt Roper > >> > --- > >> > Although it's the direction we want to move, I'm not brave enough to > >> > blow away > >> > the entire PP save/restore for all platforms since I don't have the HW > >> > necessary to deal with the regressions that might pop up. The consensus > >> > on IRC > >> > is that there probably will be a few regressions when we do that, so I'd > >> > rather > >> > have people with appropriate platform access make those changes. > >> > >> This is the first step we want to take anyway. > >> > >> Reviewed-by: Jani Nikula > > > > Just looked at this some more, and the code here is the only code that > > restores PP registers. Except for vlv/chv, where we have a call to > > intel_dp_init_panel_power_sequencer_registers in vlv_power_sequencer_pipe. > > > > I think we first need to sprinkle more calls to restore PP registers > > around before we can disable this here for bxt. This patch here just > > papers over a legit bug, without fixing it really. > > No, this patch doesn't paper over anything. The code that gets run for > Broxton before this patch doesn't save/restore any meaningful registers > that could possibly help with PP. This fixes unclaimed register > read/writes, and it's a valid fix as such. > > The commit message should probably be fixed to say that we're not there > yet for Broxton with the hooks. But IMO this patch is a valid fix and > orthogonal to the issue of fixing the hooks. The hooks are good enough, at least they work for vlv/chv. If we can't fix up PP restoring for bxt right away because it's too much work I think we should at least put int a FIXME. Otherwise we'll promptly forget about this again, invalidating the value unclaimed register access provides as a hint that we haven't yet completed some crucial bxt enabling work. Also, we need a jira to keep track of this. Adding Imre/Annie for that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add Backlight Control using DPCD for eDP connectors (v4)
On Wed, 16 Dec 2015, Yetunde Adebisi wrote:
> This patch adds support for eDP backlight control using DPCD registers to
> backlight hooks in intel_panel.
>
> It checks for backlight control over AUX channel capability and sets up
> function pointers to get and set the backlight brightness level if
> supported.
>
> v2: Moved backlight functions from intel_dp.c into a new file
> intel_dp_aux_backlight.c. Also moved reading of eDP display control
> registers to intel_dp_get_dpcd
>
> v3: Correct some formatting mistakes
>
> v4: Updated to use AUX backlight control if PWM control is not possible
> (Jani)
>
> This patch depends on http://patchwork.freedesktop.org/patch/64253/
>
> Cc: Bob Paauwe
> Cc: Jani Nikula
> Acked-by: Jesse Barnes
> Signed-off-by: Yetunde Adebisi
Please accept my apologies for taking so long to review the updated
patches.
This is starting to look good. There are a couple of small changes still
needed, please find the comments inline.
BR,
Jani.
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/intel_dp.c | 17 ++-
> drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 181
> ++
> drivers/gpu/drm/i915/intel_drv.h | 6 +
> drivers/gpu/drm/i915/intel_panel.c| 4 +
> 5 files changed, 203 insertions(+), 6 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/intel_dp_aux_backlight.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 0851de07..41250cc 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -77,6 +77,7 @@ i915-y += dvo_ch7017.o \
> dvo_tfp410.o \
> intel_crt.o \
> intel_ddi.o \
> + intel_dp_aux_backlight.o \
> intel_dp_link_training.o \
> intel_dp_mst.o \
> intel_dp.o \
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 31ba241..2b60b83 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3188,7 +3188,7 @@ static void chv_dp_post_pll_disable(struct
> intel_encoder *encoder)
> * Sinks are *supposed* to come up within 1ms from an off state, but we're
> also
> * supposed to retry 3 times per the spec.
> */
> -static ssize_t
> +ssize_t
> intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset,
> void *buffer, size_t size)
> {
> @@ -3855,7 +3855,6 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - uint8_t rev;
>
> if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd,
> sizeof(intel_dp->dpcd)) < 0)
> @@ -3891,6 +3890,15 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> DRM_DEBUG_KMS("PSR2 %s on sink",
> dev_priv->psr.psr2_support ? "supported" : "not
> supported");
> }
> +
> + /* Read the eDP Display control capabilities registers */
> + memset(intel_dp->edp_dpcd, 0, sizeof(intel_dp->edp_dpcd));
> + if ((intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] &
> DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
> + (intel_dp_dpcd_read_wake(&intel_dp->aux,
> DP_EDP_DPCD_REV,
> + intel_dp->edp_dpcd,
> sizeof(intel_dp->edp_dpcd)) ==
> +
> sizeof(intel_dp->edp_dpcd)))
> + DRM_DEBUG_KMS("EDP DPCD : %*ph\n", (int)
> sizeof(intel_dp->edp_dpcd),
> + intel_dp->edp_dpcd);
> }
>
> DRM_DEBUG_KMS("Display Port TPS3 support: source %s, sink %s\n",
> @@ -3898,10 +3906,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> yesno(drm_dp_tps3_supported(intel_dp->dpcd)));
>
> /* Intermediate frequency support */
> - if (is_edp(intel_dp) &&
> - (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] &
> DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
> - (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1)
> == 1) &&
> - (rev >= 0x03)) { /* eDp v1.4 or higher */
> + if (is_edp(intel_dp) && (intel_dp->edp_dpcd[0] >= 0x03)) { /* eDp v1.4
> or higher */
> __le16 sink_rates[DP_MAX_SUPPORTED_RATES];
> int i;
>
> diff --git a/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> new file mode 100644
> index 000..3bba6b5
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_dp_aux_backlight.c
> @@ -0,0 +1,181 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Soft
Re: [Intel-gfx] [PATCH 10/15] drm/i915/bios: add sequences for MIPI sequence block v2
On Thu, 07 Jan 2016, Ville Syrjälä wrote:
> On Mon, Dec 21, 2015 at 03:11:01PM +0200, Jani Nikula wrote:
>> Properly parse the new sequences added in MIPI sequence block v2.
>>
>> Signed-off-by: Jani Nikula
>> ---
>> drivers/gpu/drm/i915/intel_bios.h | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_bios.h
>> b/drivers/gpu/drm/i915/intel_bios.h
>> index 411b33794536..6146f1b0cf48 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.h
>> +++ b/drivers/gpu/drm/i915/intel_bios.h
>> @@ -960,6 +960,9 @@ enum mipi_seq {
>> MIPI_SEQ_DISPLAY_ON,
>> MIPI_SEQ_DISPLAY_OFF,
>> MIPI_SEQ_DEASSERT_RESET,
>> +MIPI_SEQ_BACKLIGHT_ON, /* sequence block v2+ */
>> +MIPI_SEQ_BACKLIGHT_OFF, /* sequence block v2+ */
>> +MIPI_SEQ_TEAR_ON, /* sequence block v2+ */
>
> Can't comment on the v2+ part since the spec fails to mention it, but
> otherwise looks sane.
>
> It's a bit hard to review w/o an explicit assignments for each, but
> assuming the MIPI_SEQ_DEASSERT_RESET value is correct these should be
> correct too.
>
> Not sure why we stopped at "tear on" though? The spec has "tear off",
> "panel on" and "panel off" listed as well.
Just for lols. The sequence block v2 stops there. The rest are added at
v3. Words fail me.
BR,
Jani.
>
>> MIPI_SEQ_MAX
>> };
>>
>> --
>> 2.1.4
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/15] drm/i915: Adding the parsing logic for the i2c element
On Mon, Dec 21, 2015 at 03:11:02PM +0200, Jani Nikula wrote:
> From: vkorjani
>
> New sequence element for i2c is been added in the
> mipi sequence block of the VBT. This patch parses
> and executes the i2c sequence.
>
> v2: Add i2c_put_adapter call(Jani), rebase
>
> v3: corrected the retry loop(Jani), rebase
>
> v4 by Jani:
> - don't put the adapter if get fails
> - print an error message if all retries exhausted
> - use a for loop
> - fix warnings for unused variables
>
> Cc: Jani Nikula
> Signed-off-by: vkorjani
> Signed-off-by: Deepak M
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 58
> ++
> 1 file changed, 58 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index ba5355506590..8fcfb0f63dc1 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include "i915_drv.h"
> @@ -104,6 +105,62 @@ static struct gpio_table gtable[] = {
> { GPIO_NC_11_PCONF0, GPIO_NC_11_PAD, 0}
> };
>
> +static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
> +{
> + struct i2c_adapter *adapter;
> + int ret, i;
> + u8 reg_offset, payload_size;
> + struct i2c_msg msg;
> + u8 *transmit_buffer;
> + u8 flag, bus_number;
> + u16 slave_add;
> +
> + flag = *data++;
> + data++; /* index, unused */
> + bus_number = *data++;
> + slave_add = *(u16 *)(data);
> + data += 2;
> + reg_offset = *data++;
> + payload_size = *data++;
> +
> + adapter = i2c_get_adapter(bus_number);
I'm trying to find who is responsible for registering i2c busses with
these magic bus numbers. So far I can't see anything that would be doing
it.
> + if (!adapter) {
> + DRM_ERROR("i2c_get_adapter(%u)\n", bus_number);
> + goto out;
> + }
> +
> + transmit_buffer = kmalloc(1 + payload_size, GFP_TEMPORARY);
> + if (!transmit_buffer)
> + goto out_put;
> +
> + transmit_buffer[0] = reg_offset;
> + memcpy(&transmit_buffer[1], data, payload_size);
> +
> + msg.addr = slave_add;
> + msg.flags = 0;
> + msg.len = payload_size + 1;
> + msg.buf = &transmit_buffer[0];
> +
> + for (i = 0; i < 6; i++) {
> + ret = i2c_transfer(adapter, &msg, 1);
> + if (ret == 1) {
> + goto out_free;
> + } else if (ret == -EAGAIN) {
> + usleep_range(1000, 2500);
> + } else {
> + break;
> + }
> + }
> +
> + DRM_ERROR("i2c transfer failed: %d\n", ret);
> +out_free:
> + kfree(transmit_buffer);
> +out_put:
> + i2c_put_adapter(adapter);
> +out:
> + return data + payload_size;
> +}
> +
> static inline enum port intel_dsi_seq_port_to_port(u8 port)
> {
> return port ? PORT_C : PORT_A;
> @@ -235,6 +292,7 @@ static const fn_mipi_elem_exec exec_elem[] = {
> [MIPI_SEQ_ELEM_SEND_PKT] = mipi_exec_send_packet,
> [MIPI_SEQ_ELEM_DELAY] = mipi_exec_delay,
> [MIPI_SEQ_ELEM_GPIO] = mipi_exec_gpio,
> + [MIPI_SEQ_ELEM_I2C] = mipi_exec_i2c,
> };
>
> /*
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/15] drm/i915/bios: add defines for v3 sequence block
On Mon, Dec 21, 2015 at 03:11:03PM +0200, Jani Nikula wrote:
> New sequences, new operations within sequences.
>
> Signed-off-by: Jani Nikula
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_bios.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> b/drivers/gpu/drm/i915/intel_bios.h
> index 6146f1b0cf48..350d4e0f75a4 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -963,6 +963,9 @@ enum mipi_seq {
> MIPI_SEQ_BACKLIGHT_ON, /* sequence block v2+ */
> MIPI_SEQ_BACKLIGHT_OFF, /* sequence block v2+ */
> MIPI_SEQ_TEAR_ON, /* sequence block v2+ */
> + MIPI_SEQ_TEAR_OFF, /* sequence block v3+ */
> + MIPI_SEQ_POWER_ON, /* sequence block v3+ */
> + MIPI_SEQ_POWER_OFF, /* sequence block v3+ */
> MIPI_SEQ_MAX
> };
>
> @@ -972,6 +975,8 @@ enum mipi_seq_element {
> MIPI_SEQ_ELEM_DELAY,
> MIPI_SEQ_ELEM_GPIO,
> MIPI_SEQ_ELEM_I2C, /* sequence block v2+ */
> + MIPI_SEQ_ELEM_SPI, /* sequence block v3+ */
> + MIPI_SEQ_ELEM_PMIC, /* sequence block v3+ */
> MIPI_SEQ_ELEM_MAX
> };
>
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ failure: Fi.CI.BAT
On 07/01/16 14:36, Daniel Vetter wrote: On Thu, Jan 07, 2016 at 03:06:13PM +0200, Jani Nikula wrote: On Thu, 07 Jan 2016, Dave Gordon wrote: On 06/01/16 10:20, Patchwork wrote: == Summary == Built on 24b053acb16b4b3b021575e4ee30ffedd3ab2920 drm-intel-nightly: 2016y-01m-06d-08h-16m-11s UTC integration manifest Test drv_getparams_basic: Subgroup basic-eu-total: pass -> DMESG-FAIL (skl-i5k-2) pass -> DMESG-FAIL (skl-i7k-2) [snip] bdw-nuci7total:132 pass:122 dwarn:1 dfail:0 fail:0 skip:9 bdw-ultratotal:132 pass:126 dwarn:0 dfail:0 fail:0 skip:6 bsw-nuc-2total:135 pass:115 dwarn:0 dfail:0 fail:0 skip:20 byt-nuc total:135 pass:121 dwarn:1 dfail:0 fail:0 skip:13 hsw-brixbox total:135 pass:128 dwarn:0 dfail:0 fail:0 skip:7 hsw-gt2 total:135 pass:131 dwarn:0 dfail:0 fail:0 skip:4 ilk-hp8440p total:135 pass:100 dwarn:0 dfail:0 fail:0 skip:35 ivb-t430stotal:135 pass:129 dwarn:0 dfail:0 fail:0 skip:6 skl-i5k-2total:135 pass:2dwarn:0 dfail:132 fail:0 skip:1 skl-i7k-2total:135 pass:2dwarn:0 dfail:132 fail:0 skip:1 snb-dellxps total:135 pass:123 dwarn:0 dfail:0 fail:0 skip:12 snb-x220ttotal:135 pass:123 dwarn:0 dfail:0 fail:1 skip:11 Results at /archive/results/CI_IGT_test/Patchwork_1094/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx Looks like the CI SKL doesn't have the GuC firmware installed? It's tons of: [ 38.169461] [drm:intel_lr_context_deferred_alloc [i915]] *ERROR* ring create req: -5 If that gets fixed by installing the GuC firmware, the answer is *not* to install the GuC firmware on the CI machines. The answer is to make the driver handle missing firmware gracefully. I kinda don't want to support 2 different ways to run things on any given platform, because we can't even support one way properly. But since it took forever to get guc enabled people will indeed scream if guc isn't there, so either we enable this only for bxt and later or we indeed need to support both cases on skl :( Yeah, we used to have that ability, until it was vetoed :( See <20150706142822.GJ2156@phenom.ffwll.local> or http://www.mail-archive.com/intel-gfx%40lists.freedesktop.org/msg63553.html Either way we do need to install guc firmware first on CI boxes, since otherwise coverage isn't there. -Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: intel_hpd_init(): Fix suspend/resume reprobing
This fixes reprobing of display connectors on resume. After some
talking with danvet on IRC, I learned that calling
drm_helper_hpd_irq_event() does actually trigger a full reprobe of each
connector's status. It turns out this is the actual reason reprobing on
resume hasn't been working (this was observed on a T440s):
- We call hpd_init()
- We check each connector for a couple of things before marking
connector->polled with DRM_CONNECTOR_POLL_HPD, one of which is an
active encoder. Of course, a disconnected port won't have an
active encoder, so we don't add the flag to any of the
connectors.
- We call drm_helper_hpd_irq_event()
- drm_helper_irq_event() checks each connector for the
DRM_CONNECTOR_POLL_HPD flag. The only one that has it is eDP-1,
so we skip reprobing each connector except that one.
In addition, we also now avoid setting connector->polled to
DRM_CONNECTOR_POLL_HPD for MST connectors, since their reprobing is
handled by the mst helpers. This is probably what was originally
intended to happen here.
Changes since V1:
* Use the explanation of the issue as the commit message instead
* Change the title of the commit, since this does more then just stop a
check for an encoder now
* Add "Fixes" line for the patch that introduced this regression
* Don't enable DRM_CONNECTOR_POLL_HPD for mst connectors
Changes since V2:
* Put patch changelog above Signed-off-by
* Follow Daniel Vetter's suggestion for making the code here a bit more
legible
Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)")
Cc: sta...@vger.kernel.org
Signed-off-by: Lyude
---
drivers/gpu/drm/i915/intel_hotplug.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
b/drivers/gpu/drm/i915/intel_hotplug.c
index b177857..d7a6437 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -468,9 +468,14 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
list_for_each_entry(connector, &mode_config->connector_list, head) {
struct intel_connector *intel_connector =
to_intel_connector(connector);
connector->polled = intel_connector->polled;
- if (connector->encoder && !connector->polled &&
I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE)
- connector->polled = DRM_CONNECTOR_POLL_HPD;
+
+ /* MST has a dynamic intel_connector->encoder and it's reprobing
+* is all handled by the MST helpers. */
if (intel_connector->mst_port)
+ continue;
+
+ if (!connector->polled && I915_HAS_HOTPLUG(dev) &&
+ intel_connector->encoder->hpd_pin > HPD_NONE)
connector->polled = DRM_CONNECTOR_POLL_HPD;
}
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
> commit ac9b8236551d1177fd07b56aef9b565d1864420d
> Author: Ville Syrjälä
> Date: Fri Nov 27 18:55:26 2015 +0200
>
> drm/i915: Introduce a gmbus power domain
>
> gmbus also needs the power domain infrastructure right from the start,
> since as soon as we register the i2c controllers someone can use them.
>
> v2: Adjust cleanup paths too (Chris).
>
> v3: Rebase onto -nightly (totally bogus tree I had lying around) and
> also move dpio init head (Ville).
>
> v4: Ville instead suggested to move gmbus setup later in the sequence,
> since it's only needed by the modeset code.
>
> v5: Move even close to the actual user, right next to the comment that
> states where we really need gmbus (and interrupts!).
>
> Cc: Ville Syrjälä
> Cc: Patrik Jakobsson
> Cc: Imre Deak
> Cc: Jani Nikula
> Cc: Meelis Roos
> Cc: Chris Wilson
> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
> Cc: sta...@vger.kernel.org
> References: http://www.spinics.net/lists/intel-gfx/msg83075.html
> Signed-off-by: Daniel Vetter
> ---
>
> Meelis, can you pls retest this one?
Tested successfully on SNB computer.
>
> Thanks, Daniel
> ---
> drivers/gpu/drm/i915/i915_dma.c | 6 +++---
> drivers/gpu/drm/i915/intel_display.c | 2 ++
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 988a3806512a..d70d96fe553b 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -406,6 +406,8 @@ static int i915_load_modeset_init(struct drm_device *dev)
> if (ret)
> goto cleanup_gem_stolen;
>
> + intel_setup_gmbus(dev);
> +
> /* Important: The output setup functions called by modeset_init need
>* working irqs for e.g. gmbus and dp aux transfers. */
> intel_modeset_init(dev);
> @@ -455,6 +457,7 @@ cleanup_gem:
> cleanup_irq:
> intel_guc_ucode_fini(dev);
> drm_irq_uninstall(dev);
> + intel_teardown_gmbus(dev);
> cleanup_gem_stolen:
> i915_gem_cleanup_stolen(dev);
> cleanup_vga_switcheroo:
> @@ -1028,7 +1031,6 @@ int i915_driver_load(struct drm_device *dev, unsigned
> long flags)
>
> /* Try to make sure MCHBAR is enabled before poking at it */
> intel_setup_mchbar(dev);
> - intel_setup_gmbus(dev);
> intel_opregion_setup(dev);
>
> i915_gem_load(dev);
> @@ -1101,7 +1103,6 @@ out_gem_unload:
> if (dev->pdev->msi_enabled)
> pci_disable_msi(dev->pdev);
>
> - intel_teardown_gmbus(dev);
> intel_teardown_mchbar(dev);
> pm_qos_remove_request(&dev_priv->pm_qos);
> destroy_workqueue(dev_priv->gpu_error.hangcheck_wq);
> @@ -1203,7 +1204,6 @@ int i915_driver_unload(struct drm_device *dev)
>
> intel_csr_ucode_fini(dev_priv);
>
> - intel_teardown_gmbus(dev);
> intel_teardown_mchbar(dev);
>
> destroy_workqueue(dev_priv->hotplug.dp_wq);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 37945ddb4dad..ac0038bf4fbf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15971,6 +15971,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
> mutex_lock(&dev->struct_mutex);
> intel_cleanup_gt_powersave(dev);
> mutex_unlock(&dev->struct_mutex);
> +
> + intel_teardown_gmbus(dev);
> }
>
> /*
>
--
Meelis Roos (mr...@ut.ee) http://www.cs.ut.ee/~mroos/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm: add support for generic zpos property
Hello,
On 2016-01-07 14:59, Daniel Vetter wrote:
On Tue, Jan 05, 2016 at 01:52:50PM +0100, Marek Szyprowski wrote:
This patch adds support for generic plane's zpos property property with
well-defined semantics:
- added zpos properties to drm core and plane state structures
- added helpers for normalizing zpos properties of given set of planes
- well defined semantics: planes are sorted by zpos values and then plane
id value if zpos equals
Signed-off-by: Marek Szyprowski
lgtm I think. Longer-term we want to think whether we don't want to
extract such extensions into separate files, and push the kerneldoc into
an overview DOC: section in there. Just to keep things more closely
together. Benjamin with drm/sti also needs this, so cc'ing him.
Besides sti and exynos, zpos is also already implemented in rcar, mdp5
and omap
drivers. I'm not sure what should be done in case of omap, which uses
this property
with different name ("zorder" instead of "zpos").
---
Documentation/DocBook/gpu.tmpl | 14 --
drivers/gpu/drm/drm_atomic.c| 4 +++
drivers/gpu/drm/drm_atomic_helper.c | 52 +
drivers/gpu/drm/drm_crtc.c | 13 ++
include/drm/drm_atomic_helper.h | 2 ++
include/drm/drm_crtc.h | 13 ++
6 files changed, 96 insertions(+), 2 deletions(-)
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 6c6e81a9eaf4..e81acd999891 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2004,7 +2004,7 @@ void intel_crt_init(struct drm_device *dev)
Description/Restrictions
- DRM
+ DRM
Generic
“rotation”
BITMASK
@@ -2256,7 +2256,7 @@ void intel_crt_init(struct drm_device *dev)
property to suggest an Y offset for a connector
- Optional
+ Optional
“scaling mode”
ENUM
{ "None", "Full", "Center", "Full aspect" }
@@ -2280,6 +2280,16 @@ void intel_crt_init(struct drm_device *dev)
TBD
+"zpos"
+ RANGE
+ Min=0, Max=255
+ Plane
+ Plane's 'z' position during blending (0 for
background, 255 for frontmost).
+ If two planes assigned to same CRTC have equal zpos values, the
plane with higher plane
+ id is treated as closer to front. Can be IMMUTABLE if driver
doesn't support changing
+ plane's order.
+
+
i915
Generic
"Broadcast RGB"
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 6a21e5c378c1..97bb069cb6a3 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -614,6 +614,8 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
state->src_h = val;
} else if (property == config->rotation_property) {
state->rotation = val;
+ } else if (property == config->zpos_property) {
+ state->zpos = val;
} else if (plane->funcs->atomic_set_property) {
return plane->funcs->atomic_set_property(plane, state,
property, val);
@@ -670,6 +672,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->src_h;
} else if (property == config->rotation_property) {
*val = state->rotation;
+ } else if (property == config->zpos_property) {
+ *val = state->zpos;
} else if (plane->funcs->atomic_get_property) {
return plane->funcs->atomic_get_property(plane, state,
property, val);
} else {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 268d37f26960..de3ca33eb696 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
/**
* DOC: overview
@@ -2781,3 +2782,54 @@ void drm_atomic_helper_connector_destroy_state(struct
drm_connector *connector,
kfree(state);
}
EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
+
+/**
+ * __drm_atomic_helper_plane_zpos_cmp - compare zpos value of two planes
+ * @a: pointer to first plane
+ * @b: pointer to second plane
Generally we don't do kerneldoc for non-exported functions. If you want
just keep the text itself as a comment, or better move it into the
official interface docs for drm_atomic_helper_normalize_zpos().
okay
+ *
+ * This function is used for comparing two planes while sorting them to assign
+ * a normalized zpos values. Planes are compared first by their zpos values,
+ * then in case they equal, by plane id.
+ */
+static int __drm_atomic_helper_plane_zpos_cmp(const void *a, const void *b)
+{
+ const struct drm_plane *pa = *(struct drm_plane **)a;
+ const struct drm_plane *pb = *(struct drm_plane **)b;
+ int zpos_a = 0, zpos
[Intel-gfx] [PATCH 6/6] drm/i915: Only grab timestamps when needed
From: Tvrtko Ursulin
No need to call ktime_get_raw_ns twice per unlimited wait and can
also elimate a local variable.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_gem.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index de98dc41fb9f..c4f69579eb7a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1246,7 +1246,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE;
DEFINE_WAIT(wait);
unsigned long timeout_expire;
- s64 before, now;
+ s64 before = 0;
int ret;
WARN(!intel_irqs_enabled(dev_priv), "IRQs disabled");
@@ -1266,14 +1266,17 @@ int __i915_wait_request(struct drm_i915_gem_request
*req,
return -ETIME;
timeout_expire = jiffies + nsecs_to_jiffies_timeout(*timeout);
+
+ /*
+* Record current time in case interrupted by signal, or wedged.
+*/
+ before = ktime_get_raw_ns();
}
if (INTEL_INFO(dev_priv)->gen >= 6)
gen6_rps_boost(dev_priv, rps, req->emitted_jiffies);
- /* Record current time in case interrupted by signal, or wedged */
trace_i915_gem_request_wait_begin(req);
- before = ktime_get_raw_ns();
/* Optimistic spin for the next jiffie before touching IRQs */
ret = __i915_spin_request(req, state);
@@ -1331,11 +1334,10 @@ int __i915_wait_request(struct drm_i915_gem_request
*req,
finish_wait(&ring->irq_queue, &wait);
out:
- now = ktime_get_raw_ns();
trace_i915_gem_request_wait_end(req);
if (timeout) {
- s64 tres = *timeout - (now - before);
+ s64 tres = *timeout - (ktime_get_raw_ns() - before);
*timeout = tres < 0 ? 0 : tres;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/6] Misc cleanups
From: Tvrtko Ursulin Just some random stuff, mostly execlists and tiny bit of wait request. Spends a little bit fewer cycles in the hot paths, shrinks the source by a bit, results with a little bit less .text, and polutes with Gen conditionals at hot code paths a little bit less. Tvrtko Ursulin (6): drm/i915/bdw+: Replace list_del+list_add_tail with list_move_tail drm/i915: Don't need a timer to wake us up drm/i915: Avoid invariant conditionals in lrc interrupt handler drm/i915: Fail engine initialization if LRCA is incorrectly aligned drm/i915: Cache LRCA in the context drm/i915: Only grab timestamps when needed drivers/gpu/drm/i915/i915_debugfs.c | 15 ++--- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 40 + drivers/gpu/drm/i915/intel_lrc.c| 100 drivers/gpu/drm/i915/intel_lrc.h| 3 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + 6 files changed, 76 insertions(+), 85 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915/bdw+: Replace list_del+list_add_tail with list_move_tail
From: Tvrtko Ursulin
Same effect for slightly less source code and resulting binary.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/intel_lrc.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 23839ff04e27..8b6071fcd743 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -431,9 +431,8 @@ static void execlists_context_unqueue(struct
intel_engine_cs *ring)
/* Same ctx: ignore first request, as second request
* will update tail past first request's workload */
cursor->elsp_submitted = req0->elsp_submitted;
- list_del(&req0->execlist_link);
- list_add_tail(&req0->execlist_link,
- &ring->execlist_retired_req_list);
+ list_move_tail(&req0->execlist_link,
+ &ring->execlist_retired_req_list);
req0 = cursor;
} else {
req1 = cursor;
@@ -485,9 +484,8 @@ static bool execlists_check_remove_request(struct
intel_engine_cs *ring,
"Never submitted head request\n");
if (--head_req->elsp_submitted <= 0) {
- list_del(&head_req->execlist_link);
- list_add_tail(&head_req->execlist_link,
- &ring->execlist_retired_req_list);
+ list_move_tail(&head_req->execlist_link,
+
&ring->execlist_retired_req_list);
return true;
}
}
@@ -608,9 +606,8 @@ static int execlists_context_queue(struct
drm_i915_gem_request *request)
if (request->ctx == tail_req->ctx) {
WARN(tail_req->elsp_submitted != 0,
"More than 2 already-submitted reqs queued\n");
- list_del(&tail_req->execlist_link);
- list_add_tail(&tail_req->execlist_link,
- &ring->execlist_retired_req_list);
+ list_move_tail(&tail_req->execlist_link,
+ &ring->execlist_retired_req_list);
}
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] drm/i915: Cache LRCA in the context
From: Tvrtko Ursulin
LRCA lifetime is well defined so cache it so it can be looked up
cheaply from the interrupt context and at command submission
time.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/i915_debugfs.c | 15 ++
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_lrc.c| 40 -
drivers/gpu/drm/i915/intel_lrc.h| 3 ++-
4 files changed, 26 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b05bd189eab..714a45cf8a51 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1976,12 +1976,13 @@ static int i915_context_status(struct seq_file *m, void
*unused)
}
static void i915_dump_lrc_obj(struct seq_file *m,
- struct intel_engine_cs *ring,
- struct drm_i915_gem_object *ctx_obj)
+ struct intel_context *ctx,
+ struct intel_engine_cs *ring)
{
struct page *page;
uint32_t *reg_state;
int j;
+ struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
unsigned long ggtt_offset = 0;
if (ctx_obj == NULL) {
@@ -1991,7 +1992,7 @@ static void i915_dump_lrc_obj(struct seq_file *m,
}
seq_printf(m, "CONTEXT: %s %u\n", ring->name,
- intel_execlists_ctx_id(ctx_obj));
+ intel_execlists_ctx_id(ctx, ring));
if (!i915_gem_obj_ggtt_bound(ctx_obj))
seq_puts(m, "\tNot bound in GGTT\n");
@@ -2040,8 +2041,7 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
list_for_each_entry(ctx, &dev_priv->context_list, link) {
for_each_ring(ring, dev_priv, i) {
if (ring->default_context != ctx)
- i915_dump_lrc_obj(m, ring,
- ctx->engine[i].state);
+ i915_dump_lrc_obj(m, ctx, ring);
}
}
@@ -2115,11 +2115,8 @@ static int i915_execlists(struct seq_file *m, void *data)
seq_printf(m, "\t%d requests in queue\n", count);
if (head_req) {
- struct drm_i915_gem_object *ctx_obj;
-
- ctx_obj = head_req->ctx->engine[ring_id].state;
seq_printf(m, "\tHead request id: %u\n",
- intel_execlists_ctx_id(ctx_obj));
+ intel_execlists_ctx_id(head_req->ctx, ring));
seq_printf(m, "\tHead request tail: %u\n",
head_req->tail);
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8cf655c6fc03..b77a5d84eac2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -881,6 +881,7 @@ struct intel_context {
struct drm_i915_gem_object *state;
struct intel_ringbuffer *ringbuf;
int pin_count;
+ u32 lrca;
} engine[I915_NUM_RINGS];
struct list_head link;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b9d5862c01c9..c61da46e0472 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -265,7 +265,8 @@ int intel_sanitize_enable_execlists(struct drm_device *dev,
int enable_execlists
/**
* intel_execlists_ctx_id() - get the Execlists Context ID
- * @ctx_obj: Logical Ring Context backing object.
+ * @ctx: Context to get the ID for
+ * @ring: Engine to get the ID for
*
* Do not confuse with ctx->id! Unfortunately we have a name overload
* here: the old context ID we pass to userspace as a handler so that
@@ -275,14 +276,12 @@ int intel_sanitize_enable_execlists(struct drm_device
*dev, int enable_execlists
*
* Return: 20-bits globally unique context ID.
*/
-u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj)
+u32 intel_execlists_ctx_id(struct intel_context *ctx,
+ struct intel_engine_cs *ring)
{
- u32 lrca = i915_gem_obj_ggtt_offset(ctx_obj) +
- LRC_PPHWSP_PN * PAGE_SIZE;
-
/* LRCA is required to be 4K aligned so the more significant 20 bits
* are globally unique */
- return lrca >> 12;
+ return ctx->engine[ring->id].lrca >> 12;
}
static bool disable_lite_restore_wa(struct intel_engine_cs *ring)
@@ -297,13 +296,11 @@ static bool disable_lite_restore_wa(struct
intel_engine_cs *ring)
uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
struct intel_engine_cs *ring)
{
- struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
+ uint64_t lrca = ctx->engine[ring->id].lrca;
uint64_t desc = ring->ctx_desc_templat
Re: [Intel-gfx] [PATCH] drm/i915: Cleaning up DDI translation tables
On Tue, Jan 05, 2016 at 11:18:55AM -0800, Rodrigo Vivi wrote:
> No functional changes.
>
> That state the obvious and just duplicate the place we
> need to change whenever the table is updated. So let's clean it.
>
> Cc: Ville Syrjälä
> Signed-off-by: Rodrigo Vivi
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 22 +++---
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index bc8a497..d3611f5 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -133,12 +133,12 @@ static const struct ddi_buf_trans
> skl_ddi_translations_dp[] = {
> { 0x2016, 0x00A0, 0x0 },
> { 0x5012, 0x009B, 0x0 },
> { 0x7011, 0x0088, 0x0 },
> - { 0x80009010, 0x00C0, 0x1 },/* Uses I_boost level 0x1 */
> + { 0x80009010, 0x00C0, 0x1 },
> { 0x2016, 0x009B, 0x0 },
> { 0x5012, 0x0088, 0x0 },
> - { 0x80007011, 0x00C0, 0x1 },/* Uses I_boost level 0x1 */
> + { 0x80007011, 0x00C0, 0x1 },
> { 0x2016, 0x00DF, 0x0 },
> - { 0x80005012, 0x00C0, 0x1 },/* Uses I_boost level 0x1 */
> + { 0x80005012, 0x00C0, 0x1 },
> };
>
> /* Skylake U */
> @@ -146,12 +146,12 @@ static const struct ddi_buf_trans
> skl_u_ddi_translations_dp[] = {
> { 0x201B, 0x00A2, 0x0 },
> { 0x5012, 0x0088, 0x0 },
> { 0x80007011, 0x00CD, 0x0 },
> - { 0x80009010, 0x00C0, 0x1 },/* Uses I_boost level 0x1 */
> + { 0x80009010, 0x00C0, 0x1 },
> { 0x201B, 0x009D, 0x0 },
> - { 0x80005012, 0x00C0, 0x1 },/* Uses I_boost level 0x1 */
> - { 0x80007011, 0x00C0, 0x1 },/* Uses I_boost level 0x1 */
> + { 0x80005012, 0x00C0, 0x1 },
> + { 0x80007011, 0x00C0, 0x1 },
> { 0x2016, 0x0088, 0x0 },
> - { 0x80005012, 0x00C0, 0x1 },/* Uses I_boost level 0x1 */
> + { 0x80005012, 0x00C0, 0x1 },
> };
>
> /* Skylake Y */
> @@ -159,12 +159,12 @@ static const struct ddi_buf_trans
> skl_y_ddi_translations_dp[] = {
> { 0x0018, 0x00A2, 0x0 },
> { 0x5012, 0x0088, 0x0 },
> { 0x80007011, 0x00CD, 0x0 },
> - { 0x80009010, 0x00C0, 0x3 },/* Uses I_boost level 0x3 */
> + { 0x80009010, 0x00C0, 0x3 },
> { 0x0018, 0x009D, 0x0 },
> - { 0x80005012, 0x00C0, 0x3 },/* Uses I_boost level 0x3 */
> - { 0x80007011, 0x00C0, 0x3 },/* Uses I_boost level 0x3 */
> + { 0x80005012, 0x00C0, 0x3 },
> + { 0x80007011, 0x00C0, 0x3 },
> { 0x0018, 0x0088, 0x0 },
> - { 0x80005012, 0x00C0, 0x3 },/* Uses I_boost level 0x3 */
> + { 0x80005012, 0x00C0, 0x3 },
> };
>
> /*
> --
> 2.4.3
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915: Cache LRCA in the context
On Thu, Jan 07, 2016 at 04:36:20PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > LRCA lifetime is well defined so cache it so it can be looked up > cheaply from the interrupt context and at command submission > time. Or track the actual vma. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Only grab timestamps when needed
On Thu, Jan 07, 2016 at 04:36:21PM +, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > No need to call ktime_get_raw_ns twice per unlimited wait and can > also elimate a local variable. > > Signed-off-by: Tvrtko Ursulin > --- > drivers/gpu/drm/i915/i915_gem.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index de98dc41fb9f..c4f69579eb7a 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1246,7 +1246,7 @@ int __i915_wait_request(struct drm_i915_gem_request > *req, > int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; > DEFINE_WAIT(wait); > unsigned long timeout_expire; > - s64 before, now; > + s64 before = 0; I prefer my version. ;) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: fix get digital port issue in intel_audio
On Wed, Jan 06, 2016 at 10:26:41AM +0800, libin.y...@linux.intel.com wrote:
> From: Libin Yang
>
> For DP MST, use enc_to_mst(encoder)->primary to get intel_digital_port,
> instead of using enc_to_dig_port(encoder).
>
> Signed-off-by: Libin Yang
> ---
> drivers/gpu/drm/i915/intel_audio.c | 21 +
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> index 31f6d21..431487a0 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -187,6 +187,16 @@ static bool intel_eld_uptodate(struct drm_connector
> *connector,
> return true;
> }
>
> +static struct intel_digital_port *
> +intel_encoder_to_dig_port(struct intel_encoder *intel_encoder)
> +{
> + struct drm_encoder *encoder = &intel_encoder->base;
> +
> + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> + return enc_to_mst(encoder)->primary;
> + return enc_to_dig_port(encoder);
> +}
> +
> static void g4x_audio_codec_disable(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> @@ -286,7 +296,7 @@ static void hsw_audio_codec_enable(struct drm_connector
> *connector,
> struct i915_audio_component *acomp = dev_priv->audio_component;
> const uint8_t *eld = connector->eld;
> struct intel_digital_port *intel_dig_port =
> - enc_to_dig_port(&encoder->base);
> + intel_encoder_to_dig_port(encoder);
This hunk makes sense since we just look at intel_dig_port->port. Might
make sense to entirely eliminate the local inte_dig_port variable from
these hooks so that there's no confusion whether it points at the fake
or primary encoder.
> enum port port = intel_dig_port->port;
> uint32_t tmp;
> int len, i;
> @@ -500,7 +510,8 @@ void intel_audio_codec_enable(struct intel_encoder
> *intel_encoder)
> struct drm_device *dev = encoder->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct i915_audio_component *acomp = dev_priv->audio_component;
> - struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> + struct intel_digital_port *intel_dig_port =
> + intel_encoder_to_dig_port(intel_encoder);
> enum port port = intel_dig_port->port;
>
> connector = drm_select_eld(encoder);
> @@ -546,7 +557,8 @@ void intel_audio_codec_disable(struct intel_encoder
> *intel_encoder)
> struct drm_device *dev = encoder->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct i915_audio_component *acomp = dev_priv->audio_component;
> - struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> + struct intel_digital_port *intel_dig_port =
> + intel_encoder_to_dig_port(intel_encoder);
> enum port port = intel_dig_port->port;
>
> if (dev_priv->display.audio_codec_disable)
> @@ -724,7 +736,8 @@ static int i915_audio_component_get_eld(struct device
> *dev, int port,
> /* intel_encoder might be NULL for DP MST */
> if (intel_encoder) {
> ret = 0;
> - intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> + intel_dig_port =
> + intel_encoder_to_dig_port(intel_encoder);
> *enabled = intel_dig_port->audio_connector != NULL;
These not so much. We'd clobber 'intel_dig_port->audio_connector' for
the primary encoder whenever a new MST stream is enabled.
Was the intention just to fix the port information passed to
.pin_eld_notify()?
This whole thing seems highlight a rather big issue with the current
component stuff; How do you tell the streams apart when all are using
the same DDI port?
> if (*enabled) {
> eld = intel_dig_port->audio_connector->eld;
> --
> 1.9.1
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: simplify allocation of driver-internal requests
On Thu, Jan 07, 2016 at 08:49:38AM -0800, Jesse Barnes wrote: > On 01/07/2016 03:58 AM, Chris Wilson wrote: > > On Thu, Jan 07, 2016 at 10:20:50AM +, Dave Gordon wrote: > >> There are a number of places where the driver needs a request, but isn't > >> working on behalf of any specific user or in a specific context. At > >> present, we associate them with the per-engine default context. A future > >> patch will abolish those per-engine context pointers; but we can already > >> eliminate a lot of the references to them, just by making the allocator > >> allow NULL as a shorthand for "an appropriate context for this ring", > >> which will mean that the callers don't need to know anything about how > >> the "appropriate context" is found (e.g. per-ring vs per-device, etc). > >> > >> So this patch renames the existing i915_gem_request_alloc(), and makes > >> it local (static inline), and replaces it with a wrapper that provides > >> a default if the context is NULL, and also has a nicer calling > >> convention (doesn't require a pointer to an output parameter). Then we > >> change all callers to use the new convention: > >> OLD: > >>err = i915_gem_request_alloc(ring, user_ctx, &req); > >>if (err) ... > >> NEW: > >>req = i915_gem_request_alloc(ring, user_ctx); > >>if (IS_ERR(req)) ... > >> OLD: > >>err = i915_gem_request_alloc(ring, ring->default_context, &req); > >>if (err) ... > >> NEW: > >>req = i915_gem_request_alloc(ring, NULL); > >>if (IS_ERR(req)) ... > > > > Nak. You haven't fixed i915_gem_request_alloc() at all. > > > > http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=breadcrumbs&id=82c72e1a2b4385f0ab07dccee45acef38303e96f > > is the patch I have been carrying ever since. > > Can we stop with the "nak"? This patch wraps the request alloc > differently than yours, but you haven't given details as to why you > think it's incorrect (see Dave's reply). I am annoyed that people do not review my patches and are duplicating work I did last year and the year before, without attempting to fix real bugs. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: simplify allocation of driver-internal requests
On Thu, Jan 07, 2016 at 12:34:39PM +, Dave Gordon wrote: > On 07/01/16 11:58, Chris Wilson wrote: > >On Thu, Jan 07, 2016 at 10:20:50AM +, Dave Gordon wrote: > >>There are a number of places where the driver needs a request, but isn't > >>working on behalf of any specific user or in a specific context. At > >>present, we associate them with the per-engine default context. A future > >>patch will abolish those per-engine context pointers; but we can already > >>eliminate a lot of the references to them, just by making the allocator > >>allow NULL as a shorthand for "an appropriate context for this ring", > >>which will mean that the callers don't need to know anything about how > >>the "appropriate context" is found (e.g. per-ring vs per-device, etc). > >> > >>So this patch renames the existing i915_gem_request_alloc(), and makes > >>it local (static inline), and replaces it with a wrapper that provides > >>a default if the context is NULL, and also has a nicer calling > >>convention (doesn't require a pointer to an output parameter). Then we > >>change all callers to use the new convention: > >>OLD: > >>err = i915_gem_request_alloc(ring, user_ctx, &req); > >>if (err) ... > >>NEW: > >>req = i915_gem_request_alloc(ring, user_ctx); > >>if (IS_ERR(req)) ... > >>OLD: > >>err = i915_gem_request_alloc(ring, ring->default_context, &req); > >>if (err) ... > >>NEW: > >>req = i915_gem_request_alloc(ring, NULL); > >>if (IS_ERR(req)) ... > > > >Nak. You haven't fixed i915_gem_request_alloc() at all. > > > >http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=breadcrumbs&id=82c72e1a2b4385f0ab07dccee45acef38303e96f > >is the patch I have been carrying ever since. > >-Chris > > I think you'll find that the version of i915_gem_request_alloc() > I've implemented is equivalent to yours, with the *additional* (and > better) semantic of not requiring the caller to specify > (ring->default_param) as the context parameter (which is the main > point, as far as I'm concerned; making the calling convention nicer > was just incidental). No. Specifying the context is crucial to request allocation. The issue in the current function call chains are the requests appear out of nowhere rather than being created with explicit context. Hiding the context is bad. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Avoid invariant conditionals in lrc interrupt handler
On 07/01/16 16:42, Chris Wilson wrote:
On Thu, Jan 07, 2016 at 04:36:18PM +, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
There is no need to check on what Gen we are running on every
interrupt and every command submission. We can instead set up
some of that when engines are initialized, store it in the
engine structure and use it directly at runtime.
Signed-off-by: Tvrtko Ursulin
---
drivers/gpu/drm/i915/intel_lrc.c| 36 ++---
drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++
2 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8b6071fcd743..84977a6e6f3f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -298,29 +298,15 @@ uint64_t intel_lr_context_descriptor(struct intel_context
*ctx,
struct intel_engine_cs *ring)
{
struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
- uint64_t desc;
+ uint64_t desc = ring->ctx_desc_template;
uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj) +
LRC_PPHWSP_PN * PAGE_SIZE;
WARN_ON(lrca & 0x0FFFULL);
- desc = GEN8_CTX_VALID;
- desc |= GEN8_CTX_ADDRESSING_MODE(dev) << GEN8_CTX_ADDRESSING_MODE_SHIFT;
- if (IS_GEN8(ctx_obj->base.dev))
- desc |= GEN8_CTX_L3LLC_COHERENT;
- desc |= GEN8_CTX_PRIVILEGE;
desc |= lrca;
desc |= (u64)intel_execlists_ctx_id(ctx_obj) << GEN8_CTX_ID_SHIFT;
- /* TODO: WaDisableLiteRestore when we start using semaphore
-* signalling between Command Streamers */
- /* desc |= GEN8_CTX_FORCE_RESTORE; */
-
- /* WaEnableForceRestoreInCtxtDescForVCS:skl */
- /* WaEnableForceRestoreInCtxtDescForVCS:bxt */
- if (disable_lite_restore_wa(ring))
- desc |= GEN8_CTX_FORCE_RESTORE;
-
return desc;
}
@@ -556,7 +542,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
}
}
- if (disable_lite_restore_wa(ring)) {
+ if (ring->disable_lite_restore_wa) {
/* Prevent a ctx to preempt itself */
if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) &&
(submit_contexts != 0))
Didn't it occur to you that this code is garbage?
No, my default position is that I don't understand it well enough. If
you mean that the two if statements here can be merged and have a single
execlists_context_unqueue call site, sure, but why not do it in simpler
steps.
@@ -1980,6 +1966,24 @@ static int logical_ring_init(struct drm_device *dev,
struct intel_engine_cs *rin
goto error;
}
+ ring->disable_lite_restore_wa = disable_lite_restore_wa(ring);
+
+ ring->ctx_desc_template = GEN8_CTX_VALID;
+ ring->ctx_desc_template |= GEN8_CTX_ADDRESSING_MODE(dev) <<
+ GEN8_CTX_ADDRESSING_MODE_SHIFT;
+ if (IS_GEN8(dev))
+ ring->ctx_desc_template |= GEN8_CTX_L3LLC_COHERENT;
+ ring->ctx_desc_template |= GEN8_CTX_PRIVILEGE;
+
+ /* TODO: WaDisableLiteRestore when we start using semaphore
+* signalling between Command Streamers */
+ /* ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE; */
+
+ /* WaEnableForceRestoreInCtxtDescForVCS:skl */
+ /* WaEnableForceRestoreInCtxtDescForVCS:bxt */
+ if (ring->disable_lite_restore_wa)
+ ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE;
+
return 0;
error:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 49574ffe54bc..0b91a4b77359 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -268,6 +268,8 @@ struct intel_engine_cs {
struct list_head execlist_queue;
struct list_head execlist_retired_req_list;
u8 next_context_status_buffer;
+ bool disable_lite_restore_wa;
You don't need that as a separate flag. You know I sent this patch last
year?
Nope. :/
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915/bdw+: Replace list_del+list_add_tail with list_move_tail
On 07/01/16 16:45, Chris Wilson wrote: On Thu, Jan 07, 2016 at 04:36:16PM +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Same effect for slightly less source code and resulting binary. How about last year's patch that removed 200 lines from this very code in lrc? If it is small enough for easy quick review you can rebase and resend? Although I have other things from you to review first. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915: Cache LRCA in the context
On 07/01/16 16:46, Chris Wilson wrote: On Thu, Jan 07, 2016 at 04:36:20PM +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin LRCA lifetime is well defined so cache it so it can be looked up cheaply from the interrupt context and at command submission time. Or track the actual vma. This looked easier and good enough. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915/bxt: Add pipe_src size property
On Thu, Jan 07, 2016 at 05:58:39PM +0100, Daniel Vetter wrote: > On Thu, Jan 07, 2016 at 06:56:35PM +0200, Ville Syrjälä wrote: > > On Wed, Jan 06, 2016 at 08:57:57AM +0100, Daniel Vetter wrote: > > > On Tue, Jan 05, 2016 at 05:18:40PM +0200, Ville Syrjälä wrote: > > > > On Tue, Jan 05, 2016 at 03:50:38PM +0100, Daniel Vetter wrote: > > > > > On Mon, Jan 04, 2016 at 07:06:15PM +0200, Ville Syrjälä wrote: > > > > > > On Mon, Jan 04, 2016 at 11:16:39AM +0100, Maarten Lankhorst wrote: > > > > > > > Hey, > > > > > > > > > > > > > > Op 23-12-15 om 12:05 schreef Nabendu Maiti: > > > > > > > > This patch is adding pipesource size as property as intel > > > > > > > > property.User > > > > > > > > application is allowed to change the pipe source size in > > > > > > > > runtime on BXT/SKL. > > > > > > > > Added the property as inteli crtc property. > > > > > > > > > > > > > > > > Comments and suggestions are requested for whether we can keep > > > > > > > > the > > > > > > > > property as intel crtc property or move to drm level. > > > > > > > > > > > > > > > This property would get lost on a modeset. But why do you need a > > > > > > > pipe_src property? > > > > > > > > > > > > It's needed if we want to use the panel fitter with non-eDP/LVDS/DSI > > > > > > displays. > > > > > > > > > > > > Last time this came up I decided that we want to expose this via a > > > > > > new > > > > > > "fixed_mode" property. Ie. userspace can choose the actual display > > > > > > timings by setting the "fixed_mode" property with a specific mode, > > > > > > and > > > > > > then the normal mode property will basically just control just the > > > > > > pipe > > > > > > src size just like it already does for eDP/LVDS/DSI where we > > > > > > already have > > > > > > the fixed_mode internally (just not exposed to usersapce). If the > > > > > > fixed_mode is not specified, things will work just as they do right > > > > > > now. Obviously for eDP/LVDS/DSI we will reject any request to > > > > > > change/unset the fixed mode. > > > > > > > > > > > > The benefit of that approach is that things will work exactly the > > > > > > same > > > > > > way as before unless the user explicitly sets the fixed_mode, and > > > > > > once > > > > > > it's set thigns will work exactly the same way as they have worked > > > > > > so > > > > > > far for eDP/LVDS/DSI. Also it keeps the policy of choosing the fixed > > > > > > mode strictly is userspace for external displays. > > > > > > > > > > > > And as a bonus we will also expose the eDP/LVDS/DSI fixed_mode to > > > > > > userspace giving userspace more information on what the actual panel > > > > > > timings are. Currently userspace has to more or less guess that one > > > > > > of the modes the connector claims to support has the actual panel > > > > > > timings. > > > > > > > > > > > > So far I've not heard any opposing opinions to this plan. > > > > > > > > > > Hm yeah, pipe_src would run into all kinds of fun in conjunction with > > > > > the > > > > > existing fixed mode stuff we have. Just exposing the fixed as a prop > > > > > might > > > > > be simpler. But then we need to figure out what to do wrt the clock > > > > > in the > > > > > mode passed in through userspace - currently the fixed mode always > > > > > wins, > > > > > but for manual DRRS it would be nice if userspace could control it, > > > > > and > > > > > doesn't have to know that there's a fixed_mode. > > > > > > > > We could have the user mode vrefresh indicate the desired refresh rate > > > > of the fixed mode as well. In fact I've been wanting to add a check to > > > > make sure the user mode refresh rate isn't too far off from the > > > > fixed/downlclock mode vrefresh, but didn't get around to it yet. > > > > > > Yeah agreed, userspace vrefresh should control (or at least be checked > > > against) the fixed mode vrefresh. > > > > > > > > So semantically I think only exposing the pipe src to expose panel > > > > > fitters > > > > > would be cleaner. Then we'd need to deal with some internal troubles > > > > > to > > > > > make sure we combine everything correctly, but that shouldn't be too > > > > > hard > > > > > really. > > > > > > > > I think it's quite a bit more work since we have to redo all the fb size > > > > checks and whatnot to use the property when specified. I once outlined > > > > a more detailed plan for his approach too (too lazy to dig up the mail), > > > > but I think the fixed mode prop is a simpler approach and won't actually > > > > require much in the way of userspace changes either. It'll be enough to > > > > set the property once and then even the legacy modeset ioctls will work > > > > exactly like they do now for eDP/LVDS/DSI. > > > > > > One downside of the fixed mode against an explicit pipe src rectangle is > > > that the explict rectangle allows us to letter/pillar/stretch/move too. > > > With just a fixed_mode that's much harder to pull off. > > > > Nope. It's the same.
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Only grab timestamps when needed
On 07/01/16 16:47, Chris Wilson wrote: On Thu, Jan 07, 2016 at 04:36:21PM +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin No need to call ktime_get_raw_ns twice per unlimited wait and can also elimate a local variable. Signed-off-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index de98dc41fb9f..c4f69579eb7a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1246,7 +1246,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req, int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE; DEFINE_WAIT(wait); unsigned long timeout_expire; - s64 before, now; + s64 before = 0; I prefer my version. ;) Advantage of this one is it can be reviewed and merged in a minute for immediate, even if imperceptible, gain. :I Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Init power domains early in driver load
> commit ac9b8236551d1177fd07b56aef9b565d1864420d
> Author: Ville Syrjälä
> Date: Fri Nov 27 18:55:26 2015 +0200
>
> drm/i915: Introduce a gmbus power domain
>
> gmbus also needs the power domain infrastructure right from the start,
> since as soon as we register the i2c controllers someone can use them.
>
> v2: Adjust cleanup paths too (Chris).
>
> v3: Rebase onto -nightly (totally bogus tree I had lying around) and
> also move dpio init head (Ville).
>
> v4: Ville instead suggested to move gmbus setup later in the sequence,
> since it's only needed by the modeset code.
>
> v5: Move even close to the actual user, right next to the comment that
> states where we really need gmbus (and interrupts!).
>
> Cc: Ville Syrjälä
> Cc: Patrik Jakobsson
> Cc: Imre Deak
> Cc: Jani Nikula
> Cc: Meelis Roos
> Cc: Chris Wilson
> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain")
> Cc: sta...@vger.kernel.org
> References: http://www.spinics.net/lists/intel-gfx/msg83075.html
> Signed-off-by: Daniel Vetter
> ---
>
> Meelis, can you pls retest this one?
I also confirmed that my 865G chipset computer was suffering from the
same problem and the patch also helps on D865GLC mainboard with my
userspace that autoloads eeprom driver.
--
Meelis Roos (mr...@linux.ee)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 i-g-t] tests/gem_softpin: Use offset addresses in canonical form
On Wed, Jan 06, 2016 at 03:00:39PM +, Michel Thierry wrote:
> i915 validates that requested offset is in canonical form, so tests need
> to convert the offsets as required.
>
> Also add test to verify non-canonical 48-bit address will be rejected.
>
> v2: Use sign_extend64 for converting to canonical form (Tvrtko)
>
> Cc: Vinay Belgaumkar
> Cc: Tvrtko Ursulin
> Reviewed-by: Vinay Belgaumkar (v1)
> Signed-off-by: Michel Thierry
> Reviewed-by: Vinay Belgaumkar
> ---
> tests/gem_softpin.c | 69
> +
> 1 file changed, 49 insertions(+), 20 deletions(-)
>
> diff --git a/tests/gem_softpin.c b/tests/gem_softpin.c
> index 0919716..1cbde4e 100644
> --- a/tests/gem_softpin.c
> +++ b/tests/gem_softpin.c
> @@ -67,7 +67,7 @@ static void *create_mem_buffer(uint64_t size);
> static int gem_call_userptr_ioctl(int fd, i915_gem_userptr *userptr);
> static void gem_pin_userptr_test(void);
> static void gem_pin_bo_test(void);
> -static void gem_pin_invalid_vma_test(bool test_decouple_flags);
> +static void gem_pin_invalid_vma_test(bool test_decouple_flags, bool
> test_canonical_offset);
> static void gem_pin_overlap_test(void);
> static void gem_pin_high_address_test(void);
>
> @@ -198,6 +198,18 @@ static void setup_exec_obj(struct
> drm_i915_gem_exec_object2 *exec,
> exec->offset = offset;
> }
>
> +/* gen8_canonical_addr
> + * Used to convert any address into canonical form, i.e. [63:48] == [47].
> + * Based on kernel's sign_extend64 implementation.
> + * @address - a virtual address
> +*/
> +#define GEN8_HIGH_ADDRESS_BIT 47
> +static uint64_t gen8_canonical_addr(uint64_t address)
> +{
> + __u8 shift = 63 - GEN8_HIGH_ADDRESS_BIT;
> + return (__s64)(address << shift) >> shift;
> +}
> +
> /* gem_store_data_svm
> * populate batch buffer with MI_STORE_DWORD_IMM command
> * @fd: drm file descriptor
> @@ -630,6 +642,7 @@ static void gem_pin_overlap_test(void)
> * Share with GPU using userptr ioctl
> * Create batch buffer to write DATA in first element of each buffer
> * Pin each buffer to varying addresses starting from 0x8000 going
> below
> + * (requires offsets in canonical form)
> * Execute Batch Buffer on Blit ring STRESS_NUM_LOOPS times
> * Validate every buffer has DATA in first element
> * Rinse and Repeat on Render ring
> @@ -637,7 +650,7 @@ static void gem_pin_overlap_test(void)
> #define STRESS_NUM_BUFFERS 10
> #define STRESS_NUM_LOOPS 100
> #define STRESS_STORE_COMMANDS 4 * STRESS_NUM_BUFFERS
> -
> +#define STRESS_START_ADDRESS 0x8000
> static void gem_softpin_stress_test(void)
> {
> i915_gem_userptr userptr;
> @@ -650,7 +663,7 @@ static void gem_softpin_stress_test(void)
> uint32_t batch_buf_handle;
> int ring, len;
> int buf, loop;
> - uint64_t pinning_offset = 0x8000;
> + uint64_t pinning_offset = STRESS_START_ADDRESS;
>
> fd = drm_open_driver(DRIVER_INTEL);
> igt_require(uses_full_ppgtt(fd, FULL_48_BIT_PPGTT));
> @@ -680,10 +693,10 @@ static void gem_softpin_stress_test(void)
> setup_exec_obj(&exec_object2[buf], shared_handle[buf],
> EXEC_OBJECT_PINNED |
> EXEC_OBJECT_SUPPORTS_48B_ADDRESS,
> -pinning_offset);
> +gen8_canonical_addr(pinning_offset));
> len += gem_store_data_svm(fd, batch_buffer + (len/4),
> - pinning_offset, buf,
> - (buf == STRESS_NUM_BUFFERS-1)? \
> + gen8_canonical_addr(pinning_offset),
> + buf, (buf == STRESS_NUM_BUFFERS-1)? \
> true:false);
>
> /* decremental 4K aligned address */
> @@ -705,10 +718,11 @@ static void gem_softpin_stress_test(void)
> for (loop = 0; loop < STRESS_NUM_LOOPS; loop++) {
> submit_and_sync(fd, &execbuf, batch_buf_handle);
> /* Set pinning offset back to original value */
> - pinning_offset = 0x8000;
> + pinning_offset = STRESS_START_ADDRESS;
> for(buf = 0; buf < STRESS_NUM_BUFFERS; buf++) {
> gem_userptr_sync(fd, shared_handle[buf]);
> - igt_assert(exec_object2[buf].offset ==
> pinning_offset);
> + igt_assert(exec_object2[buf].offset ==
> + gen8_canonical_addr(pinning_offset));
> igt_fail_on_f(*shared_buffer[buf] != buf, \
> "Mismatch in buffer %d, iteration %d:
> 0x%08X\n", \
> buf, loop, *shared_buffer[buf]);
> @@ -727,10 +741,11 @@ static void gem_softpin_stress_test(void)
>STRES
[Intel-gfx] [PATCH i-g-t] kms_psr_sink_crc: Simplify debugfs reading.
Let's start using igt_debugfs_read helper so we
can change the debugfs interface at anytime.
Signed-off-by: Rodrigo Vivi
---
tests/kms_psr_sink_crc.c | 57 +++-
1 file changed, 8 insertions(+), 49 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 5fad9b5..b18e426 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -199,63 +199,22 @@ static void fill_render(data_t *data, uint32_t handle,
unsigned char color)
static bool psr_possible(data_t *data)
{
- FILE *file;
- char buf[4096];
- int ret;
-
- if (running_with_psr_disabled)
- return true;
-
- file = igt_debugfs_fopen("i915_edp_psr_status", "r");
- igt_require(file);
-
- /* First dump the entire file into the debug log for later analysis
-* if required.
-*/
- ret = fread(buf, 1, 4095, file);
- igt_require(ret > 0);
- buf[ret] = '\0';
- igt_debug("i915_edp_psr_status:\n%s", buf);
- fseek(file, 0, SEEK_SET);
+ char buf[512];
- /* Now check that we have all the preconditions required for PSR */
- ret = fscanf(file, "Sink_Support: %s\n", buf);
- igt_require_f(ret == 1 && strcmp(buf, "yes") == 0,
- "Sink_Support: %s\n", buf);
+ igt_debugfs_read("i915_edp_psr_status", buf);
- fclose(file);
- return true;
+ return running_with_psr_disabled ||
+ strstr(buf, "Sink_Support: yes\n");
}
static bool psr_active(data_t *data)
{
- int ret;
- FILE *file;
- char str[4];
+ char buf[512];
- if (running_with_psr_disabled)
- return true;
+ igt_debugfs_read("i915_edp_psr_status", buf);
- file = igt_debugfs_fopen("i915_edp_psr_status", "r");
- igt_require(file);
-
- ret = fscanf(file, "Sink_Support: %s\n", str);
- igt_assert_neq(ret, 0);
- ret = fscanf(file, "Source_OK: %s\n", str);
- igt_assert_neq(ret, 0);
- ret = fscanf(file, "Enabled: %s\n", str);
- igt_assert_neq(ret, 0);
- ret = fscanf(file, "Active: %s\n", str);
- igt_assert_neq(ret, 0);
- ret = fscanf(file, "Busy frontbuffer bits: %s\n", str);
- igt_assert_neq(ret, 0);
- ret = fscanf(file, "Re-enable work scheduled: %s\n", str);
- igt_assert_neq(ret, 0);
- ret = fscanf(file, "HW Enabled & Active bit: %s\n", str);
- igt_assert_neq(ret, 0);
-
- fclose(file);
- return strcmp(str, "yes") == 0;
+ return running_with_psr_disabled ||
+ strstr(buf, "HW Enabled & Active bit: yes\n");
}
static bool wait_psr_entry(data_t *data)
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests/gem_guc_loading: Adding simple GuC loading test
This has been reviewed internally. LGTM.
Reviewed-by: Alex Dai
On 01/05/2016 08:17 AM, Lukasz Fiedorowicz wrote:
Test check GuC debugfs file for successful loading confirmation
Signed-off-by: Lukasz Fiedorowicz
---
tests/Makefile.sources | 1 +
tests/gem_guc_loading.c | 89 +
2 files changed, 90 insertions(+)
create mode 100644 tests/gem_guc_loading.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index d594038..331234f 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -36,6 +36,7 @@ TESTS_progs_M = \
gem_flink_basic \
gem_flink_race \
gem_linear_blits \
+ gem_guc_loading \
gem_madvise \
gem_mmap \
gem_mmap_gtt \
diff --git a/tests/gem_guc_loading.c b/tests/gem_guc_loading.c
new file mode 100644
index 000..fd53a46
--- /dev/null
+++ b/tests/gem_guc_loading.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Lukasz Fiedorowicz
+ *
+ */
+
+#include
+#include
+#include
+#include
+
+#include "igt.h"
+
+IGT_TEST_DESCRIPTION("GuC firmware loading test.");
+
+#define LOAD_STATUS_BUF_SIZE 96
+
+enum guc_status { GUC_ENABLED, GUC_DISABLED };
+
+int guc_status_fd;
+
+static void open_guc_status(void)
+{
+ guc_status_fd = igt_debugfs_open("i915_guc_load_status", O_RDONLY);
+ igt_assert_f(guc_status_fd >= 0, "Can't open i915_guc_load_status\n");
+}
+
+static enum guc_status get_guc_status(void)
+{
+ char buf[LOAD_STATUS_BUF_SIZE];
+
+ FILE *fp = fdopen(guc_status_fd, "r");
+ igt_assert_f(fp != NULL, "Can't open i915_guc_load_status file\n");
+
+ while (fgets(buf, LOAD_STATUS_BUF_SIZE, fp))
+ if ((strstr(buf, "\tload: SUCCESS\n")))
+ return GUC_ENABLED;
+
+ return GUC_DISABLED;
+}
+
+static void close_guc_status(void)
+{
+ close(guc_status_fd);
+}
+
+static void test_guc_loaded()
+{
+ igt_assert_f(get_guc_status() == GUC_ENABLED, "GuC is disabled\n");
+}
+
+igt_main
+{
+ int gfx_fd = 0;
+ int gen = 0;
+
+ igt_fixture
+ {
+ gfx_fd = drm_open_driver(DRIVER_INTEL);
+ gen = intel_gen(intel_get_drm_devid(gfx_fd));
+ igt_require(gen >= 9);
+ open_guc_status();
+ }
+
+ igt_subtest("guc_loaded") test_guc_loaded();
+
+ igt_fixture close_guc_status();
+}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915/kbl: Adding missing IS_KABYLAKE checks.
Hi Michel,
On Thu, 2016-01-07 at 16:14 +, Michel Thierry wrote:
> On 1/7/2016 1:15 AM, Rodrigo Vivi wrote:
> > When adding IS_KABYLAKE definition I didn't included the
> > DC states related because I was planing to include them
> > with the patch that fixes DMC firmware loading, but I
> > forgot them.
> >
> > Meanwhile this runtime pm code changed a lot for
> > Skylake.
> >
> > Well, I didn't expect that this would crash the machine
> > and I just noticed now that Sarah warned me our driver
> > wasn't working. Thanks Sarah.
> >
> > Cc: Sarah Sharp
> > Signed-off-by: Rodrigo Vivi
> > ---
> > drivers/gpu/drm/i915/intel_csr.c| 5 +++--
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 19 ---
> > 2 files changed, 15 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_csr.c
> > b/drivers/gpu/drm/i915/intel_csr.c
> > index 9bb63a8..3f69829 100644
> > --- a/drivers/gpu/drm/i915/intel_csr.c
> > +++ b/drivers/gpu/drm/i915/intel_csr.c
> > @@ -278,7 +278,8 @@ static uint32_t *parse_csr_fw(struct
> > drm_i915_private *dev_priv,
> >
> > csr->version = css_header->version;
> >
> > - if (IS_SKYLAKE(dev) && csr->version <
> > SKL_CSR_VERSION_REQUIRED) {
> > + if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) &&
> > +csr->version < SKL_CSR_VERSION_REQUIRED) {
> > DRM_INFO("Refusing to load old Skylake DMC
> > firmware v%u.%u,"
> > " please upgrade to v%u.%u or later"
> > " [
> > https://01.org/linuxgraphics/intel-linux-graphics-firmwares].\n";,
> > @@ -421,7 +422,7 @@ void intel_csr_ucode_init(struct
> > drm_i915_private *dev_priv)
> > if (!HAS_CSR(dev_priv))
> > return;
> >
> > - if (IS_SKYLAKE(dev_priv))
> > + if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
> > csr->fw_path = I915_CSR_SKL;
> > else if (IS_BROXTON(dev_priv))
> > csr->fw_path = I915_CSR_BXT;
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index ddbdbff..f5bf54c 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -532,7 +532,8 @@ static void assert_can_enable_dc5(struct
> > drm_i915_private *dev_priv)
> > bool pg2_enabled =
> > intel_display_power_well_is_enabled(dev_priv,
> > SKL_DISP_PW_2);
> >
> > - WARN_ONCE(!IS_SKYLAKE(dev), "Platform doesn't support
> > DC5.\n");
> > + WARN_ONCE(!IS_SKYLAKE(dev) && !IS_KABYLAKE(dev),
> > + "Platform doesn't support DC5.\n");
> > WARN_ONCE(!HAS_RUNTIME_PM(dev), "Runtime PM not
> > enabled.\n");
> > WARN_ONCE(pg2_enabled, "PG2 not disabled to enable
> > DC5.\n");
> >
> > @@ -568,7 +569,8 @@ static void assert_can_enable_dc6(struct
> > drm_i915_private *dev_priv)
> > {
> > struct drm_device *dev = dev_priv->dev;
> >
> > - WARN_ONCE(!IS_SKYLAKE(dev), "Platform doesn't support
> > DC6.\n");
> > + WARN_ONCE(!IS_SKYLAKE(dev) && !IS_KABYLAKE(dev),
> > + "Platform doesn't support DC6.\n");
> > WARN_ONCE(!HAS_RUNTIME_PM(dev), "Runtime PM not
> > enabled.\n");
> > WARN_ONCE(I915_READ(UTIL_PIN_CTL) & UTIL_PIN_ENABLE,
> > "Backlight is not disabled.\n");
> > @@ -595,7 +597,8 @@ static void gen9_disable_dc5_dc6(struct
> > drm_i915_private *dev_priv)
> > {
> > assert_can_disable_dc5(dev_priv);
> >
> > - if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 &&
> > i915.enable_dc != 1)
> > + if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> > + i915.enable_dc != 0 && i915.enable_dc != 1)
> > assert_can_disable_dc6(dev_priv);
> >
> > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > @@ -783,7 +786,8 @@ static void
> > gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> > static void gen9_dc_off_power_well_disable(struct
> > drm_i915_private *dev_priv,
> >struct i915_power_well
> > *power_well)
> > {
> > - if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 &&
> > i915.enable_dc != 1)
> > + if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> > + i915.enable_dc != 0 && i915.enable_dc != 1)
> > skl_enable_dc6(dev_priv);
> > else
> > gen9_enable_dc5(dev_priv);
> > @@ -795,7 +799,8 @@ static void
> > gen9_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
> > if (power_well->count > 0) {
> > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > } else {
> > - if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 &&
> > + if ((IS_SKYLAKE(dev_priv) ||
> > IS_KABYLAKE(dev_priv)) &&
> > + i915.enable_dc != 0 &&
> > i915.enable_dc != 1)
> > gen9_set_dc_state(dev_priv,
> > DC_STATE_EN_UPTO_DC6);
> > else
> > @@ -1851,7 +1856,7 @@ void skl_pw1_misc_io_init(struct
> > drm_i915_private *dev
[Intel-gfx] [PATCH] drm/i915/kbl: Adding missing IS_KABYLAKE checks.
When adding IS_KABYLAKE definition I didn't included the
DC states related because I was planing to include them
with the patch that fixes DMC firmware loading, but I
forgot them.
Meanwhile this runtime pm code changed a lot for
Skylake.
Well, I didn't expect that this would crash the machine
and I just noticed now that Sarah warned me our driver
wasn't working. Thanks Sarah.
Michel had found the main error first and his
fix had better details on the history and got
merged already:
commit 16fbc291cb87c7defcd13ad715d3e4af0d523e43
Author: Michel Thierry
Date: Wed Jan 6 12:08:36 2016 +
drm/i915/kbl: Enable PW1 and Misc I/O power wells
This one is a follow-up adding the other remaining
missing pieces.
v2: Rebased on top of Michel's patch as explained above.
Cc: Sarah Sharp
Signed-off-by: Rodrigo Vivi
Reviewed-by: Michel Thierry
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_csr.c| 5 +++--
drivers/gpu/drm/i915/intel_runtime_pm.c | 15 ++-
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 9bb63a8..3f69829 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -278,7 +278,8 @@ static uint32_t *parse_csr_fw(struct drm_i915_private
*dev_priv,
csr->version = css_header->version;
- if (IS_SKYLAKE(dev) && csr->version < SKL_CSR_VERSION_REQUIRED) {
+ if ((IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) &&
+csr->version < SKL_CSR_VERSION_REQUIRED) {
DRM_INFO("Refusing to load old Skylake DMC firmware v%u.%u,"
" please upgrade to v%u.%u or later"
"
[https://01.org/linuxgraphics/intel-linux-graphics-firmwares].\n";,
@@ -421,7 +422,7 @@ void intel_csr_ucode_init(struct drm_i915_private *dev_priv)
if (!HAS_CSR(dev_priv))
return;
- if (IS_SKYLAKE(dev_priv))
+ if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
csr->fw_path = I915_CSR_SKL;
else if (IS_BROXTON(dev_priv))
csr->fw_path = I915_CSR_BXT;
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 4b44e68..89a7dd8 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -532,7 +532,8 @@ static void assert_can_enable_dc5(struct drm_i915_private
*dev_priv)
bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
SKL_DISP_PW_2);
- WARN_ONCE(!IS_SKYLAKE(dev), "Platform doesn't support DC5.\n");
+ WARN_ONCE(!IS_SKYLAKE(dev) && !IS_KABYLAKE(dev),
+ "Platform doesn't support DC5.\n");
WARN_ONCE(!HAS_RUNTIME_PM(dev), "Runtime PM not enabled.\n");
WARN_ONCE(pg2_enabled, "PG2 not disabled to enable DC5.\n");
@@ -568,7 +569,8 @@ static void assert_can_enable_dc6(struct drm_i915_private
*dev_priv)
{
struct drm_device *dev = dev_priv->dev;
- WARN_ONCE(!IS_SKYLAKE(dev), "Platform doesn't support DC6.\n");
+ WARN_ONCE(!IS_SKYLAKE(dev) && !IS_KABYLAKE(dev),
+ "Platform doesn't support DC6.\n");
WARN_ONCE(!HAS_RUNTIME_PM(dev), "Runtime PM not enabled.\n");
WARN_ONCE(I915_READ(UTIL_PIN_CTL) & UTIL_PIN_ENABLE,
"Backlight is not disabled.\n");
@@ -595,7 +597,8 @@ static void gen9_disable_dc5_dc6(struct drm_i915_private
*dev_priv)
{
assert_can_disable_dc5(dev_priv);
- if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 && i915.enable_dc != 1)
+ if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
+ i915.enable_dc != 0 && i915.enable_dc != 1)
assert_can_disable_dc6(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
@@ -783,7 +786,8 @@ static void gen9_dc_off_power_well_enable(struct
drm_i915_private *dev_priv,
static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
struct i915_power_well *power_well)
{
- if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 && i915.enable_dc != 1)
+ if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
+ i915.enable_dc != 0 && i915.enable_dc != 1)
skl_enable_dc6(dev_priv);
else
gen9_enable_dc5(dev_priv);
@@ -795,7 +799,8 @@ static void gen9_dc_off_power_well_sync_hw(struct
drm_i915_private *dev_priv,
if (power_well->count > 0) {
gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
} else {
- if (IS_SKYLAKE(dev_priv) && i915.enable_dc != 0 &&
+ if ((IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) &&
+ i915.enable_dc != 0 &&
i915.enable_dc != 1)
gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC6);
else
--
2.4.3
__
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Don't warn if the workaround list is empty part 2.
Daniel, Ack? So I can merge this and the missing IS_KABYLAKE ones... The preliminary one I agree with you and will put a request for KBL on CI. Thanks in advance, Rodrigo. On Wed, Jan 6, 2016 at 5:15 PM Rodrigo Vivi wrote: > From: "Boyer, Wayne" > > Extend the same reasoning as in the patch listed below. It's not an > error for the workaround list to be empty if no workarounds are needed. > > commit 02235808b61cd9382d224b0df263193006dd9913 > Author: Francisco Jerez > Date: Wed Oct 7 14:44:01 2015 +0300 > drm/i915: Don't warn if the workaround list is empty. > > Signed-off-by: Wayne Boyer > Signed-off-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index 8096c6a..641d21c 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1100,7 +1100,7 @@ static int > intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req) > struct drm_i915_private *dev_priv = dev->dev_private; > struct i915_workarounds *w = &dev_priv->workarounds; > > - if (WARN_ON_ONCE(w->count == 0)) > + if (w->count == 0) > return 0; > > ring->gpu_caches_dirty = true; > -- > 2.4.3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: fix get digital port issue in intel_audio
Hi Ville,
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Friday, January 08, 2016 12:50 AM
> To: libin.y...@linux.intel.com
> Cc: intel-gfx@lists.freedesktop.org; conselv...@gmail.com;
> jani.nik...@linux.intel.com; Vetter, Daniel; ti...@suse.de; Yang, Libin
> Subject: Re: [PATCH 1/2] drm/i915: fix get digital port issue in intel_audio
>
> On Wed, Jan 06, 2016 at 10:26:41AM +0800, libin.y...@linux.intel.com
> wrote:
> > From: Libin Yang
> >
> > For DP MST, use enc_to_mst(encoder)->primary to get
> intel_digital_port,
> > instead of using enc_to_dig_port(encoder).
> >
> > Signed-off-by: Libin Yang
> > ---
> > drivers/gpu/drm/i915/intel_audio.c | 21 +
> > 1 file changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> b/drivers/gpu/drm/i915/intel_audio.c
> > index 31f6d21..431487a0 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -187,6 +187,16 @@ static bool intel_eld_uptodate(struct
> drm_connector *connector,
> > return true;
> > }
> >
> > +static struct intel_digital_port *
> > +intel_encoder_to_dig_port(struct intel_encoder *intel_encoder)
> > +{
> > + struct drm_encoder *encoder = &intel_encoder->base;
> > +
> > + if (intel_encoder->type == INTEL_OUTPUT_DP_MST)
> > + return enc_to_mst(encoder)->primary;
> > + return enc_to_dig_port(encoder);
> > +}
> > +
> > static void g4x_audio_codec_disable(struct intel_encoder *encoder)
> > {
> > struct drm_i915_private *dev_priv = encoder->base.dev-
> >dev_private;
> > @@ -286,7 +296,7 @@ static void hsw_audio_codec_enable(struct
> drm_connector *connector,
> > struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> > const uint8_t *eld = connector->eld;
> > struct intel_digital_port *intel_dig_port =
> > - enc_to_dig_port(&encoder->base);
> > + intel_encoder_to_dig_port(encoder);
>
> This hunk makes sense since we just look at intel_dig_port->port. Might
> make sense to entirely eliminate the local inte_dig_port variable from
> these hooks so that there's no confusion whether it points at the fake
> or primary encoder.
Do you mean we can still use enc_to_dig_port()? Maybe the new code
is better. What do you think we use the wrapper
intel_encoder_to_dig_port() here?
>
> > enum port port = intel_dig_port->port;
> > uint32_t tmp;
> > int len, i;
> > @@ -500,7 +510,8 @@ void intel_audio_codec_enable(struct
> intel_encoder *intel_encoder)
> > struct drm_device *dev = encoder->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> > - struct intel_digital_port *intel_dig_port =
> enc_to_dig_port(encoder);
> > + struct intel_digital_port *intel_dig_port =
> > + intel_encoder_to_dig_port(intel_encoder);
> > enum port port = intel_dig_port->port;
> >
> > connector = drm_select_eld(encoder);
> > @@ -546,7 +557,8 @@ void intel_audio_codec_disable(struct
> intel_encoder *intel_encoder)
> > struct drm_device *dev = encoder->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct i915_audio_component *acomp = dev_priv-
> >audio_component;
> > - struct intel_digital_port *intel_dig_port =
> enc_to_dig_port(encoder);
> > + struct intel_digital_port *intel_dig_port =
> > + intel_encoder_to_dig_port(intel_encoder);
>
> > enum port port = intel_dig_port->port;
> >
> > if (dev_priv->display.audio_codec_disable)
> > @@ -724,7 +736,8 @@ static int
> i915_audio_component_get_eld(struct device *dev, int port,
> > /* intel_encoder might be NULL for DP MST */
> > if (intel_encoder) {
> > ret = 0;
> > - intel_dig_port = enc_to_dig_port(&intel_encoder-
> >base);
> > + intel_dig_port =
> > + intel_encoder_to_dig_port(intel_encoder);
> > *enabled = intel_dig_port->audio_connector != NULL;
>
> These not so much. We'd clobber 'intel_dig_port->audio_connector' for
> the primary encoder whenever a new MST stream is enabled.
Yes. If we are using i915_audio_component_get_eld() for MST audio,
we need a parameter device_entry_id in the function. I remember
David has sent a patch to support device entry before. But MST was
not supported and he removed the device_entry_id parameter.
>
> Was the intention just to fix the port information passed to
> .pin_eld_notify()?
No. It's based on device entry.
>
> This whole thing seems highlight a rather big issue with the current
> component stuff; How do you tell the streams apart when all are using
> the same DDI port?
If we need support other device entries, we need get
the other ports besides primary port. Do you know how
to get the ports?
As Takashi has changed to get eld_info from unsol_event to using this
callback, it seems this is a must to support MST a
Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
On 1/7/2016 10:15 AM, Ville Syrjälä wrote:
On Mon, Dec 21, 2015 at 05:18:52PM -0800, abhay.ku...@intel.com wrote:
From: Abhay Kumar
Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.
v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
delay calculation(Ville).
The approach seems reasonable enough to me. There are a few issues with
the patch though, see below.
Cc: Ville Syrjälä
Signed-off-by: Abhay Kumar
---
drivers/gpu/drm/i915/intel_ddi.c | 3 +++
drivers/gpu/drm/i915/intel_dp.c | 22 ++
drivers/gpu/drm/i915/intel_drv.h | 2 +-
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5..480697d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder
*intel_encoder)
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_vdd_on(intel_dp);
intel_edp_panel_off(intel_dp);
+
+ /* storing panel power off time */
The comment seems rather pointless.
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
There appears to be a wrapper for this: ktime_get_boottime().
Not sure why you're adding this here anyway. Should be enough to just
replace the places where we currently sample jiffies to sample the boot
clock AFAICS.
}
if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..c813605 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
#include "intel_drv.h"
#include
#include "i915_drv.h"
-
Spurious change.
#define DP_LINK_CHECK_TIMEOUT (10 * 1000)
/* Compliance test status bits */
@@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
static void wait_panel_power_cycle(struct intel_dp *intel_dp)
{
+ ktime_t panel_power_on_time;
+ u32 panel_power_off_duration;
+
DRM_DEBUG_KMS("Wait for panel power cycle\n");
- /* When we disable the VDD override bit last we have to do the manual
-* wait. */
- wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
- intel_dp->panel_power_cycle_delay);
+/* take the diffrence of currrent time and panel power off time
+ and then make panel wait for t11_t12 if needed */
Indent fail. Also the comment isn't in proper format.
+ panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+ panel_power_off_duration = (panel_power_on_time.tv64 -
intel_dp->panel_power_off_time.tv64);
+ panel_power_off_duration = panel_power_off_duration / 100;
ktime_ms_delta() perhaps?
sure.
+ /* When we disable the VDD override bit last we have to do the manual
+* wait */
This comment formatting is a bit wonky too. Maybe polish it up while at
it.
+ if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+ wait_remaining_ms_from_jiffies(jiffies,
+ (intel_dp->panel_power_cycle_delay -
panel_power_off_duration));
wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
}
@@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
if ((pp & POWER_TARGET_ON) == 0)
- intel_dp->last_power_cycle = jiffies;
+ intel_dp->panel_power_off_time =
ktime_get_with_offset(TK_OFFS_BOOT);
power_domain = intel_display_port_aux_power_domain(intel_encoder);
intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2126,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
- intel_dp->last_power_cycle = jiffies;
Removing this doens't seem correct. Instead we should sample the clock
here as well.
Do we really need "last_power_cycle" ? As the ktime_get_bootime() will
always calculate the boot time in fresh boot from zero and we only need
to track the delta time when we go to suspend and resume.
this is the reason i removed last_power_cycle sampling and also
initialization. Please let me know if this is ok and make sense?
wait_panel_off(intel_dp);
/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5129,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct
drm_connector *connect
static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
{
- intel_dp->last_power_cycle = jiffies;
and I suppose here too.
intel_dp->last_power_on = jiffies;
intel_dp->last_backlight_off = jiffies;
}
diff --gi
Re: [Intel-gfx] [PATCH i-g-t] tests/gem_guc_loading: Adding simple GuC loading test
On Tue, Jan 05, 2016 at 05:17:07PM +0100, Lukasz Fiedorowicz wrote:
> Test check GuC debugfs file for successful loading confirmation
>
> Signed-off-by: Lukasz Fiedorowicz
What's the value of this testcase? What happens on a system without guc?
Seems more like a "is your system configured correctly" testcase, and thus
far we haven't done those as separate tests, but just as a pile of
igt_require (or maybe igt_fail) in a relevant functional testcase.
-Daniel
> ---
> tests/Makefile.sources | 1 +
> tests/gem_guc_loading.c | 89
> +
> 2 files changed, 90 insertions(+)
> create mode 100644 tests/gem_guc_loading.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index d594038..331234f 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -36,6 +36,7 @@ TESTS_progs_M = \
> gem_flink_basic \
> gem_flink_race \
> gem_linear_blits \
> + gem_guc_loading \
> gem_madvise \
> gem_mmap \
> gem_mmap_gtt \
> diff --git a/tests/gem_guc_loading.c b/tests/gem_guc_loading.c
> new file mode 100644
> index 000..fd53a46
> --- /dev/null
> +++ b/tests/gem_guc_loading.c
> @@ -0,0 +1,89 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Lukasz Fiedorowicz
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include "igt.h"
> +
> +IGT_TEST_DESCRIPTION("GuC firmware loading test.");
> +
> +#define LOAD_STATUS_BUF_SIZE 96
> +
> +enum guc_status { GUC_ENABLED, GUC_DISABLED };
> +
> +int guc_status_fd;
> +
> +static void open_guc_status(void)
> +{
> + guc_status_fd = igt_debugfs_open("i915_guc_load_status", O_RDONLY);
> + igt_assert_f(guc_status_fd >= 0, "Can't open i915_guc_load_status\n");
> +}
> +
> +static enum guc_status get_guc_status(void)
> +{
> + char buf[LOAD_STATUS_BUF_SIZE];
> +
> + FILE *fp = fdopen(guc_status_fd, "r");
> + igt_assert_f(fp != NULL, "Can't open i915_guc_load_status file\n");
> +
> + while (fgets(buf, LOAD_STATUS_BUF_SIZE, fp))
> + if ((strstr(buf, "\tload: SUCCESS\n")))
> + return GUC_ENABLED;
> +
> + return GUC_DISABLED;
> +}
> +
> +static void close_guc_status(void)
> +{
> + close(guc_status_fd);
> +}
> +
> +static void test_guc_loaded()
> +{
> + igt_assert_f(get_guc_status() == GUC_ENABLED, "GuC is disabled\n");
> +}
> +
> +igt_main
> +{
> + int gfx_fd = 0;
> + int gen = 0;
> +
> + igt_fixture
> + {
> + gfx_fd = drm_open_driver(DRIVER_INTEL);
> + gen = intel_gen(intel_get_drm_devid(gfx_fd));
> + igt_require(gen >= 9);
> + open_guc_status();
> + }
> +
> + igt_subtest("guc_loaded") test_guc_loaded();
> +
> + igt_fixture close_guc_status();
> +}
> --
> 2.4.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
