Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
Op 17-11-15 om 18:44 schreef Daniel Vetter:
> On Mon, Nov 02, 2015 at 05:38:07PM +0200, Ville Syrjälä wrote:
>> On Mon, Nov 02, 2015 at 01:41:03PM +0100, Maarten Lankhorst wrote:
>>> Hey,
>>>
>>> Op 30-10-15 om 22:06 schreef ville.syrj...@linux.intel.com:
From: Ville Syrjälä
This reverts commit b26a6b35581c84124bd78b68cc02d171fbd572c9.
commit b26a6b35581c ("drm/i915: Make prepare_plane_fb fully
interruptible.")
breaks GPU reset on gen3/4 machines. Go back to to non-interruptible.
>>> I've done some digging and by forcing an unconditional modeset during reset
>>> I was able to trigger it on my system.
>> Hmm, maybe we should add some kind of debug knob to do just that. That
>> way we could test most of the gen3/4 reset path with gen5+.
>>
>> I thought we already had that for load detection, but I may have
>> imagined it considering that load detection seems to have been
>> broken now for a while.
> We still have it for load detect, including an igt. No one looks at igt
> results though :(
>
> Maarten, can you please take care of both of these before pushing more
> atomic work? And I guess that means that we should apply the revert first.
> So Acked-by: Daniel Vetter on the revert.
>
> Cheers, Daniel
>
Reset is already fixed upstream so reverting wont help.
What igt is used for load detection then?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Unpin framebuffers when crtc is deconfigured.
Op 17-11-15 om 19:54 schreef Daniel Vetter: > On Wed, Nov 04, 2015 at 02:43:31PM +0100, Maarten Lankhorst wrote: >> When setcrtc is called and steals the last connector away from a crtc >> it's turned off because it can't stay configured without connectors. >> >> The framebuffer is still preserved however, and that causes troubles >> in the IGT stress test kms_flip.flips-vs-fences which tries to use >> as many pins as possible and hangs on the third crtc because of >> framebuffer pins on the first 2 crtc's. >> >> Cc: sta...@vger.kernel.org #v4.3 >> Signed-off-by: Maarten Lankhorst > drm_atomic_helper_set_config clears the fb for the primary plane if we > clear the mode. Where is the leak? Steal the last connector from another crtc. The crtc is deconfigured because it has no connectors any more, its the planes are left untouched. > Also where exactly does it hang - we should have plenty of fences left for > 3x2 pinned framebuffers. If you allow more than 2 pinned fb per plane then > that's a bug somewhere with pageflips getting ahead of the unpin worker. > -Daniel > Just try that stress test, it submits max_fences-a few so you will get -ENOSPC because it ran out out of fences. :) ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
On Wed, Nov 18, 2015 at 09:36:46AM +0100, Maarten Lankhorst wrote:
> Op 17-11-15 om 18:44 schreef Daniel Vetter:
> > On Mon, Nov 02, 2015 at 05:38:07PM +0200, Ville Syrjälä wrote:
> >> On Mon, Nov 02, 2015 at 01:41:03PM +0100, Maarten Lankhorst wrote:
> >>> Hey,
> >>>
> >>> Op 30-10-15 om 22:06 schreef ville.syrj...@linux.intel.com:
> From: Ville Syrjälä
>
> This reverts commit b26a6b35581c84124bd78b68cc02d171fbd572c9.
>
> commit b26a6b35581c ("drm/i915: Make prepare_plane_fb fully
> interruptible.")
> breaks GPU reset on gen3/4 machines. Go back to to non-interruptible.
>
> >>> I've done some digging and by forcing an unconditional modeset during
> >>> reset I was able to trigger it on my system.
> >> Hmm, maybe we should add some kind of debug knob to do just that. That
> >> way we could test most of the gen3/4 reset path with gen5+.
> >>
> >> I thought we already had that for load detection, but I may have
> >> imagined it considering that load detection seems to have been
> >> broken now for a while.
> > We still have it for load detect, including an igt. No one looks at igt
> > results though :(
> >
> > Maarten, can you please take care of both of these before pushing more
> > atomic work? And I guess that means that we should apply the revert first.
> > So Acked-by: Daniel Vetter on the revert.
> >
> > Cheers, Daniel
> >
> Reset is already fixed upstream so reverting wont help.
Sorry, I missed that the patch landed already.
> What igt is used for load detection then?
We're missing it it seems. Iirc Ander was working on it, but only the
debugfs part to force-enable load-detect logic was merged into the kernel.
-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 4/8] drm/i915: Add a modeset power domain
On Sat, Nov 07, 2015 at 08:29:49AM +1000, Dave Airlie wrote: > On 3 November 2015 at 22:31, Patrik Jakobsson > wrote: > > We need DC5/DC6 to be disabled around modesets to prevent confusing the > > DMC. Also, we've run out of bits in the 32 bit power domain mask so now > > it's a 64 bit mask. > > > > It was bad enough the original code used unsigned long so was > different on 32/64 bit, > > but don't keep going with it. uint64_t already please. Concurred. Also I don't like typing ;-) -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 v2 2/4] scripts/kernel-doc: Replacing highlights hash by an array
Em Tue, 17 Nov 2015 17:21:32 -0700 Jonathan Corbet escreveu: > On Tue, 17 Nov 2015 13:29:49 -0200 > Mauro Carvalho Chehab wrote: > > > The enclosed patch should do the trick. I tested it with perl 5.10 and > > perl 5.22 it worked fine with both versions. > > Indeed it seems to work - thanks! Applied to the docs tree, I'll get it > upstream before too long. Thanks, Jon! Regards, Mauro ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] PCI / PM: tune down RPM suspend error message with EBUSY and EAGAIN retval
The runtime PM core doesn't treat EBUSY and EAGAIN retvals from the driver
suspend hooks as errors, but they still show up as errors in dmesg. Tune
them down.
One problem caused by this was noticed by Daniel: the i915 driver
returns EAGAIN to signal a temporary failure to suspend and as a request
towards the RPM core for scheduling a suspend again. This is a normal
event, but the resulting error message flags a breakage during the
driver's automated testing which parses dmesg and picks up the error.
Reported-by: Daniel Vetter
Signed-off-by: Imre Deak
---
drivers/base/power/main.c | 7 +--
drivers/pci/pci-driver.c | 2 +-
include/linux/pm.h| 10 --
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 1710c26..39d2090 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1679,9 +1679,12 @@ int dpm_suspend_start(pm_message_t state)
}
EXPORT_SYMBOL_GPL(dpm_suspend_start);
-void __suspend_report_result(const char *function, void *fn, int ret)
+void __suspend_report_result(const char *function, void *fn, int ret,
+bool runtime_pm)
{
- if (ret)
+ if (runtime_pm && (ret == -EBUSY || ret == -EAGAIN))
+ printk(KERN_DEBUG "%s(): %pF returns %d\n", function, fn, ret);
+ else if (ret)
printk(KERN_ERR "%s(): %pF returns %d\n", function, fn, ret);
}
EXPORT_SYMBOL_GPL(__suspend_report_result);
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 108a311..9569572 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1142,7 +1142,7 @@ static int pci_pm_runtime_suspend(struct device *dev)
pci_dev->state_saved = false;
pci_dev->no_d3cold = false;
error = pm->runtime_suspend(dev);
- suspend_report_result(pm->runtime_suspend, error);
+ rpm_suspend_report_result(pm->runtime_suspend, error);
if (error)
return error;
if (!pci_dev->d3cold_allowed)
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 35d599e..f3dca18 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -702,11 +702,17 @@ extern int dpm_suspend_late(pm_message_t state);
extern int dpm_suspend(pm_message_t state);
extern int dpm_prepare(pm_message_t state);
-extern void __suspend_report_result(const char *function, void *fn, int ret);
+extern void __suspend_report_result(const char *function, void *fn, int ret,
+ bool runtime_pm);
#define suspend_report_result(fn, ret) \
do {\
- __suspend_report_result(__func__, fn, ret); \
+ __suspend_report_result(__func__, fn, ret, false); \
+ } while (0)
+
+#define rpm_suspend_report_result(fn, ret) \
+ do {\
+ __suspend_report_result(__func__, fn, ret, true); \
} while (0)
extern int device_pm_wait_for_dev(struct device *sub, struct device *dev);
--
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: Add some more bits to CURSOR_POS_MASK
On Wed, Nov 04, 2015 at 10:59:28AM +0100, Patrik Jakobsson wrote:
> On Wed, Nov 04, 2015 at 10:35:19AM +0100, Robert Fekete wrote:
> > The old value of 0x7FF will wrap the position at 2048 giving wrong
> > coordinate values on panels larger than 2048 pixels in any direction.
> > Used in i915_debugfs atm. Looking at all hw specs available at 01.org
> > shows that X position is bit 0:11, and even 0:12 on some hw where
> > remaining bits up to bit 14 is MBZ. For Y position it is bits 16-27
> > where bits 28:30 is MBZ. It should be safe to increase CURSOR_POS_MASK
> > to 13 bits (0x1FFF) making 8192 as a new wrap around value still getting
> > valid cursor positions on platforms with only 12bits available thanks to
> > MBZ on adjacent bits above.
>
> I cannot find documentation for older hardware and this only touches
> debugfs, so in worst case we get wrong values for really old hardware but good
> ones for newer. I think that's a fair tradeoff.
>
> Reviewed-by: Patrik Jakobsson
If it's only used in debugfs then imo just drop it. Having a _MASK which
isn't valid on all platforms, but where we don't have differnt #defines
for the different platforms is really confusing.
-Daniel
>
> >
> > Signed-off-by: Robert Fekete
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 894253228947..f351f46f8cb9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4883,7 +4883,7 @@ enum skl_disp_power_wells {
> > #define CURSOR_TRICKLE_FEED_DISABLE (1 << 14)
> > #define _CURABASE 0x70084
> > #define _CURAPOS 0x70088
> > -#define CURSOR_POS_MASK 0x007FF
> > +#define CURSOR_POS_MASK 0x01FFF
> > #define CURSOR_POS_SIGN 0x8000
> > #define CURSOR_X_SHIFT0
> > #define CURSOR_Y_SHIFT16
> > --
> > 1.9.1
> >
> > ___
> > 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
--
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 0/6] Fixing sink count related detection over
On Wed, 2015-11-18 at 11:12 +0530, Shubhangi Shrivastava wrote: > > On Wednesday 18 November 2015 01:23 AM, Daniel Vetter wrote: > > On Mon, Nov 02, 2015 at 06:25:10PM +0530, Shubhangi Shrivastava wrote: > > > This patch set cleans up DP detection logic to bring all DPCD > > > operations at one place and to create a clear demarcation > > > between handling of long and short pulses. This simplifies > > > fixing of sink count related detection for DP panels. > > > > > > Patches: > > > 1. First two patches clean up intel_dp_detect and form a new > > > function which will include all DPCD related operations. > > > 2. Second patch splits up intel_dp_check_link_status to form > > > a new function which will handle short pulse requests. > > > 3. Last three patches fixes the detection logic related to > > > sink count i.e detect changes in sink count and handle them > > > appropriately. > > > > > > Note: this is tested on BXT with non-mst panels, > > > will get back ASAP with results for MST panels too. > > Sivakumar and Ander are working on reorganizing all the DP hpd handling, > > at least they have been before I went on vacation. I think it would make > > sense to land that work first, before we start to apply functional fixes. > > > > Please coordinate with them. > > -Daniel > Yes.. The reorganization of DP HPD handling is in progress. > I am working with Siva on the same. :) I thought this was the series that reorganizes HPD. Should I be looking at some other patches too? Ander > We have found certain bugs in MST related code, pointed out > by Ander. Once it is fixed, we will re-upload the patches. > > > > > Shubhangi Shrivastava (6): > > >drm/i915: Splitting intel_dp_detect > > >drm/i915: Cleaning up intel_dp_hpd_pulse > > >drm/i915: Splitting intel_dp_check_link_status > > >drm/i915: Save sink_count for tracking changes to it > > >drm/i915: read sink_count dpcd always > > >drm/i915: force full detect on sink count change > > > > > > drivers/gpu/drm/i915/intel_dp.c | 170 + > > > -- > > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > > 2 files changed, 110 insertions(+), 61 deletions(-) > > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
On Wed, 2015-11-18 at 09:55 +0100, Daniel Vetter wrote:
> On Wed, Nov 18, 2015 at 09:36:46AM +0100, Maarten Lankhorst wrote:
> > Op 17-11-15 om 18:44 schreef Daniel Vetter:
> > > On Mon, Nov 02, 2015 at 05:38:07PM +0200, Ville Syrjälä wrote:
> > > > On Mon, Nov 02, 2015 at 01:41:03PM +0100, Maarten Lankhorst wrote:
> > > > > Hey,
> > > > >
> > > > > Op 30-10-15 om 22:06 schreef ville.syrj...@linux.intel.com:
> > > > > > From: Ville Syrjälä
> > > > > >
> > > > > > This reverts commit b26a6b35581c84124bd78b68cc02d171fbd572c9.
> > > > > >
> > > > > > commit b26a6b35581c ("drm/i915: Make prepare_plane_fb fully
> > > > > > interruptible.")
> > > > > > breaks GPU reset on gen3/4 machines. Go back to to non
> > > > > > -interruptible.
> > > > > >
> > > > > I've done some digging and by forcing an unconditional modeset during
> > > > > reset I was able to trigger it on my system.
> > > > Hmm, maybe we should add some kind of debug knob to do just that. That
> > > > way we could test most of the gen3/4 reset path with gen5+.
> > > >
> > > > I thought we already had that for load detection, but I may have
> > > > imagined it considering that load detection seems to have been
> > > > broken now for a while.
> > > We still have it for load detect, including an igt. No one looks at igt
> > > results though :(
> > >
> > > Maarten, can you please take care of both of these before pushing more
> > > atomic work? And I guess that means that we should apply the revert first.
> > > So Acked-by: Daniel Vetter on the revert.
> > >
> > > Cheers, Daniel
> > >
> > Reset is already fixed upstream so reverting wont help.
>
> Sorry, I missed that the patch landed already.
>
> > What igt is used for load detection then?
>
> We're missing it it seems. Iirc Ander was working on it, but only the
> debugfs part to force-enable load-detect logic was merged into the kernel.
That wasn't me. Perhaps it was Matt?
Ander
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/6] Fixing sink count related detection over
On Wednesday 18 November 2015 02:53 PM, Ander Conselvan De Oliveira wrote: On Wed, 2015-11-18 at 11:12 +0530, Shubhangi Shrivastava wrote: On Wednesday 18 November 2015 01:23 AM, Daniel Vetter wrote: On Mon, Nov 02, 2015 at 06:25:10PM +0530, Shubhangi Shrivastava wrote: This patch set cleans up DP detection logic to bring all DPCD operations at one place and to create a clear demarcation between handling of long and short pulses. This simplifies fixing of sink count related detection for DP panels. Patches: 1. First two patches clean up intel_dp_detect and form a new function which will include all DPCD related operations. 2. Second patch splits up intel_dp_check_link_status to form a new function which will handle short pulse requests. 3. Last three patches fixes the detection logic related to sink count i.e detect changes in sink count and handle them appropriately. Note: this is tested on BXT with non-mst panels, will get back ASAP with results for MST panels too. Sivakumar and Ander are working on reorganizing all the DP hpd handling, at least they have been before I went on vacation. I think it would make sense to land that work first, before we start to apply functional fixes. Please coordinate with them. -Daniel Yes.. The reorganization of DP HPD handling is in progress. I am working with Siva on the same. :) I thought this was the series that reorganizes HPD. Should I be looking at some other patches too? Ander Its the same Ander.. :) We have found certain bugs in MST related code, pointed out by Ander. Once it is fixed, we will re-upload the patches. Shubhangi Shrivastava (6): drm/i915: Splitting intel_dp_detect drm/i915: Cleaning up intel_dp_hpd_pulse drm/i915: Splitting intel_dp_check_link_status drm/i915: Save sink_count for tracking changes to it drm/i915: read sink_count dpcd always drm/i915: force full detect on sink count change drivers/gpu/drm/i915/intel_dp.c | 170 + -- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 110 insertions(+), 61 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/8] drm/i915: Add a modeset power domain
On Wed, Nov 18, 2015 at 10:02 AM, Daniel Vetter wrote: > On Sat, Nov 07, 2015 at 08:29:49AM +1000, Dave Airlie wrote: >> On 3 November 2015 at 22:31, Patrik Jakobsson >> wrote: >> > We need DC5/DC6 to be disabled around modesets to prevent confusing the >> > DMC. Also, we've run out of bits in the 32 bit power domain mask so now >> > it's a 64 bit mask. >> > >> >> It was bad enough the original code used unsigned long so was >> different on 32/64 bit, >> >> but don't keep going with it. uint64_t already please. > > Concurred. Also I don't like typing ;-) We removed a few unused power domains so didn't need to bump it to 64 bits. Can do that anyway or just change it to an uint32_t in the next round of PW/DMC cleanups. -Patrik > -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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/29] drm/i915: Streamline gpio_mmio_base deduction
On Wed, Nov 04, 2015 at 11:20:00PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> If we ignore the BXT situation, we can observe that the only variables
> affecting gpio_mmio_base is IS_VALLEVIEW and HAS_GMCH_DISPLAY. The BXT
> situation we can fit into the same pattern if we change gmbus_pins_bxt[]
> to house the GMCH GPIO register offsets (like we do for all other
> platfotms). So let's do that.
>
> We could even simplify the VLV situation more by including the
> display_mmio_offset in the GPIO register defines, but let's leave it be
> for now.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_i2c.c | 15 +++
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c
> b/drivers/gpu/drm/i915/intel_i2c.c
> index bd58da0..9d35589 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -63,9 +63,9 @@ static const struct gmbus_pin gmbus_pins_skl[] = {
> };
>
> static const struct gmbus_pin gmbus_pins_bxt[] = {
> - [GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
> - [GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
> - [GMBUS_PIN_3_BXT] = { "misc", PCH_GPIOD },
> + [GMBUS_PIN_1_BXT] = { "dpb", GPIOB },
> + [GMBUS_PIN_2_BXT] = { "dpc", GPIOC },
> + [GMBUS_PIN_3_BXT] = { "misc", GPIOD },
> };
>
> /* pin is expected to be valid */
> @@ -626,12 +626,11 @@ int intel_setup_gmbus(struct drm_device *dev)
>
> if (HAS_PCH_NOP(dev))
> return 0;
> - else if (HAS_PCH_SPLIT(dev))
> - dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA;
> - else if (IS_VALLEYVIEW(dev))
> +
> + if (IS_VALLEYVIEW(dev))
> dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE;
> - else
> - dev_priv->gpio_mmio_base = 0;
> + else if (!HAS_GMCH_DISPLAY(dev))
> + dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA;
>
> mutex_init(&dev_priv->gmbus_mutex);
> init_waitqueue_head(&dev_priv->gmbus_wait_queue);
> --
> 2.4.10
>
> ___
> 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
[Intel-gfx] [PATCH 3/3] drm/i915: Limit request busywaiting
If we suppose that on average it takes 1ms to do a fullscreen paint
(which is fairly typical across generations, as the GPUs get faster the
displays get larger), then the likelihood of a synchronous wait on the last
request completing within 2 microseconds is miniscule. On the other
hand, not everything attempts a fullscreen repaint (or is ratelimited by
such) and for those a short busywait is much faster than setting up the
interrupt driven waiter. The challenge is identifying such tasks. Here
we opt to never spin for an unlocked wait (such as from a set-domain or
wait ioctl) and instead only spin in circumstances where we are holding
the lock and have either already completed one unlocked wait or we are
in a situation where we are being forced to drain old requests (which we
know must be close to completion).
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 12 +++-
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f47d701f2ddb..dc8f2a87dac0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2979,6 +2979,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
s64 *timeout,
struct intel_rps_client *rps);
#define I915_WAIT_INTERRUPTIBLE 0x1
+#define I915_WAIT_MAY_SPIN 0x2
int __must_check i915_wait_request(struct drm_i915_gem_request *req);
int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
int __must_check
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d4d5c6e8c02f..cfa101fad479 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1273,9 +1273,11 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
before = ktime_get_raw_ns();
/* Optimistic spin for the next jiffie before touching IRQs */
- ret = __i915_spin_request(req, state);
- if (ret == 0)
- goto out;
+ if (flags & I915_WAIT_MAY_SPIN) {
+ ret = __i915_spin_request(req, state);
+ if (ret == 0)
+ goto out;
+ }
if (!irq_test_in_progress && WARN_ON(!ring->irq_get(ring))) {
ret = -ENODEV;
@@ -1467,7 +1469,7 @@ i915_wait_request(struct drm_i915_gem_request *req)
if (ret)
return ret;
- flags = 0;
+ flags = I915_WAIT_MAY_SPIN;
if (dev_priv->mm.interruptible)
flags |= I915_WAIT_INTERRUPTIBLE;
@@ -4107,7 +4109,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct
drm_file *file)
ret = __i915_wait_request(target,
reset_counter,
- I915_WAIT_INTERRUPTIBLE,
+ I915_WAIT_INTERRUPTIBLE | I915_WAIT_MAY_SPIN,
NULL, NULL);
if (ret == 0)
queue_delayed_work(dev_priv->wq, &dev_priv->mm.retire_work, 0);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 664ce0b20b23..d3142af0f347 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2241,7 +2241,7 @@ int intel_ring_idle(struct intel_engine_cs *ring)
struct drm_i915_gem_request,
list);
- flags = 0;
+ flags = I915_WAIT_MAY_SPIN;
if (to_i915(ring->dev)->mm.interruptible)
flags |= I915_WAIT_INTERRUPTIBLE;
--
2.6.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Convert __i915_wait_request to receive flags
Convert the bool interruptible argument over to a flags bitmask so that
we can add further bool parameters conveniently.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/i915_gem.c | 42 +++--
drivers/gpu/drm/i915/intel_display.c| 2 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +-
4 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 16095b95d2df..f47d701f2ddb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2975,9 +2975,10 @@ void __i915_add_request(struct drm_i915_gem_request *req,
__i915_add_request(req, NULL, false)
int __i915_wait_request(struct drm_i915_gem_request *req,
unsigned reset_counter,
- bool interruptible,
+ unsigned flags,
s64 *timeout,
struct intel_rps_client *rps);
+#define I915_WAIT_INTERRUPTIBLE 0x1
int __must_check i915_wait_request(struct drm_i915_gem_request *req);
int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
int __must_check
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index af9ffa11ef44..d4d5c6e8c02f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1224,7 +1224,7 @@ static int __i915_spin_request(struct
drm_i915_gem_request *req, int state)
* __i915_wait_request - wait until execution of request has finished
* @req: duh!
* @reset_counter: reset sequence associated with the given request
- * @interruptible: do an interruptible wait (normally yes)
+ * @flags: flags
* @timeout: in - how long to wait (NULL forever); out - how much time
remaining
*
* Note: It is of utmost importance that the passed in seqno and reset_counter
@@ -1239,7 +1239,7 @@ static int __i915_spin_request(struct
drm_i915_gem_request *req, int state)
*/
int __i915_wait_request(struct drm_i915_gem_request *req,
unsigned reset_counter,
- bool interruptible,
+ unsigned flags,
s64 *timeout,
struct intel_rps_client *rps)
{
@@ -1248,7 +1248,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
struct drm_i915_private *dev_priv = dev->dev_private;
const bool irq_test_in_progress =
ACCESS_ONCE(dev_priv->gpu_error.test_irq_rings) &
intel_ring_flag(ring);
- int state = interruptible ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE;
+ int state = flags & I915_WAIT_INTERRUPTIBLE ? TASK_INTERRUPTIBLE :
TASK_UNINTERRUPTIBLE;
DEFINE_WAIT(wait);
unsigned long timeout_expire;
s64 before, now;
@@ -1292,7 +1292,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
if (reset_counter !=
atomic_read(&dev_priv->gpu_error.reset_counter)) {
/* ... but upgrade the -EAGAIN to an -EIO if the gpu
* is truely gone. */
- ret = i915_gem_check_wedge(&dev_priv->gpu_error,
interruptible);
+ ret = i915_gem_check_wedge(&dev_priv->gpu_error,
+ flags &
I915_WAIT_INTERRUPTIBLE);
if (ret == 0)
ret = -EAGAIN;
break;
@@ -1451,24 +1452,28 @@ i915_wait_request(struct drm_i915_gem_request *req)
{
struct drm_device *dev;
struct drm_i915_private *dev_priv;
- bool interruptible;
+ unsigned flags;
int ret;
BUG_ON(req == NULL);
dev = req->ring->dev;
dev_priv = dev->dev_private;
- interruptible = dev_priv->mm.interruptible;
BUG_ON(!mutex_is_locked(&dev->struct_mutex));
- ret = i915_gem_check_wedge(&dev_priv->gpu_error, interruptible);
+ ret = i915_gem_check_wedge(&dev_priv->gpu_error,
+ dev_priv->mm.interruptible);
if (ret)
return ret;
+ flags = 0;
+ if (dev_priv->mm.interruptible)
+ flags |= I915_WAIT_INTERRUPTIBLE;
+
ret = __i915_wait_request(req,
atomic_read(&dev_priv->gpu_error.reset_counter),
- interruptible, NULL, NULL);
+ flags, NULL, NULL);
if (ret)
return ret;
@@ -1580,7 +1585,9 @@ i915_gem_object_wait_rendering__nonblocking(struct
drm_i915_gem_object *obj,
mutex_unlock(&dev->struct_mutex);
for (i = 0; ret == 0 && i < n; i++)
- ret = __i915_wait_request(requests[i], reset_counter, true,
+ ret = __i915_wait_request(requests[i],
+ reset_counter,
+
[Intel-gfx] [PATCH 1/3] drm/i915: Only spin whilst waiting on the current request
Limit busywaiting only to the request currently being processed by the
GPU. If the request is not currently being processed by the GPU, there
is a very low likelihood of it being completed within the 2 microsecond
spin timeout and so we will just be wasting CPU cycles.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 8 +++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8afda459a26e..16095b95d2df 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2190,7 +2190,7 @@ struct drm_i915_gem_request {
struct intel_engine_cs *ring;
/** GEM sequence number associated with this request. */
- uint32_t seqno;
+ uint32_t seqno, spin_seqno;
/** Position in the ringbuffer of the start of the request */
u32 head;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 414150a0b8d5..af9ffa11ef44 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1193,9 +1193,14 @@ static int __i915_spin_request(struct
drm_i915_gem_request *req, int state)
* takes to sleep on a request, on the order of a microsecond.
*/
- if (i915_gem_request_get_ring(req)->irq_refcount)
+ if (req->ring->irq_refcount)
return -EBUSY;
+ /* Only spin if we know the GPU is processing this request */
+ if (i915_seqno_passed(req->ring->get_seqno(req->ring, false),
+ req->spin_seqno))
+ return -EAGAIN;
+
timeout = local_clock_us(&cpu) + 2;
while (!need_resched()) {
if (i915_gem_request_completed(req, true))
@@ -2592,6 +2597,7 @@ void __i915_add_request(struct drm_i915_gem_request
*request,
request->batch_obj = obj;
request->emitted_jiffies = jiffies;
+ request->spin_seqno = ring->last_submitted_seqno;
ring->last_submitted_seqno = request->seqno;
list_add_tail(&request->list, &ring->request_list);
--
2.6.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Limit busywaiting
This should filter out all explicit wait requests from userspace and only apply busywaiting to circumstances where we are forced to drain the GPU of old requests. With the 2 microsecond timeout from before, this still seems to preserve the speed up in stress tests and cancel the busywaiting for desktop loads. -Chris ___ 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: Use insert_page for pwrite_fast
On Thu, Nov 05, 2015 at 05:15:59PM +0530, ankitprasad.r.sha...@intel.com wrote:
> From: Ankitprasad Sharma
>
> In pwrite_fast, map an object page by page if obj_ggtt_pin fails. First,
> we try a nonblocking pin for the whole object (since that is fastest if
> reused), then failing that we try to grab one page in the mappable
> aperture. It also allows us to handle objects larger than the mappable
> aperture (e.g. if we need to pwrite with vGPU restricting the aperture
> to a measely 8MiB or something like that).
We already have a fallback to the shmem pwrite. Why do we need this?
-Daniel
>
> Signed-off-by: Ankitprasad Sharma
> ---
> drivers/gpu/drm/i915/i915_gem.c | 92
> ++---
> 1 file changed, 69 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index bf5ef7a..9132240 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -766,14 +766,26 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev,
>struct drm_file *file)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_mm_node node;
> ssize_t remain;
> loff_t offset, page_base;
> char __user *user_data;
> - int page_offset, page_length, ret;
> + int page_offset, page_length, ret, i;
> + bool pinned = true;
>
> ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE | PIN_NONBLOCK);
> - if (ret)
> - goto out;
> + if (ret) {
> + pinned = false;
> + memset(&node, 0, sizeof(node));
> + ret =
> drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
> + &node, 4096, 0,
> + I915_CACHE_NONE, 0,
> +
> dev_priv->gtt.mappable_end,
> + DRM_MM_SEARCH_DEFAULT,
> +
> DRM_MM_CREATE_DEFAULT);
> + if (ret)
> + goto out;
> + }
>
> ret = i915_gem_object_set_to_gtt_domain(obj, true);
> if (ret)
> @@ -786,42 +798,76 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev,
> user_data = to_user_ptr(args->data_ptr);
> remain = args->size;
>
> - offset = i915_gem_obj_ggtt_offset(obj) + args->offset;
> -
> intel_fb_obj_invalidate(obj, ORIGIN_GTT);
>
> - while (remain > 0) {
> - /* Operation in this page
> + if (likely(pinned)) {
> + offset = i915_gem_obj_ggtt_offset(obj) + args->offset;
> + /* Operation in the page
>*
>* page_base = page offset within aperture
>* page_offset = offset within page
> - * page_length = bytes to copy for this page
> + * page_length = bytes to copy for the page
>*/
> page_base = offset & PAGE_MASK;
> page_offset = offset_in_page(offset);
> - page_length = remain;
> - if ((page_offset + remain) > PAGE_SIZE)
> - page_length = PAGE_SIZE - page_offset;
> + while (remain > 0) {
> + page_length = remain;
> + if ((page_offset + remain) > PAGE_SIZE)
> + page_length = PAGE_SIZE - page_offset;
> +
> + /* If we get a fault while copying data, then
> (presumably) our
> + * source page isn't available. Return the error and
> we'll
> + * retry in the slow path.
> + */
> + if (fast_user_write(dev_priv->gtt.mappable, page_base,
> + page_offset, user_data,
> page_length)) {
> + ret = -EFAULT;
> + goto out_flush;
> + }
>
> - /* If we get a fault while copying data, then (presumably) our
> - * source page isn't available. Return the error and we'll
> - * retry in the slow path.
> - */
> - if (fast_user_write(dev_priv->gtt.mappable, page_base,
> - page_offset, user_data, page_length)) {
> - ret = -EFAULT;
> - goto out_flush;
> + remain -= page_length;
> + user_data += page_length;
> + page_offset = 0;
> }
> + } else {
> + i = args->offset / PAGE_SIZE;
> + page_offset = offset_in_page(args->offset);
> + while (remain > 0) {
> + page_length = remain;
> + if ((page_offset + remain) > PAGE_SIZE)
> + page_length = PAGE_SIZE - page_offse
Re: [Intel-gfx] [PATCH 07/31] drm/i915: IPS Sysfs interface.
On Thu, Nov 05, 2015 at 09:04:02PM +, Chris Wilson wrote: > On Thu, Nov 05, 2015 at 10:49:59AM -0800, Rodrigo Vivi wrote: > > With the lock in place we can expose ips enabled/disable on sysfs > > for developing, debugging and information purposes. > > No. sysfs is not for developement, debugging or information. It is a > userspace ABI. Yup, this should be for debugfs. -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 1/2] drm/i915: Mark uneven memory banks on gen4 desktop as unknown swizzling
On Tue, Nov 17, 2015 at 10:40:51AM +, Chris Wilson wrote: > We have varied reports of swizzling corruption on gen4 desktop, and > confirmation that it is triggered by uneven memory banks. The > implication is that the swizzling various between the paired channels > and the remainder of memory on the single channel. As the object then > has unpredictable swizzling (it will vary depending on exact page > allocation and may even change during the object's lifetime as the pages > are replaced), we have to report to userspace that the swizzling is > unknown. > > Reported-by: Matti Hämäläinen > References: https://bugs.freedesktop.org/show_bug.cgi?id=90725 > Signed-off-by: Chris Wilson > Cc: Matti Hämäläinen > Cc: sta...@vger.kernel.org Tested-by: Matti Hämäläinen -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 10/31] drm/i915: Detatch i915.enable_psr from psr_ready
On Thu, Nov 05, 2015 at 10:50:02AM -0800, Rodrigo Vivi wrote:
> PSR will be enabled on every post primary update when it is
> ready and parameter allows.
> With this we allow test cases to continue using this parameter
> for enabling disabling the feature.
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_dp.c | 2 +-
> drivers/gpu/drm/i915/intel_psr.c | 5 -
> 2 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6ab127c..e154a2e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5334,7 +5334,7 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
> return;
> }
>
> - if (intel_crtc->config->psr_ready) {
> + if (intel_crtc->config->psr_ready && i915.enable_psr) {
I don't get this change ... The pipe config should take into account
enable_psr already. If we allow testcases to change this without a
modeset, this could result in a lot of racy fun.
-Daniel
> DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
> return;
> }
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 4a9d620..e690db3 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -314,11 +314,6 @@ bool intel_psr_ready(struct intel_dp *intel_dp,
> return false;
> }
>
> - if (!i915.enable_psr) {
> - DRM_DEBUG_KMS("PSR disable by flag\n");
> - return false;
> - }
> -
> if (IS_HASWELL(dev) &&
> I915_READ(HSW_STEREO_3D_CTL(pipe_config->cpu_transcoder)) &
> S3D_ENABLE) {
> --
> 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: Fix gpu frequency change tracing
On Tue, 17 Nov 2015, Ville Syrjälä wrote: > On Tue, Nov 17, 2015 at 06:14:26PM +0200, Mika Kuoppala wrote: >> With gen < 9 we have had always 50Mhz units as our hw >> ratio. With gen >= 9 the hw ratio changed to 16.667Mhz (50/3). >> The result was that our gpu frequency tracing started to output >> values 3 times larger than expected due to hardcoded scaling >> value. Fix this by using Use intel_gpu_freq() when generating Mhz >> value from ratio for 'intel_gpu_freq_change' trace event. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92591 >> Reported-by: Eero Tamminen >> Signed-off-by: Mika Kuoppala >> --- >> drivers/gpu/drm/i915/intel_pm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c >> b/drivers/gpu/drm/i915/intel_pm.c >> index ebd6735..656c53a 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -4330,7 +4330,7 @@ static void gen6_set_rps(struct drm_device *dev, u8 >> val) >> POSTING_READ(GEN6_RPNSWREQ); >> >> dev_priv->rps.cur_freq = val; >> -trace_intel_gpu_freq_change(val * 50); >> +trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val)); > > I was thinking maybe moving it to the caller to avoid having it in both > the gen6 and vlv set_rps functions, but there are actually so many callers > that we're better off leaving it here. > > Reviewed-by: Ville Syrjälä Pushed to drm-intel-fixes with cc: stable for v4.3+, thanks for the patch and review. BR, Jani. > >> } >> >> static void valleyview_set_rps(struct drm_device *dev, u8 val) >> -- >> 2.5.0 >> >> ___ >> 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 12/31] drm/i915: Fix PSR initialization.
On Thu, Nov 05, 2015 at 10:50:04AM -0800, Rodrigo Vivi wrote:
> PSR is still disabled by default, but even passing i915.enable_psr=1
> at this point we weren't able to get PSR working because with
> fastboot by default in place we weren't executing the path that enables
> encoder and consequently PSR.
>
> Now with psr_ready in place and PSR using crtc signature we can move
> its enable/disable sequences from the encoder enable to the post
> atomic modeset functions.
>
> i915.enable_psr parameter is still used to enable/disable psr feature
> on the next primary plane update. So current test cases that relies
> on this flow still works.
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 2 --
> drivers/gpu/drm/i915/intel_display.c | 15 +++
> drivers/gpu/drm/i915/intel_dp.c | 5 -
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 4 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index b8f8dee..36db970 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2404,7 +2404,6 @@ static void intel_enable_ddi(struct intel_encoder
> *intel_encoder)
> intel_dp_stop_link_train(intel_dp);
>
> intel_edp_backlight_on(intel_dp);
> - intel_psr_enable(intel_crtc);
> intel_edp_drrs_enable(intel_dp);
> }
>
> @@ -2432,7 +2431,6 @@ static void intel_disable_ddi(struct intel_encoder
> *intel_encoder)
> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>
> intel_edp_drrs_disable(intel_dp);
> - intel_psr_disable(intel_crtc);
> intel_edp_backlight_off(intel_dp);
> }
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 869929d..f67e2ee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4687,6 +4687,9 @@ static void intel_post_plane_update(struct intel_crtc
> *crtc)
> if (atomic->enable_ips)
> intel_ips_enable(crtc);
>
> + if (atomic->enable_psr)
> + intel_psr_enable(crtc);
What we need here is a post-modeset fixup hook for encoders/connectors.
That would avoid the layering violation of going through a crtc and then
doing the crtc->encoder lookup you do in the previous patch. And we have
other uses for this, e.g. mipi self refresh, fixing up infoframes and all
those things.
-Daniel
> +
> if (atomic->post_enable_primary)
> intel_post_enable_primary(&crtc->base);
>
> @@ -4705,6 +4708,9 @@ static void intel_pre_plane_update(struct intel_crtc
> *crtc)
> if (crtc->atomic.disable_ips)
> intel_ips_disable_if_alone(crtc);
>
> + if (crtc->atomic.disable_psr)
> + intel_psr_disable(crtc);
> +
> if (atomic->pre_disable_primary)
> intel_pre_disable_primary(&crtc->base);
>
> @@ -11560,9 +11566,18 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
> intel_crtc->atomic.disable_ips = true;
>
> intel_crtc->atomic.disable_fbc = true;
> +
> + intel_crtc->atomic.disable_psr = true;
> }
> if (visible && intel_crtc->config->ips_ready)
> intel_crtc->atomic.enable_ips = true;
> +
> + if (visible && intel_crtc->config->psr_ready) {
> + if (i915.enable_psr)
> + intel_crtc->atomic.enable_psr = true;
> + else
> + intel_crtc->atomic.disable_psr = true;
> + }
> /*
>* FBC does not work on some platforms for rotated
>* planes, so disable it when rotation is not 0 and
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 92f59cc..f0ee497 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2373,9 +2373,6 @@ static void intel_disable_dp(struct intel_encoder
> *encoder)
> if (crtc->config->has_audio)
> intel_audio_codec_disable(encoder);
>
> - if (HAS_PSR(dev) && !HAS_DDI(dev))
> - intel_psr_disable(crtc);
> -
> /* Make sure the panel is off before trying to change the mode. But also
>* ensure that we have vdd while we switch off the panel. */
> intel_edp_panel_vdd_on(intel_dp);
> @@ -2629,10 +2626,8 @@ static void g4x_enable_dp(struct intel_encoder
> *encoder)
> static void vlv_enable_dp(struct intel_encoder *encoder)
> {
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> - struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>
> intel_edp_backlight_on(intel_dp);
> - intel_psr_enable(crtc);
> }
>
> static void g4x_pre_enable_dp(struct inte
Re: [Intel-gfx] [PATCH 16/31] drm/i915: Fix DRRS initialization.
On Thu, Nov 05, 2015 at 10:50:08AM -0800, Rodrigo Vivi wrote:
> With Fastboot by default we don't necessarily do a
> full modeset enabling the primary plane.
> So DRRS enable call that was in that path wasn't being
> called anymore.
>
> So, let's relly on post atomic modeset path
> and on has_drrs to enabled DRRS when we judge necessary.
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_display.c | 11 +++
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f67e2ee..6647bfe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4681,6 +4681,9 @@ static void intel_post_plane_update(struct intel_crtc
> *crtc)
> if (crtc->atomic.update_wm_post)
> intel_update_watermarks(&crtc->base);
>
> + if (atomic->enable_drrs)
> + intel_drrs_enable(crtc);
Same comment as with psr. And encoder post_modeset fixup function would be
a lot cleaner instead of leaking this all into crtc code.
-Daniel
> +
> if (atomic->update_fbc)
> intel_fbc_update(dev_priv);
>
> @@ -4702,6 +4705,9 @@ static void intel_pre_plane_update(struct intel_crtc
> *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc_atomic_commit *atomic = &crtc->atomic;
>
> + if (crtc->atomic.disable_drrs)
> + intel_drrs_disable(crtc);
> +
> if (atomic->disable_fbc)
> intel_fbc_disable_crtc(crtc);
>
> @@ -11565,10 +11571,15 @@ int intel_plane_atomic_calc_changes(struct
> drm_crtc_state *crtc_state,
>*/
> intel_crtc->atomic.disable_ips = true;
>
> + intel_crtc->atomic.disable_drrs = true;
> +
> intel_crtc->atomic.disable_fbc = true;
>
> intel_crtc->atomic.disable_psr = true;
> }
> + if (visible && intel_crtc->config->has_drrs)
> + intel_crtc->atomic.enable_drrs = true;
> +
> if (visible && intel_crtc->config->ips_ready)
> intel_crtc->atomic.enable_ips = true;
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 48f461f..bf5e77c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -529,6 +529,7 @@ struct intel_mmio_flip {
> */
> struct intel_crtc_atomic_commit {
> /* Sleepable operations to perform before commit */
> + bool disable_drrs;
> bool disable_fbc;
> bool disable_ips;
> bool disable_psr;
> @@ -539,6 +540,7 @@ struct intel_crtc_atomic_commit {
> /* Sleepable operations to perform after commit */
> unsigned fb_bits;
> bool wait_vblank;
> + bool enable_drrs;
> bool update_fbc;
> bool enable_ips;
> bool enable_psr;
> --
> 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] PCI / PM: tune down RPM suspend error message with EBUSY and EAGAIN retval
Hi Imre, [auto build test ERROR on pci/next] [also build test ERROR on v4.4-rc1 next-20151118] url: https://github.com/0day-ci/linux/commits/Imre-Deak/PCI-PM-tune-down-RPM-suspend-error-message-with-EBUSY-and-EAGAIN-retval/20151118-171831 base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next config: i386-randconfig-a0-201546 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/pci/pci-driver.c: In function 'pci_pm_runtime_suspend': >> drivers/pci/pci-driver.c:1149:2: error: implicit declaration of function >> 'rpm_suspend_report_result' [-Werror=implicit-function-declaration] rpm_suspend_report_result(pm->runtime_suspend, error); ^ cc1: some warnings being treated as errors vim +/rpm_suspend_report_result +1149 drivers/pci/pci-driver.c 1143 if (!pm || !pm->runtime_suspend) 1144 return -ENOSYS; 1145 1146 pci_dev->state_saved = false; 1147 pci_dev->no_d3cold = false; 1148 error = pm->runtime_suspend(dev); > 1149 rpm_suspend_report_result(pm->runtime_suspend, error); 1150 if (error) 1151 return error; 1152 if (!pci_dev->d3cold_allowed) --- 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 i-g-t 2/3] Unify handling of slow/combinatorial tests
On Tue, Nov 17, 2015 at 01:49:06PM -0200, Paulo Zanoni wrote: > 2015-11-17 13:34 GMT-02:00 Daniel Vetter : [snip] > > I thought the hidden tests in kms_frontbuffer_tracking would be useful, > > just really slow, but seems I'm mistaken. In general we have a bunch of > > stress tests which we want to run, but at a lower priority. > > So it doesn't sound good to put both the kms_frontbuffer_trackign and > the slow-but-useful behind the same knob. Anyway, I think the "flags" > idea can solve the problem. Indeed it should be able to solve that problem. Obviously it cannot solve the "skipped because the feature isn't available on this platform", "blacklisted because this feature hasn't been implemented yet", and what not, but that is, I think, out of scope here. So, does anyone have any objections (philosophical, colour of bikeshed, or technical) against the current "flags" implementation? Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [drm-intel:drm-intel-next-queued 0/1] warning: (DRM_I915) selects STOP_MACHINE which has unmet direct dependencies (SMP && ..)
On Wed, Nov 18, 2015 at 02:17:17AM +0800, kbuild test robot wrote: > tree: git://anongit.freedesktop.org/drm-intel drm-intel-next-queued > head: e64e6bd0f46c78b53b236474f59bd1290b834c89 > commit: 5bab6f60cb4d1417ad7c599166bcfec87529c1a2 [0/1] drm/i915: Serialise > updates to GGTT with access through GGTT on Braswell > config: x86_64-randconfig-x011-11162304 (attached as .config) > reproduce: > git checkout 5bab6f60cb4d1417ad7c599166bcfec87529c1a2 > # save the attached .config to linux build tree > make ARCH=x86_64 > > All warnings (new ones prefixed by >>): > > warning: (DRM_I915) selects STOP_MACHINE which has unmet direct dependencies > (SMP && MODULE_UNLOAD || HOTPLUG_CPU) Note that if we do not force STOP_MACHINE then we do not get the correct stop_machine() without modules enabled. linux/stop_machine.c works on UP/SMP, with and without hotplug, with and without modules etc, so this just appears to be a very old issue resulting from when stop_machine was a part of module.c (back in pre-git era) -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
[Intel-gfx] [PATCH i-g-t] tests/gem_reset_stats.c: prepare for per engine resets
From: Tim Gore
when checking to make sure that the driver has performed
the expected number of resets, this test looks at the
reset_count, which is incremented each time the GPU is
reset. Upcoming changes in the way GPU hangs are handled
mean that in most cases (and in all the cases in this
test) only a single GPU engine is reset which does not
cause the reset_count to be incremented. This is already
causing this test to fail on Android. In this case
we can instead look at the batch_active count which is
also returned from the i915_get_reset_stats_ioctl and is
incremented by both a single engine reset and a full
gpu reset. There are differences between the reset_count
and the batch_active count, but for establishing that the
correct number of resets have occured either can be used.
This change enables this test to run successfully on
Android and will mean that the test does not break when
the TDR patches get merged into the uptream driver.
Signed-off-by: Tim Gore
---
tests/gem_reset_stats.c | 41 ++---
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index 4cbbb4e..5ec026f 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -104,9 +104,9 @@ static int gem_reset_stats(int fd, int ctx_id,
rs->ctx_id = ctx_id;
rs->flags = 0;
- rs->reset_count = rand();
- rs->batch_active = rand();
- rs->batch_pending = rand();
+ rs->reset_count = UINT32_MAX;
+ rs->batch_active = UINT32_MAX;
+ rs->batch_pending = UINT32_MAX;
rs->pad = 0;
do {
@@ -690,6 +690,18 @@ static int get_reset_count(int fd, int ctx)
return rs.reset_count;
}
+static int get_active_count(int fd, int ctx)
+{
+ int ret;
+ struct local_drm_i915_reset_stats rs;
+
+ ret = gem_reset_stats(fd, ctx, &rs);
+ if (ret)
+ return ret;
+
+ return rs.batch_active;
+}
+
static void test_close_pending_ctx(void)
{
int fd, h;
@@ -837,17 +849,16 @@ static void test_reset_count(const bool create_ctx)
assert_reset_status(fd, ctx, RS_NO_ERROR);
- c1 = get_reset_count(fd, ctx);
- igt_assert(c1 >= 0);
+ c1 = get_active_count(fd, ctx);
+ igt_assert(c1 == 0);
h = inject_hang(fd, ctx);
igt_assert_lte(0, h);
gem_sync(fd, h);
assert_reset_status(fd, ctx, RS_BATCH_ACTIVE);
- c2 = get_reset_count(fd, ctx);
- igt_assert(c2 >= 0);
- igt_assert(c2 == (c1 + 1));
+ c2 = get_active_count(fd, ctx);
+ igt_assert(c2 == 1);
igt_fork(child, 1) {
igt_drop_root();
@@ -877,9 +888,9 @@ static int _test_params(int fd, int ctx, uint32_t flags,
uint32_t pad)
rs.ctx_id = ctx;
rs.flags = flags;
- rs.reset_count = rand();
- rs.batch_active = rand();
- rs.batch_pending = rand();
+ rs.reset_count = UINT32_MAX;
+ rs.batch_active = UINT32_MAX;
+ rs.batch_pending = UINT32_MAX;
rs.pad = pad;
do {
@@ -976,14 +987,14 @@ static void defer_hangcheck(int ring_num)
igt_skip_on(next_ring == current_ring);
- count_start = get_reset_count(fd, 0);
- igt_assert_lte(0, count_start);
+ count_start = get_active_count(fd, 0);
+ igt_assert(count_start == 0);
igt_assert(inject_hang_ring(fd, 0, current_ring->exec, true));
while (--seconds) {
igt_assert(exec_valid_ring(fd, 0, next_ring->exec));
- count_end = get_reset_count(fd, 0);
+ count_end = get_active_count(fd, 0);
igt_assert_lte(0, count_end);
if (count_end > count_start)
@@ -992,7 +1003,7 @@ static void defer_hangcheck(int ring_num)
sleep(1);
}
- igt_assert_lt(count_start, count_end);
+ igt_assert(count_end == 1);
close(fd);
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 28/31] drm/i915: Make Sink crc calculation waiting for counter to reset.
On Tue, Nov 10, 2015 at 07:49:51PM -0200, Paulo Zanoni wrote:
> 2015-11-10 18:31 GMT-02:00 Paulo Zanoni :
> > 2015-11-05 16:50 GMT-02:00 Rodrigo Vivi :
> >> According to VESA DP spec TEST_CRC_COUNT (Bits 3:0) at
> >> TEST_SINK_MISC (00246h) is "Reset to 0 when TEST_SINK bit 0 = 0;
> >>
> >> So let's give few vblanks so we are really sure that this counter
> >> is really zeroed on the next sink_crc read.
> >>
> >> Signed-off-by: Rodrigo Vivi
> >> ---
> >> drivers/gpu/drm/i915/intel_dp.c | 19 ++-
> >> 1 file changed, 18 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >> b/drivers/gpu/drm/i915/intel_dp.c
> >> index c0fa90a..5d810cd 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -3806,6 +3806,8 @@ static int intel_dp_sink_crc_stop(struct intel_dp
> >> *intel_dp)
> >> struct intel_crtc *intel_crtc =
> >> to_intel_crtc(dig_port->base.base.crtc);
> >> u8 buf;
> >> int ret = 0;
> >> + int count = 0;
> >> + int attempts = 10;
> >>
> >> if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
> >> DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
> >> @@ -3820,7 +3822,22 @@ static int intel_dp_sink_crc_stop(struct intel_dp
> >> *intel_dp)
> >> goto out;
> >> }
> >>
> >> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> >> + do {
> >> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> >> +
> >> + if (drm_dp_dpcd_readb(&intel_dp->aux,
> >> + DP_TEST_SINK_MISC, &buf) < 0) {
> >> + ret = -EIO;
> >> + goto out;
> >
> > This "goto out" will make sink_crc.started remain as true even though
> > we already sent the DPCD message telling it to stop, and it
> > acknowledged our message. And it won't even print stuff on dmesg. I
> > guess I'd probably write something on dmesg and flip started to false.
>
> Now I see that patch 30 deals with this issue.
>
> >
> >> + }
> >> + count = buf & DP_TEST_COUNT_MASK;
> >> + } while (--attempts && count);
> >> +
> >> + if (attempts == 0) {
> >> + DRM_ERROR("TIMEOUT: Sink CRC counter is not zeroed\n");
> >
> > The other errors are all DRM_DEBUG_KMS. On one hand we can't do
> > anything about them since they're most likely panel errors so
> > DRM_ERROR doesn't look good. On the other hand normal users are not
> > going to ever run this code, and DRM_ERROR may make us - and our
> > testing robots - notice the possible failures, so maybe DRM_ERROR is
> > the way to go here. Anyway, we should be consistent regardless of the
> > decision.
> >
> > Besides, at intel_dp_sink_crc_start(), we read the last_count, but
> > it's supposed to be zero. Can't we use a check for this there too?
> > Maybe just an informative DRM_DEBUG_KMS("this was supposed to be zero
> > but it's not\n") without really returning.
>
> This is addressed by patch 29.
>
> >
> > Everything else looks good.
>
> So with or without the changes between the log level of the messages
> (since end users shouldn't be running them):
> Reviewed-by: Paulo Zanoni
>
> I also vote that we merge 27, 28, 29 and 30 right now since they don't
> require patches 1-26. The only conflict is the rename of the IPS
> functions, and this can be easily fixed in the patch file.
Good idea, all 4 pulled into dinq. Rodrigo, is this all we need to make
sink CRC reliable? Or is the read_wake stuff still needed?
-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 i-g-t 2/8] kms_frontbuffer_tracking: Skip on unreliable CRC.
On Thu, Nov 05, 2015 at 06:30:30PM -0200, Paulo Zanoni wrote:
> 2015-11-05 16:53 GMT-02:00 Rodrigo Vivi :
> > Even with all sink crc re-works we still have platforms
> > where after 6 vblanks it is unable to calculate the
> > sink crc. But if we don't get the sink crc it isn't true
> > that test failed, but that we have no ways to say test
> > passed or failed.
> >
> > So let's print a message and move forward in case sink crc
> > cannot help us to know if the screen has been updated.
>
> As much as I understand your reasoning here, "Try running this test
> again" will be ignored by our future bots.
>
> Instead of just skipping, isn't there something else we could do, such
> as trying again 10 times? 60 frames doesn't seem expensive. If it
> works at least sometimes, I'd say it's worth the try.
>
> Besides, did we try the AUX_MUTEX register that was suggested here:
> http://patchwork.freedesktop.org/patch/57693/ ? Maybe it would solve
> all our sink CRCs problem.
>
> Another comment: FBC doesn't really need sink CRC, but it's currently
> checking sink CRC, so it may get SKIPs. Maybe instead of a SKIP for
> failed sink CRC on FBC we could just ignore and move on? Maybe we
> could pass some flags to collect_crcs() so it can know if sink CRCs
> are ignorable.
Yeah, at least we should make FBC tests not depend upon sink crcs.
Otherwise test coverage might artificially go down.
-Daniel
>
> Another problem is: what if we fail while getting the reference CRC?
> We will leave garbage inside crc->data, and the other tests will
> compare themselves against the garbage in case reading sink CRCs end
> up working for them, so we'll have test failures that are not real
> failures. Maybe we should pass some flag to collect_crtcs() signaling
> that we're trying a reference CRC, so it can write something to
> crtc->data, just like we have the "unsupported!" string. Then we'd
> have to check this special string later.
>
> You also probably need to fix setup_sink_crc(), because it currently
> doesn't check for ETIMETDOUT.
>
> I'm not blocking the patch, just starting the discussion :)
>
> >
> > Signed-off-by: Rodrigo Vivi
> > ---
> > tests/kms_frontbuffer_tracking.c | 11 +--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/kms_frontbuffer_tracking.c
> > b/tests/kms_frontbuffer_tracking.c
> > index cd2879d..606d0a9 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -858,10 +858,17 @@ static bool psr_wait_until_enabled(void)
> >
> > static void get_sink_crc(sink_crc_t *crc)
> > {
> > + int rc, errno_;
> > +
> > lseek(sink_crc.fd, 0, SEEK_SET);
> >
> > - igt_assert(read(sink_crc.fd, crc->data, SINK_CRC_SIZE) ==
> > - SINK_CRC_SIZE);
> > + rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
> > + errno_ = errno;
> > +
> > + if (rc == -1 && errno_ == ETIMEDOUT)
> > + igt_skip("Sink CRC is unreliable on this machine. Try
> > running this test again individually\n");
> > +
> > + igt_assert(rc == SINK_CRC_SIZE);
> > }
> >
> > static bool sink_crc_equal(sink_crc_t *a, sink_crc_t *b)
> > --
> > 2.4.3
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
> ___
> 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 i-g-t 4/8] kms_frontbuffer_tracking: Allow to skip suspend_resume sub test case.
On Mon, Nov 09, 2015 at 11:52:20AM -0200, Paulo Zanoni wrote: > 2015-11-05 18:40 GMT-02:00 Ville Syrjälä : > > On Thu, Nov 05, 2015 at 06:34:07PM -0200, Paulo Zanoni wrote: > >> 2015-11-05 16:53 GMT-02:00 Rodrigo Vivi : > >> > There are few platforms with other suspend resume bugs that breaks > >> > the full execution. So let's provide a way to skip suspend resume case. > >> > >> Well, I carry a local patch that completely disables suspend subtests > >> for the tests that I usually run, so I really understand your pain. > >> Suspend subtests take a long time to run, and they usually don't work > >> on some of the preproduction machines I still use. > >> > >> But since this problem is not specific to kms_frontbuffer_tracking, > >> maybe we could adopt an igt-wide solution here? Thomas, any idea here? > > > > -x suspend is what I tell piglit on one hsw I have here which hangs on s3. > > The problem with piglit is that it runs every single subtest as a > separate test program invocation. For KMS tests this is a huge problem > since it requires generating the reference CRCs every time and it also > requires a modeset to restore fbcon every single time. For non-eDP the > cost is not big, but for eDP it adds up: running a subset of only 70 > subtests on kms_frontbuffer_tracking, we go from 3:05 to 6:21 in total > execution time. We need to fix this. Generating the reference crc is hard, but getting rid of fbcon and other setup overhead should be doable. Improving piglit runtime means CI can run more tests, which means the test you're writing might actually be used. -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] drm/i915: Don't fail rpm suspend with -EGAIN
On Tue, Nov 17, 2015 at 11:35:33PM +0200, Imre Deak wrote:
> On Tue, 2015-11-17 at 23:30 +0200, Ville Syrjälä wrote:
> > On Tue, Nov 17, 2015 at 10:18:41PM +0100, Daniel Vetter wrote:
> > > If we can't acquire dev->struct_mutex we need to fail runtime suspend,
> > > at least with the current design. Currently we do that using -EAGAIN,
> > > but that upsets the pm core, resulting in the occasional fail testcase
> > > in our CI with the following dmesg dirt:
> > >
> > > pci_pm_runtime_suspend(): intel_runtime_suspend+0x0/0x240 [i915] returns
> > > -11
> > >
> > > Chris has some ideas to improve this, but for now just shut up the
> > > error.
> > >
> > > Cc: Paulo Zanoni
> > > Cc: Chris Wilson
> > > Signed-off-by: Daniel Vetter
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 5a70aca71d6b..ab8ffbc48e2d 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -1497,8 +1497,7 @@ static int intel_runtime_suspend(struct device
> > > *device)
> > > * We could deadlock here in case another thread holding struct_mutex
> > > * calls RPM suspend concurrently, since the RPM suspend will wait
> > > * first for this RPM suspend to finish. In this case the concurrent
> > > - * RPM resume will be followed by its RPM suspend counterpart. Still
> > > - * for consistency return -EAGAIN, which will reschedule this suspend.
> > > + * RPM resume will be followed by its RPM suspend counterpart.
> > > */
> > > if (!mutex_trylock(&dev->struct_mutex)) {
> > > DRM_DEBUG_KMS("device lock contention, deffering suspend\n");
> > > @@ -1508,7 +1507,8 @@ static int intel_runtime_suspend(struct device
> > > *device)
> > > */
> > > pm_runtime_mark_last_busy(device);
> > >
> > > - return -EAGAIN;
> > > + /* Fail silently to avoid upsetting the pm core. */
> > > + return 0;
> >
> > So the core will assume we're now suspended and then resume gets called
> > while we're still powered on. Sounds like a bad plan to me. I'm
> > especially worried about VLV here with its GT no wake dance and manual
> > save/restore.
>
> Also the PCI core will put the device into D3 if we report success.
Oh right. Somehow I remembered that it was for system suspend only, but
it's there for runtime pm as well.
So I think if we want to hide the dmesg spew temporarily, the only sane
option is to reduce the loglevel in __suspend_report_result().
>
> >
> > > }
> > > /*
> > > * We are safe here against re-faults, since the fault handler takes
> > > --
> > > 2.5.1
> > >
> > > ___
> > > 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 1/2] drm/i915/bxt: Universal plane pixel format support
On Fri, Nov 06, 2015 at 07:34:46PM +0530, Vandita Kulkarni wrote:
> From: vandita kulkarni
>
> This patch adds support for RGB formats on sprites
> for BXT (as per Bspec) as we have Universal planes
> This patch also adds support for AYUV format on
> primary and sprites.
>
> Signed-off-by: vandita kulkarni
Needs an igt. Also there's a giant mess going on with all the universal
plane stuff, and we still don't have full CI and I'd really, really
prefer we get to fix that up a bit more before adding even more on top.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c |5 +
> drivers/gpu/drm/i915/intel_sprite.c |4
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 3f1b545..0e436d5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -76,6 +76,7 @@ static const uint32_t skl_primary_formats[] = {
> DRM_FORMAT_YVYU,
> DRM_FORMAT_UYVY,
> DRM_FORMAT_VYUY,
> + DRM_FORMAT_AYUV,
> };
>
> /* Cursor formats */
> @@ -3013,6 +3014,8 @@ u32 skl_plane_ctl_format(uint32_t pixel_format)
> return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_UYVY;
> case DRM_FORMAT_VYUY:
> return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
> + case DRM_FORMAT_AYUV:
> + return PLANE_CTL_FORMAT_AYUV;
> default:
> MISSING_CASE(pixel_format);
> }
> @@ -4482,6 +4485,7 @@ static int skl_update_scaler_plane(struct
> intel_crtc_state *crtc_state,
> case DRM_FORMAT_YVYU:
> case DRM_FORMAT_UYVY:
> case DRM_FORMAT_VYUY:
> + case DRM_FORMAT_AYUV:
> break;
> default:
> DRM_DEBUG_KMS("[PLANE:%d] FB:%d unsupported scaling format
> 0x%x\n",
> @@ -14418,6 +14422,7 @@ static int intel_framebuffer_init(struct drm_device
> *dev,
> case DRM_FORMAT_UYVY:
> case DRM_FORMAT_YVYU:
> case DRM_FORMAT_VYUY:
> + case DRM_FORMAT_AYUV:
> if (INTEL_INFO(dev)->gen < 5) {
> DRM_DEBUG("unsupported pixel format: %s\n",
> drm_get_format_name(mode_cmd->pixel_format));
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 4276c13..c250a82 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1029,10 +1029,14 @@ static uint32_t skl_plane_formats[] = {
> DRM_FORMAT_ARGB,
> DRM_FORMAT_XBGR,
> DRM_FORMAT_XRGB,
> + DRM_FORMAT_XRGB2101010,
> + DRM_FORMAT_XBGR2101010,
> DRM_FORMAT_YUYV,
> DRM_FORMAT_YVYU,
> DRM_FORMAT_UYVY,
> DRM_FORMAT_VYUY,
> + DRM_FORMAT_AYUV,
> + DRM_FORMAT_C8,
> };
>
> int
> --
> 1.7.9.5
>
> ___
> 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] i-g-t/libdrm email tagging & patchwork
On Sun, Nov 08, 2015 at 12:31:36AM +, Damien Lespiau wrote: > Hi all, > > I've added a feature to sort the patches sent to intel-gfx into 3 > buckets: i915, intel-gpu-tools and libdrm. This sorting relies on > tagging patches, using the subject prefixes (which is what most people > do already anyway). > > - i915 (intel-gfx): catchall project, all mails not matching any of > the other 2 projects will end up there > > - intel-gpu-tools: mails need to be tagged with i-g-t, igt or > intel-gpu-tools > > - libdrm-intel: mails need to be tagged with libdrm > > This tagging can be set up per git repository with: > > $ git config format.subjectprefix "PATCH i-g-t" Is there any way we could push this out to users somehow? I have bazillion of machines, I'll get this wrong eventually ... So will everyone else I guess. -Daniel > > And use git send-email as usual. A note of caution though, using the > command line argument --subject-prefix will override the one configured, > so the tag will have to be repeated. To limit the number of things one > needs to think about I'd suggest to use --reroll-count to tag patches > with the v2/v3/... tags. I'm more and more thinking that wrapping the > sending logic for developers into the git-pw command would be a good > thing (especially for --in-reply-to) but that'd be for another time. > > There are two new patchwork projects then: > > http://patchwork.freedesktop.org/project/intel-gpu-tools/ > http://patchwork.freedesktop.org/project/libdrm-intel/ > > I've also run the sorting on all the existing patches so the entries > that were historically in the intel-gfx project are now in those new > projects. > > There is still some work left to limit the noise of those lists of > patches, eg. some patches are still marked as New but, in reality, they > have been merged. Solving that is quite important and high-ish the TODO > list. > > HTH, > > -- > Damien > ___ > 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
[Intel-gfx] [PATCH v2] PCI / PM: tune down RPM suspend error message with EBUSY and EAGAIN retval
The runtime PM core doesn't treat EBUSY and EAGAIN retvals from the driver
suspend hooks as errors, but they still show up as errors in dmesg. Tune
them down.
One problem caused by this was noticed by Daniel: the i915 driver
returns EAGAIN to signal a temporary failure to suspend and as a request
towards the RPM core for scheduling a suspend again. This is a normal
event, but the resulting error message flags a breakage during the
driver's automated testing which parses dmesg and picks up the error.
v2:
- fix compile breake when CONFIG_PM_SLEEP=n (0-day builder)
Reported-by: Daniel Vetter
Signed-off-by: Imre Deak
---
drivers/base/power/main.c | 7 +--
drivers/pci/pci-driver.c | 2 +-
include/linux/pm.h| 11 +--
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 1710c26..39d2090 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1679,9 +1679,12 @@ int dpm_suspend_start(pm_message_t state)
}
EXPORT_SYMBOL_GPL(dpm_suspend_start);
-void __suspend_report_result(const char *function, void *fn, int ret)
+void __suspend_report_result(const char *function, void *fn, int ret,
+bool runtime_pm)
{
- if (ret)
+ if (runtime_pm && (ret == -EBUSY || ret == -EAGAIN))
+ printk(KERN_DEBUG "%s(): %pF returns %d\n", function, fn, ret);
+ else if (ret)
printk(KERN_ERR "%s(): %pF returns %d\n", function, fn, ret);
}
EXPORT_SYMBOL_GPL(__suspend_report_result);
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 108a311..9569572 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1142,7 +1142,7 @@ static int pci_pm_runtime_suspend(struct device *dev)
pci_dev->state_saved = false;
pci_dev->no_d3cold = false;
error = pm->runtime_suspend(dev);
- suspend_report_result(pm->runtime_suspend, error);
+ rpm_suspend_report_result(pm->runtime_suspend, error);
if (error)
return error;
if (!pci_dev->d3cold_allowed)
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 35d599e..54f37e3 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -702,11 +702,17 @@ extern int dpm_suspend_late(pm_message_t state);
extern int dpm_suspend(pm_message_t state);
extern int dpm_prepare(pm_message_t state);
-extern void __suspend_report_result(const char *function, void *fn, int ret);
+extern void __suspend_report_result(const char *function, void *fn, int ret,
+ bool runtime_pm);
#define suspend_report_result(fn, ret) \
do {\
- __suspend_report_result(__func__, fn, ret); \
+ __suspend_report_result(__func__, fn, ret, false); \
+ } while (0)
+
+#define rpm_suspend_report_result(fn, ret) \
+ do {\
+ __suspend_report_result(__func__, fn, ret, true); \
} while (0)
extern int device_pm_wait_for_dev(struct device *sub, struct device *dev);
@@ -744,6 +750,7 @@ static inline int dpm_suspend_start(pm_message_t state)
}
#define suspend_report_result(fn, ret) do {} while (0)
+#define rpm_suspend_report_result(fn, ret) do {} while (0)
static inline int device_pm_wait_for_dev(struct device *a, struct device *b)
{
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915 Skylake: "Invalid ROM contents"
On Tue, Nov 17, 2015 at 11:43:25AM -0800, Andy Lutomirski wrote: > Typing: > > # cat /sys/devices/pci:00/:00:02.0/rom > > Provokes: > > i915 :00:02.0: Invalid ROM contents Hmm. So there's no PCI option ROM there. I wonder what is there. I get the same on my Braswell BTW. I tried to look through the UEFI spec a bit, and it seems to say that even for non-legacy option ROMs the 0x55aa signature should be there. But this being the GPU means we may be using the shadow ROM stuff, which IIRC assumes that the shadow is at 0xc000. I'm not sure that holds anymore with UEFI, and maybe we should be using some UEFI trick instead to find out where it actually lives? BTW what does 'lspci -vv -s 00:02.0' say on your machine? > > This is on a Dell XPS 13 9350 (Skylake). This is 4.3.0 plus some > wireless-next bits. > > --Andy > > -- > Andy Lutomirski > AMA Capital Management, LLC > ___ > 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: Add BXT Sprite plane fractional scalings
On older platforms scalers/cliping used to provide destination size an
exact multiple of src size.
Gen-9 and above support fractional ratio of dst/src so that source is
not manipulated to meet the exact multiple factor.
Signed-off-by: Nabendu Maiti
---
drivers/gpu/drm/i915/intel_sprite.c | 48 +
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 2b96f33..e8c17ae 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -813,29 +813,37 @@ intel_check_sprite_plane(struct drm_plane *plane,
crtc_h = drm_rect_height(dst);
if (state->visible) {
- /* check again in case clipping clamped the results */
- hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
- if (hscale < 0) {
- DRM_DEBUG_KMS("Horizontal scaling factor out of
limits\n");
- drm_rect_debug_print("src: ", src, true);
- drm_rect_debug_print("dst: ", dst, false);
-
- return hscale;
- }
- vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
- if (vscale < 0) {
- DRM_DEBUG_KMS("Vertical scaling factor out of
limits\n");
- drm_rect_debug_print("src: ", src, true);
- drm_rect_debug_print("dst: ", dst, false);
+ /* Gen 9 and above has fractional scaling support */
+ if (INTEL_INFO(plane->dev)->gen < 9) {
- return vscale;
- }
+ /* check again in case clipping clamped the results */
+ hscale = drm_rect_calc_hscale(src, dst, min_scale,
max_scale);
+ if (hscale < 0) {
+ DRM_DEBUG_KMS("Horizontal scaling factor out of
limits\n");
+ drm_rect_debug_print("src: ", src, true);
+ drm_rect_debug_print("dst: ", dst, false);
+
+ return hscale;
+ }
+
+ vscale = drm_rect_calc_vscale(src, dst, min_scale,
max_scale);
+ if (vscale < 0) {
+ DRM_DEBUG_KMS("Vertical scaling factor out of
limits\n");
+ drm_rect_debug_print("src: ", src, true);
+ drm_rect_debug_print("dst: ", dst, false);
- /* Make the source viewport size an exact multiple of the
scaling factors. */
- drm_rect_adjust_size(src,
-drm_rect_width(dst) * hscale -
drm_rect_width(src),
-drm_rect_height(dst) * vscale -
drm_rect_height(src));
+ return vscale;
+ }
+
+ /*
+* Make the source viewport size an exact
+* multiple of the scaling factors.
+*/
+ drm_rect_adjust_size(src,
+drm_rect_width(dst) * hscale -
drm_rect_width(src),
+drm_rect_height(dst) * vscale -
drm_rect_height(src));
+ }
drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
state->base.rotation);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Correcting proper src & dst height - width comparision for 90/270 rotation.
On 90/270 rotation case source width and height was not compared properly with destination height and width check plane.Which added erroneous check while doing scaling or normal 90/270 rotation. Signed-off-by: Nabendu Maiti --- drivers/gpu/drm/i915/intel_display.c | 22 -- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 688d484..cd5bb28 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane *plane, int min_scale = DRM_PLANE_HELPER_NO_SCALING; int max_scale = DRM_PLANE_HELPER_NO_SCALING; bool can_position = false; + struct drm_rect *src = &state->src; + struct drm_rect *dest = &state->dst; + int ret = -1; /* use scaler when colorkey is not required */ if (INTEL_INFO(plane->dev)->gen >= 9 && @@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane *plane, can_position = true; } - return drm_plane_helper_check_update(plane, crtc, fb, &state->src, -&state->dst, &state->clip, + /* +* FIXME the following code does a bunch of fuzzy adjustments to the +* coordinates and sizes for rotations. We probably need some way to +* decide whether more strict checking should be done instead. +*/ + if (fb) + drm_rect_rotate(src, fb->width << 16, fb->height << 16, + state->base.rotation); + + ret = drm_plane_helper_check_update(plane, crtc, fb, src, +dest, &state->clip, min_scale, max_scale, can_position, true, &state->visible); + + /* Restore the originl unrotated co-ordinates */ + if (fb) + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, + state->base.rotation); + return ret; } static void -- 1.9.1 ___ 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_reset_stats.c: prepare for per engine resets
On Wed, Nov 18, 2015 at 10:24:43AM +, tim.g...@intel.com wrote:
> From: Tim Gore
>
> when checking to make sure that the driver has performed
> the expected number of resets, this test looks at the
> reset_count, which is incremented each time the GPU is
> reset. Upcoming changes in the way GPU hangs are handled
> mean that in most cases (and in all the cases in this
> test) only a single GPU engine is reset which does not
> cause the reset_count to be incremented. This is already
> causing this test to fail on Android. In this case
> we can instead look at the batch_active count which is
> also returned from the i915_get_reset_stats_ioctl and is
> incremented by both a single engine reset and a full
> gpu reset. There are differences between the reset_count
> and the batch_active count, but for establishing that the
> correct number of resets have occured either can be used.
> This change enables this test to run successfully on
> Android and will mean that the test does not break when
> the TDR patches get merged into the uptream driver.
>
> Signed-off-by: Tim Gore
Why doesn't TDR just count resets correctly?
-Daniel
> ---
> tests/gem_reset_stats.c | 41 ++---
> 1 file changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
> index 4cbbb4e..5ec026f 100644
> --- a/tests/gem_reset_stats.c
> +++ b/tests/gem_reset_stats.c
> @@ -104,9 +104,9 @@ static int gem_reset_stats(int fd, int ctx_id,
>
> rs->ctx_id = ctx_id;
> rs->flags = 0;
> - rs->reset_count = rand();
> - rs->batch_active = rand();
> - rs->batch_pending = rand();
> + rs->reset_count = UINT32_MAX;
> + rs->batch_active = UINT32_MAX;
> + rs->batch_pending = UINT32_MAX;
> rs->pad = 0;
>
> do {
> @@ -690,6 +690,18 @@ static int get_reset_count(int fd, int ctx)
> return rs.reset_count;
> }
>
> +static int get_active_count(int fd, int ctx)
> +{
> + int ret;
> + struct local_drm_i915_reset_stats rs;
> +
> + ret = gem_reset_stats(fd, ctx, &rs);
> + if (ret)
> + return ret;
> +
> + return rs.batch_active;
> +}
> +
> static void test_close_pending_ctx(void)
> {
> int fd, h;
> @@ -837,17 +849,16 @@ static void test_reset_count(const bool create_ctx)
>
> assert_reset_status(fd, ctx, RS_NO_ERROR);
>
> - c1 = get_reset_count(fd, ctx);
> - igt_assert(c1 >= 0);
> + c1 = get_active_count(fd, ctx);
> + igt_assert(c1 == 0);
>
> h = inject_hang(fd, ctx);
> igt_assert_lte(0, h);
> gem_sync(fd, h);
>
> assert_reset_status(fd, ctx, RS_BATCH_ACTIVE);
> - c2 = get_reset_count(fd, ctx);
> - igt_assert(c2 >= 0);
> - igt_assert(c2 == (c1 + 1));
> + c2 = get_active_count(fd, ctx);
> + igt_assert(c2 == 1);
>
> igt_fork(child, 1) {
> igt_drop_root();
> @@ -877,9 +888,9 @@ static int _test_params(int fd, int ctx, uint32_t flags,
> uint32_t pad)
>
> rs.ctx_id = ctx;
> rs.flags = flags;
> - rs.reset_count = rand();
> - rs.batch_active = rand();
> - rs.batch_pending = rand();
> + rs.reset_count = UINT32_MAX;
> + rs.batch_active = UINT32_MAX;
> + rs.batch_pending = UINT32_MAX;
> rs.pad = pad;
>
> do {
> @@ -976,14 +987,14 @@ static void defer_hangcheck(int ring_num)
>
> igt_skip_on(next_ring == current_ring);
>
> - count_start = get_reset_count(fd, 0);
> - igt_assert_lte(0, count_start);
> + count_start = get_active_count(fd, 0);
> + igt_assert(count_start == 0);
>
> igt_assert(inject_hang_ring(fd, 0, current_ring->exec, true));
> while (--seconds) {
> igt_assert(exec_valid_ring(fd, 0, next_ring->exec));
>
> - count_end = get_reset_count(fd, 0);
> + count_end = get_active_count(fd, 0);
> igt_assert_lte(0, count_end);
>
> if (count_end > count_start)
> @@ -992,7 +1003,7 @@ static void defer_hangcheck(int ring_num)
> sleep(1);
> }
>
> - igt_assert_lt(count_start, count_end);
> + igt_assert(count_end == 1);
>
> close(fd);
> }
> --
> 1.9.1
>
> ___
> 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
[Intel-gfx] [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
Uninitialized variables (width, Height) in intel_check_sprite_plane leads to compilererror in O1 level. Initialize all declared variables to fix this issue. Signed-off-by: Nabendu Maiti --- drivers/gpu/drm/i915/intel_sprite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 2b96f33..a5af384 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -747,7 +747,7 @@ intel_check_sprite_plane(struct drm_plane *plane, struct drm_framebuffer *fb = state->base.fb; int crtc_x, crtc_y; unsigned int crtc_w, crtc_h; - uint32_t src_x, src_y, src_w, src_h; + uint32_t src_x = 0, src_y = 0, src_w = 0, src_h = 0; struct drm_rect *src = &state->src; struct drm_rect *dst = &state->dst; const struct drm_rect *clip = &state->clip; -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add BXT Sprite plane fractional scalings
On Wed, Nov 18, 2015 at 05:13:01PM +0530, Nabendu Maiti wrote:
> On older platforms scalers/cliping used to provide destination size an
> exact multiple of src size.
> Gen-9 and above support fractional ratio of dst/src so that source is
> not manipulated to meet the exact multiple factor.
>
> Signed-off-by: Nabendu Maiti
> ---
> drivers/gpu/drm/i915/intel_sprite.c | 48
> +
> 1 file changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 2b96f33..e8c17ae 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -813,29 +813,37 @@ intel_check_sprite_plane(struct drm_plane *plane,
> crtc_h = drm_rect_height(dst);
>
> if (state->visible) {
> - /* check again in case clipping clamped the results */
> - hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> - if (hscale < 0) {
> - DRM_DEBUG_KMS("Horizontal scaling factor out of
> limits\n");
> - drm_rect_debug_print("src: ", src, true);
> - drm_rect_debug_print("dst: ", dst, false);
> -
> - return hscale;
> - }
>
> - vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> - if (vscale < 0) {
> - DRM_DEBUG_KMS("Vertical scaling factor out of
> limits\n");
> - drm_rect_debug_print("src: ", src, true);
> - drm_rect_debug_print("dst: ", dst, false);
> + /* Gen 9 and above has fractional scaling support */
> + if (INTEL_INFO(plane->dev)->gen < 9) {
>
> - return vscale;
> - }
> + /* check again in case clipping clamped the results */
> + hscale = drm_rect_calc_hscale(src, dst, min_scale,
> max_scale);
> + if (hscale < 0) {
> + DRM_DEBUG_KMS("Horizontal scaling factor out of
> limits\n");
> + drm_rect_debug_print("src: ", src, true);
> + drm_rect_debug_print("dst: ", dst, false);
> +
> + return hscale;
> + }
> +
> + vscale = drm_rect_calc_vscale(src, dst, min_scale,
> max_scale);
> + if (vscale < 0) {
> + DRM_DEBUG_KMS("Vertical scaling factor out of
> limits\n");
> + drm_rect_debug_print("src: ", src, true);
> + drm_rect_debug_print("dst: ", dst, false);
>
> - /* Make the source viewport size an exact multiple of the
> scaling factors. */
> - drm_rect_adjust_size(src,
> - drm_rect_width(dst) * hscale -
> drm_rect_width(src),
> - drm_rect_height(dst) * vscale -
> drm_rect_height(src));
> + return vscale;
> + }
> +
> + /*
> + * Make the source viewport size an exact
> + * multiple of the scaling factors.
> + */
> + drm_rect_adjust_size(src,
> + drm_rect_width(dst) * hscale -
> drm_rect_width(src),
> + drm_rect_height(dst) * vscale -
> drm_rect_height(src));
> + }
NAK. This code just makes sure the actual scaling ratio matches what we
calculated. As in there may have been some amount of rounding involved
etc.
The part you actually want to change is what comes after this where we
throw away the fractional part of the coordinates. And then you need to
change the actual hw programming so that we actually feed the sub-pixel
coordinates to the hardware.
>
> drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
> state->base.rotation);
> --
> 1.9.1
>
> ___
> 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: Correcting proper src & dst height - width comparision for 90/270 rotation.
On Wed, Nov 18, 2015 at 05:19:26PM +0530, Nabendu Maiti wrote: > On 90/270 rotation case source width and height was not compared > properly with destination height and width check plane.Which added > erroneous check while doing scaling or normal 90/270 rotation. > > Signed-off-by: Nabendu Maiti > --- > drivers/gpu/drm/i915/intel_display.c | 22 -- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 688d484..cd5bb28 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13723,6 +13723,9 @@ intel_check_primary_plane(struct drm_plane *plane, > int min_scale = DRM_PLANE_HELPER_NO_SCALING; > int max_scale = DRM_PLANE_HELPER_NO_SCALING; > bool can_position = false; > + struct drm_rect *src = &state->src; > + struct drm_rect *dest = &state->dst; > + int ret = -1; > > /* use scaler when colorkey is not required */ > if (INTEL_INFO(plane->dev)->gen >= 9 && > @@ -13732,11 +13735,26 @@ intel_check_primary_plane(struct drm_plane *plane, > can_position = true; > } > > - return drm_plane_helper_check_update(plane, crtc, fb, &state->src, > - &state->dst, &state->clip, > + /* > + * FIXME the following code does a bunch of fuzzy adjustments to the > + * coordinates and sizes for rotations. We probably need some way to > + * decide whether more strict checking should be done instead. > + */ > + if (fb) > + drm_rect_rotate(src, fb->width << 16, fb->height << 16, > + state->base.rotation); > + > + ret = drm_plane_helper_check_update(plane, crtc, fb, src, > + dest, &state->clip, >min_scale, max_scale, >can_position, true, >&state->visible); > + > + /* Restore the originl unrotated co-ordinates */ > + if (fb) > + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, > + state->base.rotation); We should put the rotation handling into helper. And someone should really just move all the good code from intel_sprite into the helper instead of having two totally different ways of doing things. > + return ret; > } > > static void > -- > 1.9.1 > > ___ > 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 i-g-t] tests/gem_reset_stats.c: prepare for per engine resets
Tim Gore
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Wednesday, November 18, 2015 11:54 AM
> To: Gore, Tim
> Cc: intel-gfx@lists.freedesktop.org; mika.kuopp...@linux.com; Wood,
> Thomas
> Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/gem_reset_stats.c: prepare for
> per engine resets
>
> On Wed, Nov 18, 2015 at 10:24:43AM +, tim.g...@intel.com wrote:
> > From: Tim Gore
> >
> > when checking to make sure that the driver has performed the expected
> > number of resets, this test looks at the reset_count, which is
> > incremented each time the GPU is reset. Upcoming changes in the way
> > GPU hangs are handled mean that in most cases (and in all the cases in
> > this
> > test) only a single GPU engine is reset which does not cause the
> > reset_count to be incremented. This is already causing this test to
> > fail on Android. In this case we can instead look at the batch_active
> > count which is also returned from the i915_get_reset_stats_ioctl and
> > is incremented by both a single engine reset and a full gpu reset.
> > There are differences between the reset_count and the batch_active
> > count, but for establishing that the correct number of resets have
> > occured either can be used.
> > This change enables this test to run successfully on Android and will
> > mean that the test does not break when the TDR patches get merged into
> > the uptream driver.
> >
> > Signed-off-by: Tim Gore
>
> Why doesn't TDR just count resets correctly?
> -Daniel
>
It does. It just doesn't do gpu resets for most hangs. Instead it resets
as single command streamer which is much less intrusive and loses the
minimum amount of work.
Tim
> > ---
> > tests/gem_reset_stats.c | 41
> > ++---
> > 1 file changed, 26 insertions(+), 15 deletions(-)
> >
> > diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c index
> > 4cbbb4e..5ec026f 100644
> > --- a/tests/gem_reset_stats.c
> > +++ b/tests/gem_reset_stats.c
> > @@ -104,9 +104,9 @@ static int gem_reset_stats(int fd, int ctx_id,
> >
> > rs->ctx_id = ctx_id;
> > rs->flags = 0;
> > - rs->reset_count = rand();
> > - rs->batch_active = rand();
> > - rs->batch_pending = rand();
> > + rs->reset_count = UINT32_MAX;
> > + rs->batch_active = UINT32_MAX;
> > + rs->batch_pending = UINT32_MAX;
> > rs->pad = 0;
> >
> > do {
> > @@ -690,6 +690,18 @@ static int get_reset_count(int fd, int ctx)
> > return rs.reset_count;
> > }
> >
> > +static int get_active_count(int fd, int ctx) {
> > + int ret;
> > + struct local_drm_i915_reset_stats rs;
> > +
> > + ret = gem_reset_stats(fd, ctx, &rs);
> > + if (ret)
> > + return ret;
> > +
> > + return rs.batch_active;
> > +}
> > +
> > static void test_close_pending_ctx(void) {
> > int fd, h;
> > @@ -837,17 +849,16 @@ static void test_reset_count(const bool
> > create_ctx)
> >
> > assert_reset_status(fd, ctx, RS_NO_ERROR);
> >
> > - c1 = get_reset_count(fd, ctx);
> > - igt_assert(c1 >= 0);
> > + c1 = get_active_count(fd, ctx);
> > + igt_assert(c1 == 0);
> >
> > h = inject_hang(fd, ctx);
> > igt_assert_lte(0, h);
> > gem_sync(fd, h);
> >
> > assert_reset_status(fd, ctx, RS_BATCH_ACTIVE);
> > - c2 = get_reset_count(fd, ctx);
> > - igt_assert(c2 >= 0);
> > - igt_assert(c2 == (c1 + 1));
> > + c2 = get_active_count(fd, ctx);
> > + igt_assert(c2 == 1);
> >
> > igt_fork(child, 1) {
> > igt_drop_root();
> > @@ -877,9 +888,9 @@ static int _test_params(int fd, int ctx, uint32_t
> > flags, uint32_t pad)
> >
> > rs.ctx_id = ctx;
> > rs.flags = flags;
> > - rs.reset_count = rand();
> > - rs.batch_active = rand();
> > - rs.batch_pending = rand();
> > + rs.reset_count = UINT32_MAX;
> > + rs.batch_active = UINT32_MAX;
> > + rs.batch_pending = UINT32_MAX;
> > rs.pad = pad;
> >
> > do {
> > @@ -976,14 +987,14 @@ static void defer_hangcheck(int ring_num)
> >
> > igt_skip_on(next_ring == current_ring);
> >
> > - count_start = get_reset_count(fd, 0);
> > - igt_assert_lte(0, count_start);
> > + count_start = get_active_count(fd, 0);
> > + igt_assert(count_start == 0);
> >
> > igt_assert(inject_hang_ring(fd, 0, current_ring->exec, true));
> > while (--seconds) {
> > igt_assert(exec_valid_ring(fd, 0, next_ring->exec));
> >
> > - count_end = get_reset_count(fd, 0);
> > + count_end = get_active_count(fd, 0);
> > igt_assert_lte(0, count_end);
> >
> > if (count_end > count_start)
> > @@ -992,7 +1003,7 @@ static void defer_hangcheck(int ring_num)
> > sleep(1);
> > }
> >
> > - igt_assert_lt(count_start, count_end);
> > + igt_assert(count_end == 1);
> >
> > close(fd);
> > }
> > --
> > 1.9.1
> >
> > ___
Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote: > Uninitialized variables (width, Height) in intel_check_sprite_plane > leads to compilererror in O1 level. Initialize all declared variables > to fix this issue. > > Signed-off-by: Nabendu Maiti > --- > drivers/gpu/drm/i915/intel_sprite.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > b/drivers/gpu/drm/i915/intel_sprite.c > index 2b96f33..a5af384 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -747,7 +747,7 @@ intel_check_sprite_plane(struct drm_plane *plane, > struct drm_framebuffer *fb = state->base.fb; > int crtc_x, crtc_y; > unsigned int crtc_w, crtc_h; > - uint32_t src_x, src_y, src_w, src_h; > + uint32_t src_x = 0, src_y = 0, src_w = 0, src_h = 0; Where exactly are these used without initialization? > struct drm_rect *src = &state->src; > struct drm_rect *dst = &state->dst; > const struct drm_rect *clip = &state->clip; > -- > 1.9.1 > > ___ > 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: Fix uninitialized variables in intel_check_sprite_plane
On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
> Uninitialized variables (width, Height) in intel_check_sprite_plane
> leads to compilererror in O1 level. Initialize all declared variables
> to fix this issue.
>
> Signed-off-by: Nabendu Maiti
Or perhaps:
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 2b96f336589e..8d7b4eb5b5b9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
struct drm_framebuffer *fb = state->base.fb;
int crtc_x, crtc_y;
unsigned int crtc_w, crtc_h;
- uint32_t src_x, src_y, src_w, src_h;
struct drm_rect *src = &state->src;
struct drm_rect *dst = &state->dst;
const struct drm_rect *clip = &state->clip;
@@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
crtc_h = drm_rect_height(dst);
if (state->visible) {
+ u32 src_x, src_y, src_w, src_h;
+
/* check again in case clipping clamped the results */
hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
if (hscale < 0) {
@@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (crtc_w == 0)
state->visible = false;
}
- }
/* Check size restrictions when scaling */
- if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
+ if (src_w != crtc_w || src_h != crtc_h) {
unsigned int width_bytes;
WARN_ON(!can_scale);
@@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
}
}
- if (state->visible) {
src->x1 = src_x << 16;
src->x2 = (src_x + src_w) << 16;
src->y1 = src_y << 16;
And make both the compiler and reader happier
-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
[Intel-gfx] [PATCH i-g-t v3] lib/igt_core: Prefer CLOCK_MONOTONIC_RAW
CLOCK_MONOTONIC_RAW is not affected by NTP, so it should be THE clock
used for timing execution of tests.
When fetching either the starting or ending time of a test, show the
time as -1.000s.
v3:
- Do not exit directly from handler (Chris)
- Show elapsed time as -1 if it is not calculable
v2:
- Cache the used clock (Chris)
- Do not change the clock during execution
- Spit out and error if monotonic time can not be read
Cc: Thomas Wood
Cc: Chris Wilson
Signed-off-by: Joonas Lahtinen
---
lib/igt_core.c | 51 +--
1 file changed, 41 insertions(+), 10 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 04a0ab2..b4cab42 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -220,6 +220,7 @@ static char *run_single_subtest = NULL;
static bool run_single_subtest_found = false;
static const char *in_subtest = NULL;
static struct timespec subtest_time;
+static clockid_t igt_clock = (clockid_t)-1;
static bool in_fixture = false;
static bool test_with_subtests = false;
static bool in_atexit_handler = false;
@@ -337,14 +338,32 @@ static void kmsg(const char *format, ...)
fclose(file);
}
-static void gettime(struct timespec *ts)
+static int gettime(struct timespec *ts)
{
memset(ts, 0, sizeof(*ts));
+ errno = 0;
+ // Stay on the same clock for consistency.
+ if (igt_clock != (clockid_t)-1) {
+ if (clock_gettime(igt_clock, ts))
+ goto error;
+ return 0;
+ }
+
+#ifdef CLOCK_MONOTONIC_RAW
+ if (!clock_gettime(igt_clock = CLOCK_MONOTONIC_RAW, ts))
+ return 0;
+#endif
#ifdef CLOCK_MONOTONIC_COARSE
- if (clock_gettime(CLOCK_MONOTONIC_COARSE, ts))
+ if (!clock_gettime(igt_clock = CLOCK_MONOTONIC_COARSE, ts))
+ return 0;
#endif
- clock_gettime(CLOCK_MONOTONIC, ts);
+ if (!clock_gettime(igt_clock = CLOCK_MONOTONIC, ts))
+ return 0;
+error:
+ igt_warn("Could not read monotonic time: %s\n",
+strerror(errno));
+ return -errno;
}
bool __igt_fixture(void)
@@ -832,10 +851,16 @@ static void exit_subtest(const char *result)
{
struct timespec now;
double elapsed;
+ int err;
- gettime(&now);
- elapsed = now.tv_sec - subtest_time.tv_sec;
- elapsed += (now.tv_nsec - subtest_time.tv_nsec) * 1e-9;
+ err = gettime(&now);
+ if (!err && subtest_time.tv_sec != 0 &&
+ subtest_time.tv_nsec != 0) {
+ elapsed = now.tv_sec - subtest_time.tv_sec;
+ elapsed += (now.tv_nsec - subtest_time.tv_nsec) * 1e-9;
+ } else {
+ elapsed = -1.;
+ }
printf("%sSubtest %s: %s (%.3fs)%s\n",
(!__igt_plain_output) ? "\x1b[1m" : "",
@@ -1090,10 +1115,17 @@ void igt_exit(void)
struct timespec now;
double elapsed;
const char *result;
+ int err;
- gettime(&now);
- elapsed = now.tv_sec - subtest_time.tv_sec;
- elapsed += (now.tv_nsec - subtest_time.tv_nsec) * 1e-9;
+ err = gettime(&now);
+
+ if (!err && subtest_time.tv_sec != 0 &&
+ subtest_time.tv_nsec != 0) {
+ elapsed = now.tv_sec - subtest_time.tv_sec;
+ elapsed += (now.tv_nsec - subtest_time.tv_nsec) * 1e-9;
+ } else {
+ elapsed = -1.;
+ }
switch (igt_exitcode) {
case IGT_EXIT_SUCCESS:
@@ -1109,7 +1141,6 @@ void igt_exit(void)
result = "FAIL";
}
-
printf("%s (%.3fs)\n", result, elapsed);
exit(igt_exitcode);
}
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Consider SPLL as another shared pll, v2.
On 16.11.2015 17:53, Jani Nikula wrote:
On Mon, 16 Nov 2015, Maarten Lankhorst
wrote:
When diagnosing a unrelated bug for someone on irc, it would seem the hardware
can
be brought up by the BIOS with the embedded displayport using the SPLL for
spread spectrum.
Right now this is not handled well in i915, and it calculates the crtc needs to
be reprogrammed on the first modeset without SSC, but the SPLL itself was kept
active. Fix this by exposing SPLL as a shared pll that will not be returned
by intel_get_shared_dpll; you have to know it exists to use it.
Changes since v1:
- Create a separate dpll_hw_state.spll for spll, and use
separate pll functions for spll.
Tested-by: Emil Renner Berthing #v1
Reviewed-by: Daniel Vetter
Signed-off-by: Maarten Lankhorst
---
Emil, can you retest?
Gabriel, you too please!
I retested it and it's ok.
Regards,
Gabriel.
BR,
Jani.
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/intel_crt.c | 31 ++-
drivers/gpu/drm/i915/intel_ddi.c | 75 +++-
drivers/gpu/drm/i915/intel_display.c | 15 ++--
4 files changed, 83 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 85c84c624ed1..b6b62387cbe1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -351,6 +351,8 @@ enum intel_dpll_id {
/* hsw/bdw */
DPLL_ID_WRPLL1 = 0,
DPLL_ID_WRPLL2 = 1,
+ DPLL_ID_SPLL = 2,
+
/* skl */
DPLL_ID_SKL_DPLL1 = 0,
DPLL_ID_SKL_DPLL2 = 1,
@@ -367,6 +369,7 @@ struct intel_dpll_hw_state {
/* hsw, bdw */
uint32_t wrpll;
+ uint32_t spll;
/* skl */
/*
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index b84aaa0bb48a..6a2c76e367a5 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -138,18 +138,6 @@ static void hsw_crt_get_config(struct intel_encoder
*encoder,
pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
}
-static void hsw_crt_pre_enable(struct intel_encoder *encoder)
-{
- struct drm_device *dev = encoder->base.dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL already enabled\n");
- I915_WRITE(SPLL_CTL,
- SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC);
- POSTING_READ(SPLL_CTL);
- udelay(20);
-}
-
/* Note: The caller is required to filter out dpms modes not supported by the
* platform. */
static void intel_crt_set_dpms(struct intel_encoder *encoder, int mode)
@@ -216,19 +204,6 @@ static void pch_post_disable_crt(struct intel_encoder
*encoder)
intel_disable_crt(encoder);
}
-static void hsw_crt_post_disable(struct intel_encoder *encoder)
-{
- struct drm_device *dev = encoder->base.dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- uint32_t val;
-
- DRM_DEBUG_KMS("Disabling SPLL\n");
- val = I915_READ(SPLL_CTL);
- WARN_ON(!(val & SPLL_PLL_ENABLE));
- I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
- POSTING_READ(SPLL_CTL);
-}
-
static void intel_enable_crt(struct intel_encoder *encoder)
{
struct intel_crt *crt = intel_encoder_to_crt(encoder);
@@ -280,6 +255,10 @@ static bool intel_crt_compute_config(struct intel_encoder
*encoder,
if (HAS_DDI(dev)) {
pipe_config->ddi_pll_sel = PORT_CLK_SEL_SPLL;
pipe_config->port_clock = 135000 * 2;
+
+ pipe_config->dpll_hw_state.wrpll = 0;
+ pipe_config->dpll_hw_state.spll =
+ SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC;
}
return true;
@@ -860,8 +839,6 @@ void intel_crt_init(struct drm_device *dev)
if (HAS_DDI(dev)) {
crt->base.get_config = hsw_crt_get_config;
crt->base.get_hw_state = intel_ddi_get_hw_state;
- crt->base.pre_enable = hsw_crt_pre_enable;
- crt->base.post_disable = hsw_crt_post_disable;
} else {
crt->base.get_config = intel_crt_get_config;
crt->base.get_hw_state = intel_crt_get_hw_state;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index abb4a265a6df..90fad7ad12ac 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1286,6 +1286,18 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
}
crtc_state->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
+ } else if (crtc_state->ddi_pll_sel == PORT_CLK_SEL_SPLL) {
+ struct drm_atomic_state *state = crtc_state->base.state;
+ struct intel_shared_dpll_config *spll =
+
&intel_atomic_get_shared_dpll_state(state)[DPLL_ID_SPLL];
+
+ if (
Re: [Intel-gfx] [PATCH i-g-t v3] lib/igt_core: Prefer CLOCK_MONOTONIC_RAW
On Wed, Nov 18, 2015 at 02:18:52PM +0200, Joonas Lahtinen wrote:
> CLOCK_MONOTONIC_RAW is not affected by NTP, so it should be THE clock
> used for timing execution of tests.
>
> When fetching either the starting or ending time of a test, show the
> time as -1.000s.
>
> v3:
> - Do not exit directly from handler (Chris)
> - Show elapsed time as -1 if it is not calculable
Aye, that's better for the subtest handling.
> @@ -832,10 +851,16 @@ static void exit_subtest(const char *result)
> {
> struct timespec now;
> double elapsed;
> + int err;
>
> - gettime(&now);
> - elapsed = now.tv_sec - subtest_time.tv_sec;
> - elapsed += (now.tv_nsec - subtest_time.tv_nsec) * 1e-9;
> + err = gettime(&now);
> + if (!err && subtest_time.tv_sec != 0 &&
> + subtest_time.tv_nsec != 0) {
A little paranoid? If we want the paranoia perhaps move it to gettime
and return an error?
-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 v2 17/29] drm/i915: Make the high dword offset more explicit in i915_reg_read_ioctl
On Fri, Nov 06, 2015 at 09:43:41PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Store the upper dword of the register offset in the whitelist as well. > This would allow it to read register where the two halves aren't sitting > right next to each other, and it'll make it easier to make register > access type safe. > > While at it change the register offsets to u32 from u64. Our register > space isn't quite that big, yet :) > > v2: Use ldw/udw as the suffixes, and add a note about > 64bit wide split regs (Chris) > > Cc: Chris Wilson > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -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
[Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay Fix power consumption not being displayed
The power metter was not showing up due to a check over I915_PERF_ENERGY.
ENOENT is returned when I915_PERF_ENERGY is not available, and we use
that for relaying on debugfs i915_energy_uJ.
Signed-off-by: Marius Vlad
---
overlay/power.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/overlay/power.c b/overlay/power.c
index 6c5c374..6873e7f 100644
--- a/overlay/power.c
+++ b/overlay/power.c
@@ -60,7 +60,7 @@ int power_init(struct power *power)
memset(power, 0, sizeof(*power));
power->fd = perf_open();
- if (power->fd != -1)
+ if (power->fd != -ENOENT)
return 0;
sprintf(buf, "%s/i915_energy_uJ", debugfs_dri_path);
@@ -121,7 +121,7 @@ int power_update(struct power *power)
if (power->error)
return power->error;
- if (power->fd != -1) {
+ if (power->fd != -ENOENT) {
uint64_t data[2];
int len;
--
2.6.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Consider SPLL as another shared pll, v2.
On 18 November 2015 at 13:31, Gabriel Feceoru wrote: > > > On 16.11.2015 17:53, Jani Nikula wrote: >> >> On Mon, 16 Nov 2015, Maarten Lankhorst >> wrote: >>> >>> When diagnosing a unrelated bug for someone on irc, it would seem the >>> hardware can >>> be brought up by the BIOS with the embedded displayport using the SPLL >>> for spread spectrum. >>> >>> Right now this is not handled well in i915, and it calculates the crtc >>> needs to >>> be reprogrammed on the first modeset without SSC, but the SPLL itself >>> was kept >>> active. Fix this by exposing SPLL as a shared pll that will not be >>> returned >>> by intel_get_shared_dpll; you have to know it exists to use it. >>> >>> Changes since v1: >>> - Create a separate dpll_hw_state.spll for spll, and use >>>separate pll functions for spll. >>> >>> Tested-by: Emil Renner Berthing #v1 >>> Reviewed-by: Daniel Vetter >>> Signed-off-by: Maarten Lankhorst >>> --- >>> Emil, can you retest? >> >> >> Gabriel, you too please! > > > I retested it and it's ok. It works fine for me too. I haven't run the tests yet, but I've run the patch applied to v4.3 the last couple of days. /Emil ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay Fix power consumption not being displayed
From: Marius Vlad
The power metter was not showing up due to a check over I915_PERF_ENERGY.
ENOENT is returned when I915_PERF_ENERGY is not available, and we use
that for relaying on debugfs i915_energy_uJ.
Signed-off-by: Marius Vlad
---
overlay/power.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/overlay/power.c b/overlay/power.c
index 6c5c374..6873e7f 100644
--- a/overlay/power.c
+++ b/overlay/power.c
@@ -60,7 +60,7 @@ int power_init(struct power *power)
memset(power, 0, sizeof(*power));
power->fd = perf_open();
- if (power->fd != -1)
+ if (power->fd != -ENOENT)
return 0;
sprintf(buf, "%s/i915_energy_uJ", debugfs_dri_path);
@@ -121,7 +121,7 @@ int power_update(struct power *power)
if (power->error)
return power->error;
- if (power->fd != -1) {
+ if (power->fd != -ENOENT) {
uint64_t data[2];
int len;
--
2.6.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay
From: Marius Vlad
Use sigaction() instead of signal() and add SIGINT, SIGTERM to close
the overlay window. With this change the overlay window will be
destroyed.
Signed-off-by: Marius Vlad
---
overlay/overlay.c | 42 --
1 file changed, 36 insertions(+), 6 deletions(-)
diff --git a/overlay/overlay.c b/overlay/overlay.c
index 3c0dbb4..48ba67c 100644
--- a/overlay/overlay.c
+++ b/overlay/overlay.c
@@ -804,11 +804,19 @@ static void show_gem_objects(struct overlay_context *ctx,
struct overlay_gem_obj
static int take_snapshot;
-static void signal_snapshot(int sig)
+static void
+signal_snapshot(int sig, siginfo_t *si, void *__unused)
{
take_snapshot = sig;
}
+static void
+signal_x11_destroy(int sig, siginfo_t *si, void *__unused)
+{
+ x11_overlay_stop();
+ exit(EXIT_SUCCESS);
+}
+
static int get_sample_period(struct config *config)
{
const char *value;
@@ -854,6 +862,7 @@ int main(int argc, char **argv)
};
struct overlay_context ctx;
struct config config;
+ struct sigaction sa;
int index, sample_period;
int daemonize = 1, renice = 0;
int i;
@@ -898,14 +907,22 @@ int main(int argc, char **argv)
ctx.width = 640;
ctx.height = 236;
ctx.surface = NULL;
- if (ctx.surface == NULL)
+
+ if (ctx.surface == NULL) {
ctx.surface = x11_overlay_create(&config, &ctx.width,
&ctx.height);
- if (ctx.surface == NULL)
+ }
+ if (ctx.surface == NULL) {
+ fprintf(stderr, "Failed to create X11 overlay.\n");
ctx.surface = x11_window_create(&config, &ctx.width,
&ctx.height);
- if (ctx.surface == NULL)
+ }
+ if (ctx.surface == NULL) {
+ fprintf(stderr, "Failed to create X11 window.\n");
ctx.surface = kms_overlay_create(&config, &ctx.width,
&ctx.height);
- if (ctx.surface == NULL)
+ }
+ if (ctx.surface == NULL) {
+ fprintf(stderr, "Failed to create KMS overlay.\n");
return ENXIO;
+ }
if (daemonize && daemon(0, 0))
return EINVAL;
@@ -913,7 +930,20 @@ int main(int argc, char **argv)
if (renice && (nice(renice) == -1))
fprintf(stderr, "Could not renice: %s\n", strerror(errno));
- signal(SIGUSR1, signal_snapshot);
+ sa.sa_flags = SA_SIGINFO;
+ sigemptyset(&sa.sa_mask);
+ sa.sa_sigaction = &signal_snapshot;
+
+ if (sigaction(SIGUSR1, &sa, NULL) == -1) {
+ x11_overlay_stop();
+ return EXIT_FAILURE;
+ }
+
+ sa.sa_sigaction = &signal_x11_destroy;
+ if (sigaction(SIGINT | SIGTERM, &sa, NULL) == -1) {
+ x11_overlay_stop();
+ return EXIT_FAILURE;
+ }
debugfs_init();
--
2.6.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay Fix power consumption not being displayed
From: Marius Vlad
The power metter was not showing up due to a check over I915_PERF_ENERGY.
ENOENT is returned when I915_PERF_ENERGY is not available, and we use
that for relaying on debugfs i915_energy_uJ.
Signed-off-by: Marius Vlad
---
overlay/power.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/overlay/power.c b/overlay/power.c
index 6c5c374..6873e7f 100644
--- a/overlay/power.c
+++ b/overlay/power.c
@@ -60,7 +60,7 @@ int power_init(struct power *power)
memset(power, 0, sizeof(*power));
power->fd = perf_open();
- if (power->fd != -1)
+ if (power->fd != -ENOENT)
return 0;
sprintf(buf, "%s/i915_energy_uJ", debugfs_dri_path);
@@ -121,7 +121,7 @@ int power_update(struct power *power)
if (power->error)
return power->error;
- if (power->fd != -1) {
+ if (power->fd != -ENOENT) {
uint64_t data[2];
int len;
--
2.6.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay Fix power consumption not being displayed
On Wed, Nov 18, 2015 at 02:36:22PM +0200, Marius Vlad wrote: > The power metter was not showing up due to a check over I915_PERF_ENERGY. > ENOENT is returned when I915_PERF_ENERGY is not available, and we use > that for relaying on debugfs i915_energy_uJ. > > Signed-off-by: Marius Vlad > --- > overlay/power.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/overlay/power.c b/overlay/power.c > index 6c5c374..6873e7f 100644 > --- a/overlay/power.c > +++ b/overlay/power.c > @@ -60,7 +60,7 @@ int power_init(struct power *power) > memset(power, 0, sizeof(*power)); > > power->fd = perf_open(); > - if (power->fd != -1) > + if (power->fd != -ENOENT) Nope. The bug is diff --git a/overlay/power.c b/overlay/power.c index 6c5c374..2f1521b 100644 --- a/overlay/power.c +++ b/overlay/power.c @@ -45,7 +45,7 @@ static int perf_open(void) attr.type = i915_type_id(); if (attr.type == 0) - return -ENOENT; + return -1; attr.config = I915_PERF_ENERGY; attr.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED; and the more pressing concern is that we still don't have support for the more accurate tracking from the kernel. -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: Fix uninitialized variables in intel_check_sprite_plane
agreed, this is better
On 11/18/2015 5:49 PM, Chris Wilson wrote:
On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
Uninitialized variables (width, Height) in intel_check_sprite_plane
leads to compilererror in O1 level. Initialize all declared variables
to fix this issue.
Signed-off-by: Nabendu Maiti
Or perhaps:
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 2b96f336589e..8d7b4eb5b5b9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
struct drm_framebuffer *fb = state->base.fb;
int crtc_x, crtc_y;
unsigned int crtc_w, crtc_h;
- uint32_t src_x, src_y, src_w, src_h;
struct drm_rect *src = &state->src;
struct drm_rect *dst = &state->dst;
const struct drm_rect *clip = &state->clip;
@@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
crtc_h = drm_rect_height(dst);
if (state->visible) {
+ u32 src_x, src_y, src_w, src_h;
+
/* check again in case clipping clamped the results */
hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
if (hscale < 0) {
@@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (crtc_w == 0)
state->visible = false;
}
- }
/* Check size restrictions when scaling */
- if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
+ if (src_w != crtc_w || src_h != crtc_h) {
unsigned int width_bytes;
WARN_ON(!can_scale);
@@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
}
}
- if (state->visible) {
src->x1 = src_x << 16;
src->x2 = (src_x + src_w) << 16;
src->y1 = src_y << 16;
And make both the compiler and reader happier
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay Fix power consumption not being displayed
Just trying to figure out what we have currently. I can redo with with -1, if that's OK with you. Out of curiosity noticed that I915_PERF_ENERGY is not available on my machine, is that related to i915_oa? (Haswell). -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Wednesday, November 18, 2015 2:45 PM To: Vlad, Marius C Cc: marius.vl...@gmail.com; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay Fix power consumption not being displayed On Wed, Nov 18, 2015 at 02:36:22PM +0200, Marius Vlad wrote: > The power metter was not showing up due to a check over I915_PERF_ENERGY. > ENOENT is returned when I915_PERF_ENERGY is not available, and we use > that for relaying on debugfs i915_energy_uJ. > > Signed-off-by: Marius Vlad > --- > overlay/power.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/overlay/power.c b/overlay/power.c index 6c5c374..6873e7f > 100644 > --- a/overlay/power.c > +++ b/overlay/power.c > @@ -60,7 +60,7 @@ int power_init(struct power *power) > memset(power, 0, sizeof(*power)); > > power->fd = perf_open(); > - if (power->fd != -1) > + if (power->fd != -ENOENT) Nope. The bug is diff --git a/overlay/power.c b/overlay/power.c index 6c5c374..2f1521b 100644 --- a/overlay/power.c +++ b/overlay/power.c @@ -45,7 +45,7 @@ static int perf_open(void) attr.type = i915_type_id(); if (attr.type == 0) - return -ENOENT; + return -1; attr.config = I915_PERF_ENERGY; attr.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED; and the more pressing concern is that we still don't have support for the more accurate tracking from the kernel. -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: Fix uninitialized variables in intel_check_sprite_plane
On Wed, Nov 18, 2015 at 12:19:06PM +, Chris Wilson wrote:
> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
> > Uninitialized variables (width, Height) in intel_check_sprite_plane
> > leads to compilererror in O1 level. Initialize all declared variables
> > to fix this issue.
> >
> > Signed-off-by: Nabendu Maiti
>
> Or perhaps:
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 2b96f336589e..8d7b4eb5b5b9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> struct drm_framebuffer *fb = state->base.fb;
> int crtc_x, crtc_y;
> unsigned int crtc_w, crtc_h;
> - uint32_t src_x, src_y, src_w, src_h;
> struct drm_rect *src = &state->src;
> struct drm_rect *dst = &state->dst;
> const struct drm_rect *clip = &state->clip;
> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
> crtc_h = drm_rect_height(dst);
>
> if (state->visible) {
> + u32 src_x, src_y, src_w, src_h;
> +
> /* check again in case clipping clamped the results */
> hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> if (hscale < 0) {
> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
> if (crtc_w == 0)
> state->visible = false;
> }
> - }
>
> /* Check size restrictions when scaling */
> - if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> + if (src_w != crtc_w || src_h != crtc_h) {
That would change what it does.
> unsigned int width_bytes;
>
> WARN_ON(!can_scale);
> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> }
> }
>
> - if (state->visible) {
> src->x1 = src_x << 16;
> src->x2 = (src_x + src_w) << 16;
> src->y1 = src_y << 16;
>
> And make both the compiler and reader happier
> -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
--
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 i-g-t 2/3] tests: add context param to pm_backlight tests
We'll be adding more context for the subtests than just the max
brightness.
Signed-off-by: Jani Nikula
---
tests/pm_backlight.c | 39 ++-
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
index bdc0e33de94c..c199b9bf4baf 100644
--- a/tests/pm_backlight.c
+++ b/tests/pm_backlight.c
@@ -35,6 +35,10 @@
#include
#include
+struct context {
+ int max;
+};
+
#define TOLERANCE 5 /* percent */
#define BACKLIGHT_PATH "/sys/class/backlight/intel_backlight"
@@ -107,41 +111,41 @@ static void test_and_verify(int val)
igt_assert_lte(val - tolerance, result);
}
-static void test_brightness(int max)
+static void test_brightness(struct context *context)
{
test_and_verify(0);
- test_and_verify(max);
- test_and_verify(max / 2);
+ test_and_verify(context->max);
+ test_and_verify(context->max / 2);
}
-static void test_bad_brightness(int max)
+static void test_bad_brightness(struct context *context)
{
int val;
/* First write some sane value */
- backlight_write(max / 2, "brightness");
+ backlight_write(context->max / 2, "brightness");
/* Writing invalid values should fail and not change the value */
igt_assert_lt(backlight_write(-1, "brightness"), 0);
backlight_read(&val, "brightness");
- igt_assert_eq(val, max / 2);
- igt_assert_lt(backlight_write(max + 1, "brightness"), 0);
+ igt_assert_eq(val, context->max / 2);
+ igt_assert_lt(backlight_write(context->max + 1, "brightness"), 0);
backlight_read(&val, "brightness");
- igt_assert_eq(val, max / 2);
+ igt_assert_eq(val, context->max / 2);
igt_assert_lt(backlight_write(INT_MAX, "brightness"), 0);
backlight_read(&val, "brightness");
- igt_assert_eq(val, max / 2);
+ igt_assert_eq(val, context->max / 2);
}
-static void test_fade(int max)
+static void test_fade(struct context *context)
{
int i;
static const struct timespec ts = { .tv_sec = 0, .tv_nsec =
FADESPEED*100 };
/* Fade out, then in */
- for (i = max; i > 0; i -= max / FADESTEPS) {
+ for (i = context->max; i > 0; i -= context->max / FADESTEPS) {
test_and_verify(i);
nanosleep(&ts, NULL);
}
- for (i = 0; i <= max; i += max / FADESTEPS) {
+ for (i = 0; i <= context->max; i += context->max / FADESTEPS) {
test_and_verify(i);
nanosleep(&ts, NULL);
}
@@ -149,22 +153,23 @@ static void test_fade(int max)
igt_main
{
- int max, old;
+ struct context context = {0};
+ int old;
igt_skip_on_simulation();
igt_fixture {
/* Get the max value and skip the whole test if sysfs interface
not available */
igt_skip_on(backlight_read(&old, "brightness"));
- igt_assert(backlight_read(&max, "max_brightness") > -1);
+ igt_assert(backlight_read(&context.max, "max_brightness") > -1);
}
igt_subtest("basic-brightness")
- test_brightness(max);
+ test_brightness(&context);
igt_subtest("bad-brightness")
- test_bad_brightness(max);
+ test_bad_brightness(&context);
igt_subtest("fade")
- test_fade(max);
+ test_fade(&context);
igt_fixture {
/* Restore old brightness */
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/3] tests: use igt_assert_lte to verify pm_backlight test results
Gives out better diagnostics than just igt_asssert.
Signed-off-by: Jani Nikula
---
tests/pm_backlight.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
index d24fb1034198..bdc0e33de94c 100644
--- a/tests/pm_backlight.c
+++ b/tests/pm_backlight.c
@@ -94,6 +94,7 @@ static int backlight_write(int value, const char *fname)
static void test_and_verify(int val)
{
int result;
+ int tolerance = val * TOLERANCE / 100;
igt_assert_eq(backlight_write(val, "brightness"), 0);
igt_assert_eq(backlight_read(&result, "brightness"), 0);
@@ -102,7 +103,8 @@ static void test_and_verify(int val)
igt_assert_eq(backlight_read(&result, "actual_brightness"), 0);
/* Some rounding may happen depending on hw. Just check that it's close
enough. */
- igt_assert(result <= val + val * TOLERANCE / 100 && result >= val - val
* TOLERANCE / 100);
+ igt_assert_lte(result, val + tolerance);
+ igt_assert_lte(val - tolerance, result);
}
static void test_brightness(int max)
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/3] tests: limit pm_backlight actual brightness tolerance between 0 and max
Signed-off-by: Jani Nikula
---
tests/pm_backlight.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
index c199b9bf4baf..8258d4e4c124 100644
--- a/tests/pm_backlight.c
+++ b/tests/pm_backlight.c
@@ -95,7 +95,7 @@ static int backlight_write(int value, const char *fname)
return 0;
}
-static void test_and_verify(int val)
+static void test_and_verify(struct context *context, int val)
{
int result;
int tolerance = val * TOLERANCE / 100;
@@ -107,15 +107,15 @@ static void test_and_verify(int val)
igt_assert_eq(backlight_read(&result, "actual_brightness"), 0);
/* Some rounding may happen depending on hw. Just check that it's close
enough. */
- igt_assert_lte(result, val + tolerance);
- igt_assert_lte(val - tolerance, result);
+ igt_assert_lte(result, min(context->max, val + tolerance));
+ igt_assert_lte(max(0, val - tolerance), result);
}
static void test_brightness(struct context *context)
{
- test_and_verify(0);
- test_and_verify(context->max);
- test_and_verify(context->max / 2);
+ test_and_verify(context, 0);
+ test_and_verify(context, context->max);
+ test_and_verify(context, context->max / 2);
}
static void test_bad_brightness(struct context *context)
@@ -142,11 +142,11 @@ static void test_fade(struct context *context)
/* Fade out, then in */
for (i = context->max; i > 0; i -= context->max / FADESTEPS) {
- test_and_verify(i);
+ test_and_verify(context, i);
nanosleep(&ts, NULL);
}
for (i = 0; i <= context->max; i += context->max / FADESTEPS) {
- test_and_verify(i);
+ test_and_verify(context, i);
nanosleep(&ts, NULL);
}
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v3] lib/igt_core: Prefer CLOCK_MONOTONIC_RAW
On ke, 2015-11-18 at 12:28 +, Chris Wilson wrote:
> On Wed, Nov 18, 2015 at 02:18:52PM +0200, Joonas Lahtinen wrote:
> > CLOCK_MONOTONIC_RAW is not affected by NTP, so it should be THE
> > clock
> > used for timing execution of tests.
> >
> > When fetching either the starting or ending time of a test, show
> > the
> > time as -1.000s.
> >
> > v3:
> > - Do not exit directly from handler (Chris)
> > - Show elapsed time as -1 if it is not calculable
>
> Aye, that's better for the subtest handling.
>
> > @@ -832,10 +851,16 @@ static void exit_subtest(const char *result)
> > {
> > struct timespec now;
> > double elapsed;
> > + int err;
> >
> > - gettime(&now);
> > - elapsed = now.tv_sec - subtest_time.tv_sec;
> > - elapsed += (now.tv_nsec - subtest_time.tv_nsec) * 1e-9;
> > + err = gettime(&now);
> > + if (!err && subtest_time.tv_sec != 0 &&
> > + subtest_time.tv_nsec != 0) {
>
> A little paranoid? If we want the paranoia perhaps move it to gettime
> and return an error?
>
I'm just checking that the starting time which is in the subtest_time
did not fail (which leads to both being zero). I'm not looking at the
now values :)
Regards, Joonas
> -Chris
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay Fix power consumption not being displayed
On Wed, Nov 18, 2015 at 12:50:10PM +, Vlad, Marius C wrote: > Just trying to figure out what we have currently. I can redo with with -1, if > that's > OK with you. Already pushed. > Out of curiosity noticed that I915_PERF_ENERGY is not available on my > machine, > is that related to i915_oa? (Haswell). Kind off (except that i915_oa itself doesn't try to implement the generic load/client tracking). -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 v1 1/2] drm/i915:skl: Add WaEnableGapsTsvCreditFix
Hi,
Just applying this patch is enough to make gem_ringfill --r render work
on my SKL 2+2 DT GT2 system I have at Espoo. I do not need the second
patch which seems to be required for Arun's SKL.
Also working with just that patch on one SKL Y system here at Egham
office. It's some months older than Arun's SKL U.
Regards, Joonas
On ke, 2015-08-05 at 11:17 +0200, Daniel Vetter wrote:
> On Tue, Aug 04, 2015 at 10:01:43AM +0100, Siluvery, Arun wrote:
> > On 04/08/2015 09:58, Mika Kuoppala wrote:
> > > Ben Widawsky writes:
> > >
> > > > On Mon, Aug 03, 2015 at 08:24:56PM +0100, Arun Siluvery wrote:
> > > > > Cc: Ben Widawsky
> > > > > Cc: Joonas Lahtinen
> > > > > Signed-off-by: Arun Siluvery
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > > > > drivers/gpu/drm/i915/intel_pm.c | 6 ++
> > > > > 2 files changed, 9 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > index 77967ca..8991cd5 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > @@ -6849,6 +6849,9 @@ enum skl_disp_power_wells {
> > > > > #define GEN7_MISCCPCTL (0x9424)
> > > > > #define GEN7_DOP_CLOCK_GATE_ENABLE (1<<0)
> > > > >
> > > > > +#define GEN8_GARBCNTL 0xB004
> > > > > +#define GEN9_GAPS_TSV_CREDIT_DISABLE (1<<7)
> > > > > +
> > > > > /* IVYBRIDGE DPF */
> > > > > #define GEN7_L3CDERRST1 0xB008 /*
> > > > > L3CD Error Status 1 */
> > > > > #define HSW_L3CDERRST11 0xB208 /*
> > > > > L3CD Error Status register 1 slice 1 */
> > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > > > b/drivers/gpu/drm/i915/intel_pm.c
> > > > > index c23cab6..9152113 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > > @@ -106,6 +106,12 @@ static void skl_init_clock_gating(struct
> > > > > drm_device *dev)
> > > > > /* WaDisableLSQCROPERFforOCL:skl */
> > > > > I915_WRITE(GEN8_L3SQCREG4,
> > > > > I915_READ(GEN8_L3SQCREG4) |
> > > > > GEN8_LQSC_RO_PERF_DIS);
> > > > > +
> > > > > + /* WaEnableGapsTsvCreditFix:skl */
> > > > > + if (IS_SKYLAKE(dev) && (INTEL_REVID(dev) >=
> > > > > SKL_REVID_C0)) {
> > > > > + I915_WRITE(GEN8_GARBCNTL,
> > > > > (I915_READ(GEN8_GARBCNTL) |
> > > > > +
> > > > > GEN9_GAPS_TSV_CREDIT_DISABLE));
> > > > > + }
> > > > > }
> > > > >
> > > > > static void bxt_init_clock_gating(struct drm_device *dev)
> > > >
> > > > FWIW, the docs make it sound like BIOS should be doing this.
> > > > Did you verify we
> > > > actually don't have the bit set with more recent BKC?
> > > >
> > >
> > > I have pretty recent BIOS and the bit was not set.
> >
> > I checked about this, it should be done in driver.
> >
> > regards
> > Arun
> >
> > >
> > > > Tested-by: Ben Widawsky
> > > > Reviewed-by: Ben Widawsky
> > > >
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90854
> > > Tested-by: Mika Kuoppala
>
> Queued for -next, thanks for the patch.
> -Daniel
>
> > >
> > > > --
> > > > Ben Widawsky, Intel Open Source Technology Center
> > > > ___
> > > > 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
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay
On Wed, Nov 18, 2015 at 02:40:29PM +0200, marius.c.v...@intel.com wrote:
> From: Marius Vlad
>
> Use sigaction() instead of signal() and add SIGINT, SIGTERM to close
> the overlay window. With this change the overlay window will be
> destroyed.
>
> Signed-off-by: Marius Vlad
> ---
> overlay/overlay.c | 42 --
> 1 file changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/overlay/overlay.c b/overlay/overlay.c
> index 3c0dbb4..48ba67c 100644
> --- a/overlay/overlay.c
> +++ b/overlay/overlay.c
> @@ -804,11 +804,19 @@ static void show_gem_objects(struct overlay_context
> *ctx, struct overlay_gem_obj
>
> static int take_snapshot;
>
> -static void signal_snapshot(int sig)
> +static void
> +signal_snapshot(int sig, siginfo_t *si, void *__unused)
> {
> take_snapshot = sig;
> }
>
> +static void
> +signal_x11_destroy(int sig, siginfo_t *si, void *__unused)
> +{
> + x11_overlay_stop();
is not signalsafe.
> + if (ctx.surface == NULL) {
> + fprintf(stderr, "Failed to create X11 overlay.\n");
Spurious changes.
> ctx.surface = x11_window_create(&config, &ctx.width,
> &ctx.height);
> - if (ctx.surface == NULL)
> + }
> + if (ctx.surface == NULL) {
> + fprintf(stderr, "Failed to create X11 window.\n");
> ctx.surface = kms_overlay_create(&config, &ctx.width,
> &ctx.height);
> - if (ctx.surface == NULL)
> + }
> + if (ctx.surface == NULL) {
> + fprintf(stderr, "Failed to create KMS overlay.\n");
> return ENXIO;
> + }
>
> if (daemonize && daemon(0, 0))
> return EINVAL;
> @@ -913,7 +930,20 @@ int main(int argc, char **argv)
> if (renice && (nice(renice) == -1))
> fprintf(stderr, "Could not renice: %s\n", strerror(errno));
>
> - signal(SIGUSR1, signal_snapshot);
> + sa.sa_flags = SA_SIGINFO;
> + sigemptyset(&sa.sa_mask);
> + sa.sa_sigaction = &signal_snapshot;
> +
> + if (sigaction(SIGUSR1, &sa, NULL) == -1) {
Any particular reason for a fondness here for sigaction?
-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 i-g-t v3] lib/igt_core: Prefer CLOCK_MONOTONIC_RAW
On Wed, Nov 18, 2015 at 02:54:20PM +0200, Joonas Lahtinen wrote:
> On ke, 2015-11-18 at 12:28 +, Chris Wilson wrote:
> > On Wed, Nov 18, 2015 at 02:18:52PM +0200, Joonas Lahtinen wrote:
> > > CLOCK_MONOTONIC_RAW is not affected by NTP, so it should be THE
> > > clock
> > > used for timing execution of tests.
> > >
> > > When fetching either the starting or ending time of a test, show
> > > the
> > > time as -1.000s.
> > >
> > > v3:
> > > - Do not exit directly from handler (Chris)
> > > - Show elapsed time as -1 if it is not calculable
> >
> > Aye, that's better for the subtest handling.
> >
> > > @@ -832,10 +851,16 @@ static void exit_subtest(const char *result)
> > > {
> > > struct timespec now;
> > > double elapsed;
> > > + int err;
> > >
> > > - gettime(&now);
> > > - elapsed = now.tv_sec - subtest_time.tv_sec;
> > > - elapsed += (now.tv_nsec - subtest_time.tv_nsec) * 1e-9;
> > > + err = gettime(&now);
> > > + if (!err && subtest_time.tv_sec != 0 &&
> > > + subtest_time.tv_nsec != 0) {
> >
> > A little paranoid? If we want the paranoia perhaps move it to gettime
> > and return an error?
> >
>
> I'm just checking that the starting time which is in the subtest_time
> did not fail (which leads to both being zero). I'm not looking at the
> now values :)
Ok, now I see.
#define time_valid(ts) ((ts)->tv_sec || (ts)->tv_nsec)
gettime(&now);
if (time_valid(&now) && time_valid(&subtest_time)) {
elapsed = ...;
} else {
elapsed = -1;
}
Perhaps?
-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: Add BXT Sprite plane fractional scalings
On 11/18/2015 5:41 PM, Ville Syrjälä wrote:
On Wed, Nov 18, 2015 at 05:13:01PM +0530, Nabendu Maiti wrote:
On older platforms scalers/cliping used to provide destination size an
exact multiple of src size.
Gen-9 and above support fractional ratio of dst/src so that source is
not manipulated to meet the exact multiple factor.
Signed-off-by: Nabendu Maiti
---
drivers/gpu/drm/i915/intel_sprite.c | 48 +
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 2b96f33..e8c17ae 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -813,29 +813,37 @@ intel_check_sprite_plane(struct drm_plane *plane,
crtc_h = drm_rect_height(dst);
if (state->visible) {
- /* check again in case clipping clamped the results */
- hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
- if (hscale < 0) {
- DRM_DEBUG_KMS("Horizontal scaling factor out of
limits\n");
- drm_rect_debug_print("src: ", src, true);
- drm_rect_debug_print("dst: ", dst, false);
-
- return hscale;
- }
- vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
- if (vscale < 0) {
- DRM_DEBUG_KMS("Vertical scaling factor out of
limits\n");
- drm_rect_debug_print("src: ", src, true);
- drm_rect_debug_print("dst: ", dst, false);
+ /* Gen 9 and above has fractional scaling support */
+ if (INTEL_INFO(plane->dev)->gen < 9) {
- return vscale;
- }
+ /* check again in case clipping clamped the results */
+ hscale = drm_rect_calc_hscale(src, dst, min_scale,
max_scale);
+ if (hscale < 0) {
+ DRM_DEBUG_KMS("Horizontal scaling factor out of
limits\n");
+ drm_rect_debug_print("src: ", src, true);
+ drm_rect_debug_print("dst: ", dst, false);
+
+ return hscale;
+ }
+
+ vscale = drm_rect_calc_vscale(src, dst, min_scale,
max_scale);
+ if (vscale < 0) {
+ DRM_DEBUG_KMS("Vertical scaling factor out of
limits\n");
+ drm_rect_debug_print("src: ", src, true);
+ drm_rect_debug_print("dst: ", dst, false);
- /* Make the source viewport size an exact multiple of the scaling factors. */
- drm_rect_adjust_size(src,
-drm_rect_width(dst) * hscale -
drm_rect_width(src),
-drm_rect_height(dst) * vscale -
drm_rect_height(src));
+ return vscale;
+ }
+
+ /*
+* Make the source viewport size an exact
+* multiple of the scaling factors.
+*/
+ drm_rect_adjust_size(src,
+drm_rect_width(dst) * hscale -
drm_rect_width(src),
+drm_rect_height(dst) * vscale -
drm_rect_height(src));
+ }
NAK. This code just makes sure the actual scaling ratio matches what we
calculated. As in there may have been some amount of rounding involved
etc.
The part you actually want to change is what comes after this where we
throw away the fractional part of the coordinates. And then you need to
change the actual hw programming so that we actually feed the sub-pixel
coordinates to the hardware.
In line
drm_rect_adjust_size(src,
+drm_rect_width(dst) * hscale -
drm_rect_width(src),
+drm_rect_height(dst) * vscale -
drm_rect_height(src));
I think we throw away the fractional part, which modifies the src,rectangle. In
commit we use the modified state->src size.
drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
state->base.rotation);
--
1.9.1
___
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] drm/i915: Consider SPLL as another shared pll, v2.
On Wed, 18 Nov 2015, Emil Renner Berthing wrote: > On 18 November 2015 at 13:31, Gabriel Feceoru > wrote: >> >> >> On 16.11.2015 17:53, Jani Nikula wrote: >>> >>> On Mon, 16 Nov 2015, Maarten Lankhorst >>> wrote: When diagnosing a unrelated bug for someone on irc, it would seem the hardware can be brought up by the BIOS with the embedded displayport using the SPLL for spread spectrum. Right now this is not handled well in i915, and it calculates the crtc needs to be reprogrammed on the first modeset without SSC, but the SPLL itself was kept active. Fix this by exposing SPLL as a shared pll that will not be returned by intel_get_shared_dpll; you have to know it exists to use it. Changes since v1: - Create a separate dpll_hw_state.spll for spll, and use separate pll functions for spll. Tested-by: Emil Renner Berthing #v1 Reviewed-by: Daniel Vetter Signed-off-by: Maarten Lankhorst --- Emil, can you retest? >>> >>> >>> Gabriel, you too please! >> >> >> I retested it and it's ok. > > It works fine for me too. I haven't run the tests yet, but I've run > the patch applied to v4.3 the last couple of days. Pushed to drm-intel-fixes, thanks for the patch, review and testing. 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] drm/i915/bxt: Fix uninitialized variables in intel_check_sprite_plane
On 11/18/2015 6:22 PM, Ville Syrjälä wrote:
On Wed, Nov 18, 2015 at 12:19:06PM +, Chris Wilson wrote:
On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
Uninitialized variables (width, Height) in intel_check_sprite_plane
leads to compilererror in O1 level. Initialize all declared variables
to fix this issue.
Signed-off-by: Nabendu Maiti
Or perhaps:
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 2b96f336589e..8d7b4eb5b5b9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
struct drm_framebuffer *fb = state->base.fb;
int crtc_x, crtc_y;
unsigned int crtc_w, crtc_h;
- uint32_t src_x, src_y, src_w, src_h;
struct drm_rect *src = &state->src;
struct drm_rect *dst = &state->dst;
const struct drm_rect *clip = &state->clip;
@@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
crtc_h = drm_rect_height(dst);
if (state->visible) {
+ u32 src_x, src_y, src_w, src_h;
+
/* check again in case clipping clamped the results */
hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
if (hscale < 0) {
@@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (crtc_w == 0)
state->visible = false;
}
- }
/* Check size restrictions when scaling */
- if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
+ if (src_w != crtc_w || src_h != crtc_h) {
That would change what it does.
yes, checked the code where inside each if condition loop we may be
changing the state->visible var itself. Next condition check it may be
false too.
The place giving compiler error
src->x1 = src_x << 16;
src->x2 = (src_x + src_w) << 16;
src->y1 = src_y << 16;
src->y2 = (src_y + src_h) << 16;
Then just one line change of initializing the variables is better?
unsigned int width_bytes;
WARN_ON(!can_scale);
@@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
}
}
- if (state->visible) {
src->x1 = src_x << 16;
src->x2 = (src_x + src_w) << 16;
src->y1 = src_y << 16;
And make both the compiler and reader happier
-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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add BXT Sprite plane fractional scalings
On Wed, Nov 18, 2015 at 06:37:17PM +0530, Maiti, Nabendu Bikash wrote:
>
>
> On 11/18/2015 5:41 PM, Ville Syrjälä wrote:
> > On Wed, Nov 18, 2015 at 05:13:01PM +0530, Nabendu Maiti wrote:
> >> On older platforms scalers/cliping used to provide destination size an
> >> exact multiple of src size.
> >> Gen-9 and above support fractional ratio of dst/src so that source is
> >> not manipulated to meet the exact multiple factor.
> >>
> >> Signed-off-by: Nabendu Maiti
> >> ---
> >> drivers/gpu/drm/i915/intel_sprite.c | 48
> >> +
> >> 1 file changed, 28 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >> b/drivers/gpu/drm/i915/intel_sprite.c
> >> index 2b96f33..e8c17ae 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -813,29 +813,37 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >>crtc_h = drm_rect_height(dst);
> >>
> >>if (state->visible) {
> >> - /* check again in case clipping clamped the results */
> >> - hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >> - if (hscale < 0) {
> >> - DRM_DEBUG_KMS("Horizontal scaling factor out of
> >> limits\n");
> >> - drm_rect_debug_print("src: ", src, true);
> >> - drm_rect_debug_print("dst: ", dst, false);
> >> -
> >> - return hscale;
> >> - }
> >>
> >> - vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> >> - if (vscale < 0) {
> >> - DRM_DEBUG_KMS("Vertical scaling factor out of
> >> limits\n");
> >> - drm_rect_debug_print("src: ", src, true);
> >> - drm_rect_debug_print("dst: ", dst, false);
> >> + /* Gen 9 and above has fractional scaling support */
> >> + if (INTEL_INFO(plane->dev)->gen < 9) {
> >>
> >> - return vscale;
> >> - }
> >> + /* check again in case clipping clamped the results */
> >> + hscale = drm_rect_calc_hscale(src, dst, min_scale,
> >> max_scale);
> >> + if (hscale < 0) {
> >> + DRM_DEBUG_KMS("Horizontal scaling factor out of
> >> limits\n");
> >> + drm_rect_debug_print("src: ", src, true);
> >> + drm_rect_debug_print("dst: ", dst, false);
> >> +
> >> + return hscale;
> >> + }
> >> +
> >> + vscale = drm_rect_calc_vscale(src, dst, min_scale,
> >> max_scale);
> >> + if (vscale < 0) {
> >> + DRM_DEBUG_KMS("Vertical scaling factor out of
> >> limits\n");
> >> + drm_rect_debug_print("src: ", src, true);
> >> + drm_rect_debug_print("dst: ", dst, false);
> >>
> >> - /* Make the source viewport size an exact multiple of the
> >> scaling factors. */
> >> - drm_rect_adjust_size(src,
> >> - drm_rect_width(dst) * hscale -
> >> drm_rect_width(src),
> >> - drm_rect_height(dst) * vscale -
> >> drm_rect_height(src));
> >> + return vscale;
> >> + }
> >> +
> >> + /*
> >> + * Make the source viewport size an exact
> >> + * multiple of the scaling factors.
> >> + */
> >> + drm_rect_adjust_size(src,
> >> + drm_rect_width(dst) * hscale -
> >> drm_rect_width(src),
> >> + drm_rect_height(dst) * vscale -
> >> drm_rect_height(src));
> >> + }
> > NAK. This code just makes sure the actual scaling ratio matches what we
> > calculated. As in there may have been some amount of rounding involved
> > etc.
> >
> > The part you actually want to change is what comes after this where we
> > throw away the fractional part of the coordinates. And then you need to
> > change the actual hw programming so that we actually feed the sub-pixel
> > coordinates to the hardware.
>
> In line
>
> drm_rect_adjust_size(src,
> + drm_rect_width(dst) * hscale -
> drm_rect_width(src),
> + drm_rect_height(dst) * vscale -
> drm_rect_height(src));
>
> I think we throw away the fractional part,
No, we don't.
> which modifies the src,rectangle. In commit we use the modified state->src
> size.
>
> >>
> >>drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16,
> >>state->base.rotation);
> >> --
> >> 1.9.1
> >>
> >> ___
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Re: [Intel-gfx] [PATCH v2] PCI / PM: tune down RPM suspend error message with EBUSY and EAGAIN retval
On ke, 2015-11-18 at 12:56 +0200, Imre Deak wrote:
> The runtime PM core doesn't treat EBUSY and EAGAIN retvals from the driver
> suspend hooks as errors, but they still show up as errors in dmesg. Tune
> them down.
>
> One problem caused by this was noticed by Daniel: the i915 driver
> returns EAGAIN to signal a temporary failure to suspend and as a request
> towards the RPM core for scheduling a suspend again. This is a normal
> event, but the resulting error message flags a breakage during the
> driver's automated testing which parses dmesg and picks up the error.
>
> v2:
> - fix compile breake when CONFIG_PM_SLEEP=n (0-day builder)
>
> Reported-by: Daniel Vetter
> Signed-off-by: Imre Deak
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92992
> ---
> drivers/base/power/main.c | 7 +--
> drivers/pci/pci-driver.c | 2 +-
> include/linux/pm.h| 11 +--
> 3 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 1710c26..39d2090 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1679,9 +1679,12 @@ int dpm_suspend_start(pm_message_t state)
> }
> EXPORT_SYMBOL_GPL(dpm_suspend_start);
>
> -void __suspend_report_result(const char *function, void *fn, int ret)
> +void __suspend_report_result(const char *function, void *fn, int ret,
> + bool runtime_pm)
> {
> - if (ret)
> + if (runtime_pm && (ret == -EBUSY || ret == -EAGAIN))
> + printk(KERN_DEBUG "%s(): %pF returns %d\n", function, fn, ret);
> + else if (ret)
> printk(KERN_ERR "%s(): %pF returns %d\n", function, fn, ret);
> }
> EXPORT_SYMBOL_GPL(__suspend_report_result);
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 108a311..9569572 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1142,7 +1142,7 @@ static int pci_pm_runtime_suspend(struct device *dev)
> pci_dev->state_saved = false;
> pci_dev->no_d3cold = false;
> error = pm->runtime_suspend(dev);
> - suspend_report_result(pm->runtime_suspend, error);
> + rpm_suspend_report_result(pm->runtime_suspend, error);
> if (error)
> return error;
> if (!pci_dev->d3cold_allowed)
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 35d599e..54f37e3 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -702,11 +702,17 @@ extern int dpm_suspend_late(pm_message_t state);
> extern int dpm_suspend(pm_message_t state);
> extern int dpm_prepare(pm_message_t state);
>
> -extern void __suspend_report_result(const char *function, void *fn, int ret);
> +extern void __suspend_report_result(const char *function, void *fn, int ret,
> + bool runtime_pm);
>
> #define suspend_report_result(fn, ret)
> \
> do {\
> - __suspend_report_result(__func__, fn, ret); \
> + __suspend_report_result(__func__, fn, ret, false); \
> + } while (0)
> +
> +#define rpm_suspend_report_result(fn, ret) \
> + do {\
> + __suspend_report_result(__func__, fn, ret, true); \
> } while (0)
>
> extern int device_pm_wait_for_dev(struct device *sub, struct device *dev);
> @@ -744,6 +750,7 @@ static inline int dpm_suspend_start(pm_message_t state)
> }
>
> #define suspend_report_result(fn, ret) do {} while (0)
> +#define rpm_suspend_report_result(fn, ret) do {} while (0)
>
> static inline int device_pm_wait_for_dev(struct device *a, struct device *b)
> {
___
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 uninitialized variables in intel_check_sprite_plane
On Wed, Nov 18, 2015 at 06:48:37PM +0530, Maiti, Nabendu Bikash wrote:
>
>
>
> On 11/18/2015 6:22 PM, Ville Syrjälä wrote:
> > On Wed, Nov 18, 2015 at 12:19:06PM +, Chris Wilson wrote:
> >> On Wed, Nov 18, 2015 at 05:43:52PM +0530, Nabendu Maiti wrote:
> >>> Uninitialized variables (width, Height) in intel_check_sprite_plane
> >>> leads to compilererror in O1 level. Initialize all declared variables
> >>> to fix this issue.
> >>>
> >>> Signed-off-by: Nabendu Maiti
> >> Or perhaps:
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >> b/drivers/gpu/drm/i915/intel_sprite.c
> >> index 2b96f336589e..8d7b4eb5b5b9 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -747,7 +747,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >> struct drm_framebuffer *fb = state->base.fb;
> >> int crtc_x, crtc_y;
> >> unsigned int crtc_w, crtc_h;
> >> - uint32_t src_x, src_y, src_w, src_h;
> >> struct drm_rect *src = &state->src;
> >> struct drm_rect *dst = &state->dst;
> >> const struct drm_rect *clip = &state->clip;
> >> @@ -813,6 +812,8 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >> crtc_h = drm_rect_height(dst);
> >>
> >> if (state->visible) {
> >> + u32 src_x, src_y, src_w, src_h;
> >> +
> >> /* check again in case clipping clamped the results */
> >> hscale = drm_rect_calc_hscale(src, dst, min_scale,
> >> max_scale);
> >> if (hscale < 0) {
> >> @@ -871,10 +872,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >> if (crtc_w == 0)
> >> state->visible = false;
> >> }
> >> - }
> >>
> >> /* Check size restrictions when scaling */
> >> - if (state->visible && (src_w != crtc_w || src_h != crtc_h)) {
> >> + if (src_w != crtc_w || src_h != crtc_h) {
> > That would change what it does.
> yes, checked the code where inside each if condition loop we may be
> changing the state->visible var itself. Next condition check it may be
> false too.
>
> The place giving compiler error
> src->x1 = src_x << 16;
> src->x2 = (src_x + src_w) << 16;
> src->y1 = src_y << 16;
> src->y2 = (src_y + src_h) << 16;
>
> Then just one line change of initializing the variables is better?
Or maybe fix your compiler instead? I don't get any warning/errors from
this. What version of gcc are you using?
>
> >> unsigned int width_bytes;
> >>
> >> WARN_ON(!can_scale);
> >> @@ -898,7 +898,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
> >> }
> >> }
> >>
> >> - if (state->visible) {
> >> src->x1 = src_x << 16;
> >> src->x2 = (src_x + src_w) << 16;
> >> src->y1 = src_y << 16;
> >>
> >> And make both the compiler and reader happier
> >> -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
--
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] i915 Skylake: "Invalid ROM contents"
On Wed, Nov 18, 2015 at 5:59 AM, Ville Syrjälä wrote: > On Tue, Nov 17, 2015 at 11:43:25AM -0800, Andy Lutomirski wrote: >> Typing: >> >> # cat /sys/devices/pci:00/:00:02.0/rom >> >> Provokes: >> >> i915 :00:02.0: Invalid ROM contents > > Hmm. So there's no PCI option ROM there. I wonder what is there. I > get the same on my Braswell BTW. I tried to look through the UEFI > spec a bit, and it seems to say that even for non-legacy option ROMs > the 0x55aa signature should be there. > > But this being the GPU means we may be using the shadow ROM stuff, > which IIRC assumes that the shadow is at 0xc000. I'm not sure that > holds anymore with UEFI, and maybe we should be using some UEFI > trick instead to find out where it actually lives? We started getting the same message on radeon even though we are eventually able to find the rom. It was due to some core pci change in the handling of roms, but I don't remember the details. Alex > > BTW what does 'lspci -vv -s 00:02.0' say on your machine? > >> >> This is on a Dell XPS 13 9350 (Skylake). This is 4.3.0 plus some >> wireless-next bits. >> >> --Andy >> >> -- >> Andy Lutomirski >> AMA Capital Management, LLC >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 00/29] drm/i915: Type safe register read/write (v2) and more prep work
On Wed, Nov 04, 2015 at 11:19:48PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Another round of stuff for the type safe register access. Some of these
> were already posted during the first round, but a lot of it is new
> (LRI stuff, cmd parser, lrc setup etc., vgpu, etc.)
>
> Available as a git branch too:
> git://github.com/vsyrjala/linux.git type_safe_reg_access_8
>
> Ville Syrjälä (29):
> pci: Decouple quirks.c from i915_reg.h
> drm/i915: Remove the magic AUX_CTL is at DP + foo tricks
> drm/i915: Replace the aux ddc name switch statement with a table
> drm/i915: Parametrize AUX registers
> drm/i915: Add dev_priv->psr_mmio_base
> drm/i915: Store aux data reg offsets in intel_dp->aux_ch_data_reg[]
> drm/i915: Model PSR AUX register selection more like the normal AUX
> code
> drm/i915: s/PCH_DP_/PORT_/ in intel_trans_dp_port_sel() and move it
> next to its only user
> drm/i915: Replace aux_ch_ctl_reg check with port check
> drm/i915: s/is_sdvob/enum port/
> drm/i915: Store DVO SRCDIM register offset under intel_dvo_device
> drm/i915: Streamline gpio_mmio_base deduction
> drm/i915: Prefix raw register defines with underscore
> drm/i915: Parametrize L3 error registers
> drm/i915: Parametrize MOCS registers
> drm/i915: s/0x50/RING_PSMI_CTL/
> drm/i915: Make the high dword offset more explicit in
> i915_reg_read_ioctl
> drm/i915: Make the cmd parser 64bit regs explicit
> drm/i915: Add functions to emit register offsets to the ring
> drm/i915: Add wa_ctx_emit_reg()
> drm/i915: Wrap ASSIGN_CTX_{PDP,PM4L} in do {} while(0)
> drm/i915: Give names to more ring registers
> drm/i915: Wrap context LRI init in a macro
> drm/i915: Turn vgpu pdps into an array
> drm/i915: Pull the vgpu uncore funcs apart from the rest of gen6+
> drm/i915: Add 'offset' to uncore funcs
> drm/i915: Add save/restore of SWF for ILK+
> drm/i915: Add missing ')' to SKL_PS_ECC_STAT define
> drm/i915: Type safe register read/write
Pushed all the remaining patches, except for
"drm/i915: Add save/restore of SWF for ILK+" which can be left for
future studies/experiments.
Thank you for the reviews.
--
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 i-g-t] overlay/intel-gpu-overlay
-Original Message-
From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
Sent: Wednesday, November 18, 2015 2:59 PM
To: Vlad, Marius C
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay
On Wed, Nov 18, 2015 at 02:40:29PM +0200, marius.c.v...@intel.com wrote:
> From: Marius Vlad
>
> Use sigaction() instead of signal() and add SIGINT, SIGTERM to close
> the overlay window. With this change the overlay window will be
> destroyed.
>
> Signed-off-by: Marius Vlad
> ---
> overlay/overlay.c | 42 --
> 1 file changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/overlay/overlay.c b/overlay/overlay.c index
> 3c0dbb4..48ba67c 100644
> --- a/overlay/overlay.c
> +++ b/overlay/overlay.c
> @@ -804,11 +804,19 @@ static void show_gem_objects(struct
> overlay_context *ctx, struct overlay_gem_obj
>
> static int take_snapshot;
>
> -static void signal_snapshot(int sig)
> +static void
> +signal_snapshot(int sig, siginfo_t *si, void *__unused)
> {
> take_snapshot = sig;
> }
>
> +static void
> +signal_x11_destroy(int sig, siginfo_t *si, void *__unused) {
> + x11_overlay_stop();
is not signalsafe.
Indeed. Any ideas then? I'm hitting a weird system hang if the overlay window
is open
for a period of time (although period ranges to minutes it seems to happen each
time). If the window
is closed and process exits I don't see this behavior. This is entirely
different bug but meanwhile
wanted to keep my machine running.
> + if (ctx.surface == NULL) {
> + fprintf(stderr, "Failed to create X11 overlay.\n");
Spurious changes.
Right, just debugging.
> ctx.surface = x11_window_create(&config, &ctx.width,
> &ctx.height);
> - if (ctx.surface == NULL)
> + }
> + if (ctx.surface == NULL) {
> + fprintf(stderr, "Failed to create X11 window.\n");
> ctx.surface = kms_overlay_create(&config, &ctx.width,
> &ctx.height);
> - if (ctx.surface == NULL)
> + }
> + if (ctx.surface == NULL) {
> + fprintf(stderr, "Failed to create KMS overlay.\n");
> return ENXIO;
> + }
>
> if (daemonize && daemon(0, 0))
> return EINVAL;
> @@ -913,7 +930,20 @@ int main(int argc, char **argv)
> if (renice && (nice(renice) == -1))
> fprintf(stderr, "Could not renice: %s\n", strerror(errno));
>
> - signal(SIGUSR1, signal_snapshot);
> + sa.sa_flags = SA_SIGINFO;
> + sigemptyset(&sa.sa_mask);
> + sa.sa_sigaction = &signal_snapshot;
> +
> + if (sigaction(SIGUSR1, &sa, NULL) == -1) {
Any particular reason for a fondness here for sigaction?
Well signal(2) states to avoid it and instead use sigaction(). I'm not that
fond of it :)
-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
[Intel-gfx] [PATCH 1/2] drm/i915/ddi: fix intel_display_port_aux_power_domain() after HDMI detect
Due to the current sharing of the DDI encoder between DP and HDMI
connectors we can run the DP detection after the HDMI detection has
already set the shared encoder's type. For now solve this keeping the
current behavior and running the detection in this case too. For a proper
solution Ville suggested to split the encoder into an HDMI and DP one, that
can be done as a follow-up.
This issue triggers the WARN in intel_display_port_aux_power_domain() and
was introduced in:
commit 25f78f58e5bfb46a270ce4d690fb49dc104558b1
Author: Ville Syrjälä
Date: Mon Nov 16 15:01:04 2015 +0100
drm/i915: Clean up AUX power domain handling
CC: Patrik Jakobsson
CC: Ville Syrjälä
Signed-off-by: Imre Deak
Reviewed-by: Patrik Jakobsson
---
drivers/gpu/drm/i915/intel_display.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 688d484..cdc1761 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5230,7 +5230,14 @@ intel_display_port_aux_power_domain(struct intel_encoder
*intel_encoder)
switch (intel_encoder->type) {
case INTEL_OUTPUT_UNKNOWN:
- /* Only DDI platforms should ever use this output type */
+ case INTEL_OUTPUT_HDMI:
+ /*
+* Only DDI platforms should ever use these output types.
+* We can get here after the HDMI detect code has already set
+* the type of the shared encoder. Since we can't be sure
+* what's the status of the given connectors, play safe and
+* run the DP detection too.
+*/
WARN_ON_ONCE(!HAS_DDI(dev));
case INTEL_OUTPUT_DISPLAYPORT:
case INTEL_OUTPUT_EDP:
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: add MISSING_CASE to a few port/aux power domain helpers
MISSING_CASE() would have been useful to track down a recent problem in intel_display_port_aux_power_domain(), so add it there and a few related helpers. This was also suggested by Ville in his review of the latest DMC/DC changes, we forgot to address that. Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/intel_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index cdc1761..b3d0557 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5166,7 +5166,7 @@ static enum intel_display_power_domain port_to_power_domain(enum port port) case PORT_E: return POWER_DOMAIN_PORT_DDI_E_LANES; default: - WARN_ON_ONCE(1); + MISSING_CASE(port); return POWER_DOMAIN_PORT_OTHER; } } @@ -5186,7 +5186,7 @@ static enum intel_display_power_domain port_to_aux_power_domain(enum port port) /* FIXME: Check VBT for actual wiring of PORT E */ return POWER_DOMAIN_AUX_D; default: - WARN_ON_ONCE(1); + MISSING_CASE(port); return POWER_DOMAIN_AUX_A; } } @@ -5247,7 +5247,7 @@ intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder) intel_dig_port = enc_to_mst(&intel_encoder->base)->primary; return port_to_aux_power_domain(intel_dig_port->port); default: - WARN_ON_ONCE(1); + MISSING_CASE(intel_encoder->type); return POWER_DOMAIN_AUX_A; } } -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay
On Wed, Nov 18, 2015 at 01:51:57PM +, Vlad, Marius C wrote:
>
>
> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Wednesday, November 18, 2015 2:59 PM
> To: Vlad, Marius C
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH i-g-t] overlay/intel-gpu-overlay
>
> On Wed, Nov 18, 2015 at 02:40:29PM +0200, marius.c.v...@intel.com wrote:
> > From: Marius Vlad
> >
> > Use sigaction() instead of signal() and add SIGINT, SIGTERM to close
> > the overlay window. With this change the overlay window will be
> > destroyed.
> >
> > Signed-off-by: Marius Vlad
> > ---
> > overlay/overlay.c | 42 --
> > 1 file changed, 36 insertions(+), 6 deletions(-)
> >
> > diff --git a/overlay/overlay.c b/overlay/overlay.c index
> > 3c0dbb4..48ba67c 100644
> > --- a/overlay/overlay.c
> > +++ b/overlay/overlay.c
> > @@ -804,11 +804,19 @@ static void show_gem_objects(struct
> > overlay_context *ctx, struct overlay_gem_obj
> >
> > static int take_snapshot;
> >
> > -static void signal_snapshot(int sig)
> > +static void
> > +signal_snapshot(int sig, siginfo_t *si, void *__unused)
> > {
> > take_snapshot = sig;
> > }
> >
> > +static void
> > +signal_x11_destroy(int sig, siginfo_t *si, void *__unused) {
> > + x11_overlay_stop();
>
> is not signalsafe.
>
> Indeed. Any ideas then? I'm hitting a weird system hang if the overlay window
> is open
> for a period of time (although period ranges to minutes it seems to happen
> each time). If the window
> is closed and process exits I don't see this behavior. This is entirely
> different bug but meanwhile
> wanted to keep my machine running.
Yes, it is a known hw (on ivb/hsw) issue with concurrent access to
registers. We need the i915 perf interface to avoid it (as then we can
serialise all register access).
-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 i-g-t] tests/gem_reset_stats.c: prepare for per engine resets
On Wed, Nov 18, 2015 at 12:15:11PM +, Gore, Tim wrote:
>
>
> Tim Gore
> Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ
>
>
> > -Original Message-
> > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Wednesday, November 18, 2015 11:54 AM
> > To: Gore, Tim
> > Cc: intel-gfx@lists.freedesktop.org; mika.kuopp...@linux.com; Wood,
> > Thomas
> > Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/gem_reset_stats.c: prepare for
> > per engine resets
> >
> > On Wed, Nov 18, 2015 at 10:24:43AM +, tim.g...@intel.com wrote:
> > > From: Tim Gore
> > >
> > > when checking to make sure that the driver has performed the expected
> > > number of resets, this test looks at the reset_count, which is
> > > incremented each time the GPU is reset. Upcoming changes in the way
> > > GPU hangs are handled mean that in most cases (and in all the cases in
> > > this
> > > test) only a single GPU engine is reset which does not cause the
> > > reset_count to be incremented. This is already causing this test to
> > > fail on Android. In this case we can instead look at the batch_active
> > > count which is also returned from the i915_get_reset_stats_ioctl and
> > > is incremented by both a single engine reset and a full gpu reset.
> > > There are differences between the reset_count and the batch_active
> > > count, but for establishing that the correct number of resets have
> > > occured either can be used.
> > > This change enables this test to run successfully on Android and will
> > > mean that the test does not break when the TDR patches get merged into
> > > the uptream driver.
> > >
> > > Signed-off-by: Tim Gore
> >
> > Why doesn't TDR just count resets correctly?
> > -Daniel
> >
> It does. It just doesn't do gpu resets for most hangs. Instead it resets
> as single command streamer which is much less intrusive and loses the
> minimum amount of work.
Again: Why does TDR not count engine resets as resets? Afaiui that seems
to be the problem you're working around here.
-Daniel
> Tim
>
> > > ---
> > > tests/gem_reset_stats.c | 41
> > > ++---
> > > 1 file changed, 26 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c index
> > > 4cbbb4e..5ec026f 100644
> > > --- a/tests/gem_reset_stats.c
> > > +++ b/tests/gem_reset_stats.c
> > > @@ -104,9 +104,9 @@ static int gem_reset_stats(int fd, int ctx_id,
> > >
> > > rs->ctx_id = ctx_id;
> > > rs->flags = 0;
> > > - rs->reset_count = rand();
> > > - rs->batch_active = rand();
> > > - rs->batch_pending = rand();
> > > + rs->reset_count = UINT32_MAX;
> > > + rs->batch_active = UINT32_MAX;
> > > + rs->batch_pending = UINT32_MAX;
> > > rs->pad = 0;
> > >
> > > do {
> > > @@ -690,6 +690,18 @@ static int get_reset_count(int fd, int ctx)
> > > return rs.reset_count;
> > > }
> > >
> > > +static int get_active_count(int fd, int ctx) {
> > > + int ret;
> > > + struct local_drm_i915_reset_stats rs;
> > > +
> > > + ret = gem_reset_stats(fd, ctx, &rs);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + return rs.batch_active;
> > > +}
> > > +
> > > static void test_close_pending_ctx(void) {
> > > int fd, h;
> > > @@ -837,17 +849,16 @@ static void test_reset_count(const bool
> > > create_ctx)
> > >
> > > assert_reset_status(fd, ctx, RS_NO_ERROR);
> > >
> > > - c1 = get_reset_count(fd, ctx);
> > > - igt_assert(c1 >= 0);
> > > + c1 = get_active_count(fd, ctx);
> > > + igt_assert(c1 == 0);
> > >
> > > h = inject_hang(fd, ctx);
> > > igt_assert_lte(0, h);
> > > gem_sync(fd, h);
> > >
> > > assert_reset_status(fd, ctx, RS_BATCH_ACTIVE);
> > > - c2 = get_reset_count(fd, ctx);
> > > - igt_assert(c2 >= 0);
> > > - igt_assert(c2 == (c1 + 1));
> > > + c2 = get_active_count(fd, ctx);
> > > + igt_assert(c2 == 1);
> > >
> > > igt_fork(child, 1) {
> > > igt_drop_root();
> > > @@ -877,9 +888,9 @@ static int _test_params(int fd, int ctx, uint32_t
> > > flags, uint32_t pad)
> > >
> > > rs.ctx_id = ctx;
> > > rs.flags = flags;
> > > - rs.reset_count = rand();
> > > - rs.batch_active = rand();
> > > - rs.batch_pending = rand();
> > > + rs.reset_count = UINT32_MAX;
> > > + rs.batch_active = UINT32_MAX;
> > > + rs.batch_pending = UINT32_MAX;
> > > rs.pad = pad;
> > >
> > > do {
> > > @@ -976,14 +987,14 @@ static void defer_hangcheck(int ring_num)
> > >
> > > igt_skip_on(next_ring == current_ring);
> > >
> > > - count_start = get_reset_count(fd, 0);
> > > - igt_assert_lte(0, count_start);
> > > + count_start = get_active_count(fd, 0);
> > > + igt_assert(count_start == 0);
> > >
> > > igt_assert(inject_hang_ring(fd, 0, current_ring->exec, true));
> > > while (--seconds) {
> > > igt_assert(exec_valid_ring(fd, 0, next_ring->exec));
> > >
> > > - count_end = get_reset_count(fd, 0);
> > > + count_end = get_active_count(fd, 0);
> > > igt_assert_lte(0,
Re: [Intel-gfx] [PATCH v2] PCI / PM: tune down RPM suspend error message with EBUSY and EAGAIN retval
On Wed, Nov 18, 2015 at 03:28:38PM +0200, Imre Deak wrote:
> On ke, 2015-11-18 at 12:56 +0200, Imre Deak wrote:
> > The runtime PM core doesn't treat EBUSY and EAGAIN retvals from the driver
> > suspend hooks as errors, but they still show up as errors in dmesg. Tune
> > them down.
> >
> > One problem caused by this was noticed by Daniel: the i915 driver
> > returns EAGAIN to signal a temporary failure to suspend and as a request
> > towards the RPM core for scheduling a suspend again. This is a normal
> > event, but the resulting error message flags a breakage during the
> > driver's automated testing which parses dmesg and picks up the error.
> >
> > v2:
> > - fix compile breake when CONFIG_PM_SLEEP=n (0-day builder)
> >
> > Reported-by: Daniel Vetter
> > Signed-off-by: Imre Deak
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92992
Reviewed-by: Daniel Vetter
Rafael, can you please pick this up for 4.4? The spurious KERN_ERR noise
in dmesg is causing a lot fo spurious fail in our (very recently put into
place) i915 CI system.
Thanks, Daniel
>
> > ---
> > drivers/base/power/main.c | 7 +--
> > drivers/pci/pci-driver.c | 2 +-
> > include/linux/pm.h| 11 +--
> > 3 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > index 1710c26..39d2090 100644
> > --- a/drivers/base/power/main.c
> > +++ b/drivers/base/power/main.c
> > @@ -1679,9 +1679,12 @@ int dpm_suspend_start(pm_message_t state)
> > }
> > EXPORT_SYMBOL_GPL(dpm_suspend_start);
> >
> > -void __suspend_report_result(const char *function, void *fn, int ret)
> > +void __suspend_report_result(const char *function, void *fn, int ret,
> > + bool runtime_pm)
> > {
> > - if (ret)
> > + if (runtime_pm && (ret == -EBUSY || ret == -EAGAIN))
> > + printk(KERN_DEBUG "%s(): %pF returns %d\n", function, fn, ret);
> > + else if (ret)
> > printk(KERN_ERR "%s(): %pF returns %d\n", function, fn, ret);
> > }
> > EXPORT_SYMBOL_GPL(__suspend_report_result);
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index 108a311..9569572 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -1142,7 +1142,7 @@ static int pci_pm_runtime_suspend(struct device *dev)
> > pci_dev->state_saved = false;
> > pci_dev->no_d3cold = false;
> > error = pm->runtime_suspend(dev);
> > - suspend_report_result(pm->runtime_suspend, error);
> > + rpm_suspend_report_result(pm->runtime_suspend, error);
> > if (error)
> > return error;
> > if (!pci_dev->d3cold_allowed)
> > diff --git a/include/linux/pm.h b/include/linux/pm.h
> > index 35d599e..54f37e3 100644
> > --- a/include/linux/pm.h
> > +++ b/include/linux/pm.h
> > @@ -702,11 +702,17 @@ extern int dpm_suspend_late(pm_message_t state);
> > extern int dpm_suspend(pm_message_t state);
> > extern int dpm_prepare(pm_message_t state);
> >
> > -extern void __suspend_report_result(const char *function, void *fn, int
> > ret);
> > +extern void __suspend_report_result(const char *function, void *fn, int
> > ret,
> > + bool runtime_pm);
> >
> > #define suspend_report_result(fn, ret)
> > \
> > do {\
> > - __suspend_report_result(__func__, fn, ret); \
> > + __suspend_report_result(__func__, fn, ret, false); \
> > + } while (0)
> > +
> > +#define rpm_suspend_report_result(fn, ret) \
> > + do {\
> > + __suspend_report_result(__func__, fn, ret, true); \
> > } while (0)
> >
> > extern int device_pm_wait_for_dev(struct device *sub, struct device *dev);
> > @@ -744,6 +750,7 @@ static inline int dpm_suspend_start(pm_message_t state)
> > }
> >
> > #define suspend_report_result(fn, ret) do {} while (0)
> > +#define rpm_suspend_report_result(fn, ret) do {} while (0)
> >
> > static inline int device_pm_wait_for_dev(struct device *a, struct device
> > *b)
> > {
--
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 MISSING_CASE to a few port/aux power domain helpers
On Wed, Nov 18, 2015 at 03:57:25PM +0200, Imre Deak wrote: > MISSING_CASE() would have been useful to track down a recent problem in > intel_display_port_aux_power_domain(), so add it there and a few related > helpers. This was also suggested by Ville in his review of the latest > DMC/DC changes, we forgot to address that. > > Signed-off-by: Imre Deak Reviewed-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index cdc1761..b3d0557 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5166,7 +5166,7 @@ static enum intel_display_power_domain > port_to_power_domain(enum port port) > case PORT_E: > return POWER_DOMAIN_PORT_DDI_E_LANES; > default: > - WARN_ON_ONCE(1); > + MISSING_CASE(port); > return POWER_DOMAIN_PORT_OTHER; > } > } > @@ -5186,7 +5186,7 @@ static enum intel_display_power_domain > port_to_aux_power_domain(enum port port) > /* FIXME: Check VBT for actual wiring of PORT E */ > return POWER_DOMAIN_AUX_D; > default: > - WARN_ON_ONCE(1); > + MISSING_CASE(port); > return POWER_DOMAIN_AUX_A; > } > } > @@ -5247,7 +5247,7 @@ intel_display_port_aux_power_domain(struct > intel_encoder *intel_encoder) > intel_dig_port = enc_to_mst(&intel_encoder->base)->primary; > return port_to_aux_power_domain(intel_dig_port->port); > default: > - WARN_ON_ONCE(1); > + MISSING_CASE(intel_encoder->type); > return POWER_DOMAIN_AUX_A; > } > } > -- > 2.5.0 -- 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 v2 3/4] drm/i915: make assert_device_not_suspended more precise
On Mon, Nov 09, 2015 at 09:13:45PM +0200, Imre Deak wrote:
> Atm, we assert that the device is not suspended after the point when the
> HW is truly put to a suspended state. This is fine, but we can catch
> more problems if we check the RPM refcount. After that one drops to zero
> we shouldn't access the HW any more, although the actual suspend may be
> delayed. The only complication is that we want to avoid asserts while
> the suspend handler itself is running, so add a flag to handle this
> case.
Why do we want to avoid asserts firing while we go through the suspend
handler? Calling assert_device_not_suspended from within rpm
suspend/resume code sounds like a bug. Where/why does this happen?
-Daniel
>
> While at it remove the HAS_RUNTIME_PM check, the pm.suspended flag is
> false and the RPM refcount is non-zero on all platforms that don't
> support RPM.
>
> This caught additional WARNs from the atomic path, those will be fixed
> as a follow-up.
>
> v2:
> - remove the redundant HAS_RUNTIME_PM check (Ville)
>
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/i915_drv.c | 5 +
> drivers/gpu/drm/i915/i915_drv.h | 5 +
> drivers/gpu/drm/i915/intel_runtime_pm.c | 14 --
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 77d183d..caeb218 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1494,6 +1494,9 @@ static int intel_runtime_suspend(struct device *device)
>
> return -EAGAIN;
> }
> +
> + dev_priv->pm.disable_suspended_assert = true;
> +
> /*
>* We are safe here against re-faults, since the fault handler takes
>* an RPM reference.
> @@ -1518,6 +1521,8 @@ static int intel_runtime_suspend(struct device *device)
> intel_uncore_forcewake_reset(dev, false);
> dev_priv->pm.suspended = true;
>
> + dev_priv->pm.disable_suspended_assert = false;
> +
> /*
>* FIXME: We really should find a document that references the arguments
>* used below!
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5628c5a..43fd341 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1599,6 +1599,11 @@ struct skl_wm_level {
> * For more, read the Documentation/power/runtime_pm.txt.
> */
> struct i915_runtime_pm {
> + /*
> + * Used for the duration of runtime suspend to avoid false device
> + * suspended asserts.
> + */
> + bool disable_suspended_assert;
> bool suspended;
> bool irqs_enabled;
> };
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 4d39b3c..2bdbcd4 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2120,8 +2120,18 @@ void intel_power_domains_init_hw(struct
> drm_i915_private *dev_priv, bool resume)
>
> void assert_device_not_suspended(struct drm_i915_private *dev_priv)
> {
> - WARN_ONCE(HAS_RUNTIME_PM(dev_priv->dev) && dev_priv->pm.suspended,
> - "Device suspended\n");
> + int rpm_usage;
> +
> + if (dev_priv->pm.disable_suspended_assert)
> + return;
> +
> +#ifdef CONFIG_PM
> + rpm_usage = atomic_read(&dev_priv->dev->dev->power.usage_count);
> +#else
> + rpm_usage = 1;
> +#endif
> +
> + WARN_ONCE(dev_priv->pm.suspended || !rpm_usage, "Device suspended\n");
> }
>
> /**
> --
> 2.5.0
>
> ___
> 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 2/2] drm/i915: add MISSING_CASE to a few port/aux power domain helpers
On Wed, Nov 18, 2015 at 03:57:25PM +0200, Imre Deak wrote: > MISSING_CASE() would have been useful to track down a recent problem in > intel_display_port_aux_power_domain(), so add it there and a few related > helpers. This was also suggested by Ville in his review of the latest > DMC/DC changes, we forgot to address that. > > Signed-off-by: Imre Deak Reviewed-by: Patrik Jakobsson > --- > drivers/gpu/drm/i915/intel_display.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index cdc1761..b3d0557 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5166,7 +5166,7 @@ static enum intel_display_power_domain > port_to_power_domain(enum port port) > case PORT_E: > return POWER_DOMAIN_PORT_DDI_E_LANES; > default: > - WARN_ON_ONCE(1); > + MISSING_CASE(port); > return POWER_DOMAIN_PORT_OTHER; > } > } > @@ -5186,7 +5186,7 @@ static enum intel_display_power_domain > port_to_aux_power_domain(enum port port) > /* FIXME: Check VBT for actual wiring of PORT E */ > return POWER_DOMAIN_AUX_D; > default: > - WARN_ON_ONCE(1); > + MISSING_CASE(port); > return POWER_DOMAIN_AUX_A; > } > } > @@ -5247,7 +5247,7 @@ intel_display_port_aux_power_domain(struct > intel_encoder *intel_encoder) > intel_dig_port = enc_to_mst(&intel_encoder->base)->primary; > return port_to_aux_power_domain(intel_dig_port->port); > default: > - WARN_ON_ONCE(1); > + MISSING_CASE(intel_encoder->type); > return POWER_DOMAIN_AUX_A; > } > } > -- > 2.5.0 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/4] drm/i915: make assert_device_not_suspended more precise
On ke, 2015-11-18 at 15:37 +0100, Daniel Vetter wrote:
> On Mon, Nov 09, 2015 at 09:13:45PM +0200, Imre Deak wrote:
> > Atm, we assert that the device is not suspended after the point
> > when the
> > HW is truly put to a suspended state. This is fine, but we can
> > catch
> > more problems if we check the RPM refcount. After that one drops to
> > zero
> > we shouldn't access the HW any more, although the actual suspend
> > may be
> > delayed. The only complication is that we want to avoid asserts
> > while
> > the suspend handler itself is running, so add a flag to handle this
> > case.
>
> Why do we want to avoid asserts firing while we go through the
> suspend
> handler? Calling assert_device_not_suspended from within rpm
> suspend/resume code sounds like a bug. Where/why does this happen?
Yea, disable_rpm_asserts() is misnamed. Should be
disable_rpm_wakelock_asserts(). Will change that in the next iteration.
> -Daniel
>
> >
> > While at it remove the HAS_RUNTIME_PM check, the pm.suspended flag
> > is
> > false and the RPM refcount is non-zero on all platforms that don't
> > support RPM.
> >
> > This caught additional WARNs from the atomic path, those will be
> > fixed
> > as a follow-up.
> >
> > v2:
> > - remove the redundant HAS_RUNTIME_PM check (Ville)
> >
> > Signed-off-by: Imre Deak
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 5 +
> > drivers/gpu/drm/i915/i915_drv.h | 5 +
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 14 --
> > 3 files changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 77d183d..caeb218 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1494,6 +1494,9 @@ static int intel_runtime_suspend(struct
> > device *device)
> >
> > return -EAGAIN;
> > }
> > +
> > + dev_priv->pm.disable_suspended_assert = true;
> > +
> > /*
> > * We are safe here against re-faults, since the fault
> > handler takes
> > * an RPM reference.
> > @@ -1518,6 +1521,8 @@ static int intel_runtime_suspend(struct
> > device *device)
> > intel_uncore_forcewake_reset(dev, false);
> > dev_priv->pm.suspended = true;
> >
> > + dev_priv->pm.disable_suspended_assert = false;
> > +
> > /*
> > * FIXME: We really should find a document that references
> > the arguments
> > * used below!
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 5628c5a..43fd341 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1599,6 +1599,11 @@ struct skl_wm_level {
> > * For more, read the Documentation/power/runtime_pm.txt.
> > */
> > struct i915_runtime_pm {
> > + /*
> > + * Used for the duration of runtime suspend to avoid false
> > device
> > + * suspended asserts.
> > + */
> > + bool disable_suspended_assert;
> > bool suspended;
> > bool irqs_enabled;
> > };
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 4d39b3c..2bdbcd4 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -2120,8 +2120,18 @@ void intel_power_domains_init_hw(struct
> > drm_i915_private *dev_priv, bool resume)
> >
> > void assert_device_not_suspended(struct drm_i915_private
> > *dev_priv)
> > {
> > - WARN_ONCE(HAS_RUNTIME_PM(dev_priv->dev) && dev_priv-
> > >pm.suspended,
> > - "Device suspended\n");
> > + int rpm_usage;
> > +
> > + if (dev_priv->pm.disable_suspended_assert)
> > + return;
> > +
> > +#ifdef CONFIG_PM
> > + rpm_usage = atomic_read(&dev_priv->dev->dev-
> > >power.usage_count);
> > +#else
> > + rpm_usage = 1;
> > +#endif
> > +
> > + WARN_ONCE(dev_priv->pm.suspended || !rpm_usage, "Device
> > suspended\n");
> > }
> >
> > /**
> > --
> > 2.5.0
> >
> > ___
> > 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 i-g-t] tests/gem_reset_stats.c: prepare for per engine resets
Tim Gore
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Wednesday, November 18, 2015 2:12 PM
> To: Gore, Tim
> Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org;
> mika.kuopp...@linux.com; Wood, Thomas
> Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/gem_reset_stats.c: prepare for
> per engine resets
>
> On Wed, Nov 18, 2015 at 12:15:11PM +, Gore, Tim wrote:
> >
> >
> > Tim Gore
> > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon
> > SN3 1RJ
> >
> >
> > > -Original Message-
> > > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of
> > > Daniel Vetter
> > > Sent: Wednesday, November 18, 2015 11:54 AM
> > > To: Gore, Tim
> > > Cc: intel-gfx@lists.freedesktop.org; mika.kuopp...@linux.com; Wood,
> > > Thomas
> > > Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/gem_reset_stats.c:
> > > prepare for per engine resets
> > >
> > > On Wed, Nov 18, 2015 at 10:24:43AM +, tim.g...@intel.com wrote:
> > > > From: Tim Gore
> > > >
> > > > when checking to make sure that the driver has performed the
> > > > expected number of resets, this test looks at the reset_count,
> > > > which is incremented each time the GPU is reset. Upcoming changes
> > > > in the way GPU hangs are handled mean that in most cases (and in
> > > > all the cases in this
> > > > test) only a single GPU engine is reset which does not cause the
> > > > reset_count to be incremented. This is already causing this test
> > > > to fail on Android. In this case we can instead look at the
> > > > batch_active count which is also returned from the
> > > > i915_get_reset_stats_ioctl and is incremented by both a single engine
> reset and a full gpu reset.
> > > > There are differences between the reset_count and the batch_active
> > > > count, but for establishing that the correct number of resets have
> > > > occured either can be used.
> > > > This change enables this test to run successfully on Android and
> > > > will mean that the test does not break when the TDR patches get
> > > > merged into the uptream driver.
> > > >
> > > > Signed-off-by: Tim Gore
> > >
> > > Why doesn't TDR just count resets correctly?
> > > -Daniel
> > >
> > It does. It just doesn't do gpu resets for most hangs. Instead it
> > resets as single command streamer which is much less intrusive and
> > loses the minimum amount of work.
>
> Again: Why does TDR not count engine resets as resets? Afaiui that seems to
> be the problem you're working around here.
> -Daniel
>
This was not intended as a workaround, just to align the test with the way TDR
is being implemented. I assume TDR chose not the alter the meaning of the
reset_counter as this is implicitly part of the interface to userland. We can
change
reset_counter to count both GPU resets and engine resets but I think this
may have knock on effects for the openGL robustness interface which uses this
Ioctl. I'll check with the openGL team as I know they are still working on this.
> > Tim
> >
> > > > ---
> > > > tests/gem_reset_stats.c | 41
> > > > ++---
> > > > 1 file changed, 26 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
> > > > index 4cbbb4e..5ec026f 100644
> > > > --- a/tests/gem_reset_stats.c
> > > > +++ b/tests/gem_reset_stats.c
> > > > @@ -104,9 +104,9 @@ static int gem_reset_stats(int fd, int ctx_id,
> > > >
> > > > rs->ctx_id = ctx_id;
> > > > rs->flags = 0;
> > > > - rs->reset_count = rand();
> > > > - rs->batch_active = rand();
> > > > - rs->batch_pending = rand();
> > > > + rs->reset_count = UINT32_MAX;
> > > > + rs->batch_active = UINT32_MAX;
> > > > + rs->batch_pending = UINT32_MAX;
> > > > rs->pad = 0;
> > > >
> > > > do {
> > > > @@ -690,6 +690,18 @@ static int get_reset_count(int fd, int ctx)
> > > > return rs.reset_count;
> > > > }
> > > >
> > > > +static int get_active_count(int fd, int ctx) {
> > > > + int ret;
> > > > + struct local_drm_i915_reset_stats rs;
> > > > +
> > > > + ret = gem_reset_stats(fd, ctx, &rs);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + return rs.batch_active;
> > > > +}
> > > > +
> > > > static void test_close_pending_ctx(void) {
> > > > int fd, h;
> > > > @@ -837,17 +849,16 @@ static void test_reset_count(const bool
> > > > create_ctx)
> > > >
> > > > assert_reset_status(fd, ctx, RS_NO_ERROR);
> > > >
> > > > - c1 = get_reset_count(fd, ctx);
> > > > - igt_assert(c1 >= 0);
> > > > + c1 = get_active_count(fd, ctx);
> > > > + igt_assert(c1 == 0);
> > > >
> > > > h = inject_hang(fd, ctx);
> > > > igt_assert_lte(0, h);
> > > > gem_sync(fd, h);
> > > >
> > > > assert_reset_status(
Re: [Intel-gfx] [PATCH i-g-t] tests/gem_buffered_svm_test: New tests for buffered SVM feature
On Tue, Nov 10, 2015 at 07:37:54AM -0800, Vinay Belgaumkar wrote: > v1: These tests exercise the userptr ioctl to create shared buffers > between CPU and GPU. They contain error and normal usage scenarios. > They also contain a couple of stress tests which copy buffers between > CPU and GPU. These tests rely on the softpin patch in order to pin buffers > to a certain VA. > > Caveat: These tests were designed to run on 64-bit system. Future work > includes adding logic to ensure these tests can run on 32-bit systems with > PPGTT support. Some tests are currently disabled for 32-bit systems for that > reason. > > Testcase: igt/gem_buffered_svm_test > --- > tests/Makefile.sources|1 + > tests/gem_buffered_svm_test.c | 1490 > + > 2 files changed, 1491 insertions(+) > create mode 100644 tests/gem_buffered_svm_test.c > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index 8fb2de8..2ce4216 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -11,6 +11,7 @@ TESTS_progs_M = \ > drv_hangman \ > gem_bad_reloc \ > gem_basic \ > + gem_buffered_svm_test \ I think it'd make more sense to call this gem_softpin, since that's the kernel feature it tests. That softpin+userptr is used to implement buffered svm in opencl is kinda irrelevant for low-level testing. Also there's lots of overlap with other gem tests, which will just be wasting CI resources. The only thing we need is to exercise softpin specifically - we have hundreds of testcases for basic gem, coherency, copying and userptr already. What I'm looking for here is overlapping testcases, invalid flags tests, and maybe softpin stress tests (e.g. create 100k 4096 byte objecsts with softpinned randomized offsets all over the place). Also no compile-time skipping of testcases please. If they don't work on 32bit then please skip at runtime using igt_skip_f (and print a useful reason for why). -Daniel > gem_caching \ > gem_close_race \ > gem_concurrent_blit \ > diff --git a/tests/gem_buffered_svm_test.c b/tests/gem_buffered_svm_test.c > new file mode 100644 > index 000..44d342a > --- /dev/null > +++ b/tests/gem_buffered_svm_test.c > @@ -0,0 +1,1490 @@ > +/* > + * 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: > + *Vinay Belgaumkar + Thomas Daniel > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "drm.h" > +#include "ioctl_wrappers.h" > +#include "drmtest.h" > +#include "intel_chipset.h" > +#include "intel_io.h" > +#include "i915_drm.h" > +#include > +#include > +#include > +#include > +#include "igt_kms.h" > +#include > +#include > +#include > + > + > +#if (INTPTR_MAX == INT32_MAX) > + #define IS_32BIT_USER > +#endif > + > +#define OBJECT_SIZE 16384 > +#define BO_SIZE 4 * 4096 > +#define STORE_BATCH_BUFFER_SIZE 6 > +#define STRESS_BATCH_BUFFER_SIZE 5 > +#define EXEC_OBJECT_PINNED (1<<4) > +#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3) > +#define SHARED_BUFFER_SIZE 4096 > +#define NUM_EXEC_OBJECTS 2 > + > +typedef struct drm_i915_gem_userptr i915_gem_userptr; > +static i915_gem_userptr* gem_create_userptr_struct(void* ptr, int size, int > read_only); > + > +static void* gem_create_mem_buffer(int size); > +static void gem_invalid_userptr_test(void); > +static int gem_call_userptr_ioctl(int fd, i915_gem_userptr* userptr); > +static void gem_basic_test(bool); > +static void gem_pin_invalid_vma_test(void); > +static void gem_pin_overlap_test(void); > +static void gem_shmem_test(void); > +static void gem_pin_high_address_test(void); > +static void gem_pin_mmap_anonymous_test(void); > +static void gem_pin_mmap_file_test(vo
Re: [Intel-gfx] [PATCH 0/5] Sink CRC stabilization
On Mon, Nov 16, 2015 at 03:59:15PM +, Rodrigo Vivi wrote: > Hi Daniel, > > could you please ignore patch 5 in this series and merge the 4 first > patches. I merged an earlier instance of the first 4 patches. Has anything changed? Please double-check the right versions landed in dinq. Thanks, Daniel > > The aux mutex is really unreliable and doesn't help on all SKLs so better > to just ignore that. > > Thanks, > Rodrigo. > > On Thu, Nov 12, 2015 at 11:03 AM Rodrigo Vivi > wrote: > > > On Thu, Nov 12, 2015 at 10:56 AM Vivi, Rodrigo > > wrote: > > > >> On Thu, 2015-11-12 at 12:17 +0200, Jani Nikula wrote: > >> > On Wed, 11 Nov 2015, Rodrigo Vivi wrote: > >> > > Let's start spliting that big series that enables PSR with this > >> > > sink crc > >> > > stabilization. > >> > > > >> > > Also I'm adding Wayne's mutex that stabilizes sink CRC on Skylake. > >> > > > >> > > All patches already reviewed and ready to merge. > >> > > >> > Disagreed on the hw mutex patch. > >> > >> I wasn't considered it as nacked. But it seems that we have more to > >> discuss around it. > >> > >> Although I don't see a reason of blocking a patch that follow spec, it > >> is reviewed, and it is used to fix a bug, unblock validation and only > >> used for the particular case that is just used on automated validation > >> and not broadly to all aux communications. > >> > > > > Oh, now I saw you had comments there on the patch. So please ignore this > > complain and accept my apologies. > > > > > >> > >> > > >> > BR, > >> > Jani. > >> > > >> > > > >> > > Thank you very much Paulo for the review and Thank you Wayne for > >> > > the > >> > > SKL aux mutex. > >> > > > >> > > Thanks, > >> > > Rodrigo. > >> > > > >> > > Boyer, Wayne (1): > >> > > drm/i915/skl: implement DP Aux Mutex framework > >> > > > >> > > Rodrigo Vivi (4): > >> > > drm/i915: Allow 1 vblank to let Sink CRC calculation to start or > >> > > stop. > >> > > drm/i915: Make Sink crc calculation waiting for counter to reset. > >> > > drm/i915: Stop tracking last calculated Sink CRC. > >> > > drm/i915: Rely on TEST_SINK_START instead of tracking Sink CRC > >> > > state > >> > > on dev_priv. > >> > > > >> > > drivers/gpu/drm/i915/i915_drv.h | 1 + > >> > > drivers/gpu/drm/i915/i915_reg.h | 5 ++ > >> > > drivers/gpu/drm/i915/intel_dp.c | 126 +++ > >> > > > >> > > drivers/gpu/drm/i915/intel_drv.h | 7 --- > >> > > 4 files changed, 93 insertions(+), 46 deletions(-) > >> > > >> ___ > >> 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 0/4] PSR Critical fixes
On Mon, Nov 16, 2015 at 04:00:35PM +, Rodrigo Vivi wrote: > Hi Daniel, > > All 4 patches in this series are reviewed and ready to merge, > could you please merge them? All merged to dinq, thanks for patches&review. -Daniel > > Thanks, > Rodrigo. > > On Fri, Nov 13, 2015 at 10:42 AM Vivi, Rodrigo > wrote: > > > On Fri, 2015-11-13 at 17:08 +0200, Ville Syrjälä wrote: > > > On Wed, Nov 11, 2015 at 11:37:06AM -0800, Rodrigo Vivi wrote: > > > > Let's split critical PSR fixes from the series that contains other > > > > reworks, stabilization and improvements. > > > > > > > > The second patch in this series isn't considered critical in terms > > > > of functionality, but it depends on the first one and it can be > > > > consider > > > > a fix for PSR residency on VLV/CHV. > > > > > > FYI I recently glanced at the psr code and a few things that left me > > > scratching my head: > > > > Thanks for spotting this. > > > > > - hsw_psr_enable_sink() frobs at the AUX registers without holding > > > the hw_mutex. > > > On SKL+ it seems to use the normal AUX registers here. Before it > > > used > > > the special PSR registers, so that may have been OK, but the SKL+ > > > thing seems rather questionable. > > > > Yes, I agree. I'll take a look. > > > > > - intel_psr_enable() calls intel_psr_activate() on SKL+ but not on > > > HSW/BDW. I'm thinking there should be a comment there to make it > > > clear > > > why, if it's even correct. > > > > Indeed. Also Durga when reviewing mentioned it would be good to make > > only worker calling _activate(). So I will follow-up with a patch to > > fix this. > > > > > > > > > ___ > > 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 v2 3/4] drm/i915: make assert_device_not_suspended more precise
On ke, 2015-11-18 at 16:44 +0200, Imre Deak wrote:
> On ke, 2015-11-18 at 15:37 +0100, Daniel Vetter wrote:
> > On Mon, Nov 09, 2015 at 09:13:45PM +0200, Imre Deak wrote:
> > > Atm, we assert that the device is not suspended after the point
> > > when the
> > > HW is truly put to a suspended state. This is fine, but we can
> > > catch
> > > more problems if we check the RPM refcount. After that one drops
> > > to
> > > zero
> > > we shouldn't access the HW any more, although the actual suspend
> > > may be
> > > delayed. The only complication is that we want to avoid asserts
> > > while
> > > the suspend handler itself is running, so add a flag to handle
> > > this
> > > case.
> >
> > Why do we want to avoid asserts firing while we go through the
> > suspend
> > handler? Calling assert_device_not_suspended from within rpm
> > suspend/resume code sounds like a bug. Where/why does this happen?
>
> Yea, disable_rpm_asserts() is misnamed. Should be
> disable_rpm_wakelock_asserts(). Will change that in the next
> iteration.
Ok, misunderstood your question. assert_device_not_suspended() is
called during runtime suspend since we're accessing the HW until the
point we set dev_priv->pm.suspended = true. Atm this wouldn't trigger a
WARN, since assert_device_not_suspended() only checks pm.suspended and
that will check out fine, but once we start to check HW accesses
against the actual RPM refcount we want to disable the asserts on those
in the handlers, since there the refcount is zero. Hence disabling it
explicitly around the handlers, but we would still keep checking
pm.suspended.
Hope this explains,
Imre
>
> > -Daniel
> >
> > >
> > > While at it remove the HAS_RUNTIME_PM check, the pm.suspended
> > > flag
> > > is
> > > false and the RPM refcount is non-zero on all platforms that
> > > don't
> > > support RPM.
> > >
> > > This caught additional WARNs from the atomic path, those will be
> > > fixed
> > > as a follow-up.
> > >
> > > v2:
> > > - remove the redundant HAS_RUNTIME_PM check (Ville)
> > >
> > > Signed-off-by: Imre Deak
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.c | 5 +
> > > drivers/gpu/drm/i915/i915_drv.h | 5 +
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 14 --
> > > 3 files changed, 22 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 77d183d..caeb218 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -1494,6 +1494,9 @@ static int intel_runtime_suspend(struct
> > > device *device)
> > >
> > > return -EAGAIN;
> > > }
> > > +
> > > + dev_priv->pm.disable_suspended_assert = true;
> > > +
> > > /*
> > > * We are safe here against re-faults, since the fault
> > > handler takes
> > > * an RPM reference.
> > > @@ -1518,6 +1521,8 @@ static int intel_runtime_suspend(struct
> > > device *device)
> > > intel_uncore_forcewake_reset(dev, false);
> > > dev_priv->pm.suspended = true;
> > >
> > > + dev_priv->pm.disable_suspended_assert = false;
> > > +
> > > /*
> > > * FIXME: We really should find a document that
> > > references
> > > the arguments
> > > * used below!
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 5628c5a..43fd341 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1599,6 +1599,11 @@ struct skl_wm_level {
> > > * For more, read the Documentation/power/runtime_pm.txt.
> > > */
> > > struct i915_runtime_pm {
> > > + /*
> > > + * Used for the duration of runtime suspend to avoid
> > > false
> > > device
> > > + * suspended asserts.
> > > + */
> > > + bool disable_suspended_assert;
> > > bool suspended;
> > > bool irqs_enabled;
> > > };
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 4d39b3c..2bdbcd4 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -2120,8 +2120,18 @@ void intel_power_domains_init_hw(struct
> > > drm_i915_private *dev_priv, bool resume)
> > >
> > > void assert_device_not_suspended(struct drm_i915_private
> > > *dev_priv)
> > > {
> > > - WARN_ONCE(HAS_RUNTIME_PM(dev_priv->dev) && dev_priv-
> > > > pm.suspended,
> > > - "Device suspended\n");
> > > + int rpm_usage;
> > > +
> > > + if (dev_priv->pm.disable_suspended_assert)
> > > + return;
> > > +
> > > +#ifdef CONFIG_PM
> > > + rpm_usage = atomic_read(&dev_priv->dev->dev-
> > > > power.usage_count);
> > > +#else
> > > + rpm_usage = 1;
> > > +#endif
> > > +
> > > + WARN_ONCE(dev_priv->pm.suspended || !rpm_usage, "Device
> > > suspended\n");
> > > }
> > >
> > > /**
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/4] drm/i915: make assert_device_not_suspended more precise
On Wed, Nov 18, 2015 at 04:58:46PM +0200, Imre Deak wrote: > On ke, 2015-11-18 at 16:44 +0200, Imre Deak wrote: > > On ke, 2015-11-18 at 15:37 +0100, Daniel Vetter wrote: > > > On Mon, Nov 09, 2015 at 09:13:45PM +0200, Imre Deak wrote: > > > > Atm, we assert that the device is not suspended after the point > > > > when the > > > > HW is truly put to a suspended state. This is fine, but we can > > > > catch > > > > more problems if we check the RPM refcount. After that one drops > > > > to > > > > zero > > > > we shouldn't access the HW any more, although the actual suspend > > > > may be > > > > delayed. The only complication is that we want to avoid asserts > > > > while > > > > the suspend handler itself is running, so add a flag to handle > > > > this > > > > case. > > > > > > Why do we want to avoid asserts firing while we go through the > > > suspend > > > handler? Calling assert_device_not_suspended from within rpm > > > suspend/resume code sounds like a bug. Where/why does this happen? > > > > Yea, disable_rpm_asserts() is misnamed. Should be > > disable_rpm_wakelock_asserts(). Will change that in the next > > iteration. > > Ok, misunderstood your question. assert_device_not_suspended() is > called during runtime suspend since we're accessing the HW until the > point we set dev_priv->pm.suspended = true. Atm this wouldn't trigger a > WARN, since assert_device_not_suspended() only checks pm.suspended and > that will check out fine, but once we start to check HW accesses > against the actual RPM refcount we want to disable the asserts on those > in the handlers, since there the refcount is zero. Hence disabling it > explicitly around the handlers, but we would still keep checking > pm.suspended. That seems like we're mixing up 2 asserts: - assert_device_not_suspended: To be used in runtime_suspend code. - assert_holding_rpm_wakelock (or whatever, I'm bad at names): check the count. What am I missing? -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 v2 3/4] drm/i915: make assert_device_not_suspended more precise
On ke, 2015-11-18 at 16:01 +0100, Daniel Vetter wrote: > On Wed, Nov 18, 2015 at 04:58:46PM +0200, Imre Deak wrote: > > On ke, 2015-11-18 at 16:44 +0200, Imre Deak wrote: > > > On ke, 2015-11-18 at 15:37 +0100, Daniel Vetter wrote: > > > > On Mon, Nov 09, 2015 at 09:13:45PM +0200, Imre Deak wrote: > > > > > Atm, we assert that the device is not suspended after the point > > > > > when the > > > > > HW is truly put to a suspended state. This is fine, but we can > > > > > catch > > > > > more problems if we check the RPM refcount. After that one drops > > > > > to > > > > > zero > > > > > we shouldn't access the HW any more, although the actual suspend > > > > > may be > > > > > delayed. The only complication is that we want to avoid asserts > > > > > while > > > > > the suspend handler itself is running, so add a flag to handle > > > > > this > > > > > case. > > > > > > > > Why do we want to avoid asserts firing while we go through the > > > > suspend > > > > handler? Calling assert_device_not_suspended from within rpm > > > > suspend/resume code sounds like a bug. Where/why does this happen? > > > > > > Yea, disable_rpm_asserts() is misnamed. Should be > > > disable_rpm_wakelock_asserts(). Will change that in the next > > > iteration. > > > > Ok, misunderstood your question. assert_device_not_suspended() is > > called during runtime suspend since we're accessing the HW until the > > point we set dev_priv->pm.suspended = true. Atm this wouldn't trigger a > > WARN, since assert_device_not_suspended() only checks pm.suspended and > > that will check out fine, but once we start to check HW accesses > > against the actual RPM refcount we want to disable the asserts on those > > in the handlers, since there the refcount is zero. Hence disabling it > > explicitly around the handlers, but we would still keep checking > > pm.suspended. > > That seems like we're mixing up 2 asserts: > - assert_device_not_suspended: To be used in runtime_suspend code. > - assert_holding_rpm_wakelock (or whatever, I'm bad at names): check the > count. We call this assert (atm assert_device_not_suspended()) from low level register access helpers, so we can't distinguish between calling one or the other assert depending on whether we are on the rpm suspend path or not. What this patch does is to switch all the places where call assert_device_not_suspended() to assert_rpm_wakelock_held(), since that one provides a bigger coverage. Since this change will also affect the low level reg access functions which are called during rpm suspend, we need to disable part of the assert that checks for the refcount which is known to be zero there. Otherwise assert_rpm_wakelock_held() also includes assert_device_not_suspended(), since that should be true in all other cases. > > What am I missing? > -Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Remove platform specific *_dp_detect() functions
Their logic is exactly the same: check if the digital port is connected
and then call intel_dp_detect_dpcd(). So just put that logic in their
only caller: intel_dp_detect().
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_dp.c | 33 +
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2b28762..9217705 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4534,31 +4534,6 @@ bool intel_digital_port_connected(struct
drm_i915_private *dev_priv,
return g4x_digital_port_connected(dev_priv, port);
}
-static enum drm_connector_status
-ironlake_dp_detect(struct intel_dp *intel_dp)
-{
- struct drm_device *dev = intel_dp_to_dev(intel_dp);
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-
- if (!intel_digital_port_connected(dev_priv, intel_dig_port))
- return connector_status_disconnected;
-
- return intel_dp_detect_dpcd(intel_dp);
-}
-
-static enum drm_connector_status
-g4x_dp_detect(struct intel_dp *intel_dp)
-{
- struct drm_device *dev = intel_dp_to_dev(intel_dp);
- struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-
- if (!intel_digital_port_connected(dev->dev_private, intel_dig_port))
- return connector_status_disconnected;
-
- return intel_dp_detect_dpcd(intel_dp);
-}
-
static struct edid *
intel_dp_get_edid(struct intel_dp *intel_dp)
{
@@ -4631,10 +4606,12 @@ intel_dp_detect(struct drm_connector *connector, bool
force)
/* Can't disconnect eDP, but you can close the lid... */
if (is_edp(intel_dp))
status = edp_detect(intel_dp);
- else if (HAS_PCH_SPLIT(dev))
- status = ironlake_dp_detect(intel_dp);
+ else if (intel_digital_port_connected(to_i915(dev),
+ dp_to_dig_port(intel_dp)))
+ status = intel_dp_detect_dpcd(intel_dp);
else
- status = g4x_dp_detect(intel_dp);
+ status = connector_status_disconnected;
+
if (status != connector_status_connected) {
intel_dp->compliance_test_active = 0;
intel_dp->compliance_test_type = 0;
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Move Braswell stop_machine GGTT insertion workaround
There was a silent conflict between
commit 0a878716265e9af9f697264dc2e858fcc060d833
Author: Daniel Vetter
Date: Thu Oct 15 14:23:01 2015 +0200
drm/i915: restore ggtt double-bind avoidance
and
commit 5bab6f60cb4d1417ad7c599166bcfec87529c1a2
Author: Chris Wilson
Date: Fri Oct 23 18:43:32 2015 +0100
drm/i915: Serialise updates to GGTT with access through GGTT on Braswell
thankfully caught by the extra WARN safegaurd in 0a878716. Since we now
override the GGTT insert_pages callback when installing the aliasing
ppgtt, we assert that the callback is the original ggtt routine.
However, on Braswell we now use a different insertion routine to
serialise access through the GGTT with updating the PTE and hence the
conflict. To avoid the conflict, move the custom insertion routine for
Braswell down a level.
Reported-by: Ville Syrjälä
Cc: Ville Syrjälä
Cc: Michel Thierry
Cc: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 50 +
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4d357e18e5d1..8cd271fa2396 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2386,6 +2386,32 @@ static void gen8_ggtt_insert_entries(struct
i915_address_space *vm,
POSTING_READ(GFX_FLSH_CNTL_GEN6);
}
+struct insert_entries {
+ struct i915_address_space *vm;
+ struct sg_table *st;
+ uint64_t start;
+ enum i915_cache_level level;
+ u32 flags;
+};
+
+static int gen8_ggtt_insert_entries__cb(void *_arg)
+{
+ struct insert_entries *arg = _arg;
+ gen8_ggtt_insert_entries(arg->vm, arg->st,
+arg->start, arg->level, arg->flags);
+ return 0;
+}
+
+static void gen8_ggtt_insert_entries__BKL(struct i915_address_space *vm,
+ struct sg_table *st,
+ uint64_t start,
+ enum i915_cache_level level,
+ u32 flags)
+{
+ struct insert_entries arg = { vm, st, start, level, flags };
+ stop_machine(gen8_ggtt_insert_entries__cb, &arg, NULL);
+}
+
/*
* Binds an object into the global gtt with the specified cache level. The
object
* will be accessible to the GPU via commands whose operands reference offsets
@@ -2534,26 +2560,6 @@ static int ggtt_bind_vma(struct i915_vma *vma,
return 0;
}
-struct ggtt_bind_vma__cb {
- struct i915_vma *vma;
- enum i915_cache_level cache_level;
- u32 flags;
-};
-
-static int ggtt_bind_vma__cb(void *_arg)
-{
- struct ggtt_bind_vma__cb *arg = _arg;
- return ggtt_bind_vma(arg->vma, arg->cache_level, arg->flags);
-}
-
-static int ggtt_bind_vma__BKL(struct i915_vma *vma,
- enum i915_cache_level cache_level,
- u32 flags)
-{
- struct ggtt_bind_vma__cb arg = { vma, cache_level, flags };
- return stop_machine(ggtt_bind_vma__cb, &arg, NULL);
-}
-
static int aliasing_gtt_bind_vma(struct i915_vma *vma,
enum i915_cache_level cache_level,
u32 flags)
@@ -3021,8 +3027,8 @@ static int gen8_gmch_probe(struct drm_device *dev,
dev_priv->gtt.base.bind_vma = ggtt_bind_vma;
dev_priv->gtt.base.unbind_vma = ggtt_unbind_vma;
- if (IS_CHERRYVIEW(dev))
- dev_priv->gtt.base.bind_vma = ggtt_bind_vma__BKL;
+ if (IS_CHERRYVIEW(dev_priv))
+ dev_priv->gtt.base.insert_entries =
gen8_ggtt_insert_entries__BKL;
return ret;
}
--
2.6.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Don't do edp panel detection in g4x_dp_detect()
That call was moved to intel_dp_detect() in
commit d410b56d74bc706f414158cb0149e2a149ee1650
Author: Chris Wilson
Date: Tue Sep 2 20:03:59 2014 +0100
drm/i915/dp: Refactor common eDP lid detection
but it seem to have been resurrected in the following commit, probably
due to a wrong merge conflict resolution.
commit 2a592bec50994597716c633191ed6bf7af14defc
Author: Dave Airlie
Date: Mon Sep 1 16:58:12 2014 +1000
drm/i915: handle G45/GM45 pulse detection connected state.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_dp.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 72d099d..2b28762 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4553,16 +4553,6 @@ g4x_dp_detect(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dp_to_dev(intel_dp);
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
- /* Can't disconnect eDP, but you can close the lid... */
- if (is_edp(intel_dp)) {
- enum drm_connector_status status;
-
- status = intel_panel_detect(dev);
- if (status == connector_status_unknown)
- status = connector_status_connected;
- return status;
- }
-
if (!intel_digital_port_connected(dev->dev_private, intel_dig_port))
return connector_status_disconnected;
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] A wrong DDI encoder override from HDMI to DP at hotplug
Hi, currently a DDI port may register both DP and HDMI and it shares the same encoder. The bug we've got a report is about this encoder type: namely, a machine using DDI port D for HDMI is screwed up because the encoder is switched to DP suddently. The details are found in: http://bugzilla.opensuse.org/show_bug.cgi?id=955190 The problem happens in intel_dp_hpd_pulse(). Since the machine declares both DP and HDMI, the driver registers this callback. And at a hotplug event, the function changes the encoder type like: if (intel_dig_port->base.type != INTEL_OUTPUT_EDP) intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT; After this point, the encoder is handled as DP although the same HDMI monitor is connected. Changing this to exclude INTEL_OUTPUT_HDMI makes the thing working in the bug report above, but I'm not sure what's the right fix. Any suggestions? thanks, Takashi ___ 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: Don't do edp panel detection in g4x_dp_detect()
On Wed, Nov 18, 2015 at 05:19:29PM +0200, Ander Conselvan de Oliveira wrote:
> That call was moved to intel_dp_detect() in
>
> commit d410b56d74bc706f414158cb0149e2a149ee1650
> Author: Chris Wilson
> Date: Tue Sep 2 20:03:59 2014 +0100
>
> drm/i915/dp: Refactor common eDP lid detection
>
> but it seem to have been resurrected in the following commit, probably
> due to a wrong merge conflict resolution.
>
> commit 2a592bec50994597716c633191ed6bf7af14defc
> Author: Dave Airlie
> Date: Mon Sep 1 16:58:12 2014 +1000
>
> drm/i915: handle G45/GM45 pulse detection connected state.
>
> Signed-off-by: Ander Conselvan de Oliveira
>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 10 --
> 1 file changed, 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 72d099d..2b28762 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4553,16 +4553,6 @@ g4x_dp_detect(struct intel_dp *intel_dp)
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>
> - /* Can't disconnect eDP, but you can close the lid... */
> - if (is_edp(intel_dp)) {
> - enum drm_connector_status status;
> -
> - status = intel_panel_detect(dev);
> - if (status == connector_status_unknown)
> - status = connector_status_connected;
> - return status;
> - }
> -
Yup, dead code now.
Reviewed-by: Ville Syrjälä
> if (!intel_digital_port_connected(dev->dev_private, intel_dig_port))
> return connector_status_disconnected;
>
> --
> 2.4.3
>
> ___
> 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/skl: enable PC9/10 power states during suspend-to-idle
During suspend-to-idle we need to keep the DMC firmware active and DC6
enabled, since otherwise we won't reach deep system power states like
PC9/10. The lead for this came from Nivedita who noticed that the
kernel's turbostat tool didn't report any PC9/10 residency change
across an 'echo freeze > /sys/power/state'.
Reported-by: Nivedita Swaminathan
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_drv.c | 44 +++--
drivers/gpu/drm/i915/i915_drv.h | 1 +
2 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6344dfb..649e20a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -624,6 +624,14 @@ static int vlv_resume_prepare(struct drm_i915_private
*dev_priv,
bool rpm_resume);
static int bxt_resume_prepare(struct drm_i915_private *dev_priv);
+static bool suspend_to_idle(struct drm_i915_private *dev_priv)
+{
+#if IS_ENABLED(CONFIG_ACPI_SLEEP)
+ if (acpi_target_system_state() < ACPI_STATE_S3)
+ return true;
+#endif
+ return false;
+}
static int i915_drm_suspend(struct drm_device *dev)
{
@@ -676,11 +684,7 @@ static int i915_drm_suspend(struct drm_device *dev)
i915_save_state(dev);
- opregion_target_state = PCI_D3cold;
-#if IS_ENABLED(CONFIG_ACPI_SLEEP)
- if (acpi_target_system_state() < ACPI_STATE_S3)
- opregion_target_state = PCI_D1;
-#endif
+ opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
intel_opregion_notify_adapter(dev, opregion_target_state);
intel_uncore_forcewake_reset(dev, false);
@@ -701,15 +705,26 @@ static int i915_drm_suspend(struct drm_device *dev)
static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation)
{
struct drm_i915_private *dev_priv = drm_dev->dev_private;
+ bool fw_csr;
int ret;
- intel_power_domains_suspend(dev_priv);
+ fw_csr = suspend_to_idle(dev_priv) && dev_priv->csr.dmc_payload;
+ /*
+* In case of firmware assisted context save/restore don't manually
+* deinit the power domains. This also means the CSR/DMC firmware will
+* stay active, it will power down any HW resources as required and
+* also enable deeper system power states that would be blocked if the
+* firmware was inactive.
+*/
+ if (!fw_csr)
+ intel_power_domains_suspend(dev_priv);
ret = intel_suspend_complete(dev_priv);
if (ret) {
DRM_ERROR("Suspend complete failed: %d\n", ret);
- intel_power_domains_init_hw(dev_priv, true);
+ if (!fw_csr)
+ intel_power_domains_init_hw(dev_priv, true);
return ret;
}
@@ -730,6 +745,8 @@ static int i915_drm_suspend_late(struct drm_device
*drm_dev, bool hibernation)
if (!(hibernation && INTEL_INFO(dev_priv)->gen < 6))
pci_set_power_state(drm_dev->pdev, PCI_D3hot);
+ dev_priv->suspended_to_idle = suspend_to_idle(dev_priv);
+
return 0;
}
@@ -842,8 +859,10 @@ static int i915_drm_resume_early(struct drm_device *dev)
* FIXME: This should be solved with a special hdmi sink device or
* similar so that power domains can be employed.
*/
- if (pci_enable_device(dev->pdev))
- return -EIO;
+ if (pci_enable_device(dev->pdev)) {
+ ret = -EIO;
+ goto out;
+ }
pci_set_master(dev->pdev);
@@ -861,7 +880,12 @@ static int i915_drm_resume_early(struct drm_device *dev)
hsw_disable_pc8(dev_priv);
intel_uncore_sanitize(dev);
- intel_power_domains_init_hw(dev_priv, true);
+
+ if (!(dev_priv->suspended_to_idle && dev_priv->csr.dmc_payload))
+ intel_power_domains_init_hw(dev_priv, true);
+
+out:
+ dev_priv->suspended_to_idle = false;
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a32571f..4a50382 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1882,6 +1882,7 @@ struct drm_i915_private {
u32 chv_phy_control;
u32 suspend_count;
+ bool suspended_to_idle;
struct i915_suspend_saved_registers regfile;
struct vlv_s0ix_state vlv_s0ix_state;
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] A wrong DDI encoder override from HDMI to DP at hotplug
On Wed, Nov 18, 2015 at 04:23:06PM +0100, Takashi Iwai wrote: > Hi, > > currently a DDI port may register both DP and HDMI and it shares the > same encoder. The bug we've got a report is about this encoder type: > namely, a machine using DDI port D for HDMI is screwed up because the > encoder is switched to DP suddently. The details are found in: > http://bugzilla.opensuse.org/show_bug.cgi?id=955190 > > The problem happens in intel_dp_hpd_pulse(). Since the machine > declares both DP and HDMI, the driver registers this callback. And at > a hotplug event, the function changes the encoder type like: > > if (intel_dig_port->base.type != INTEL_OUTPUT_EDP) > intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT; > > After this point, the encoder is handled as DP although the same HDMI > monitor is connected. > > Changing this to exclude INTEL_OUTPUT_HDMI makes the thing working in > the bug report above, but I'm not sure what's the right fix. > > Any suggestions? This has been causing all sorts of troubles for years. I've suggested several times that we should just split the encoder into two, like we have for older platforms, but no one has taken the bait thus far. This particular piece of code to change the encoder type in the hpd_pulse hook came about with the mst code drop from Dave IIRC. At least I never figured out what it's trying to do since -ENOCOMMENT. -- 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] A wrong DDI encoder override from HDMI to DP at hotplug
On Wed, Nov 18, 2015 at 04:23:06PM +0100, Takashi Iwai wrote: > Hi, > > currently a DDI port may register both DP and HDMI and it shares the > same encoder. The bug we've got a report is about this encoder type: > namely, a machine using DDI port D for HDMI is screwed up because the > encoder is switched to DP suddently. The details are found in: > http://bugzilla.opensuse.org/show_bug.cgi?id=955190 > > The problem happens in intel_dp_hpd_pulse(). Since the machine > declares both DP and HDMI, the driver registers this callback. And at > a hotplug event, the function changes the encoder type like: > > if (intel_dig_port->base.type != INTEL_OUTPUT_EDP) > intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT; > > After this point, the encoder is handled as DP although the same HDMI > monitor is connected. > > Changing this to exclude INTEL_OUTPUT_HDMI makes the thing working in > the bug report above, but I'm not sure what's the right fix. > > Any suggestions? The right fix is to adjust the encoder type in the modeset code and stop frobbing it in the detect functions. Userspace tells us at modeset time which connector it wants to us, so it's very well defined. Trying to autodetect it will just result in bugs like the above. Problem is a bit that encoder->type should be tracked in the intel_crtc_config structure (since it can change). I though Maarten is working on this ... -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 i-g-t] Add dmesg capture and dumping to tests and a test for it.
On Mon, Nov 16, 2015 at 03:22:23PM +0200, Joonas Lahtinen wrote:
> Cc: Thomas Wood
> Cc: Chris Wilson
> Cc: Damien Lespiau
> Signed-off-by: Joonas Lahtinen
Given that we have all that in piglit already the commit message is a bit
thin on justification. Why do we need this in igt too? How does this
interact with the piglit dmesg capture?
> ---
> lib/igt_core.c | 113
> ++---
> tests/Makefile.sources | 1 +
> tests/igt_capture.c| 93
> 3 files changed, 200 insertions(+), 7 deletions(-)
> create mode 100644 tests/igt_capture.c
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 04a0ab2..e73175a 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -211,6 +211,8 @@
> * "--help" command line option.
> */
>
> +#define IGT_KMSG_CAPTURE_DUMP_BUF_SIZE 4096
> +
> static unsigned int exit_handler_count;
> const char *igt_interactive_debug;
>
> @@ -247,6 +249,10 @@ enum {
> static int igt_exitcode = IGT_EXIT_SUCCESS;
> static const char *command_str;
>
> +static int igt_kmsg_capture_fd;
> +static char* igt_kmsg_capture_dump_buf;
> +static pthread_mutex_t kmsg_mutex = PTHREAD_MUTEX_INITIALIZER;
> +
> static char* igt_log_domain_filter;
> static struct {
> char *entries[256];
> @@ -312,6 +318,71 @@ static void _igt_log_buffer_dump(void)
> pthread_mutex_unlock(&log_buffer_mutex);
> }
>
> +static void _igt_kmsg_capture_reset(void)
> +{
> + if (igt_kmsg_capture_fd == -1)
> + return;
> +
> + pthread_mutex_lock(&kmsg_mutex);
> +
> + lseek(igt_kmsg_capture_fd, 0, SEEK_END);
> +
> + pthread_mutex_unlock(&kmsg_mutex);
> +}
> +
> +static void _igt_kmsg_capture_dump(void)
> +{
> + size_t nbytes;
> + int nlines;
> + char *p;
> + char *p0;
> + int c;
> +
> + if (igt_kmsg_capture_fd == -1 ||
> + igt_kmsg_capture_dump_buf == NULL)
> + return;
> +
> + pthread_mutex_lock(&kmsg_mutex);
> +
> +
> + nlines = 0;
> + do {
> + errno = 0;
> + nbytes = read(igt_kmsg_capture_fd,
> + igt_kmsg_capture_dump_buf,
> + IGT_KMSG_CAPTURE_DUMP_BUF_SIZE);
> +
> + if (nbytes == -1)
> + continue;
> +
> + if (!nlines)
> + fprintf(stderr, " KMSG \n");
> +
> + p = p0 = strchr(igt_kmsg_capture_dump_buf, ';') + 1;
> + while (p - p0 < nbytes) {
> + if (*p != '\\') {
> + fputc(*p++, stderr);
> + continue;
> + }
> + sscanf(p, "\\x%x", &c);
> + fputc(c, stderr);
> + p += 4;
> + }
> + //fputs(strchr(igt_kmsg_capture_dump_buf, ';') + 1, stderr);
> + nlines++;
> + } while(errno == 0);
> +
> + if (nlines)
> + fprintf(stderr, " END \n");
> + else
> + fprintf(stderr, "No kmsg.\n");
> +
> + if (errno != EAGAIN)
> + fprintf(stderr, "Incomplete kmsg.\n");
> +
> + pthread_mutex_unlock(&kmsg_mutex);
> +}
> +
> __attribute__((format(printf, 1, 2)))
> static void kmsg(const char *format, ...)
> #define KERN_EMER"<0>"
> @@ -330,11 +401,15 @@ static void kmsg(const char *format, ...)
> if (file == NULL)
> return;
>
> + pthread_mutex_lock(&kmsg_mutex);
> +
> va_start(ap, format);
> vfprintf(file, format, ap);
> va_end(ap);
>
> fclose(file);
> +
> + pthread_mutex_unlock(&kmsg_mutex);
> }
>
> static void gettime(struct timespec *ts)
> @@ -527,6 +602,15 @@ static int common_init(int *argc, char **argv,
> int ret = 0;
> const char *env;
>
> + igt_kmsg_capture_fd = open("/dev/kmsg", O_RDWR | O_NONBLOCK);
> + if (igt_kmsg_capture_fd != -1) {
> + igt_kmsg_capture_dump_buf =
> + malloc(IGT_KMSG_CAPTURE_DUMP_BUF_SIZE);
> + if(igt_kmsg_capture_dump_buf == NULL)
> + igt_warn("Unable to allocate memory, "
> + "will not dump kmsg.\n");
> + }
> +
> if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT"))
> __igt_plain_output = true;
>
> @@ -796,6 +880,7 @@ bool __igt_run_subtest(const char *subtest_name)
> igt_debug("Starting subtest: %s\n", subtest_name);
>
> _igt_log_buffer_reset();
> + _igt_kmsg_capture_reset();
>
> gettime(&subtest_time);
> return (in_subtest = subtest_name);
> @@ -972,6 +1057,7 @@ void igt_fail(int exitcode)
> exit(exitcode);
>
> _igt_log_buffer_dump();
> + _igt_kmsg_capture_dump();
>
> if (in_subtest) {
> if (exitcode == IGT_EXIT_TIMEOUT)
> @@ -1072,16 +1158,21 @@ void __igt_fail_assert(const char *domain, const char
> *fi
Re: [Intel-gfx] [PATCH v2 3/4] drm/i915: make assert_device_not_suspended more precise
On Wed, Nov 18, 2015 at 05:11:03PM +0200, Imre Deak wrote: > On ke, 2015-11-18 at 16:01 +0100, Daniel Vetter wrote: > > On Wed, Nov 18, 2015 at 04:58:46PM +0200, Imre Deak wrote: > > > On ke, 2015-11-18 at 16:44 +0200, Imre Deak wrote: > > > > On ke, 2015-11-18 at 15:37 +0100, Daniel Vetter wrote: > > > > > On Mon, Nov 09, 2015 at 09:13:45PM +0200, Imre Deak wrote: > > > > > > Atm, we assert that the device is not suspended after the point > > > > > > when the > > > > > > HW is truly put to a suspended state. This is fine, but we can > > > > > > catch > > > > > > more problems if we check the RPM refcount. After that one drops > > > > > > to > > > > > > zero > > > > > > we shouldn't access the HW any more, although the actual suspend > > > > > > may be > > > > > > delayed. The only complication is that we want to avoid asserts > > > > > > while > > > > > > the suspend handler itself is running, so add a flag to handle > > > > > > this > > > > > > case. > > > > > > > > > > Why do we want to avoid asserts firing while we go through the > > > > > suspend > > > > > handler? Calling assert_device_not_suspended from within rpm > > > > > suspend/resume code sounds like a bug. Where/why does this happen? > > > > > > > > Yea, disable_rpm_asserts() is misnamed. Should be > > > > disable_rpm_wakelock_asserts(). Will change that in the next > > > > iteration. > > > > > > Ok, misunderstood your question. assert_device_not_suspended() is > > > called during runtime suspend since we're accessing the HW until the > > > point we set dev_priv->pm.suspended = true. Atm this wouldn't trigger a > > > WARN, since assert_device_not_suspended() only checks pm.suspended and > > > that will check out fine, but once we start to check HW accesses > > > against the actual RPM refcount we want to disable the asserts on those > > > in the handlers, since there the refcount is zero. Hence disabling it > > > explicitly around the handlers, but we would still keep checking > > > pm.suspended. > > > > That seems like we're mixing up 2 asserts: > > - assert_device_not_suspended: To be used in runtime_suspend code. > > - assert_holding_rpm_wakelock (or whatever, I'm bad at names): check the > > count. > > We call this assert (atm assert_device_not_suspended()) from low level > register access helpers, so we can't distinguish between calling one or > the other assert depending on whether we are on the rpm suspend path or > not. What this patch does is to switch all the places where call > assert_device_not_suspended() to assert_rpm_wakelock_held(), since that > one provides a bigger coverage. Since this change will also affect the > low level reg access functions which are called during rpm suspend, we > need to disable part of the assert that checks for the refcount which > is known to be zero there. > > Otherwise assert_rpm_wakelock_held() also includes > assert_device_not_suspended(), since that should be true in all other > cases. Ok, that makes sense. Should be in the commit message ;-) Instead of cooking our own, what about checking pci_dev->base.power.runtim_status == PM_SUSPENDING plus a comment? -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
