Re: [Intel-gfx] [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect
On Wed, 07 Dec 2016, Manasi Navare wrote:
> The detect_done flag was introduced in the commit
> 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid
The preferred format to cite commits is:
7d23e3c37bb3 ("drm/i915: Cleaning up intel_dp_hpd_pulse")
or
commit 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c
Author: Shubhangi Shrivastava
Date: Wed Mar 30 18:05:23 2016 +0530
drm/i915: Cleaning up intel_dp_hpd_pulse
> multiple detects on hotplug where intel_dp_long_pulse()
> was called from HPD handler as well as in intel_dp_detect.
> So this detect_done flag was required to make sure intel_dp_detect()
> did not call long pulse handler again if it was already been called
> from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb
1015811609c0 ("drm/i915: Move long hpd handling into the hotplug work")
> differs the long hpd handling entirely until the hotplug work runs to
> avoid the double long hpd handling the "detect_done" flag is trying
> to prevent.
>
> So now we do not need to reset the detect_done flag to false in
> intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so
> that intel_dp_detect does a full detect. However if the .detect
> gets called during mode enumeration then we do not need to do a
> full detect. This patch avoids the WARNS_ONS during connected boot
Please include such a backtrace in the commit message; it makes matching
bugs and fixes so much easier.
IIUC the warnings were introduced by 1015811609c0 ("drm/i915: Move long
hpd handling into the hotplug work"), and that's cc: stable, so this one
should be too, along with
Fixes: 1015811609c0 ("drm/i915: Move long hpd handling into the hotplug work")
Cc: sta...@vger.kernel.org
Of course, someone(tm) will still need to make sure this is the right
fix...
BR,
Jani.
> case when it calls intel_dp_check_link_status() due to multiple
> detects and also avoids DP compliance failures. It avoids doing
> a full detect every single time on .detect().
>
> Cc: Ville Syrjala
> Cc: Daniel Vetter
> Cc: Jani Nikula
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/intel_dp.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index db75bb9..9c9277e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct
> drm_i915_private *dev_priv,
> if (!intel_dp->detect_done)
> status = intel_dp_long_pulse(intel_dp->attached_connector);
>
> - intel_dp->detect_done = false;
> -
> return status;
> }
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] dim: Try git-merge --ff-only before git-rebase -i when updating branches.
On Tue, 06 Dec 2016, Maarten Lankhorst wrote: > When a branch can be fast-forwarded, try it first before rebasing. > This will prevent a whole lot of editor windows opening with 'noop' > when running dim ub. > > Signed-off-by: Maarten Lankhorst > --- > dim | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/dim b/dim > index fa63ae8c8a79..b2e6841e23d7 100755 > --- a/dim > +++ b/dim > @@ -1286,9 +1286,7 @@ function dim_update_branches > repo=`branch_to_repo $branch` > remote=`repo_to_remote $repo` > > - if git diff --quiet $remote/$branch; then > - $DRY git rebase > - else > + if ! $DRY git merge --ff-only ; then What does it assume for branches to merge when none are provided? Would it be better to be explicit? Thanks for this, I've been meaning to do this forever... BR, Jani. > $DRY git rebase -i > fi > done -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [GLK MIPI DSI V1 0/9] GLK MIPI DSI VIDEO MODE PATCHES
On Wed, 07 Dec 2016, Madhav Chauhan wrote: > The patches in this list enable MIPI DSI video mode > support for GLK platform. Tesed locally. The patches never made it to the intel-gfx list. They got lost before reaching fdo servers: 11:39 daniels j4ni: ok, so we have never received an attempt (even with greylisting) to send mail from madhav Please try to resend. BR, Jani. > > Deepak M (7): > drm/i915/glk: Add new bit fields in MIPI CTRL register > drm/i915/glk: Program new MIPI DSI PHY registers for GLK > drm/i915/glk: Add MIPIIO Enable/disable sequence > drm/i915: Set the Z inversion overlap field > drm/i915/glk: Add DSI PLL divider range for glk > drm/i915i/glk: Program MIPI_CLOCK_CTRL only for BXT > drm/i915/glk: Program txesc clock divider for GLK > > Madhav Chauhan (1): > drm/i915/glk: Program dphy param reg for GLK > > Vincente Tsou (1): > drm/915: Parsing the missed out DTD fields from the VBT > > drivers/gpu/drm/i915/i915_reg.h| 42 > drivers/gpu/drm/i915/intel_bios.c | 8 +- > drivers/gpu/drm/i915/intel_dsi.c | 157 > - > drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 33 -- > drivers/gpu/drm/i915/intel_dsi_pll.c | 106 +++ > drivers/gpu/drm/i915/intel_vbt_defs.h | 6 +- > 6 files changed, 318 insertions(+), 34 deletions(-) -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: replace platform flags with a platform enum
On to, 2016-12-01 at 14:49 +0200, Jani Nikula wrote: > The platform flags in device info are (mostly) mutually > exclusive. Replace the flags with an enum. Add the platform enum also > for platforms that previously didn't have a flag, and give them codename > logging in dmesg. > > Pineview remains an exception, the platform being G33 for that. > > v2: Sort enum by gen and date > > v3: rebase on geminilake enabling > > Signed-off-by: Jani Nikula Still looking good; Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tools/backlight_helper: #include "config.h"
On Wed, 07 Dec 2016, Mike Frysinger wrote: > From: Mike Gilbert > Uh, which project is this for...? The commit message could use a few words, and you'll need to add Signed-off-by. BR, Jani. > --- > tools/backlight_helper.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/tools/backlight_helper.c b/tools/backlight_helper.c > index a00f0d6bd8a2..aadb8fac92ba 100644 > --- a/tools/backlight_helper.c > +++ b/tools/backlight_helper.c > @@ -1,3 +1,7 @@ > +#ifdef HAVE_CONFIG_H > +#include "config.h" > +#endif > + > #include > #include > #include -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib: Use igt_assert_eq in CHECK_RETURN
On 12/07/2016 08:36 AM, Tomeu Vizoso wrote:
> So that debug logs contain the unexpected value.
>
> Signed-off-by: Tomeu Vizoso
Reviewed-by: Abdiel Janulgue
> ---
> lib/igt_kms.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 989704e14803..1e30ddcc5373 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1681,7 +1681,7 @@ static uint32_t igt_plane_get_fb_gem_handle(igt_plane_t
> *plane)
> #define CHECK_RETURN(r, fail) { \
> if (r && !fail) \
> return r; \
> - igt_assert(r == 0); \
> + igt_assert_eq(r, 0);\
> }
>
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Reorder phys backing storage release
In commit a4f5ea64f0a8 ("drm/i915: Refactor object page API"), I
reordered the object->pages teardown to be more friendly wrt to a
separate obj->mm.lock. However, I overlooked the phys object and left it
with a dangling use-after-free of its phys_handle. Move the allocation
of the phys handle to get_pages and it release to put_pages to prevent
the invalid access and to improve symmetry.
Testcase: igt/drv_selftest/objects
Reported-by: Ville Syrjälä
Fixes: a4f5ea64f0a8 ("drm/i915: Refactor object page API")
Signed-off-by: Chris Wilson
Cc: Ville Syrjälä
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
Cc: intel-gfx@lists.freedesktop.org
---
drivers/gpu/drm/i915/i915_gem.c | 43 +++--
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ca0bb837a57f..f05c48dce7d0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -176,21 +176,29 @@ static struct sg_table *
i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
{
struct address_space *mapping = obj->base.filp->f_mapping;
- char *vaddr = obj->phys_handle->vaddr;
+ drm_dma_handle_t *phys;
struct sg_table *st;
struct scatterlist *sg;
+ char *vaddr;
int i;
if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
return ERR_PTR(-EINVAL);
+ phys = drm_pci_alloc(obj->base.dev, obj->base.size, obj->base.size);
+ if (!phys)
+ return ERR_PTR(-ENOMEM);
+
+ vaddr = phys->vaddr;
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
struct page *page;
char *src;
page = shmem_read_mapping_page(mapping, i);
- if (IS_ERR(page))
- return ERR_CAST(page);
+ if (IS_ERR(page)) {
+ st = ERR_CAST(page);
+ goto err_phys;
+ }
src = kmap_atomic(page);
memcpy(vaddr, src, PAGE_SIZE);
@@ -204,21 +212,29 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object
*obj)
i915_gem_chipset_flush(to_i915(obj->base.dev));
st = kmalloc(sizeof(*st), GFP_KERNEL);
- if (st == NULL)
- return ERR_PTR(-ENOMEM);
+ if (st == NULL) {
+ st = ERR_PTR(-ENOMEM);
+ goto err_phys;
+ }
if (sg_alloc_table(st, 1, GFP_KERNEL)) {
kfree(st);
- return ERR_PTR(-ENOMEM);
+ st = ERR_PTR(-ENOMEM);
+ goto err_phys;
}
sg = st->sgl;
sg->offset = 0;
sg->length = obj->base.size;
- sg_dma_address(sg) = obj->phys_handle->busaddr;
+ sg_dma_address(sg) = phys->busaddr;
sg_dma_len(sg) = obj->base.size;
+ obj->phys_handle = phys;
+ return st;
+
+err_phys:
+ drm_pci_free(obj->base.dev, phys);
return st;
}
@@ -274,12 +290,13 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object
*obj,
sg_free_table(pages);
kfree(pages);
+
+ drm_pci_free(obj->base.dev, obj->phys_handle);
}
static void
i915_gem_object_release_phys(struct drm_i915_gem_object *obj)
{
- drm_pci_free(obj->base.dev, obj->phys_handle);
i915_gem_object_unpin_pages(obj);
}
@@ -540,9 +557,11 @@ int
i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
int align)
{
- drm_dma_handle_t *phys;
int ret;
+ if (align > obj->base.size)
+ return -EINVAL;
+
if (obj->phys_handle) {
if ((unsigned long)obj->phys_handle->vaddr & (align -1))
return -EBUSY;
@@ -564,12 +583,6 @@ i915_gem_object_attach_phys(struct drm_i915_gem_object
*obj,
if (obj->mm.pages)
return -EBUSY;
- /* create a new object */
- phys = drm_pci_alloc(obj->base.dev, obj->base.size, align);
- if (!phys)
- return -ENOMEM;
-
- obj->phys_handle = phys;
obj->ops = &i915_gem_phys_ops;
return i915_gem_object_pin_pages(obj);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: replace platform flags with a platform enum
On Wed, 07 Dec 2016, Joonas Lahtinen wrote: > On to, 2016-12-01 at 14:49 +0200, Jani Nikula wrote: >> The platform flags in device info are (mostly) mutually >> exclusive. Replace the flags with an enum. Add the platform enum also >> for platforms that previously didn't have a flag, and give them codename >> logging in dmesg. >> >> Pineview remains an exception, the platform being G33 for that. >> >> v2: Sort enum by gen and date >> >> v3: rebase on geminilake enabling >> >> Signed-off-by: Jani Nikula > > Still looking good; > > Reviewed-by: Joonas Lahtinen Pushed this one, thanks for the review - which I failed to add to the commit itself. /o\ Apologies! Dare I still ask for review on patch 2/6...? BR, Jani. > > Regards, Joonas -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: rename BROADWATER and CRESTLINE to I965G and I965GM, respectively
Add more consistency to our naming. Pineview remains the outlier. Keep
using code names for gen5+.
v2: rebased
Reviewed-by: Joonas Lahtinen
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 8
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_internal.c | 2 +-
drivers/gpu/drm/i915/i915_pci.c | 4 ++--
drivers/gpu/drm/i915/intel_device_info.c | 4 ++--
drivers/gpu/drm/i915/intel_display.c | 8
drivers/gpu/drm/i915/intel_pm.c | 6 +++---
9 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 95f7a5ef0e36..00a36bf87993 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1734,7 +1734,7 @@ static int i915_sr_status(struct seq_file *m, void
*unused)
if (HAS_PCH_SPLIT(dev_priv))
sr_enabled = I915_READ(WM1_LP_ILK) & WM1_LP_SR_EN;
- else if (IS_CRESTLINE(dev_priv) || IS_G4X(dev_priv) ||
+ else if (IS_I965GM(dev_priv) || IS_G4X(dev_priv) ||
IS_I945G(dev_priv) || IS_I945GM(dev_priv))
sr_enabled = I915_READ(FW_BLC_SELF) & FW_BLC_SELF_EN;
else if (IS_I915GM(dev_priv))
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ae583c79c19f..1a7ac2eefe97 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1033,7 +1033,7 @@ static int i915_driver_init_hw(struct drm_i915_private
*dev_priv)
* behaviour if any general state is accessed within a page above 4GB,
* which also needs to be handled carefully.
*/
- if (IS_BROADWATER(dev_priv) || IS_CRESTLINE(dev_priv)) {
+ if (IS_I965G(dev_priv) || IS_I965GM(dev_priv)) {
ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
if (ret) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dc59670160e1..e5465b330886 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -755,8 +755,8 @@ enum intel_platform {
INTEL_I945GM,
INTEL_G33,
INTEL_PINEVIEW,
- INTEL_BROADWATER,
- INTEL_CRESTLINE,
+ INTEL_I965G,
+ INTEL_I965GM,
INTEL_G4X,
INTEL_IRONLAKE,
INTEL_SANDYBRIDGE,
@@ -2522,8 +2522,8 @@ intel_info(const struct drm_i915_private *dev_priv)
#define IS_I915GM(dev_priv)(INTEL_DEVID(dev_priv) == 0x2592)
#define IS_I945G(dev_priv) (INTEL_DEVID(dev_priv) == 0x2772)
#define IS_I945GM(dev_priv)((dev_priv)->info.platform == INTEL_I945GM)
-#define IS_BROADWATER(dev_priv)((dev_priv)->info.platform ==
INTEL_BROADWATER)
-#define IS_CRESTLINE(dev_priv) ((dev_priv)->info.platform == INTEL_CRESTLINE)
+#define IS_I965G(dev_priv) ((dev_priv)->info.platform == INTEL_I965G)
+#define IS_I965GM(dev_priv)((dev_priv)->info.platform == INTEL_I965GM)
#define IS_GM45(dev_priv) (INTEL_DEVID(dev_priv) == 0x2A42)
#define IS_G4X(dev_priv) ((dev_priv)->info.platform == INTEL_G4X)
#define IS_PINEVIEW_G(dev_priv)(INTEL_DEVID(dev_priv) == 0xa001)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ca0bb837a57f..dd1a34ac830f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3999,7 +3999,7 @@ i915_gem_object_create(struct drm_i915_private *dev_priv,
u64 size)
goto fail;
mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
- if (IS_CRESTLINE(dev_priv) || IS_BROADWATER(dev_priv)) {
+ if (IS_I965GM(dev_priv) || IS_I965G(dev_priv)) {
/* 965gm cannot relocate objects above 4GiB. */
mask &= ~__GFP_HIGHMEM;
mask |= __GFP_DMA32;
diff --git a/drivers/gpu/drm/i915/i915_gem_internal.c
b/drivers/gpu/drm/i915/i915_gem_internal.c
index 08d26306d40e..863e505f 100644
--- a/drivers/gpu/drm/i915/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/i915_gem_internal.c
@@ -71,7 +71,7 @@ i915_gem_object_get_pages_internal(struct drm_i915_gem_object
*obj)
#endif
gfp = GFP_KERNEL | __GFP_HIGHMEM | __GFP_RECLAIMABLE;
- if (IS_CRESTLINE(i915) || IS_BROADWATER(i915)) {
+ if (IS_I965GM(i915) || IS_I965G(i915)) {
/* 965gm cannot relocate objects above 4GiB. */
gfp &= ~__GFP_HIGHMEM;
gfp |= __GFP_DMA32;
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index c0bcf323dbf0..99e8eed0a1fc 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -156,14 +156,14 @@ static const struct intel_device_info intel_pineview_info
= {
static const struct intel_device_info intel_i965g_info = {
GEN4_FEATURES,
- .platform = INTEL_BROADWATER,
+ .platform
Re: [Intel-gfx] [PATCH 2/6] drm/i915: keep intel device info structs in gen based order
On ke, 2016-11-30 at 17:43 +0200, Jani Nikula wrote: > Move G33 and Pineview higher up in the list. Add a couple of blank lines > for OCD while at it. > > Signed-off-by: Jani Nikula Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v10 17/21] lib/sw_sync: Add igt_require_sw_sync to enable skipping on no sw_sync support
On Tue, Dec 06, 2016 at 09:52:09PM -0500, Robert Foss wrote:
> Add igt_require_sw_sync to provide tests to skip if sw_sync support isn't
> available on the host machine.
>
> Signed-off-by: Robert Foss
> Reviewed-by: Tomeu Vizoso
> ---
> lib/sw_sync.c | 22 ++
> lib/sw_sync.h | 1 +
> 2 files changed, 23 insertions(+)
>
> diff --git a/lib/sw_sync.c b/lib/sw_sync.c
> index a2168f78..d4ecc898 100644
> --- a/lib/sw_sync.c
> +++ b/lib/sw_sync.c
> @@ -194,3 +194,25 @@ int sync_fence_count_status(int fd, int status)
> igt_assert_f(count >= 0, "No fences with supplied status found");
> return count;
> }
> +
> +static bool kernel_has_sw_sync(void)
> +{
> + bool err;
> +
> + igt_ignore_warn(system("/sbin/modprobe -s r sw_sync"));
> +
> + err = false;
> + if (access(DEVFS_SW_SYNC, R_OK | W_OK) < 0) {
> + char buf[128];
> +
> + snprintf(buf, sizeof(buf), "%s/sw_sync", igt_debugfs_mount());
> + err = access(DEBUGFS_SW_SYNC, R_OK | W_OK) < 0;
> + }
Did you mean access(buf, R_OK | W_OK) here?
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC i-g-t v3 5/5] Add support for hotplug testing with the Chamelium
Hi Lyude, this looks very good. Some minor comments below. Regards, Tomeu On 1 December 2016 at 02:24, Lyude wrote: > For the purpose of testing things such as hotplugging and bad monitors, > the ChromeOS team ended up designing a neat little device known as the > Chamelium. More information on this can be found here: > > https://www.chromium.org/chromium-os/testing/chamelium > > This adds support for a couple of things to intel-gpu-tools: > - igt library functions for connecting to udev and monitoring it for >hotplug events, loosely based off of the unfinished hotplugging >implementation in testdisplay > - Library functions for controlling the chamelium in tests using >xmlrpc. A couple of RPC calls were ommitted here, mainly because they >didn't seem very useful for our needs (yet) > - A set of basic tests using the chamelium. > > Signed-off-by: Lyude > > Changes since v1: > - Don't try to guess connector mappings, have the user specify them > manually using a configuration file > - Open DRM fd using DRIVER_ANY, not DRIVER_INTEL > - Lower the hotplug timeout a little bit, since 30 seconds was leftover > from debugging these tests anyway > - Don't try to keep track of the original state of the chamelium ports, > and just leave them plugged in after each run. This makes more sense > to me, since I'd imagine in automated testing setups using chameliums > that all of the extra monitors will probably be provided by the > Chamelium to begin with, so keeping them plugged in would make sure > tests running afterwards that require >1 monitor don't get skipped. > - Add wait_for_connector() to the chamelium tests. After some more > testing, I found that depending on the system some tests would throw > false negatives due to us not waiting long enough for the system to > detect that we connected something to it. This mainly happened with > VGA connectors, since their lack of HPD makes them take significantly > longer for the hardware to notice. wait_for_connector() fixes this by > continually reprobing the status of the desired connector (without > relying on a hpd event happening, since that might never come) until > we get what we want, or we time out and fail. > - Use kmstest_get_property() for retrieving EDIDs instead of doing it by > hand > - Don't hardcode PIPE_A for bringing up the display, use kmstest to find > an appropriate CRTC to use. > Changes since v2: > - Fix incorrect usage of the list helpers when recording new EDIDs > - Add missing documentation > - Make sure documentation actually appears > - Since we finally got video capture working, add CRC functions and fix > the ones we couldn't actually test before > - In the exit handler, reset the xmlrpc env so we can properly reset the > Chamelium even after an RPC error > - Make sure compiling without Chamelium support still works > --- > configure.ac | 13 + > .../intel-gpu-tools/intel-gpu-tools-docs.xml | 1 + > lib/Makefile.am| 4 +- > lib/Makefile.sources | 10 +- > lib/igt.h | 4 + > lib/igt_chamelium.c| 849 > + > lib/igt_chamelium.h| 73 ++ > scripts/run-tests.sh | 4 +- > tests/Makefile.am | 5 +- > tests/Makefile.sources | 9 +- > tests/chamelium.c | 407 ++ > 11 files changed, 1371 insertions(+), 8 deletions(-) > create mode 100644 lib/igt_chamelium.c > create mode 100644 lib/igt_chamelium.h > create mode 100644 tests/chamelium.c > > diff --git a/configure.ac b/configure.ac > index b62b8fc..1920609 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -263,6 +263,18 @@ if test "x$with_libunwind" = xyes; then > AC_MSG_ERROR([libunwind not found. Use > --without-libunwind to disable libunwind support.])) > fi > > +# enable support for using the chamelium > +AC_ARG_ENABLE(chamelium, > + AS_HELP_STRING([--disable-chamelium], > +[Build tests without chamelium support]), > + [], [enable_chamelium=yes]) > + > +AM_CONDITIONAL(HAVE_CHAMELIUM, [test "x$enable_chamelium" = xyes]) > +if test "x$enable_chamelium" = xyes; then > + AC_DEFINE(HAVE_CHAMELIUM, 1, [chamelium suport]) > + PKG_CHECK_MODULES(XMLRPC, xmlrpc_client) > +fi > + IMO libxmlrpc is a very common dependency and thus adding the possibility of disabling Chamelium support isn't worth the cruft we have to add later. Plus, alternative build configurations tend to break and remain broken as most people hacking on the project won't be using them. > # enable debug symbols > AC_ARG_ENABLE(debug, > AS_HELP_STRING([--disable-debug], > @@ -360,6 +
Re: [Intel-gfx] Does display engine read contents from LLC of scanout buffer?
Hi Ville, Thanks for your useful info. How about L3 cache? In my scenario, Beignet will use Read Write Data Port to write data (typed write) to bo. So the cache path is L3 -> LLC -> memory. So only leave LLC cache ability the same as PTEs is not enough. In order to make data access efficient, I set L3 to WB. So is there a way to flush L3 cache to memory? Thanks, Chuanbo Weng -Original Message- From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] Sent: Friday, November 25, 2016 3:07 AM To: Weng, Chuanbo Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] Does display engine read contents from LLC of scanout buffer? On Thu, Nov 24, 2016 at 06:20:22PM +, Weng, Chuanbo wrote: > Hi all, >I have a question of display (forgive me if it's too simple > because I'm not familiar with display): > >My scenario is as below: > gbm_bo_create to create gbm_bo > Get its handle > drmModeAddFB to specify this bo as scanout > buffer > do rendering to this bo by OpenCL(Beignet) > drmModePageFlip to do page flip > >Does display engine directly read contents from scanout > buffer or read contents from LLC of this scanout buffer? The display engine sits between the LLC and main memory effectively, so the answer is that it always reads directly from memory. When you make a bo a scanout buffer the kernel will flush the caches and reconfigure the PTEs to UC/WC so that subsequent rendering will hit memory directly. And that also means you should never override potential scanout buffers to WB via MOCS, and instead you should leave the choice up to the PTEs. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH RFC i-g-t] igt: Add a dpms property test
This testcase will set the dpms drm property to
DRM_MODE_DPMS_ON and DPMS_MODE_OFF and check that the
property values was updated and that the new state corresponds
to the crtc active property.
Signed-off-by: Marta Lofstedt
---
tests/Makefile.sources | 1 +
tests/drm_dpms.c | 168 +
2 files changed, 169 insertions(+)
create mode 100644 tests/drm_dpms.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 04dd2d5..6d6e3db 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -14,6 +14,7 @@ VC4_TESTS_M = \
TESTS_progs_M = \
core_get_client_auth \
drm_mm \
+ drm_dpms \
drv_getparams_basic \
drv_selftest \
drv_suspend \
diff --git a/tests/drm_dpms.c b/tests/drm_dpms.c
new file mode 100644
index 000..0175e7b
--- /dev/null
+++ b/tests/drm_dpms.c
@@ -0,0 +1,168 @@
+/*
+ * Copyright © 2016 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.
+ */
+
+#include "igt.h"
+
+static void prepare_pipe(igt_display_t *display, enum pipe pipe,
+igt_output_t *output, struct igt_fb *fb)
+{
+ drmModeModeInfo *mode = igt_output_get_mode(output);
+
+ igt_create_pattern_fb(display->drm_fd,
+ mode->hdisplay,
+ mode->vdisplay,
+ DRM_FORMAT_XRGB,
+ LOCAL_DRM_FORMAT_MOD_NONE,
+ fb);
+
+ igt_output_set_pipe(output, pipe);
+
+ igt_plane_set_fb(igt_output_get_plane(output, IGT_PLANE_PRIMARY), fb);
+
+ igt_display_commit2(display,
+ display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
+}
+
+static void cleanup_pipe(igt_display_t *display, enum pipe pipe,
+igt_output_t *output, struct igt_fb *fb)
+{
+ igt_plane_t *plane;
+
+ for_each_plane_on_pipe(display, pipe, plane)
+ igt_plane_set_fb(plane, NULL);
+
+ igt_output_set_pipe(output, PIPE_NONE);
+
+ igt_display_commit2(display,
+ display->is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
+
+ igt_remove_fb(display->drm_fd, fb);
+}
+
+static void test_dpms(igt_display_t *display, drmModeConnector *connector,
+ int crtc_id)
+{
+ uint64_t dpms_state, active;
+
+ kmstest_get_property(display->drm_fd, connector->connector_id,
+DRM_MODE_OBJECT_CONNECTOR, "DPMS",
+NULL, &dpms_state, NULL);
+
+ if (dpms_state == DRM_MODE_DPMS_OFF) {
+ kmstest_set_connector_dpms(display->drm_fd, connector,
+ DRM_MODE_DPMS_ON);
+ kmstest_get_property(display->drm_fd, connector->connector_id,
+DRM_MODE_OBJECT_CONNECTOR, "DPMS",
+NULL, &dpms_state, NULL);
+ igt_assert_eq(dpms_state, DRM_MODE_DPMS_ON);
+ kmstest_get_property(display->drm_fd, crtc_id,
+DRM_MODE_OBJECT_CRTC, "ACTIVE",
+NULL, &active, NULL);
+ igt_assert(active);
+ } else {
+ kmstest_set_connector_dpms(display->drm_fd, connector,
+ DRM_MODE_DPMS_OFF);
+ kmstest_get_property(display->drm_fd, connector->connector_id,
+DRM_MODE_OBJECT_CONNECTOR, "DPMS",
+NULL, &dpms_state, NULL);
+ igt_assert_eq(dpms_state, DRM_MODE_DPMS_OFF);
+ kmstest_get_property(display->drm_fd, crtc_id,
+DRM_MODE_OBJECT_CRTC, "ACTIVE",
+NULL, &active, NULL);
+ igt_assert(!ac
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Reorder phys backing storage release
== Series Details == Series: drm/i915: Reorder phys backing storage release URL : https://patchwork.freedesktop.org/series/16468/ State : success == Summary == Series 16468v1 drm/i915: Reorder phys backing storage release https://patchwork.freedesktop.org/api/1.0/series/16468/revisions/1/mbox/ fi-bdw-5557u total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28 fi-bsw-n3050 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-bxt-t5700 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-byt-j1900 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-hsw-4770 total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-hsw-4770r total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-ilk-650 total:247 pass:181 dwarn:0 dfail:0 fail:0 skip:66 fi-ivb-3520m total:247 pass:213 dwarn:0 dfail:0 fail:0 skip:34 fi-ivb-3770 total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-kbl-7500u total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-skl-6260u total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-skl-6700hqtotal:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-skl-6700k total:247 pass:210 dwarn:3 dfail:0 fail:0 skip:34 fi-skl-6770hqtotal:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-snb-2520m total:247 pass:202 dwarn:0 dfail:0 fail:0 skip:45 fi-snb-2600 total:247 pass:201 dwarn:0 dfail:0 fail:0 skip:46 7955526172a2154393986eb78de44e61cabd81f3 drm-tip: 2016y-12m-07d-10h-06m-32s UTC integration manifest bd87780 drm/i915: Reorder phys backing storage release == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3213/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH RFC i-g-t] igt: Add a dpms property test
On Wed, Dec 07, 2016 at 01:42:59PM +0200, Marta Lofstedt wrote:
> This testcase will set the dpms drm property to
> DRM_MODE_DPMS_ON and DPMS_MODE_OFF and check that the
> property values was updated and that the new state corresponds
> to the crtc active property.
>
> Signed-off-by: Marta Lofstedt
> ---
> tests/Makefile.sources | 1 +
> tests/drm_dpms.c | 168
> +
> 2 files changed, 169 insertions(+)
> create mode 100644 tests/drm_dpms.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 04dd2d5..6d6e3db 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -14,6 +14,7 @@ VC4_TESTS_M = \
> TESTS_progs_M = \
> core_get_client_auth \
> drm_mm \
> + drm_dpms \
This should be a kms_ test
> drv_getparams_basic \
> drv_selftest \
> drv_suspend \
> +static void dpms_property(igt_display_t *display)
> +{
> + enum pipe pipe;
> + igt_output_t *output;
> +
> + for_each_connected_output(display, output) {
> + bool found = false;
> +
> + for_each_pipe(display, pipe) {
> + if (!igt_pipe_connector_valid(pipe, output))
> + continue;
> +
> + found = true;
> + run_dpms_test(display, pipe, output);
> + break;
> + }
> +
> + igt_assert_f(found,
> + "Connected output should have at least 1 valid
> crtc\n");
That's a requirement for the test to run (igt_require_f) not an
assertion. If the hardware doesn't meet the test criteria igt_require_f
will SKIP, but igt_assert_f will FAIL.
> + }
> +}
> +
> +igt_main
> +{
> + igt_display_t display;
> +
> + igt_skip_on_simulation();
> +
> + igt_fixture {
> + display.drm_fd = drm_open_driver_master(DRIVER_ANY);
> +
> + kmstest_set_vt_graphics_mode();
> +
> + igt_display_init(&display, display.drm_fd);
> + }
> +
> + igt_subtest("dpms_property")
> + dpms_property(&display);
Doesn't this want to a subtest for each connector?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reorder phys backing storage release
On ke, 2016-12-07 at 10:07 +, Chris Wilson wrote:
> In commit a4f5ea64f0a8 ("drm/i915: Refactor object page API"), I
> reordered the object->pages teardown to be more friendly wrt to a
> separate obj->mm.lock. However, I overlooked the phys object and left it
> with a dangling use-after-free of its phys_handle. Move the allocation
> of the phys handle to get_pages and it release to put_pages to prevent
> the invalid access and to improve symmetry.
>
> Testcase: igt/drv_selftest/objects
> Reported-by: Ville Syrjälä
> Fixes: a4f5ea64f0a8 ("drm/i915: Refactor object page API")
> Signed-off-by: Chris Wilson
> Cc: Ville Syrjälä
> Cc: Tvrtko Ursulin
> Cc: Joonas Lahtinen
> Cc: intel-gfx@lists.freedesktop.org
> i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
> {
> struct address_space *mapping = obj->base.filp->f_mapping;
> - char *vaddr = obj->phys_handle->vaddr;
> + drm_dma_handle_t *phys;
> struct sg_table *st;
> struct scatterlist *sg;
> + char *vaddr;
> int i;
>
> if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
> return ERR_PTR(-EINVAL);
>
> + phys = drm_pci_alloc(obj->base.dev, obj->base.size, obj->base.size);
Aligning to object size sounds bit rough without any comments.
> @@ -204,21 +212,29 @@ i915_gem_object_get_pages_phys(struct
> drm_i915_gem_object *obj)
> i915_gem_chipset_flush(to_i915(obj->base.dev));
>
> st = kmalloc(sizeof(*st), GFP_KERNEL);
> - if (st == NULL)
> - return ERR_PTR(-ENOMEM);
> + if (st == NULL) {
Could convert to (!st) when touching, pleases checkpatch.pl.
With the align propagated or explained in a comment;
Reviewed-by: Joonas Lahtinen
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: introduce platform enum (rev3)
== Series Details == Series: drm/i915: introduce platform enum (rev3) URL : https://patchwork.freedesktop.org/series/16170/ State : success == Summary == Series 16170v3 drm/i915: introduce platform enum https://patchwork.freedesktop.org/api/1.0/series/16170/revisions/3/mbox/ fi-bdw-5557u total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28 fi-bsw-n3050 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-bxt-t5700 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-byt-j1900 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-hsw-4770 total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-hsw-4770r total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-ilk-650 total:247 pass:181 dwarn:0 dfail:0 fail:0 skip:66 fi-ivb-3520m total:247 pass:213 dwarn:0 dfail:0 fail:0 skip:34 fi-ivb-3770 total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-kbl-7500u total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-skl-6260u total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-skl-6700hqtotal:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-skl-6700k total:247 pass:210 dwarn:3 dfail:0 fail:0 skip:34 fi-skl-6770hqtotal:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-snb-2520m total:247 pass:202 dwarn:0 dfail:0 fail:0 skip:45 fi-snb-2600 total:247 pass:201 dwarn:0 dfail:0 fail:0 skip:46 7955526172a2154393986eb78de44e61cabd81f3 drm-tip: 2016y-12m-07d-10h-06m-32s UTC integration manifest d352e11 drm/i915: use platform enum instead of duplicating PCI ID if possible f301e5f drm/i915: give G45 and GM45 their own platform enums 86049bd drm/i915: add some more "i" in platform names for consistency 044813e drm/i915: rename BROADWATER and CRESTLINE to I965G and I965GM, respectively 83a0187 drm/i915: keep intel device info structs in gen based order == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3214/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reorder phys backing storage release
On Wed, Dec 07, 2016 at 02:10:34PM +0200, Joonas Lahtinen wrote:
> On ke, 2016-12-07 at 10:07 +, Chris Wilson wrote:
> > In commit a4f5ea64f0a8 ("drm/i915: Refactor object page API"), I
> > reordered the object->pages teardown to be more friendly wrt to a
> > separate obj->mm.lock. However, I overlooked the phys object and left it
> > with a dangling use-after-free of its phys_handle. Move the allocation
> > of the phys handle to get_pages and it release to put_pages to prevent
> > the invalid access and to improve symmetry.
> >
> > Testcase: igt/drv_selftest/objects
> > Reported-by: Ville Syrjälä
> > Fixes: a4f5ea64f0a8 ("drm/i915: Refactor object page API")
> > Signed-off-by: Chris Wilson
> > Cc: Ville Syrjälä
> > Cc: Tvrtko Ursulin
> > Cc: Joonas Lahtinen
> > Cc: intel-gfx@lists.freedesktop.org
>
>
>
> > i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
> > {
> > struct address_space *mapping = obj->base.filp->f_mapping;
> > - char *vaddr = obj->phys_handle->vaddr;
> > + drm_dma_handle_t *phys;
> > struct sg_table *st;
> > struct scatterlist *sg;
> > + char *vaddr;
> > int i;
> >
> > if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
> > return ERR_PTR(-EINVAL);
> >
> > + phys = drm_pci_alloc(obj->base.dev, obj->base.size, obj->base.size);
>
> Aligning to object size sounds bit rough without any comments.
Not that rough really, since the phys allocation will be power-of-two
aligned both in size and address. The alignment can only be power-of-two
up to the size of the object (rounded up).
The choice is adding an extra parameter for a rarely used feature or
just picking an alignment that must work for all callers. Since the
objects are either 4k or 16k and either 4k or 16k aligned (though only
830 cursors are 16k aligned), the alignment falls out of the actual
buddy allocation for the contiguous pages.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: Allow CAP_PRIME on !MODESET
vgem (and our igt tests using vgem) need this. I suspect etnaviv will
fare similarly.
Fixes: d5264ed3823a ("drm: Return -ENOTSUPP when called for KMS cap with a
non-KMS driver")
Cc: Michel Dänzer
Cc: Alex Deucher
Cc: Chris Wilson
Signed-off-by: Daniel Vetter
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_ioctl.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 0a900bd4575d..60bf95644739 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -234,10 +234,15 @@ static int drm_getcap(struct drm_device *dev, void *data,
struct drm_file *file_
req->value = 0;
- /* Only one cap makes sense with a UMS driver: */
- if (req->capability == DRM_CAP_TIMESTAMP_MONOTONIC) {
+ /* Only some caps make sense with UMS/render-only drivers. */
+ switch (req->capability) ) {
+ case DRM_CAP_TIMESTAMP_MONOTONIC:
req->value = drm_timestamp_monotonic;
return 0;
+ case DRM_CAP_PRIME:
+ req->value |= dev->driver->prime_fd_to_handle ?
DRM_PRIME_CAP_IMPORT : 0;
+ req->value |= dev->driver->prime_handle_to_fd ?
DRM_PRIME_CAP_EXPORT : 0;
+ return 0;
}
/* Other caps only work with KMS drivers */
@@ -258,10 +263,6 @@ static int drm_getcap(struct drm_device *dev, void *data,
struct drm_file *file_
case DRM_CAP_DUMB_PREFER_SHADOW:
req->value = dev->mode_config.prefer_shadow;
break;
- case DRM_CAP_PRIME:
- req->value |= dev->driver->prime_fd_to_handle ?
DRM_PRIME_CAP_IMPORT : 0;
- req->value |= dev->driver->prime_handle_to_fd ?
DRM_PRIME_CAP_EXPORT : 0;
- break;
case DRM_CAP_ASYNC_PAGE_FLIP:
req->value = dev->mode_config.async_page_flip;
break;
--
2.10.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Allow CAP_PRIME on !MODESET
On Wed, Dec 07, 2016 at 02:13:23PM +0100, Daniel Vetter wrote:
> vgem (and our igt tests using vgem) need this. I suspect etnaviv will
> fare similarly.
>
> Fixes: d5264ed3823a ("drm: Return -ENOTSUPP when called for KMS cap with a
> non-KMS driver")
> Cc: Michel Dänzer
> Cc: Alex Deucher
> Cc: Chris Wilson
> Signed-off-by: Daniel Vetter
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_ioctl.c | 13 +++--
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 0a900bd4575d..60bf95644739 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -234,10 +234,15 @@ static int drm_getcap(struct drm_device *dev, void
> *data, struct drm_file *file_
>
> req->value = 0;
>
> - /* Only one cap makes sense with a UMS driver: */
> - if (req->capability == DRM_CAP_TIMESTAMP_MONOTONIC) {
> + /* Only some caps make sense with UMS/render-only drivers. */
> + switch (req->capability) ) {
> + case DRM_CAP_TIMESTAMP_MONOTONIC:
> req->value = drm_timestamp_monotonic;
> return 0;
> + case DRM_CAP_PRIME:
> + req->value |= dev->driver->prime_fd_to_handle ?
> DRM_PRIME_CAP_IMPORT : 0;
> + req->value |= dev->driver->prime_handle_to_fd ?
> DRM_PRIME_CAP_EXPORT : 0;
> + return 0;
Slightly changes old behaviour but shouldn't we also be checking
driver->features?
if (drm_core_check_feature(dev, DRIVER_PRIME)) {
req->value |= dev->driver->prime_fd_to_handle ? DRM_PRIME_CAP_IMPORT :
0;
req->value |= dev->driver->prime_handle_to_fd ? DRM_PRIME_CAP_EXPORT :
0;
}
Paranoia says second patch as a potential change of abi. So,
Reviewed-by: Chris Wilson
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: replace platform flags with a platform enum
On Wed, 07 Dec 2016, Jani Nikula wrote: > On Wed, 07 Dec 2016, Joonas Lahtinen wrote: >> On to, 2016-12-01 at 14:49 +0200, Jani Nikula wrote: >>> The platform flags in device info are (mostly) mutually >>> exclusive. Replace the flags with an enum. Add the platform enum also >>> for platforms that previously didn't have a flag, and give them codename >>> logging in dmesg. >>> >>> Pineview remains an exception, the platform being G33 for that. >>> >>> v2: Sort enum by gen and date >>> >>> v3: rebase on geminilake enabling >>> >>> Signed-off-by: Jani Nikula >> >> Still looking good; >> >> Reviewed-by: Joonas Lahtinen > > Pushed this one, thanks for the review - which I failed to add to the > commit itself. /o\ Apologies! > > Dare I still ask for review on patch 2/6...? And pushed the rest, thanks for the review. BR, Jani. > > BR, > Jani. > > >> >> Regards, Joonas -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [CI] drm/i915: Reorder phys backing storage release
In commit a4f5ea64f0a8 ("drm/i915: Refactor object page API"), I
reordered the object->pages teardown to be more friendly wrt to a
separate obj->mm.lock. However, I overlooked the phys object and left it
with a dangling use-after-free of its phys_handle. Move the allocation
of the phys handle to get_pages and it release to put_pages to prevent
the invalid access and to improve symmetry.
v2: Add commentary about always aligning to page size.
Testcase: igt/drv_selftest/objects
Reported-by: Ville Syrjälä
Fixes: a4f5ea64f0a8 ("drm/i915: Refactor object page API")
Signed-off-by: Chris Wilson
Cc: Ville Syrjälä
Cc: Tvrtko Ursulin
Cc: Joonas Lahtinen
Reviewed-by: Joonas Lahtinen
---
drivers/gpu/drm/i915/i915_gem.c | 53 ++---
1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index dd1a34ac830f..9794dd655877 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -176,21 +176,35 @@ static struct sg_table *
i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
{
struct address_space *mapping = obj->base.filp->f_mapping;
- char *vaddr = obj->phys_handle->vaddr;
+ drm_dma_handle_t *phys;
struct sg_table *st;
struct scatterlist *sg;
+ char *vaddr;
int i;
if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
return ERR_PTR(-EINVAL);
+ /* Always aligning to the object size, allows a single allocation
+* to handle all possible callers, and given typical object sizes,
+* the alignment of the buddy allocation will naturally match.
+*/
+ phys = drm_pci_alloc(obj->base.dev,
+obj->base.size,
+roundup_pow_of_two(obj->base.size));
+ if (!phys)
+ return ERR_PTR(-ENOMEM);
+
+ vaddr = phys->vaddr;
for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
struct page *page;
char *src;
page = shmem_read_mapping_page(mapping, i);
- if (IS_ERR(page))
- return ERR_CAST(page);
+ if (IS_ERR(page)) {
+ st = ERR_CAST(page);
+ goto err_phys;
+ }
src = kmap_atomic(page);
memcpy(vaddr, src, PAGE_SIZE);
@@ -204,21 +218,29 @@ i915_gem_object_get_pages_phys(struct drm_i915_gem_object
*obj)
i915_gem_chipset_flush(to_i915(obj->base.dev));
st = kmalloc(sizeof(*st), GFP_KERNEL);
- if (st == NULL)
- return ERR_PTR(-ENOMEM);
+ if (st == NULL) {
+ st = ERR_PTR(-ENOMEM);
+ goto err_phys;
+ }
if (sg_alloc_table(st, 1, GFP_KERNEL)) {
kfree(st);
- return ERR_PTR(-ENOMEM);
+ st = ERR_PTR(-ENOMEM);
+ goto err_phys;
}
sg = st->sgl;
sg->offset = 0;
sg->length = obj->base.size;
- sg_dma_address(sg) = obj->phys_handle->busaddr;
+ sg_dma_address(sg) = phys->busaddr;
sg_dma_len(sg) = obj->base.size;
+ obj->phys_handle = phys;
+ return st;
+
+err_phys:
+ drm_pci_free(obj->base.dev, phys);
return st;
}
@@ -274,12 +296,13 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object
*obj,
sg_free_table(pages);
kfree(pages);
+
+ drm_pci_free(obj->base.dev, obj->phys_handle);
}
static void
i915_gem_object_release_phys(struct drm_i915_gem_object *obj)
{
- drm_pci_free(obj->base.dev, obj->phys_handle);
i915_gem_object_unpin_pages(obj);
}
@@ -540,15 +563,13 @@ int
i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
int align)
{
- drm_dma_handle_t *phys;
int ret;
- if (obj->phys_handle) {
- if ((unsigned long)obj->phys_handle->vaddr & (align -1))
- return -EBUSY;
+ if (align > obj->base.size)
+ return -EINVAL;
+ if (obj->ops == &i915_gem_phys_ops)
return 0;
- }
if (obj->mm.madv != I915_MADV_WILLNEED)
return -EFAULT;
@@ -564,12 +585,6 @@ i915_gem_object_attach_phys(struct drm_i915_gem_object
*obj,
if (obj->mm.pages)
return -EBUSY;
- /* create a new object */
- phys = drm_pci_alloc(obj->base.dev, obj->base.size, align);
- if (!phys)
- return -ENOMEM;
-
- obj->phys_handle = phys;
obj->ops = &i915_gem_phys_ops;
return i915_gem_object_pin_pages(obj);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Catch non-existent registers in find_fw_domain
Add WARN_ON to find_fw_domain to registers related to uninitialized hardware. Cc: Imre Deak Cc: Wang Elaine Cc: Chris Wilson Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_uncore.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 07779d0..b20b58e 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -625,7 +625,12 @@ find_fw_domain(struct drm_i915_private *dev_priv, u32 offset) dev_priv->uncore.fw_domains_table_entries, fw_range_cmp); - return entry ? entry->domains : 0; + if (!entry) + return 0; + + WARN_ON(entry->domains & ~dev_priv->uncore.fw_domains); + + return entry->domains; } static void -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Respect ring_mask when initializing forcewake domains
On ma, 2016-12-05 at 21:16 +0800, Wang Elaine wrote: > From: Elaine Wang > > Some platforms don't have render or blitter. So no need to call > render domain or blitter domain forcewake init function. > > Cc: Joonas Lahtinen > Signed-off-by: Elaine Wang I sent an additional patch to the mailing list that would catch any rogue accesses. If you could test with it in combination to this change, I would be ready to merge this patch if no problems arise. The patch is; Author: Joonas Lahtinen Date: Wed Dec 7 15:40:13 2016 +0200 drm/i915: Catch non-existent registers in find_fw_domain Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reorder phys backing storage release
On ke, 2016-12-07 at 12:27 +, Chris Wilson wrote:
> Since the
> objects are either 4k or 16k and either 4k or 16k aligned (though only
> 830 cursors are 16k aligned), the alignment falls out of the actual
> buddy allocation for the contiguous pages.
This should be enough information to add. Maybe even an explicit
{WARN,GEM_BUG}_ON...
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/16] drm/i915: Add unit tests for the breadcrumb rbtree, completion
Second retroactive test, make sure that the waiters are removed from the
global wait-tree when their seqno completes.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_breadcrumbs.c | 110 +++
1 file changed, 110 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c
b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index c768608974e1..fc950f7ff322 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -705,6 +705,12 @@ static struct intel_engine_cs *mock_engine(const char
*name)
return engine;
}
+static void mock_seqno_advance(struct intel_engine_cs *engine, u32 seqno)
+{
+ intel_write_status_page(engine, I915_GEM_HWS_INDEX, seqno);
+ intel_engine_wakeup(engine);
+}
+
static int *get_random_order(int count)
{
int *order;
@@ -766,6 +772,27 @@ static int check_rbtree(struct intel_engine_cs *engine,
return 0;
}
+static int check_completion(struct intel_engine_cs *engine,
+ const unsigned long *bitmap,
+ const struct intel_wait *waiters,
+ const int count)
+{
+ int n;
+
+ for (n = 0; n < count; n++) {
+ if (intel_wait_complete(&waiters[n]) != !!test_bit(n, bitmap))
+ continue;
+
+ pr_err("waiter[%d, seqno=%d] is %s, but expected %s\n",
+ n, waiters[n].seqno,
+ intel_wait_complete(&waiters[n]) ? "complete" : "active",
+ test_bit(n, bitmap) ? "active" : "complete");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int check_rbtree_empty(struct intel_engine_cs *engine)
{
struct intel_breadcrumbs *b = &engine->breadcrumbs;
@@ -857,10 +884,93 @@ static int igt_random_insert_remove(void *ignore)
return err;
}
+static int igt_insert_complete(void *ignore)
+{
+ struct intel_engine_cs *engine;
+ struct intel_wait *waiters;
+ const int count = 4096;
+ unsigned long *bitmap;
+ int err = -ENOMEM;
+ int n, m;
+
+ engine = mock_engine("mock");
+ if (!engine)
+ goto out;
+
+ waiters = drm_malloc_gfp(count, sizeof(*waiters), GFP_TEMPORARY);
+ if (!waiters)
+ goto out_engines;
+
+ bitmap = kcalloc(DIV_ROUND_UP(count, BITS_PER_LONG), sizeof(*bitmap),
+GFP_TEMPORARY);
+ if (!bitmap)
+ goto out_waiters;
+
+ for (n = 0; n < count; n++) {
+ intel_wait_init(&waiters[n], n + 0x1000);
+ intel_engine_add_wait(engine, &waiters[n]);
+ __set_bit(n, bitmap);
+ }
+ err = check_rbtree(engine, bitmap, waiters, count);
+ if (err)
+ goto err;
+
+ for (n = 0; n < count; n = m) {
+ int seqno = 2 * n;
+
+ GEM_BUG_ON(find_first_bit(bitmap, count) != n);
+
+ if (intel_wait_complete(&waiters[n])) {
+ pr_err("waiter[%d, seqno=%d] completed too early\n",
+ n, waiters[n].seqno);
+ err = -EINVAL;
+ goto err;
+ }
+
+ /* complete the following waiters */
+ mock_seqno_advance(engine, seqno + 0x1000);
+ for (m = n; m <= seqno; m++) {
+ if (m == count)
+ break;
+
+ GEM_BUG_ON(!test_bit(m, bitmap));
+ __clear_bit(m, bitmap);
+ }
+
+ intel_engine_remove_wait(engine, &waiters[n]);
+ RB_CLEAR_NODE(&waiters[n].node);
+
+ err = check_rbtree(engine, bitmap, waiters, count);
+ if (err) {
+ pr_err("rbtree corrupt after seqno advance to %d\n",
+ seqno + 0x1000);
+ goto err;
+ }
+
+ err = check_completion(engine, bitmap, waiters, count);
+ if (err) {
+ pr_err("completions after seqno advance to %d failed\n",
+ seqno + 0x1000);
+ goto err;
+ }
+ }
+
+ err = check_rbtree_empty(engine);
+err:
+ kfree(bitmap);
+out_waiters:
+ drm_free_large(waiters);
+out_engines:
+ kfree(engine);
+out:
+ return err;
+}
+
int intel_breadcrumbs_selftest(void)
{
static const struct i915_subtest tests[] = {
SUBTEST(igt_random_insert_remove),
+ SUBTEST(igt_insert_complete),
};
return i915_subtests(tests, NULL);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/16] kselftests: Exercise hw-independent mock tests for i915.ko
Although being a GPU driver most functionality of i915.ko depends upon real hardware, many of its internal interfaces can be "mocked" and so tested independently of any hardware. Expanding the test coverage is not only useful for i915.ko, but should provide some integration tests for core infrastructure as well. Loading i915.ko with mock_selftests=-1 will cause it to execute its mock tests then fail with -ENOTTY. If the driver is already loaded and bound to hardware, it requires a few more steps to unbind, and so the simple preliminary modprobe -r will fail. Signed-off-by: Chris Wilson Cc: Shuah Khan Cc: linux-kselft...@vger.kernel.org --- tools/testing/selftests/drivers/gpu/i915.sh | 14 ++ 1 file changed, 14 insertions(+) create mode 100755 tools/testing/selftests/drivers/gpu/i915.sh diff --git a/tools/testing/selftests/drivers/gpu/i915.sh b/tools/testing/selftests/drivers/gpu/i915.sh new file mode 100755 index ..d407f0fa1e3a --- /dev/null +++ b/tools/testing/selftests/drivers/gpu/i915.sh @@ -0,0 +1,14 @@ +#!/bin/sh +# Runs hardware independent tests for i915 (drivers/gpu/drm/i915) + +if ! /sbin/modprobe -q -r i915; then + echo "drivers/gpu/i915: [SKIP]" + exit 77 +fi + +if /sbin/modprobe -q i915 mock_selftests=-1; then + echo "drivers/gpu/i915: ok" +else + echo "drivers/gpu/i915: [FAIL]" + exit 1 +fi -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC] Smattering of selftests
More changes to GEM are on the cards, so before touching it again, let's try and nail down how the internals are meant to work. The advantage of mock testing is that we can write a universal test independent of the hw (e.g. testing physical object creation) and we can inspect internal state which should be able to spot subtle bugs easier than mashing the uabi. The downside to mock testing is that it doubles the upfront cost of every patch submission (if you change internal state, you're likely going to upset a test) and adds maintainance burden tracking change to external API (on the other hand it catches those silent changes that would lead to broken code). In addition to mock tests, I thought it would be useful to start writing a few run-once tests against real hardware. These are not as interesting as probing uabi (I don't envisage having kms_flip inside the kernel), but we might like to exercise runtime suspend once upon startup, for example. Again, we have access to internal state that may prove impossible to capture even in debugfs. Is this a totally insane and impractical proposal? -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/16] drm/i915: Move intel_lrc_context_pin() to avoid the forward declaration
Just a simple move to avoid a forward declaration.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_lrc.c | 132 +++
1 file changed, 65 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8b412880e88c..5cabe4e9d22f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -230,8 +230,6 @@ enum {
static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
struct intel_engine_cs *engine);
-static int intel_lr_context_pin(struct i915_gem_context *ctx,
- struct intel_engine_cs *engine);
static void execlists_init_reg_state(u32 *reg_state,
struct i915_gem_context *ctx,
struct intel_engine_cs *engine,
@@ -774,71 +772,6 @@ static void execlists_schedule(struct drm_i915_gem_request
*request, int prio)
/* XXX Do we need to preempt to make room for us and our deps? */
}
-int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request
*request)
-{
- struct intel_engine_cs *engine = request->engine;
- struct intel_context *ce = &request->ctx->engine[engine->id];
- int ret;
-
- /* Flush enough space to reduce the likelihood of waiting after
-* we start building the request - in which case we will just
-* have to repeat work.
-*/
- request->reserved_space += EXECLISTS_REQUEST_SIZE;
-
- if (!ce->state) {
- ret = execlists_context_deferred_alloc(request->ctx, engine);
- if (ret)
- return ret;
- }
-
- request->ring = ce->ring;
-
- ret = intel_lr_context_pin(request->ctx, engine);
- if (ret)
- return ret;
-
- if (i915.enable_guc_submission) {
- /*
-* Check that the GuC has space for the request before
-* going any further, as the i915_add_request() call
-* later on mustn't fail ...
-*/
- ret = i915_guc_wq_reserve(request);
- if (ret)
- goto err_unpin;
- }
-
- ret = intel_ring_begin(request, 0);
- if (ret)
- goto err_unreserve;
-
- if (!ce->initialised) {
- ret = engine->init_context(request);
- if (ret)
- goto err_unreserve;
-
- ce->initialised = true;
- }
-
- /* Note that after this point, we have committed to using
-* this request as it is being used to both track the
-* state of engine initialisation and liveness of the
-* golden renderstate above. Think twice before you try
-* to cancel/unwind this request now.
-*/
-
- request->reserved_space -= EXECLISTS_REQUEST_SIZE;
- return 0;
-
-err_unreserve:
- if (i915.enable_guc_submission)
- i915_guc_wq_unreserve(request);
-err_unpin:
- intel_lr_context_unpin(request->ctx, engine);
- return ret;
-}
-
static int intel_lr_context_pin(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
{
@@ -911,6 +844,71 @@ void intel_lr_context_unpin(struct i915_gem_context *ctx,
i915_gem_context_put(ctx);
}
+int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request
*request)
+{
+ struct intel_engine_cs *engine = request->engine;
+ struct intel_context *ce = &request->ctx->engine[engine->id];
+ int ret;
+
+ /* Flush enough space to reduce the likelihood of waiting after
+* we start building the request - in which case we will just
+* have to repeat work.
+*/
+ request->reserved_space += EXECLISTS_REQUEST_SIZE;
+
+ if (!ce->state) {
+ ret = execlists_context_deferred_alloc(request->ctx, engine);
+ if (ret)
+ return ret;
+ }
+
+ request->ring = ce->ring;
+
+ ret = intel_lr_context_pin(request->ctx, engine);
+ if (ret)
+ return ret;
+
+ if (i915.enable_guc_submission) {
+ /*
+* Check that the GuC has space for the request before
+* going any further, as the i915_add_request() call
+* later on mustn't fail ...
+*/
+ ret = i915_guc_wq_reserve(request);
+ if (ret)
+ goto err_unpin;
+ }
+
+ ret = intel_ring_begin(request, 0);
+ if (ret)
+ goto err_unreserve;
+
+ if (!ce->initialised) {
+ ret = engine->init_context(request);
+ if (ret)
+ goto err_unreserve;
+
+ ce->initialised = true;
+ }
+
+ /* Note that after this point, we have committed to using
[Intel-gfx] [PATCH 03/16] drm/i915: Add unit tests for the breadcrumb rbtree, insert/remove
First retroactive test, make sure that the waiters are in global seqno
order after random inserts and removals.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_mock_selftests.h | 1 +
drivers/gpu/drm/i915/intel_breadcrumbs.c | 205 +
drivers/gpu/drm/i915/intel_ringbuffer.h| 2 +
3 files changed, 208 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_mock_selftests.h
b/drivers/gpu/drm/i915/i915_mock_selftests.h
index 9bead7b496b0..1603fd35d190 100644
--- a/drivers/gpu/drm/i915/i915_mock_selftests.h
+++ b/drivers/gpu/drm/i915/i915_mock_selftests.h
@@ -8,4 +8,5 @@
*
* Tests are executed in reverse order by igt/drv_selftest
*/
+selftest(breadcrumbs, intel_breadcrumbs_selftest)
selftest(sanitycheck, i915_mock_sanitycheck) /* keep last */
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c
b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 53ae7884babd..c768608974e1 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -109,6 +109,18 @@ static void __intel_breadcrumbs_enable_irq(struct
intel_breadcrumbs *b)
if (b->rpm_wakelock)
return;
+ if (I915_SELFTEST_ONLY(b->mock)) {
+ /* For our mock objects we want to avoid interaction
+* with the real hardware (which is not set up). So
+* we simply pretend we have enabled the powerwell
+* and the irq, and leave it up to the mock
+* implementation to call intel_engine_wakeup()
+* itself when it wants to simulate a user interrupt,
+*/
+ b->rpm_wakelock = true;
+ return;
+ }
+
/* Since we are waiting on a request, the GPU should be busy
* and should have its own rpm reference. For completeness,
* record an rpm reference for ourselves to cover the
@@ -143,6 +155,11 @@ static void __intel_breadcrumbs_disable_irq(struct
intel_breadcrumbs *b)
if (!b->rpm_wakelock)
return;
+ if (I915_SELFTEST_ONLY(b->mock)) {
+ b->rpm_wakelock = false;
+ return;
+ }
+
if (b->irq_enabled) {
irq_disable(engine);
b->irq_enabled = false;
@@ -661,3 +678,191 @@ unsigned int intel_breadcrumbs_busy(struct
drm_i915_private *i915)
return mask;
}
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include
+
+#include "i915_selftest.h"
+
+static struct intel_engine_cs *mock_engine(const char *name)
+{
+ struct intel_engine_cs *engine;
+ static int id;
+
+ engine = kzalloc(sizeof(*engine) + 4096, GFP_TEMPORARY);
+ if (!engine)
+ return NULL;
+
+ /* minimal engine setup for seqno */
+ engine->name = name;
+ engine->id = id++;
+ engine->status_page.page_addr = (void *)(engine + 1);
+
+ /* minimal breadcrumbs init */
+ spin_lock_init(&engine->breadcrumbs.lock);
+ engine->breadcrumbs.mock = true;
+
+ return engine;
+}
+
+static int *get_random_order(int count)
+{
+ int *order;
+ int n, r, tmp;
+
+ order = kmalloc_array(count, sizeof(*order), GFP_TEMPORARY);
+ if (!order)
+ return order;
+
+ for (n = 0; n < count; n++)
+ order[n] = n;
+
+ for (n = count - 1; n > 1; n--) {
+ r = get_random_int() % (n + 1);
+ if (r != n) {
+ tmp = order[n];
+ order[n] = order[r];
+ order[r] = tmp;
+ }
+ }
+
+ return order;
+}
+
+static int check_rbtree(struct intel_engine_cs *engine,
+ const unsigned long *bitmap,
+ const struct intel_wait *waiters,
+ const int count)
+{
+ struct intel_breadcrumbs *b = &engine->breadcrumbs;
+ struct rb_node *rb;
+ int n;
+
+ if (&b->first_wait->node != rb_first(&b->waiters)) {
+ pr_err("First waiter does not match first element of
wait-tree\n");
+ return -EINVAL;
+ }
+
+ n = find_first_bit(bitmap, count);
+ for (rb = rb_first(&b->waiters); rb; rb = rb_next(rb)) {
+ struct intel_wait *w = container_of(rb, typeof(*w), node);
+ int idx = w - waiters;
+
+ if (!test_bit(idx, bitmap)) {
+ pr_err("waiter[%d, seqno=%d] removed but still in
wait-tree\n",
+ idx, w->seqno);
+ return -EINVAL;
+ }
+
+ if (n != idx) {
+ pr_err("waiter[%d, seqno=%d] does not match expected
next element in tree [%d]\n",
+ idx, w->seqno, n);
+ return -EINVAL;
+ }
+
+ n = find_next_bit(bitmap, count, n + 1);
+ }
+
+ return 0;
+}
+
+static int check_rbtree_emp
[Intel-gfx] [PATCH 10/16] drm/i915: Mark the shadow gvt context as closed
As the shadow gvt is not user accessible and does not have an associated
vm, we can mark it as closed during its construction. This saves leaking
the internal knowledge of i915_gem_context into gvt/.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/gvt/scheduler.c| 10 +-
drivers/gpu/drm/i915/i915_gem_context.c | 1 +
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index 4db242250235..fd2b026f7ecd 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -549,18 +549,10 @@ int intel_gvt_init_workload_scheduler(struct intel_gvt
*gvt)
void intel_vgpu_clean_gvt_context(struct intel_vgpu *vgpu)
{
- struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
-
atomic_notifier_chain_unregister(&vgpu->shadow_ctx->status_notifier,
&vgpu->shadow_ctx_notifier_block);
- mutex_lock(&dev_priv->drm.struct_mutex);
-
- /* a little hacky to mark as ctx closed */
- vgpu->shadow_ctx->closed = true;
- i915_gem_context_put(vgpu->shadow_ctx);
-
- mutex_unlock(&dev_priv->drm.struct_mutex);
+ i915_gem_context_put_unlocked(vgpu->shadow_ctx);
}
int intel_vgpu_init_gvt_context(struct intel_vgpu *vgpu)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 95812c26767c..042befd263fe 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -409,6 +409,7 @@ i915_gem_context_create_gvt(struct drm_device *dev)
if (IS_ERR(ctx))
goto out;
+ ctx->closed = true; /* not user accessible */
ctx->execlists_force_single_submission = true;
ctx->ring_size = 512 * PAGE_SIZE; /* Max ring buffer size */
out:
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/16] drm/i915: Add a simple fence selftest to i915_gem_request
Do a quick selftest on in the interoperability of dma_fence_wait on a
i915_gem_request.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_request.c | 51 +
1 file changed, 51 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c
b/drivers/gpu/drm/i915/i915_gem_request.c
index 9ba17d3e35cb..2bde3fc8e8bf 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1648,11 +1648,62 @@ static int igt_wait_request(void *ignore)
return err;
}
+static int igt_fence_wait(void *ignore)
+{
+ struct drm_i915_private *i915;
+ struct drm_i915_gem_request *request;
+ int err = -ENOMEM;
+
+ i915 = mock_device();
+ if (!i915)
+ goto out;
+
+ err = -EINVAL;
+ mutex_lock(&i915->drm.struct_mutex);
+ request = mock_request(i915->engine[RCS],
+ i915->kernel_context,
+ HZ);
+ if (!request) {
+ mutex_unlock(&i915->drm.struct_mutex);
+ goto out_device;
+ }
+
+ i915_add_request(request);
+ mutex_unlock(&i915->drm.struct_mutex);
+
+ if (dma_fence_is_signaled(&request->fence)) {
+ pr_err("fence signaled immediately!\n");
+ goto out_device;
+ }
+
+ if (dma_fence_wait_timeout(&request->fence, false, 1) != -ETIME) {
+ pr_err("fence wait success after submit (expected timeout)!\n");
+ goto out_device;
+ }
+
+ if (dma_fence_wait_timeout(&request->fence, false, 2 * HZ) <= 0) {
+ pr_err("fence wait timed out (expected success)!\n");
+ goto out_device;
+ }
+
+ if (!dma_fence_is_signaled(&request->fence)) {
+ pr_err("fence unsignaled after waiting!\n");
+ goto out_device;
+ }
+
+ err = 0;
+out_device:
+ mock_device_free(i915);
+out:
+ return err;
+}
+
int i915_gem_request_selftest(void)
{
static const struct i915_subtest tests[] = {
SUBTEST(igt_add_request),
SUBTEST(igt_wait_request),
+ SUBTEST(igt_fence_wait),
};
return i915_subtests(tests, NULL);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/16] drm/i915: Simplify releasing context reference
A few users only take the struct_mutex in order to release a reference
to a context. We can expose a kref_put_mutex() wrapper in order to
simplify these users, and optimise taking of the mutex to the final
unref.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h | 7 +++
drivers/gpu/drm/i915/i915_perf.c | 16
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7c228622716a..3411e38e32af 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3320,6 +3320,13 @@ static inline void i915_gem_context_put(struct
i915_gem_context *ctx)
kref_put(&ctx->ref, i915_gem_context_free);
}
+static inline void i915_gem_context_put_unlocked(struct i915_gem_context *ctx)
+{
+ kref_put_mutex(&ctx->ref,
+ i915_gem_context_free,
+ &ctx->i915->drm.struct_mutex);
+}
+
static inline struct intel_timeline *
i915_gem_context_lookup_timeline(struct i915_gem_context *ctx,
struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index cfe4152212b9..ee6271fe96de 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1298,8 +1298,6 @@ static long i915_perf_ioctl(struct file *file,
static void i915_perf_destroy_locked(struct i915_perf_stream *stream)
{
- struct drm_i915_private *dev_priv = stream->dev_priv;
-
if (stream->enabled)
i915_perf_disable_locked(stream);
@@ -1308,11 +1306,8 @@ static void i915_perf_destroy_locked(struct
i915_perf_stream *stream)
list_del(&stream->link);
- if (stream->ctx) {
- mutex_lock(&dev_priv->drm.struct_mutex);
- i915_gem_context_put(stream->ctx);
- mutex_unlock(&dev_priv->drm.struct_mutex);
- }
+ if (stream->ctx)
+ i915_gem_context_put_unlocked(stream->ctx);
kfree(stream);
}
@@ -1446,11 +1441,8 @@ i915_perf_open_ioctl_locked(struct drm_i915_private
*dev_priv,
err_alloc:
kfree(stream);
err_ctx:
- if (specific_ctx) {
- mutex_lock(&dev_priv->drm.struct_mutex);
- i915_gem_context_put(specific_ctx);
- mutex_unlock(&dev_priv->drm.struct_mutex);
- }
+ if (specific_ctx)
+ i915_gem_context_put_unlocked(specific_ctx);
err:
return ret;
}
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/16] drm/i915: Provide a hook for selftests
Some pieces of code are independent of hardware but are very tricky to exercise through the normal userspace ABI or via debugfs hooks. Being able to create mock unit tests and execute them through CI is vital. Start by adding a central point where we can execute unit tests and a parameter to enable them. This is disabled by default as the expectation is that these tests will occasionally explode. To facilitate integration with igt, any parameter beginning with i915.igt__ is interpreted as a subtest executable independently via igt/drv_selftest. Two classes of selftests are recognised: mock unit tests and integration tests. Mock unit tests are run as soon as the module is loaded, before the device is probed. At that point there is no driver instantiated and all hw interactions must be "mocked". This is very useful for writing universal tests to exercise code not typically run on a broad range of architectures. Alternatively, you can hook into the late selftests and run when the device has been instantiated - hw interactions are real. v2: Add a macro for compiling conditional code for mock objects inside real objects. v3: Differentiate between mock unit tests and late integration test. Signed-off-by: Chris Wilson Reviewed-by: Tvrtko Ursulin #v1 --- drivers/gpu/drm/i915/Kconfig.debug | 15 +++ drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_late_selftests.h | 11 ++ drivers/gpu/drm/i915/i915_mock_selftests.h | 11 ++ drivers/gpu/drm/i915/i915_params.c | 8 ++ drivers/gpu/drm/i915/i915_params.h | 4 + drivers/gpu/drm/i915/i915_pci.c| 19 +++- drivers/gpu/drm/i915/i915_selftest.c | 173 + drivers/gpu/drm/i915/i915_selftest.h | 77 + 9 files changed, 318 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/i915_late_selftests.h create mode 100644 drivers/gpu/drm/i915/i915_mock_selftests.h create mode 100644 drivers/gpu/drm/i915/i915_selftest.c create mode 100644 drivers/gpu/drm/i915/i915_selftest.h diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug index 597648c7a645..76af8774cf70 100644 --- a/drivers/gpu/drm/i915/Kconfig.debug +++ b/drivers/gpu/drm/i915/Kconfig.debug @@ -25,6 +25,7 @@ config DRM_I915_DEBUG select DRM_VGEM # used by igt/prime_vgem (dmabuf interop checks) select DRM_DEBUG_MM if DRM=y select DRM_I915_SW_FENCE_DEBUG_OBJECTS + select DRM_I915_SELFTEST default n help Choose this option to turn on extra driver debugging that may affect @@ -58,3 +59,17 @@ config DRM_I915_SW_FENCE_DEBUG_OBJECTS Recommended for driver developers only. If in doubt, say "N". + +config DRM_I915_SELFTEST + bool "Enable selftests upon driver load" + depends on DRM_I915 + default n + help + Choose this option to allow the driver to perform selftests upon + loading; also requires the i915.selftest=1 module parameter. To + exit the module after running the selftests (i.e. to prevent normal + module initialisation afterwards) use i915.selftest=-1. + + Recommended for driver developers only. + + If in doubt, say "N". diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 3c30916727fb..7c3b4f0c836c 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -114,6 +114,7 @@ i915-y += dvo_ch7017.o \ # Post-mortem debug and GPU hang state capture i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o +i915-$(CONFIG_DRM_I915_SELFTEST) += i915_selftest.o # virtual gpu code i915-y += i915_vgpu.o diff --git a/drivers/gpu/drm/i915/i915_late_selftests.h b/drivers/gpu/drm/i915/i915_late_selftests.h new file mode 100644 index ..e6645d08d964 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_late_selftests.h @@ -0,0 +1,11 @@ +/* List each unit test as selftest(name, function) + * + * The name is used as both an enum and expanded as subtest__name to create + * a module parameter. It must be unique and legal for a C identifier. + * + * The function should be of type int function(void). It may be conditionally + * compiled using #if IS_ENABLED(DRM_I915_SELFTEST). + * + * Tests are executed in reverse order by igt/drv_selftest + */ +selftest(sanitycheck, i915_late_sanitycheck) /* keep last */ diff --git a/drivers/gpu/drm/i915/i915_mock_selftests.h b/drivers/gpu/drm/i915/i915_mock_selftests.h new file mode 100644 index ..9bead7b496b0 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_mock_selftests.h @@ -0,0 +1,11 @@ +/* List each unit test as selftest(name, function) + * + * The name is used as both an enum and expanded as subtest__name to create + * a module parameter. It must be unique and legal for a C identifier. + * + * The function should be of type int function(void). It may be conditionally + * compiled using #if IS_ENABLED(
[Intel-gfx] [PATCH 05/16] drm/i915: Add unit tests for the breadcrumb rbtree, wakeups
Third retroactive test, make sure that the seqno waiters are woken.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_breadcrumbs.c | 171 +++
1 file changed, 171 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c
b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index fc950f7ff322..1374a54e41c9 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -966,11 +966,182 @@ static int igt_insert_complete(void *ignore)
return err;
}
+struct igt_wakeup {
+ struct task_struct *tsk;
+ atomic_t *ready, *set, *done;
+ struct intel_engine_cs *engine;
+ unsigned long flags;
+ wait_queue_head_t *wq;
+ u32 seqno;
+};
+
+static int wait_atomic(atomic_t *p)
+{
+ schedule();
+ return 0;
+}
+
+static int wait_atomic_timeout(atomic_t *p)
+{
+ return schedule_timeout(10 * HZ) ? 0 : -ETIMEDOUT;
+}
+
+static int igt_wakeup_thread(void *arg)
+{
+ struct igt_wakeup *w = arg;
+ struct intel_wait wait;
+
+ while (!kthread_should_stop()) {
+ DEFINE_WAIT(ready);
+
+ for (;;) {
+ prepare_to_wait(w->wq, &ready, TASK_INTERRUPTIBLE);
+ if (atomic_read(w->ready) == 0)
+ break;
+
+ schedule();
+ }
+ finish_wait(w->wq, &ready);
+ if (atomic_dec_and_test(w->set))
+ wake_up_atomic_t(w->set);
+
+ if (test_bit(0, &w->flags))
+ break;
+
+ intel_wait_init(&wait, w->seqno);
+ intel_engine_add_wait(w->engine, &wait);
+ for (;;) {
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if (i915_seqno_passed(intel_engine_get_seqno(w->engine),
+ w->seqno))
+ break;
+
+ schedule();
+ }
+ intel_engine_remove_wait(w->engine, &wait);
+ __set_current_state(TASK_RUNNING);
+
+ if (atomic_dec_and_test(w->done))
+ wake_up_atomic_t(w->done);
+ }
+
+ if (atomic_dec_and_test(w->done))
+ wake_up_atomic_t(w->done);
+ return 0;
+}
+
+static void igt_wake_all_sync(atomic_t *ready,
+ atomic_t *set,
+ atomic_t *done,
+ wait_queue_head_t *wq,
+ int count)
+{
+ atomic_set(set, count);
+ atomic_set(done, count);
+
+ atomic_set(ready, 0);
+ wake_up_all(wq);
+
+ wait_on_atomic_t(set, wait_atomic, TASK_UNINTERRUPTIBLE);
+ atomic_set(ready, count);
+}
+
+static int igt_wakeup(void *ignore)
+{
+ const int state = TASK_UNINTERRUPTIBLE;
+ struct intel_engine_cs *engine;
+ struct igt_wakeup *waiters;
+ DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
+ const int count = 4096;
+ const u32 max_seqno = count / 4;
+ atomic_t ready, set, done;
+ int err = -ENOMEM;
+ int n, step;
+
+ engine = mock_engine("mock");
+ if (!engine)
+ goto out;
+
+ waiters = drm_malloc_gfp(count, sizeof(*waiters), GFP_TEMPORARY);
+ if (!waiters)
+ goto out_engines;
+
+ atomic_set(&ready, count);
+ for (n = 0; n < count; n++) {
+ waiters[n].wq = &wq;
+ waiters[n].ready = &ready;
+ waiters[n].set = &set;
+ waiters[n].done = &done;
+ waiters[n].engine = engine;
+ waiters[n].flags = 0;
+
+ waiters[n].tsk = kthread_run(igt_wakeup_thread, &waiters[n],
+"i915/igt:%d", n);
+ if (IS_ERR(waiters[n].tsk))
+ goto out_waiters;
+
+ get_task_struct(waiters[n].tsk);
+ }
+
+ for (step = 1; step <= max_seqno; step <<= 1) {
+ u32 seqno;
+
+ for (n = 0; n < count; n++)
+ waiters[n].seqno = 1 + get_random_int() % max_seqno;
+
+ mock_seqno_advance(engine, 0);
+ igt_wake_all_sync(&ready, &set, &done, &wq, count);
+
+ for (seqno = 1; seqno <= max_seqno + step; seqno += step) {
+ usleep_range(50, 500);
+ mock_seqno_advance(engine, seqno);
+ }
+ GEM_BUG_ON(intel_engine_get_seqno(engine) < 1 + max_seqno);
+
+ err = wait_on_atomic_t(&done, wait_atomic_timeout, state);
+ if (err) {
+ pr_err("Timed out waiting for %d remaining waiters\n",
+ atomic_read(&done));
+ break;
+ }
+
+ err = check_rbtree_empty(engine);
+
[Intel-gfx] [PATCH 16/16] drm/i915: Add selftests for object allocation, phys
The phys object is a rarely used device (only very old machines require
a chunk of physically contiguous pages for a few hardware interactions).
As such, it is not exercised by CI and to combat that we want to add a
test that exercises the phys object on all platforms.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem.c| 168 +
drivers/gpu/drm/i915/i915_mock_selftests.h | 1 +
2 files changed, 169 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9794dd655877..1ab5fdae2d47 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4949,3 +4949,171 @@ i915_gem_object_get_dma_address(struct
drm_i915_gem_object *obj,
sg = i915_gem_object_get_sg(obj, n, &offset);
return sg_dma_address(sg) + (offset << PAGE_SHIFT);
}
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include
+
+#include "i915_selftest.h"
+
+static struct drm_driver mock_driver = {
+ .name = "mock",
+ .driver_features = DRIVER_GEM,
+
+ .gem_close_object = i915_gem_close_object,
+ .gem_free_object_unlocked = i915_gem_free_object,
+};
+
+struct mock_object {
+ struct drm_i915_gem_object base;
+};
+
+static void release_dev(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ kfree(pdev);
+}
+
+static struct drm_i915_private *mock_gem_device(void)
+{
+ struct drm_i915_private *i915;
+ struct pci_dev *pdev;
+ int err;
+
+ i915 = kzalloc(sizeof(*i915), GFP_TEMPORARY);
+ if (!i915)
+ return NULL;
+
+ pdev = kzalloc(sizeof(*pdev), GFP_TEMPORARY);
+ if (!pdev) {
+ kfree(i915);
+ return NULL;
+ }
+
+ device_initialize(&pdev->dev);
+ pdev->dev.release = release_dev;
+ dev_set_name(&pdev->dev, "mock");
+ dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+
+ pm_runtime_dont_use_autosuspend(&pdev->dev);
+ pm_runtime_get_sync(&pdev->dev);
+ pci_set_drvdata(pdev, i915);
+
+ err = drm_dev_init(&i915->drm, &mock_driver, &pdev->dev);
+ if (err) {
+ pr_err("Failed to initialise mock GEM device: err=%d\n", err);
+ put_device(&pdev->dev);
+ kfree(i915);
+ return NULL;
+ }
+ i915->drm.pdev = pdev;
+ i915->drm.dev_private = i915;
+
+ mkwrite_device_info(i915)->gen = -1;
+
+ spin_lock_init(&i915->mm.object_stat_lock);
+
+ INIT_WORK(&i915->mm.free_work, __i915_gem_free_work);
+ init_llist_head(&i915->mm.free_list);
+
+ i915->objects = KMEM_CACHE(mock_object, SLAB_HWCACHE_ALIGN);
+ if (!i915->objects)
+ goto err_device;
+
+ return i915;
+
+err_device:
+ kfree(i915);
+ return NULL;
+}
+
+static void mock_device_free(struct drm_i915_private *i915)
+{
+ struct pci_dev *pdev = i915->drm.pdev;
+
+ rcu_barrier();
+ while (flush_work(&i915->mm.free_work))
+ rcu_barrier();
+
+ drm_dev_unref(&i915->drm);
+ put_device(&pdev->dev);
+}
+
+static int igt_gem_object(void *ignore)
+{
+ struct drm_i915_gem_object *obj;
+ struct drm_i915_private *i915;
+ int err = -ENOMEM;
+
+ i915 = mock_gem_device();
+ if (!i915)
+ goto out;
+
+ obj = i915_gem_object_create(i915, 4096);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ pr_err("i915_gem_object_create failed, err=%d\n", err);
+ goto out_device;
+ }
+
+ err = 0;
+ i915_gem_object_put(obj);
+out_device:
+ mock_device_free(i915);
+out:
+ return err;
+}
+
+static int igt_phys_object(void *ignore)
+{
+ struct drm_i915_gem_object *obj;
+ struct drm_i915_private *i915;
+ int err = -ENOMEM;
+
+ i915 = mock_gem_device();
+ if (!i915)
+ goto out;
+
+ obj = i915_gem_object_create(i915, 4096);
+ if (IS_ERR(obj)) {
+ err = PTR_ERR(obj);
+ pr_err("i915_gem_object_create failed, err=%d\n", err);
+ goto out_device;
+ }
+
+ err = -EINVAL;
+ mutex_lock(&i915->drm.struct_mutex);
+ err = i915_gem_object_attach_phys(obj, PAGE_SIZE);
+ mutex_unlock(&i915->drm.struct_mutex);
+ if (err) {
+ pr_err("i915_gem_object_attach_phys failed, err=%d\n", err);
+ goto err;
+ }
+
+ if (obj->ops != &i915_gem_phys_ops) {
+ pr_err("i915_gem_object_attach_phys did not create a phys
object\n");
+ goto err;
+ }
+
+ /* Make the object work during teardown */
+ obj->mm.dirty = true;
+
+ err = 0;
+err:
+ i915_gem_object_put(obj);
+out_device:
+ mock_device_free(i915);
+out:
+ return err;
+}
+
+int i915_gem_object_selftests(void)
+{
+ static const struct i915_subtest tests[] = {
[Intel-gfx] [PATCH 12/16] drm/i915: Swap if(enable_execlists) in i915_gem_request_alloc for a vfunc
A fairly trivial move of a matching pair of routines (for preparing a
request for construction) onto an engine vfunc. The ulterior motive is
to be able to create a mock request implementation.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_request.c | 5 +
drivers/gpu/drm/i915/intel_lrc.c| 4 +++-
drivers/gpu/drm/i915/intel_lrc.h| 2 --
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +++-
drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +--
5 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c
b/drivers/gpu/drm/i915/i915_gem_request.c
index 06e9a607d934..881bed5347fb 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -622,10 +622,7 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
req->reserved_space = MIN_SPACE_FOR_ADD_REQUEST;
GEM_BUG_ON(req->reserved_space < engine->emit_breadcrumb_sz);
- if (i915.enable_execlists)
- ret = intel_logical_ring_alloc_request_extras(req);
- else
- ret = intel_ring_alloc_request_extras(req);
+ ret = engine->request_alloc(req);
if (ret)
goto err_ctx;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 22ded92d0346..3f536ef37968 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -845,7 +845,7 @@ static void execlists_context_unpin(struct intel_engine_cs
*engine,
i915_gem_context_put(ctx);
}
-int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request
*request)
+static int execlists_request_alloc(struct drm_i915_gem_request *request)
{
struct intel_engine_cs *engine = request->engine;
struct intel_context *ce = &request->ctx->engine[engine->id];
@@ -1816,6 +1816,8 @@ logical_ring_default_vfuncs(struct intel_engine_cs
*engine)
engine->context_pin = execlists_context_pin;
engine->context_unpin = execlists_context_unpin;
+ engine->request_alloc = execlists_request_alloc;
+
engine->emit_flush = gen8_emit_flush;
engine->emit_breadcrumb = gen8_emit_breadcrumb;
engine->emit_breadcrumb_sz = gen8_emit_breadcrumb_sz;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index b5630331086a..01ba36ea125e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -63,8 +63,6 @@ enum {
};
/* Logical Rings */
-int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request
*request);
-int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
void intel_logical_ring_stop(struct intel_engine_cs *engine);
void intel_logical_ring_cleanup(struct intel_engine_cs *engine);
int logical_render_ring_init(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a57eb5dec991..a42b9bf45b42 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2102,7 +2102,7 @@ void intel_legacy_submission_resume(struct
drm_i915_private *dev_priv)
}
}
-int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
+static int ring_request_alloc(struct drm_i915_gem_request *request)
{
int ret;
@@ -2597,6 +2597,8 @@ static void intel_ring_default_vfuncs(struct
drm_i915_private *dev_priv,
engine->context_pin = intel_ring_context_pin;
engine->context_unpin = intel_ring_context_unpin;
+ engine->request_alloc = ring_request_alloc;
+
engine->emit_breadcrumb = i9xx_emit_breadcrumb;
engine->emit_breadcrumb_sz = i9xx_emit_breadcrumb_sz;
if (i915.semaphores) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0b69a50ab833..12f4dd1cbf9c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -272,6 +272,7 @@ struct intel_engine_cs {
struct i915_gem_context *ctx);
void(*context_unpin)(struct intel_engine_cs *engine,
struct i915_gem_context *ctx);
+ int (*request_alloc)(struct drm_i915_gem_request *req);
int (*init_context)(struct drm_i915_gem_request *req);
int (*emit_flush)(struct drm_i915_gem_request *request,
@@ -476,8 +477,6 @@ void intel_engine_cleanup(struct intel_engine_cs *engine);
void intel_legacy_submission_resume(struct drm_i915_private *dev_priv);
-int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request);
-
int __must_check intel_ring_begin(struct drm_i915_gem_request *req, int n);
int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedes
[Intel-gfx] [PATCH 06/16] drm/i915: Add a reminder that i915_vma_move_to_active() requires struct_mutex
i915_vma_move_to_active() requires the struct_mutex for serialisation with retirement, so mark it up with lockdep_assert_held(). Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index d665a33229bd..c64438f8171c 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1259,6 +1259,7 @@ void i915_vma_move_to_active(struct i915_vma *vma, struct drm_i915_gem_object *obj = vma->obj; const unsigned int idx = req->engine->id; + lockdep_assert_held(&req->i915->drm.struct_mutex); GEM_BUG_ON(!drm_mm_node_allocated(&vma->node)); /* Add a reference if we're newly entering the active list. -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/16] drm/i915/execlists: Request the kernel context be pinned high
PIN_HIGH is an expensive operation (in comparison to allocating from the
hole stack) unsuitable for frequent use (such as switching between
contexts). However, the kernel context should be pinned just once for
the lifetime of the driver, and here it is appropriate to keep it out of
the mappable range (in order to maximise mappable space for users).
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/intel_lrc.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 58cea1f9ad27..22ded92d0346 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -767,6 +767,7 @@ static int execlists_context_pin(struct intel_engine_cs
*engine,
struct i915_gem_context *ctx)
{
struct intel_context *ce = &ctx->engine[engine->id];
+ unsigned int flags;
void *vaddr;
int ret;
@@ -781,8 +782,11 @@ static int execlists_context_pin(struct intel_engine_cs
*engine,
goto err;
}
- ret = i915_vma_pin(ce->state, 0, GEN8_LR_CONTEXT_ALIGN,
- PIN_OFFSET_BIAS | GUC_WOPCM_TOP | PIN_GLOBAL);
+ flags = PIN_OFFSET_BIAS | GUC_WOPCM_TOP | PIN_GLOBAL;
+ if (ctx == ctx->i915->kernel_context)
+ flags |= PIN_HIGH;
+
+ ret = i915_vma_pin(ce->state, 0, GEN8_LR_CONTEXT_ALIGN, flags);
if (ret)
goto err;
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/16] drm/i915: Add a simple request selftest for waiting
A trivial kselftest to submit a request and wait upon it.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_request.c | 48 +
1 file changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c
b/drivers/gpu/drm/i915/i915_gem_request.c
index 6553457adc77..9ba17d3e35cb 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1601,10 +1601,58 @@ static int igt_add_request(void *ignore)
return err;
}
+static int igt_wait_request(void *ignore)
+{
+ struct drm_i915_private *i915;
+ struct drm_i915_gem_request *request;
+ int err = -ENOMEM;
+
+ i915 = mock_device();
+ if (!i915)
+ goto out;
+
+ mutex_lock(&i915->drm.struct_mutex);
+ request = mock_request(i915->engine[RCS],
+ i915->kernel_context,
+ HZ / 2);
+ if (!request)
+ goto out_unlock;
+
+ i915_add_request(request);
+
+ if (i915_gem_request_completed(request)) {
+ pr_err("request completed immediately!\n");
+ goto out_unlock;
+ }
+
+ if (i915_wait_request(request, I915_WAIT_LOCKED, HZ / 4) != -ETIME) {
+ pr_err("request wait succeeded (expected tiemout!)\n");
+ goto out_unlock;
+ }
+
+ if (i915_wait_request(request, I915_WAIT_LOCKED, HZ / 2) == -ETIME) {
+ pr_err("request wait timed out!\n");
+ goto out_unlock;
+ }
+
+ if (!i915_gem_request_completed(request)) {
+ pr_err("request not complete after waiting!\n");
+ goto out_unlock;
+ }
+
+ err = 0;
+out_unlock:
+ mutex_unlock(&i915->drm.struct_mutex);
+ mock_device_free(i915);
+out:
+ return err;
+}
+
int i915_gem_request_selftest(void)
{
static const struct i915_subtest tests[] = {
SUBTEST(igt_add_request),
+ SUBTEST(igt_wait_request),
};
return i915_subtests(tests, NULL);
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/16] drm/i915: Unify active context tracking between legacy/execlists/guc
The requests conversion introduced a nasty bug where we could generate a
new request in the middle of constructing a request. The new request
would be executed (and waited upon) before the current one, creating a
minor havoc in the seqno accounting. (Prior to deferred seqno
assignment, this would mean that the seqno would be out of order, and
the current request would be deemed complete even before it was
submitted.)
We also employed two different mechanisms to track the active context
until it was switched out. The legacy method allowed for waiting upon an
active context (it could forcibly evict any vma, including context's),
but the execlists method took a step backwards by pinning the vma for
the entire active lifespan of the context (the only way to evict was to
idle the entire GPU, not individual contexts). However, to circumvent
the tricky issue of locking (i.e. we cannot take struct_mutex at the
time of i915_gem_request_submit(), where we would want to move the
previous context onto the active tracker and unpin it), we take the
execlists approach and keep the contexts pinned until retirement.
The benefit of the execlists approach, more important for execlists than
legacy, was the reduction in work in pinning the context for each
request - as the context was kept pinned until idle, it could short
circuit the pinning for all active contexts.
We introduce new engine vfuncs to pin and unpin the context
respectively. The context is pinned at the start of the request, and
only unpinned when the following request is retired (this ensures that
the context is idle and coherent in main memory before we unpin it). We
move the engine->last_context tracking into the retirement itself
(rather than during request submission) in order to allow the submission
to be reordered or unwound without undue difficultly.
And finally an ulterior motive for unifying context handling was to
prepare for mock requests.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_drv.h| 4 --
drivers/gpu/drm/i915/i915_gem_context.c| 102 +++--
drivers/gpu/drm/i915/i915_gem_request.c| 38 +++
drivers/gpu/drm/i915/i915_gem_request.h| 11
drivers/gpu/drm/i915/i915_guc_submission.c | 11
drivers/gpu/drm/i915/i915_perf.c | 18 ++---
drivers/gpu/drm/i915/intel_engine_cs.c | 21 +-
drivers/gpu/drm/i915/intel_lrc.c | 62 ++
drivers/gpu/drm/i915/intel_lrc.h | 5 +-
drivers/gpu/drm/i915/intel_ringbuffer.c| 57 +---
drivers/gpu/drm/i915/intel_ringbuffer.h| 4 ++
11 files changed, 122 insertions(+), 211 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8daa4fb13b52..7c228622716a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2250,7 +2250,6 @@ struct drm_i915_private {
struct i915_perf_stream *exclusive_stream;
u32 specific_ctx_id;
- struct i915_vma *pinned_rcs_vma;
struct hrtimer poll_check_timer;
wait_queue_head_t poll_wq;
@@ -3290,9 +3289,6 @@ int i915_gem_context_open(struct drm_device *dev, struct
drm_file *file);
void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
int i915_switch_context(struct drm_i915_gem_request *req);
int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv);
-struct i915_vma *
-i915_gem_context_pin_legacy(struct i915_gem_context *ctx,
- unsigned int flags);
void i915_gem_context_free(struct kref *ctx_ref);
struct i915_gem_context *
i915_gem_context_create_gvt(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index a57c22659a3c..95812c26767c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -416,21 +416,6 @@ i915_gem_context_create_gvt(struct drm_device *dev)
return ctx;
}
-static void i915_gem_context_unpin(struct i915_gem_context *ctx,
- struct intel_engine_cs *engine)
-{
- if (i915.enable_execlists) {
- intel_lr_context_unpin(ctx, engine);
- } else {
- struct intel_context *ce = &ctx->engine[engine->id];
-
- if (ce->state)
- i915_vma_unpin(ce->state);
-
- i915_gem_context_put(ctx);
- }
-}
-
int i915_gem_context_init(struct drm_i915_private *dev_priv)
{
struct i915_gem_context *ctx;
@@ -490,10 +475,11 @@ void i915_gem_context_lost(struct drm_i915_private
*dev_priv)
lockdep_assert_held(&dev_priv->drm.struct_mutex);
for_each_engine(engine, dev_priv, id) {
- if (engine->last_context) {
- i915_gem_context_unpin(engine->last_context, engine);
- e
[Intel-gfx] [PATCH 13/16] drm/i915: Add selftests for i915_gem_request
Simple starting point for adding seltests for i915_gem_request, first
mock a device (with engines and contexts) that allows us to construct
and execute a request, along with waiting for the request to complete.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_request.c| 402 +
drivers/gpu/drm/i915/i915_mock_selftests.h | 1 +
2 files changed, 403 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c
b/drivers/gpu/drm/i915/i915_gem_request.c
index 881bed5347fb..6553457adc77 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1208,3 +1208,405 @@ void i915_gem_retire_requests(struct drm_i915_private
*dev_priv)
for_each_engine(engine, dev_priv, id)
engine_retire_requests(engine);
}
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "i915_selftest.h"
+
+struct mock_engine {
+ struct intel_engine_cs base;
+
+ spinlock_t hw_lock;
+ struct list_head hw_queue;
+ struct timer_list hw_delay;
+};
+
+struct mock_request {
+ struct drm_i915_gem_request base;
+
+ struct list_head link;
+ unsigned long delay;
+};
+
+static void mock_seqno_advance(struct intel_engine_cs *engine, u32 seqno)
+{
+ intel_write_status_page(engine, I915_GEM_HWS_INDEX, seqno);
+ intel_engine_wakeup(engine);
+}
+
+static void hw_delay_complete(unsigned long data)
+{
+ struct mock_engine *engine = (typeof(engine))data;
+ struct mock_request *request;
+
+ spin_lock(&engine->hw_lock);
+ request = list_first_entry_or_null(&engine->hw_queue,
+ typeof(*request),
+ link);
+ if (request) {
+ list_del(&request->link);
+ mock_seqno_advance(&engine->base, request->base.global_seqno);
+ }
+
+ request = list_first_entry_or_null(&engine->hw_queue,
+ typeof(*request),
+ link);
+ if (request)
+ mod_timer(&engine->hw_delay, jiffies + request->delay);
+ spin_unlock(&engine->hw_lock);
+}
+
+static void mock_engine_flush(struct intel_engine_cs *engine)
+{
+ struct mock_engine *mock =
+ container_of(engine, typeof(*mock), base);
+ struct mock_request *request, *rn;
+
+ del_timer_sync(&mock->hw_delay);
+
+ spin_lock_irq(&mock->hw_lock);
+ list_for_each_entry_safe(request, rn, &mock->hw_queue, link) {
+ list_del_init(&request->link);
+ mock_seqno_advance(&mock->base, request->base.global_seqno);
+ }
+ spin_unlock_irq(&mock->hw_lock);
+}
+
+static int mock_context_pin(struct intel_engine_cs *engine,
+ struct i915_gem_context *ctx)
+{
+ i915_gem_context_get(ctx);
+ return 0;
+}
+
+static void mock_context_unpin(struct intel_engine_cs *engine,
+ struct i915_gem_context *ctx)
+{
+ i915_gem_context_put(ctx);
+}
+
+static int mock_request_alloc(struct drm_i915_gem_request *request)
+{
+ request->ring = request->engine->buffer;
+ return 0;
+}
+
+static int mock_emit_flush(struct drm_i915_gem_request *request,
+ unsigned int flags)
+{
+ return 0;
+}
+
+static void mock_emit_breadcrumb(struct drm_i915_gem_request *request,
+u32 *flags)
+{
+}
+
+static void mock_submit_request(struct drm_i915_gem_request *request)
+{
+ struct mock_request *mock = container_of(request, typeof(*mock), base);
+ struct mock_engine *engine =
+ container_of(request->engine, typeof(*engine), base);
+
+ i915_gem_request_submit(request);
+
+ spin_lock_irq(&engine->hw_lock);
+ list_add_tail(&mock->link, &engine->hw_queue);
+ if (list_first_entry(&engine->hw_queue, typeof(*mock), link) == mock)
+ mod_timer(&engine->hw_delay, jiffies + mock->delay);
+ spin_unlock_irq(&engine->hw_lock);
+}
+
+static struct drm_i915_gem_request *
+mock_request(struct intel_engine_cs *engine,
+struct i915_gem_context *context,
+unsigned long delay)
+{
+ struct drm_i915_gem_request *request;
+ struct mock_request *mock;
+
+ request = i915_gem_request_alloc(engine, context);
+ if (!request)
+ return NULL;
+
+ mock = container_of(request, typeof(*mock), base);
+ INIT_LIST_HEAD(&mock->link);
+ mock->delay = delay;
+
+ return &mock->base;
+}
+
+static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
+{
+ struct intel_ring *ring;
+
+ ring = kzalloc(sizeof(*ring) + 4096, GFP_KERNEL);
+ if (!ring)
+ return NULL;
+
+ ring->engine = engine;
+ ring->size = 4096;
+ ring->effective_size = ring->size;
+ ring->vaddr = (void *)(ring + 1);
+
+
Re: [Intel-gfx] [PATCH] drm/i915: Catch non-existent registers in find_fw_domain
On Wed, Dec 07, 2016 at 03:49:39PM +0200, Joonas Lahtinen wrote: > Add WARN_ON to find_fw_domain to registers related to uninitialized > hardware. > > Cc: Imre Deak > Cc: Wang Elaine > Cc: Chris Wilson > Signed-off-by: Joonas Lahtinen > --- > drivers/gpu/drm/i915/intel_uncore.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index 07779d0..b20b58e 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -625,7 +625,12 @@ find_fw_domain(struct drm_i915_private *dev_priv, u32 > offset) > dev_priv->uncore.fw_domains_table_entries, > fw_range_cmp); > > - return entry ? entry->domains : 0; > + if (!entry) > + return 0; > + > + WARN_ON(entry->domains & ~dev_priv->uncore.fw_domains); Ok, this is going to be hard to diagnose in the wild, how about WARN(entry->domains & ~fw_domains, "Attempting to use register 0x%04x from uninitialised fw domain %x\n", offset, entry->domains & ~fw_domains); ? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] Smattering of selftests
On Wed, Dec 07, 2016 at 01:58:17PM +, Chris Wilson wrote: > More changes to GEM are on the cards, so before touching it again, let's > try and nail down how the internals are meant to work. The advantage > of mock testing is that we can write a universal test independent of the > hw (e.g. testing physical object creation) and we can inspect internal > state which should be able to spot subtle bugs easier than mashing the > uabi. The downside to mock testing is that it doubles the upfront cost > of every patch submission (if you change internal state, you're likely > going to upset a test) and adds maintainance burden tracking change to > external API (on the other hand it catches those silent changes that > would lead to broken code). I should also say it does not alleviate the need for uabi behaviour testing as well. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] FOR_UPSTREAM [VPG]: drm/i915: Parse panel BL controller from VBT
Currently the backlight controller is taken as 0. It needs to derive
value from the VBT. Adding the necessary changes.
Signed-off-by: Uma Shankar
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/i915_drv.h| 1 +
drivers/gpu/drm/i915/intel_bios.c | 5 +
drivers/gpu/drm/i915/intel_panel.c | 2 +-
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8daa4fb..6a85fdf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1633,6 +1633,7 @@ struct intel_vbt_data {
bool present;
bool active_low_pwm;
u8 min_brightness; /* min_brightness/255 of max */
+ u8 controller; /* brightness controller number */
enum intel_backlight_type type;
} backlight;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index eaade27..130db0f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -330,6 +330,8 @@ static u32 get_blocksize(const void *block_data)
method = &backlight_data->backlight_control[panel_type];
dev_priv->vbt.backlight.type = method->type;
+ dev_priv->vbt.backlight.controller = 0;
+ dev_priv->vbt.backlight.controller = method->controller;
}
dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
@@ -341,6 +343,9 @@ static u32 get_blocksize(const void *block_data)
dev_priv->vbt.backlight.active_low_pwm ? "low" : "high",
dev_priv->vbt.backlight.min_brightness,
backlight_data->level[panel_type]);
+
+ DRM_DEBUG_KMS("VBT BL controller %u\n",
+ dev_priv->vbt.backlight.controller);
}
/* Try to find sdvo panel data */
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 3578b40..6a7d4c3 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1612,7 +1612,7 @@ static int vlv_setup_backlight(struct intel_connector
*connector, enum pipe pipe
* For BXT hard coding the Backlight controller to 0.
* TODO : Read the controller value from VBT and generalize
*/
- panel->backlight.controller = 0;
+ panel->backlight.controller = dev_priv->vbt.backlight.controller;
pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect
On Tue, Dec 06, 2016 at 04:43:51PM -0800, Manasi Navare wrote: > The detect_done flag was introduced in the commit > 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid > multiple detects on hotplug where intel_dp_long_pulse() > was called from HPD handler as well as in intel_dp_detect. > So this detect_done flag was required to make sure intel_dp_detect() > did not call long pulse handler again if it was already been called > from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb > differs the long hpd handling entirely until the hotplug work runs to > avoid the double long hpd handling the "detect_done" flag is trying > to prevent. That sentence doesn't parse here. Anyways, the flag indeed is now a nop and your patch is pretty much the same what I did here: https://patchwork.freedesktop.org/patch/101476/ > > So now we do not need to reset the detect_done flag to false in > intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so > that intel_dp_detect does a full detect. However if the .detect > gets called during mode enumeration then we do not need to do a > full detect. This patch avoids the WARNS_ONS during connected boot > case when it calls intel_dp_check_link_status() due to multiple > detects How exactly does it do that? Also we shouldn't sweep that under the rug anyway. Instead someone should actually fix the problem that causes the WARN. > and also avoids DP compliance failures. It avoids doing > a full detect every single time on .detect(). > > Cc: Ville Syrjala > Cc: Daniel Vetter > Cc: Jani Nikula > Signed-off-by: Manasi Navare > --- > drivers/gpu/drm/i915/intel_dp.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index db75bb9..9c9277e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct > drm_i915_private *dev_priv, > if (!intel_dp->detect_done) > status = intel_dp_long_pulse(intel_dp->attached_connector); > > - intel_dp->detect_done = false; > - > return status; > } > > -- > 1.9.1 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: make Pineview a platform of its own
Pineview deserves to use its own platform enum (which was already added,
unused, previously).
Cc: Ville Syrjälä
Cc: Chris Wilson
Cc: Joonas Lahtinen
Signed-off-by: Jani Nikula
---
The wrinkle here is that IS_G33() still has to match both G33 and
Pineview. I'd prefer it if G33 *only* matched G33 and *not* Pineview. To
fix this, the alternatives are to a) add an additional IS_FOO() that
matches both, or b) replaces all references to IS_G33() with IS_G33() ||
IS_PINEVIEW(). I don't like either, but I also don't like
.is_pineview. We should reserve those for feature-ish things. Or just
apply this and leave it at that. Opinions?
---
drivers/gpu/drm/i915/i915_drv.h | 6 +++---
drivers/gpu/drm/i915/i915_pci.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8daa4fb13b52..4236cdd25b32 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -688,7 +688,6 @@ struct intel_csr {
#define DEV_INFO_FOR_EACH_FLAG(func) \
func(is_mobile); \
- func(is_pineview); \
func(is_lp); \
func(is_alpha_support); \
/* Keep has_* in alphabetical order */ \
@@ -2530,8 +2529,9 @@ intel_info(const struct drm_i915_private *dev_priv)
#define IS_G4X(dev_priv) (IS_G45(dev_priv) || IS_GM45(dev_priv))
#define IS_PINEVIEW_G(dev_priv)(INTEL_DEVID(dev_priv) == 0xa001)
#define IS_PINEVIEW_M(dev_priv)(INTEL_DEVID(dev_priv) == 0xa011)
-#define IS_PINEVIEW(dev_priv) ((dev_priv)->info.is_pineview)
-#define IS_G33(dev_priv) ((dev_priv)->info.platform == INTEL_G33)
+#define IS_PINEVIEW(dev_priv) ((dev_priv)->info.platform == INTEL_PINEVIEW)
+#define IS_G33(dev_priv) ((dev_priv)->info.platform == INTEL_G33 || \
+IS_PINEVIEW(dev_priv))
#define IS_IRONLAKE_M(dev_priv)(INTEL_DEVID(dev_priv) == 0x0046)
#define IS_IVYBRIDGE(dev_priv) ((dev_priv)->info.platform == INTEL_IVYBRIDGE)
#define IS_IVB_GT1(dev_priv) (INTEL_DEVID(dev_priv) == 0x0156 || \
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f7ec6e944e09..93f50ef2a309 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -141,7 +141,7 @@ static const struct intel_device_info intel_g33_info = {
static const struct intel_device_info intel_pineview_info = {
GEN3_FEATURES,
- .platform = INTEL_G33, .is_pineview = 1, .is_mobile = 1,
+ .platform = INTEL_PINEVIEW, .is_mobile = 1,
.has_hotplug = 1,
.has_overlay = 1,
};
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Catch non-existent registers in find_fw_domain
On ke, 2016-12-07 at 14:01 +, Chris Wilson wrote: > On Wed, Dec 07, 2016 at 03:49:39PM +0200, Joonas Lahtinen wrote: > > > > + WARN_ON(entry->domains & ~dev_priv->uncore.fw_domains); > > Ok, this is going to be hard to diagnose in the wild, how about > > WARN(entry->domains & ~fw_domains, > "Attempting to use register 0x%04x from uninitialised fw domain %x\n", > offset, entry->domains & ~fw_domains); ? That's an excellent idea. Although I'll call it "forcewake domain" for clarity. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/16] kselftests: Exercise hw-independent mock tests for i915.ko
On Wed, Dec 07, 2016 at 01:58:19PM +, Chris Wilson wrote: > Although being a GPU driver most functionality of i915.ko depends upon > real hardware, many of its internal interfaces can be "mocked" and so > tested independently of any hardware. Expanding the test coverage is not > only useful for i915.ko, but should provide some integration tests for > core infrastructure as well. > > Loading i915.ko with mock_selftests=-1 will cause it to execute its mock > tests then fail with -ENOTTY. If the driver is already loaded and bound > to hardware, it requires a few more steps to unbind, and so the simple > preliminary modprobe -r will fail. I changed the exit condition to return 0 after successfully completing the mock tests (when passed mock_selftests=-1) so modprobe reports success/fail clearly. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Does display engine read contents from LLC of scanout buffer?
On Wed, Dec 07, 2016 at 11:30:37AM +, Weng, Chuanbo wrote: > Hi Ville, > Thanks for your useful info. > How about L3 cache? In my scenario, Beignet will use Read Write Data > Port to write data (typed write) to bo. > So the cache path is L3 -> LLC -> memory. So only leave LLC cache ability the > same as PTEs is not enough. > In order to make data access efficient, I set L3 to WB. So is there a > way to flush L3 cache to memory? I don't recall. But actually caching in L3 is dangerous anyway since IIRC L3 evictions can land in LLC despite the LLC cacheability being set to UC/WT. Or at least there was a note stating that in bspec at some point. So to be totally correct you should not use L3 either. Well, I suppose you could use L3 as a RO cache, but sounds like you want to write as well. > > Thanks, > Chuanbo Weng > > -Original Message- > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com] > Sent: Friday, November 25, 2016 3:07 AM > To: Weng, Chuanbo > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] Does display engine read contents from LLC of > scanout buffer? > > On Thu, Nov 24, 2016 at 06:20:22PM +, Weng, Chuanbo wrote: > > Hi all, > >I have a question of display (forgive me if it's too simple > > because I'm not familiar with display): > > > >My scenario is as below: > > gbm_bo_create to create gbm_bo > > Get its handle > > drmModeAddFB to specify this bo as scanout > > buffer > > do rendering to this bo by OpenCL(Beignet) > > drmModePageFlip to do page flip > > > >Does display engine directly read contents from scanout > > buffer or read contents from LLC of this scanout buffer? > > The display engine sits between the LLC and main memory effectively, so the > answer is that it always reads directly from memory. When you make a bo a > scanout buffer the kernel will flush the caches and reconfigure the PTEs to > UC/WC so that subsequent rendering will hit memory directly. > And that also means you should never override potential scanout buffers to WB > via MOCS, and instead you should leave the choice up to the PTEs. > > -- > Ville Syrjälä > Intel OTC -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: make Pineview a platform of its own
On Wed, Dec 07, 2016 at 04:06:48PM +0200, Jani Nikula wrote:
> Pineview deserves to use its own platform enum (which was already added,
> unused, previously).
>
> Cc: Ville Syrjälä
> Cc: Chris Wilson
> Cc: Joonas Lahtinen
> Signed-off-by: Jani Nikula
>
> ---
>
> The wrinkle here is that IS_G33() still has to match both G33 and
> Pineview. I'd prefer it if G33 *only* matched G33 and *not* Pineview. To
> fix this, the alternatives are to a) add an additional IS_FOO() that
> matches both, or b) replaces all references to IS_G33() with IS_G33() ||
> IS_PINEVIEW().
This would be in line with what we did for VLV vs. CHV. So I think it
would be OK.
> I don't like either, but I also don't like
> .is_pineview. We should reserve those for feature-ish things. Or just
> apply this and leave it at that. Opinions?
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 +++---
> drivers/gpu/drm/i915/i915_pci.c | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8daa4fb13b52..4236cdd25b32 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -688,7 +688,6 @@ struct intel_csr {
>
> #define DEV_INFO_FOR_EACH_FLAG(func) \
> func(is_mobile); \
> - func(is_pineview); \
> func(is_lp); \
> func(is_alpha_support); \
> /* Keep has_* in alphabetical order */ \
> @@ -2530,8 +2529,9 @@ intel_info(const struct drm_i915_private *dev_priv)
> #define IS_G4X(dev_priv) (IS_G45(dev_priv) || IS_GM45(dev_priv))
> #define IS_PINEVIEW_G(dev_priv) (INTEL_DEVID(dev_priv) == 0xa001)
> #define IS_PINEVIEW_M(dev_priv) (INTEL_DEVID(dev_priv) == 0xa011)
> -#define IS_PINEVIEW(dev_priv)((dev_priv)->info.is_pineview)
> -#define IS_G33(dev_priv) ((dev_priv)->info.platform == INTEL_G33)
> +#define IS_PINEVIEW(dev_priv)((dev_priv)->info.platform ==
> INTEL_PINEVIEW)
> +#define IS_G33(dev_priv) ((dev_priv)->info.platform == INTEL_G33 || \
> + IS_PINEVIEW(dev_priv))
> #define IS_IRONLAKE_M(dev_priv) (INTEL_DEVID(dev_priv) == 0x0046)
> #define IS_IVYBRIDGE(dev_priv) ((dev_priv)->info.platform ==
> INTEL_IVYBRIDGE)
> #define IS_IVB_GT1(dev_priv) (INTEL_DEVID(dev_priv) == 0x0156 || \
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index f7ec6e944e09..93f50ef2a309 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -141,7 +141,7 @@ static const struct intel_device_info intel_g33_info = {
>
> static const struct intel_device_info intel_pineview_info = {
> GEN3_FEATURES,
> - .platform = INTEL_G33, .is_pineview = 1, .is_mobile = 1,
> + .platform = INTEL_PINEVIEW, .is_mobile = 1,
> .has_hotplug = 1,
> .has_overlay = 1,
> };
> --
> 2.1.4
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Catch non-existent registers in find_fw_domain
Add WARN_ON to find_fw_domain to registers related to uninitialized hardware. v2: - Print the uninitialized domains and register (Chris) Cc: Imre Deak Cc: Wang Elaine Cc: Chris Wilson Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_uncore.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 07779d0..88f3611 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -625,7 +625,14 @@ find_fw_domain(struct drm_i915_private *dev_priv, u32 offset) dev_priv->uncore.fw_domains_table_entries, fw_range_cmp); - return entry ? entry->domains : 0; + if (!entry) + return 0; + + WARN(entry->domains & ~dev_priv->uncore.fw_domains, +"Uninitialized forcewake domain(s) 0x%x accessed at 0x%x\n", +entry->domains & ~dev_priv->uncore.fw_domains, offset); + + return entry->domains; } static void -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm: Allow CAP_PRIME on !MODESET
== Series Details == Series: drm: Allow CAP_PRIME on !MODESET URL : https://patchwork.freedesktop.org/series/16479/ State : failure == Summary == CC drivers/acpi/acpica/utobject.o CC drivers/acpi/acpica/utownerid.o LD [M] drivers/ssb/ssb.o CC drivers/acpi/acpica/utpredef.o CC drivers/acpi/acpica/utosi.o CC drivers/acpi/acpica/utstate.o CC drivers/acpi/acpica/utresrc.o CC drivers/acpi/acpica/utstring.o CC drivers/acpi/acpica/utstrtoul64.o CC drivers/acpi/acpica/utxferror.o CC drivers/acpi/acpica/utxface.o CC drivers/acpi/acpica/utxfmutex.o CC drivers/acpi/acpica/utxfinit.o LD drivers/pci/pcie/pcieportdrv.o LD kernel/events/built-in.o LD drivers/pnp/pnpacpi/pnp.o LD kernel/sched/built-in.o scripts/Makefile.build:544: recipe for target 'drivers/gpu/drm' failed make[2]: *** [drivers/gpu/drm] Error 2 scripts/Makefile.build:544: recipe for target 'drivers/gpu' failed make[1]: *** [drivers/gpu] Error 2 make[1]: *** Waiting for unfinished jobs LD drivers/pnp/pnpacpi/built-in.o LD drivers/pnp/built-in.o LD [M] drivers/net/ethernet/broadcom/genet/genet.o LD kernel/built-in.o LD drivers/iommu/built-in.o LD net/packet/built-in.o LD net/xfrm/built-in.o LD drivers/usb/storage/usb-storage.o LD drivers/usb/storage/built-in.o LD lib/raid6/raid6_pq.o LD drivers/pci/pcie/aer/aerdriver.o LD lib/raid6/built-in.o LD drivers/pci/pcie/aer/built-in.o LD drivers/pci/pcie/built-in.o LD drivers/acpi/acpica/acpi.o LD [M] drivers/net/ethernet/intel/igbvf/igbvf.o LD drivers/md/dm-mod.o LD drivers/video/fbdev/core/fb.o LD drivers/video/fbdev/core/built-in.o LD [M] drivers/usb/serial/usbserial.o LD drivers/acpi/acpica/built-in.o LD drivers/usb/gadget/libcomposite.o LD drivers/video/fbdev/built-in.o LD drivers/pci/built-in.o LD drivers/acpi/built-in.o LD [M] drivers/misc/mei/mei-me.o LD drivers/misc/built-in.o LD drivers/tty/serial/8250/8250.o LD drivers/thermal/thermal_sys.o LD drivers/scsi/scsi_mod.o LD drivers/thermal/built-in.o LD [M] sound/pci/hda/snd-hda-codec-generic.o LD sound/pci/built-in.o LD sound/built-in.o LD drivers/usb/gadget/udc/udc-core.o LD drivers/spi/built-in.o LD drivers/usb/gadget/udc/built-in.o LD drivers/usb/gadget/built-in.o LD drivers/video/console/built-in.o LD drivers/video/built-in.o LD [M] drivers/net/ethernet/intel/e1000/e1000.o LD drivers/tty/serial/8250/8250_base.o LD drivers/tty/serial/8250/built-in.o LD drivers/tty/serial/built-in.o AR lib/lib.a EXPORTS lib/lib-ksyms.o LD lib/built-in.o CC arch/x86/kernel/cpu/capflags.o LD arch/x86/kernel/cpu/built-in.o LD arch/x86/kernel/built-in.o LD net/ipv6/ipv6.o LD net/ipv4/built-in.o LD drivers/usb/core/usbcore.o LD net/ipv6/built-in.o LD drivers/scsi/sd_mod.o LD drivers/scsi/built-in.o LD drivers/usb/core/built-in.o LD arch/x86/built-in.o LD fs/btrfs/btrfs.o LD drivers/md/md-mod.o LD drivers/md/built-in.o LD fs/btrfs/built-in.o LD drivers/tty/vt/built-in.o LD drivers/tty/built-in.o LD [M] drivers/net/ethernet/intel/igb/igb.o LD drivers/usb/host/xhci-hcd.o LD drivers/usb/host/built-in.o LD drivers/usb/built-in.o LD net/core/built-in.o LD fs/ext4/ext4.o LD fs/ext4/built-in.o LD net/built-in.o LD fs/built-in.o LD [M] drivers/net/ethernet/intel/e1000e/e1000e.o LD drivers/net/ethernet/built-in.o LD drivers/net/built-in.o Makefile:988: recipe for target 'drivers' failed make: *** [drivers] Error 2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Catch non-existent registers in find_fw_domain
On Wed, Dec 07, 2016 at 04:22:39PM +0200, Joonas Lahtinen wrote: > Add WARN_ON to find_fw_domain to registers related to uninitialized > hardware. > > v2: > - Print the uninitialized domains and register (Chris) > > Cc: Imre Deak > Cc: Wang Elaine > Cc: Chris Wilson > Signed-off-by: Joonas Lahtinen Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: Allow CAP_PRIME on !MODESET
vgem (and our igt tests using vgem) need this. I suspect etnaviv will
fare similarly.
v2. Make it build. Oops.
Fixes: d5264ed3823a ("drm: Return -ENOTSUPP when called for KMS cap with a
non-KMS driver")
Cc: Michel Dänzer
Cc: Alex Deucher
Cc: Chris Wilson
Reviewed-by: Chris Wilson
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_ioctl.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 0a900bd4575d..a16b6fbd1004 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -234,10 +234,15 @@ static int drm_getcap(struct drm_device *dev, void *data,
struct drm_file *file_
req->value = 0;
- /* Only one cap makes sense with a UMS driver: */
- if (req->capability == DRM_CAP_TIMESTAMP_MONOTONIC) {
+ /* Only some caps make sense with UMS/render-only drivers. */
+ switch (req->capability) {
+ case DRM_CAP_TIMESTAMP_MONOTONIC:
req->value = drm_timestamp_monotonic;
return 0;
+ case DRM_CAP_PRIME:
+ req->value |= dev->driver->prime_fd_to_handle ?
DRM_PRIME_CAP_IMPORT : 0;
+ req->value |= dev->driver->prime_handle_to_fd ?
DRM_PRIME_CAP_EXPORT : 0;
+ return 0;
}
/* Other caps only work with KMS drivers */
@@ -258,10 +263,6 @@ static int drm_getcap(struct drm_device *dev, void *data,
struct drm_file *file_
case DRM_CAP_DUMB_PREFER_SHADOW:
req->value = dev->mode_config.prefer_shadow;
break;
- case DRM_CAP_PRIME:
- req->value |= dev->driver->prime_fd_to_handle ?
DRM_PRIME_CAP_IMPORT : 0;
- req->value |= dev->driver->prime_handle_to_fd ?
DRM_PRIME_CAP_EXPORT : 0;
- break;
case DRM_CAP_ASYNC_PAGE_FLIP:
req->value = dev->mode_config.async_page_flip;
break;
--
2.10.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] FOR_UPSTREAM [VPG]: drm/i915: Parse panel BL controller from VBT
Currently the backlight controller is taken as 0. It needs to derive
value from the VBT. Adding the necessary changes.
Signed-off-by: Uma Shankar
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/i915_drv.h| 1 +
drivers/gpu/drm/i915/intel_bios.c | 5 +
drivers/gpu/drm/i915/intel_panel.c | 2 +-
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8daa4fb..6a85fdf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1633,6 +1633,7 @@ struct intel_vbt_data {
bool present;
bool active_low_pwm;
u8 min_brightness; /* min_brightness/255 of max */
+ u8 controller; /* brightness controller number */
enum intel_backlight_type type;
} backlight;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index eaade27..130db0f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -330,6 +330,8 @@ static u32 get_blocksize(const void *block_data)
method = &backlight_data->backlight_control[panel_type];
dev_priv->vbt.backlight.type = method->type;
+ dev_priv->vbt.backlight.controller = 0;
+ dev_priv->vbt.backlight.controller = method->controller;
}
dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
@@ -341,6 +343,9 @@ static u32 get_blocksize(const void *block_data)
dev_priv->vbt.backlight.active_low_pwm ? "low" : "high",
dev_priv->vbt.backlight.min_brightness,
backlight_data->level[panel_type]);
+
+ DRM_DEBUG_KMS("VBT BL controller %u\n",
+ dev_priv->vbt.backlight.controller);
}
/* Try to find sdvo panel data */
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 3578b40..6a7d4c3 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1612,7 +1612,7 @@ static int vlv_setup_backlight(struct intel_connector
*connector, enum pipe pipe
* For BXT hard coding the Backlight controller to 0.
* TODO : Read the controller value from VBT and generalize
*/
- panel->backlight.controller = 0;
+ panel->backlight.controller = dev_priv->vbt.backlight.controller;
pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Parse panel BL controller from VBT
Currently the backlight controller is taken as 0. It needs to derive
value from the VBT. Adding the necessary changes.
v2: Updated the commit header
Signed-off-by: Uma Shankar
Signed-off-by: Vidya Srinivas
---
drivers/gpu/drm/i915/i915_drv.h| 1 +
drivers/gpu/drm/i915/intel_bios.c | 5 +
drivers/gpu/drm/i915/intel_panel.c | 2 +-
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8daa4fb..6a85fdf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1633,6 +1633,7 @@ struct intel_vbt_data {
bool present;
bool active_low_pwm;
u8 min_brightness; /* min_brightness/255 of max */
+ u8 controller; /* brightness controller number */
enum intel_backlight_type type;
} backlight;
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index eaade27..130db0f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -330,6 +330,8 @@ static u32 get_blocksize(const void *block_data)
method = &backlight_data->backlight_control[panel_type];
dev_priv->vbt.backlight.type = method->type;
+ dev_priv->vbt.backlight.controller = 0;
+ dev_priv->vbt.backlight.controller = method->controller;
}
dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
@@ -341,6 +343,9 @@ static u32 get_blocksize(const void *block_data)
dev_priv->vbt.backlight.active_low_pwm ? "low" : "high",
dev_priv->vbt.backlight.min_brightness,
backlight_data->level[panel_type]);
+
+ DRM_DEBUG_KMS("VBT BL controller %u\n",
+ dev_priv->vbt.backlight.controller);
}
/* Try to find sdvo panel data */
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 3578b40..6a7d4c3 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1612,7 +1612,7 @@ static int vlv_setup_backlight(struct intel_connector
*connector, enum pipe pipe
* For BXT hard coding the Backlight controller to 0.
* TODO : Read the controller value from VBT and generalize
*/
- panel->backlight.controller = 0;
+ panel->backlight.controller = dev_priv->vbt.backlight.controller;
pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] tests/kms_plane_lowres: Plane visibility after atomic modesets
Hi Mika,
Thanks for respinning!
On 23 November 2016 at 11:49, Mika Kahola wrote:
> +bool kmstest_plane_visible(void)
> +{
> + char tmp[256];
> + FILE *fid;
> + bool visible = false;
> + struct kmstest_resolution resolution;
> + const char *mode = "r";
> + int ret;
> +
> + fid = igt_debugfs_fopen("i915_display_info", mode);
> +
> + igt_assert(fid != NULL);
This, however, breaks non-Intel drivers: we declare DRIVER_ANY, but
later assert that we can open this file. Maybe what would be better
would be to have:
void igt_assert_plane_visible(plane_id, should_be_visible)
which would then internally do an igt_assert() on the result, or
igt_skip() if that file is not present.
Also, it seems kind of fragile, as it only looks for the first plane
marked 'OVL'. What happens if we have multiple overlay planes?
Regardless, I think it's very close, and am happy to test a v3 on
non-i915. Thanks a lot for the rework!
Cheers,
Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Allow CAP_PRIME on !MODESET
On 07/12/16 11:49 PM, Daniel Vetter wrote:
> vgem (and our igt tests using vgem) need this. I suspect etnaviv will
> fare similarly.
>
> v2. Make it build. Oops.
>
> Fixes: d5264ed3823a ("drm: Return -ENOTSUPP when called for KMS cap with a
> non-KMS driver")
> Cc: Michel Dänzer
> Cc: Alex Deucher
> Cc: Chris Wilson
> Reviewed-by: Chris Wilson
> Signed-off-by: Daniel Vetter
Thanks Daniel, and sorry I missed this, guess I was thinking of !MODESET
too much in terms of UMS and too little in terms of render-only drivers.
Reviewed-by: Michel Dänzer
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH] drm/i915: Add a cursor hack to allow converting legacy page flip to atomic.
On Thu, Nov 24, 2016 at 04:35:35PM +0100, Maarten Lankhorst wrote:
> Do something similar to vc4, only allow updating the cursor state
> in-place through a fastpath when the watermarks are unaffected. This
> will allow cursor movement to be smooth, but changing cursor size or
> showing/hiding cursor will still fall back so watermarks can be updated.
>
> Not completely tested yet and doesn't handle fence structs well,
> quite likely leaks.. so I'm sending this out as RFC only.
>
> Signed-off-by: Maarten Lankhorst
fwiw I think this is the most reasonable short-term path to fix this. Long
term we probably want to codify this (refactoring together with what vc4
has). Didn't review, but
Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_atomic_plane.c | 47 +++-
> drivers/gpu/drm/i915/intel_display.c | 116
> +-
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 3 files changed, 146 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index dbe9fb41ae53..60d75ce8a989 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -103,36 +103,24 @@ intel_plane_destroy_state(struct drm_plane *plane,
> drm_atomic_helper_plane_destroy_state(plane, state);
> }
>
> -static int intel_plane_atomic_check(struct drm_plane *plane,
> - struct drm_plane_state *state)
> +int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
> + struct intel_plane_state *intel_state)
> {
> + struct drm_plane *plane = intel_state->base.plane;
> struct drm_i915_private *dev_priv = to_i915(plane->dev);
> - struct drm_crtc *crtc = state->crtc;
> - struct intel_crtc *intel_crtc;
> - struct intel_crtc_state *crtc_state;
> + struct drm_plane_state *state = &intel_state->base;
> struct intel_plane *intel_plane = to_intel_plane(plane);
> - struct intel_plane_state *intel_state = to_intel_plane_state(state);
> - struct drm_crtc_state *drm_crtc_state;
> int ret;
>
> - crtc = crtc ? crtc : plane->state->crtc;
> - intel_crtc = to_intel_crtc(crtc);
> -
> /*
>* Both crtc and plane->crtc could be NULL if we're updating a
>* property while the plane is disabled. We don't actually have
>* anything driver-specific we need to test in that case, so
>* just return success.
>*/
> - if (!crtc)
> + if (!intel_state->base.crtc && !plane->state->crtc)
> return 0;
>
> - drm_crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc);
> - if (WARN_ON(!drm_crtc_state))
> - return -EINVAL;
> -
> - crtc_state = to_intel_crtc_state(drm_crtc_state);
> -
> /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
> intel_state->clip.x1 = 0;
> intel_state->clip.y1 = 0;
> @@ -184,6 +172,31 @@ static int intel_plane_atomic_check(struct drm_plane
> *plane,
> return intel_plane_atomic_calc_changes(&crtc_state->base, state);
> }
>
> +static int intel_plane_atomic_check(struct drm_plane *plane,
> + struct drm_plane_state *state)
> +{
> + struct drm_crtc *crtc = state->crtc;
> + struct drm_crtc_state *drm_crtc_state;
> +
> + crtc = crtc ? crtc : plane->state->crtc;
> +
> + /*
> + * Both crtc and plane->crtc could be NULL if we're updating a
> + * property while the plane is disabled. We don't actually have
> + * anything driver-specific we need to test in that case, so
> + * just return success.
> + */
> + if (!crtc)
> + return 0;
> +
> + drm_crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc);
> + if (WARN_ON(!drm_crtc_state))
> + return -EINVAL;
> +
> + return
> intel_plane_atomic_check_with_state(to_intel_crtc_state(drm_crtc_state),
> +to_intel_plane_state(state));
> +}
> +
> static void intel_plane_atomic_update(struct drm_plane *plane,
> struct drm_plane_state *old_state)
> {
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e9741fc8b04e..919d30996f3f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14689,7 +14689,7 @@ static const struct drm_crtc_funcs intel_crtc_funcs =
> {
> .set_config = drm_atomic_helper_set_config,
> .set_property = drm_atomic_helper_crtc_set_property,
> .destroy = intel_crtc_destroy,
> - .page_flip = intel_crtc_page_flip,
> + .page_flip = drm_atomic_helper_page_flip,
> .atomic_duplicate_state = intel_crtc_duplicate_state,
> .atomic_destroy_state = intel_crtc_destroy_state,
> };
> @@ -14955,6 +14955,118 @@ const struct
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Reorder phys backing storage release (rev2)
== Series Details == Series: drm/i915: Reorder phys backing storage release (rev2) URL : https://patchwork.freedesktop.org/series/16468/ State : success == Summary == Series 16468v2 drm/i915: Reorder phys backing storage release https://patchwork.freedesktop.org/api/1.0/series/16468/revisions/2/mbox/ fi-bdw-5557u total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28 fi-bsw-n3050 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-bxt-t5700 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-byt-j1900 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-byt-n2820 total:247 pass:202 dwarn:0 dfail:0 fail:0 skip:45 fi-hsw-4770 total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-hsw-4770r total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-ilk-650 total:247 pass:181 dwarn:0 dfail:0 fail:0 skip:66 fi-ivb-3520m total:247 pass:213 dwarn:0 dfail:0 fail:0 skip:34 fi-ivb-3770 total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-kbl-7500u total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-skl-6260u total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-skl-6700hqtotal:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-skl-6700k total:247 pass:210 dwarn:3 dfail:0 fail:0 skip:34 fi-skl-6770hqtotal:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-snb-2520m total:247 pass:202 dwarn:0 dfail:0 fail:0 skip:45 fi-snb-2600 total:247 pass:201 dwarn:0 dfail:0 fail:0 skip:46 c5421da3288090b260e882bdfa5754a122ba6263 drm-tip: 2016y-12m-07d-13h-56m-01s UTC integration manifest abae835 drm/i915: Reorder phys backing storage release == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3216/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v10 17/21] lib/sw_sync: Add igt_require_sw_sync to enable skipping on no sw_sync support
On 2016-12-07 06:06 AM, Petri Latvala wrote:
On Tue, Dec 06, 2016 at 09:52:09PM -0500, Robert Foss wrote:
Add igt_require_sw_sync to provide tests to skip if sw_sync support isn't
available on the host machine.
Signed-off-by: Robert Foss
Reviewed-by: Tomeu Vizoso
---
lib/sw_sync.c | 22 ++
lib/sw_sync.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/lib/sw_sync.c b/lib/sw_sync.c
index a2168f78..d4ecc898 100644
--- a/lib/sw_sync.c
+++ b/lib/sw_sync.c
@@ -194,3 +194,25 @@ int sync_fence_count_status(int fd, int status)
igt_assert_f(count >= 0, "No fences with supplied status found");
return count;
}
+
+static bool kernel_has_sw_sync(void)
+{
+ bool err;
+
+ igt_ignore_warn(system("/sbin/modprobe -s r sw_sync"));
+
+ err = false;
+ if (access(DEVFS_SW_SYNC, R_OK | W_OK) < 0) {
+ char buf[128];
+
+ snprintf(buf, sizeof(buf), "%s/sw_sync", igt_debugfs_mount());
+ err = access(DEBUGFS_SW_SYNC, R_OK | W_OK) < 0;
+ }
Did you mean access(buf, R_OK | W_OK) here?
Yes, fixing this in v11.
Thanks!
--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH xf86-video-intel v2] tools/backlight_helper: #include "config.h"
From: Mike Gilbert The file uses defines from config.h but never actually includes it. Signed-off-by: Mike Gilbert Signed-off-by: Mike Frysinger --- tools/backlight_helper.c | 4 1 file changed, 4 insertions(+) diff --git a/tools/backlight_helper.c b/tools/backlight_helper.c index a00f0d6bd8a2..aadb8fac92ba 100644 --- a/tools/backlight_helper.c +++ b/tools/backlight_helper.c @@ -1,3 +1,7 @@ +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + #include #include #include -- 2.11.0.rc2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [01/16] drm/i915: Provide a hook for selftests
== Series Details == Series: series starting with [01/16] drm/i915: Provide a hook for selftests URL : https://patchwork.freedesktop.org/series/16488/ State : success == Summary == Series 16488v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/16488/revisions/1/mbox/ fi-bdw-5557u total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28 fi-bsw-n3050 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-bxt-t5700 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-byt-j1900 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-byt-n2820 total:247 pass:202 dwarn:0 dfail:0 fail:0 skip:45 fi-hsw-4770 total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-hsw-4770r total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-ilk-650 total:247 pass:181 dwarn:0 dfail:0 fail:0 skip:66 fi-ivb-3520m total:247 pass:213 dwarn:0 dfail:0 fail:0 skip:34 fi-ivb-3770 total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-kbl-7500u total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-skl-6260u total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-skl-6700hqtotal:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-skl-6700k total:247 pass:210 dwarn:3 dfail:0 fail:0 skip:34 fi-skl-6770hqtotal:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-snb-2520m total:247 pass:202 dwarn:0 dfail:0 fail:0 skip:45 fi-snb-2600 total:247 pass:201 dwarn:0 dfail:0 fail:0 skip:46 c5421da3288090b260e882bdfa5754a122ba6263 drm-tip: 2016y-12m-07d-13h-56m-01s UTC integration manifest 3326f57 drm/i915: Add selftests for object allocation, phys b524565 drm/i915: Add a simple fence selftest to i915_gem_request 76ba51f drm/i915: Add a simple request selftest for waiting 5daaf73 drm/i915: Add selftests for i915_gem_request 1b56da3 drm/i915: Swap if(enable_execlists) in i915_gem_request_alloc for a vfunc ee582d5 drm/i915/execlists: Request the kernel context be pinned high e571ba1 drm/i915: Mark the shadow gvt context as closed 4473ea1 drm/i915: Simplify releasing context reference 0a3bad5 drm/i915: Unify active context tracking between legacy/execlists/guc fe08f31 drm/i915: Move intel_lrc_context_pin() to avoid the forward declaration 1cf26bd drm/i915: Add a reminder that i915_vma_move_to_active() requires struct_mutex 9996660 drm/i915: Add unit tests for the breadcrumb rbtree, wakeups 52bd7cd drm/i915: Add unit tests for the breadcrumb rbtree, completion e86a257 drm/i915: Add unit tests for the breadcrumb rbtree, insert/remove 58127e4 kselftests: Exercise hw-independent mock tests for i915.ko 0fd7803 drm/i915: Provide a hook for selftests == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3217/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 11/15] drm/i915: Protect DSPARB registers with a spinlock
On Tue, Dec 06, 2016 at 09:26:22AM +0100, Maarten Lankhorst wrote: > Op 05-12-16 om 15:13 schreef ville.syrj...@linux.intel.com: > > From: Ville Syrjälä > > > > Each DSPARB register can house bits for two separate pipes, hence > > we must protect the registers during reprogramming so that parallel > > FIFO reconfigurations happening simultaneosly on multiple pipes won't > > corrupt each others values. > > > > We'll use a new spinlock for this instead of the wm_mutex since we'll > > have to move the DSPARB programming to happen from the vblank evade > > critical section, and we can't use mutexes in there. > > > > v2: Document why we use a spinlock instead of a mutex (Maarten) > > > > Cc: Maarten Lankhorst > > Signed-off-by: Ville Syrjälä > Reviewed-by: Maarten Lankhorst Thanks. Pushed to dinq. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/perf: use DRM_DEBUG for userspace issues
On Fri, Dec 02, 2016 at 12:18:44PM +, Robert Bragg wrote:
> On Fri, Dec 2, 2016 at 8:35 AM, Daniel Vetter wrote:
>
> > On Thu, Dec 01, 2016 at 05:21:52PM +, Robert Bragg wrote:
> > > Avoid using DRM_ERROR for conditions userspace can trigger with a bad
> > > config when opening a stream or from not reading data in a timely
> > > fashion (whereby the OA buffer fills up). These conditions are tested
> > > by i-g-t which treats error messages as failures if using the test
> > > runner. This wasn't an issue while the i915-perf igt tests were being
> > > run in isolation.
> > >
> > > One message relating to seeing a spurious zeroed report was changed to
> > > use DRM_NOTE instead of DRM_ERROR. Ideally this warning shouldn't be
> > > seen, but it's not a serious problem if it is. Considering that the
> > > tail margin mechanism is only a heuristic it's possible we might see
> > > this from time to time.
> > >
> > > Signed-off-by: Robert Bragg > > Cc: Daniel Vetter
> > >
> > > fix i915_perf dbg messages
> > > ---
> > > drivers/gpu/drm/i915/i915_perf.c | 42 --
> > --
> > > 1 file changed, 21 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_perf.c
> > b/drivers/gpu/drm/i915/i915_perf.c
> > > index 9551282..5705005 100644
> > > --- a/drivers/gpu/drm/i915/i915_perf.c
> > > +++ b/drivers/gpu/drm/i915/i915_perf.c
> > > @@ -474,7 +474,7 @@ static int gen7_append_oa_reports(struct
> > i915_perf_stream *stream,
> > >* copying it to userspace...
> > >*/
> > > if (report32[0] == 0) {
> > > - DRM_ERROR("Skipping spurious, invalid OA
> > report\n");
> > > + DRM_NOTE("Skipping spurious, invalid OA report\n");
> > > continue;
> >
> > The above looks like a genuine hw/kernel fail, which we shouldn't put
> > under the carpet. I'd leave it at DRM_ERROR - I can bikeshed that while
> > applying if you're ok. Otherwise lgtm, will apply as soon as we've
> > clarified that.
> >
>
> It's something that is unfortunately expected to be possible from time to
> time due to a hardware race condition between the OA unit updating the tail
> pointer for a new report and that report actually becoming visible to the
> cpu in memory.
>
> If/when it happens it's not really a significant problem for userspace
> (assuming it's rare/intermittent given what the driver does as a
> best-effort workaround here). Userspace sees a briefly lower sampling
> resolution but the metrics can still be normalized.
>
> We wouldn't want i-g-t failing in this case, so that's why I was changing
> it.
>
> It's not really something you want to see ideally (it implies our
> heuristic-based software workaround isn't perfect). If it's seen a lot then
> that certainly should be considered a warning that we need to try and
> improve how we workaround the race condition. If you see it rarely then is
> somewhere between a note, and a warning I suppose.
Ok makes sense, patch applied.
-Daniel
>
> Regards,
> - Robert
>
>
> > -Daniel
> >
> > > }
> > >
> > > @@ -551,7 +551,7 @@ static int gen7_oa_read(struct i915_perf_stream
> > *stream,
> > > if (ret)
> > > return ret;
> > >
> > > - DRM_ERROR("OA buffer overflow: force restart\n");
> > > + DRM_DEBUG("OA buffer overflow: force restart\n");
> > >
> > > dev_priv->perf.oa.ops.oa_disable(dev_priv);
> > > dev_priv->perf.oa.ops.oa_enable(dev_priv);
> > > @@ -1000,17 +1000,17 @@ static int i915_oa_stream_init(struct
> > i915_perf_stream *stream,
> > >* IDs
> > >*/
> > > if (!dev_priv->perf.metrics_kobj) {
> > > - DRM_ERROR("OA metrics weren't advertised via sysfs\n");
> > > + DRM_DEBUG("OA metrics weren't advertised via sysfs\n");
> > > return -EINVAL;
> > > }
> > >
> > > if (!(props->sample_flags & SAMPLE_OA_REPORT)) {
> > > - DRM_ERROR("Only OA report sampling supported\n");
> > > + DRM_DEBUG("Only OA report sampling supported\n");
> > > return -EINVAL;
> > > }
> > >
> > > if (!dev_priv->perf.oa.ops.init_oa_buffer) {
> > > - DRM_ERROR("OA unit not supported\n");
> > > + DRM_DEBUG("OA unit not supported\n");
> > > return -ENODEV;
> > > }
> > >
> > > @@ -1019,17 +1019,17 @@ static int i915_oa_stream_init(struct
> > i915_perf_stream *stream,
> > >* we currently only allow exclusive access
> > >*/
> > > if (dev_priv->perf.oa.exclusive_stream) {
> > > - DRM_ERROR("OA unit already in use\n");
> > > + DRM_DEBUG("OA unit already in use\n");
> > > return -EBUSY;
> > > }
> > >
> > > if (!props->metrics_set) {
> > > - DRM_ERROR("OA metric set not specified\n");
> > > + DRM_DEBUG("OA metric set n
[Intel-gfx] [PATCH] drm/i915: Consolidate checks for memcpy-from-wc support
In order to silence sparse:
../drivers/gpu/drm/i915/i915_gpu_error.c:200:39: warning: Using plain integer
as NULL pointer
add a helper to check whether we have sse4.1 and that the desired
alignment is valid for acceleration.
Reported-by: Ville Syrjälä
Signed-off-by: Chris Wilson
Cc: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h| 3 +++
drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
4 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index ebeb13ba81d7..9f52fef5c76a 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1076,7 +1076,7 @@ static u32 *copy_batch(struct drm_i915_gem_object
*dst_obj,
src = ERR_PTR(-ENODEV);
if (src_needs_clflush &&
- i915_memcpy_from_wc((void *)(uintptr_t)batch_start_offset, NULL,
0)) {
+ i915_has_memcpy_from_wc((uintptr_t)batch_start_offset)) {
src = i915_gem_object_pin_map(src_obj, I915_MAP_WC);
if (!IS_ERR(src)) {
i915_memcpy_from_wc(dst,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d455e9b22388..0746d5574044 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3913,6 +3913,9 @@ __i915_request_irq_complete(const struct
drm_i915_gem_request *req)
void i915_memcpy_init_early(struct drm_i915_private *dev_priv);
bool i915_memcpy_from_wc(void *dst, const void *src, unsigned long len);
+#define i915_has_memcpy_from_wc(align) \
+ i915_memcpy_from_wc((void *)(align), NULL, 0)
+
/* i915_mm.c */
int remap_io_mapping(struct vm_area_struct *vma,
unsigned long addr, unsigned long pfn, unsigned long size,
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index a3f90536c38e..25909bff2790 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -197,7 +197,7 @@ static bool compress_init(struct compress *c)
}
c->tmp = NULL;
- if (i915_memcpy_from_wc(NULL, 0, 0))
+ if (i915_has_memcpy_from_wc(0))
c->tmp = (void *)__get_free_page(GFP_ATOMIC | __GFP_NOWARN);
return true;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
b/drivers/gpu/drm/i915/i915_guc_submission.c
index e3bca814aaf9..24c0b0d543cc 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1224,7 +1224,7 @@ static void guc_log_create(struct intel_guc *guc)
* it should be present on the chipsets supporting GuC based
* submisssions.
*/
- if (WARN_ON(!i915_memcpy_from_wc(NULL, NULL, 0))) {
+ if (WARN_ON(!i915_has_memcpy_from_wc(0))) {
/* logging will not be enabled */
i915.guc_log_level = -1;
return;
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove instructions to file a bug report.
On Mon, Dec 05, 2016 at 04:55:47PM -0800, Matt Turner wrote: > On Sat, Dec 3, 2016 at 1:52 AM, Jani Nikula > wrote: > > On Sat, 03 Dec 2016, Matt Turner wrote: > >> From these instructions, users assume that /sys/class/drm/card0/error > >> contains all the information a developer needs to diagnose and fix a GPU > >> hang. > >> > >> In fact it doesn't, and we have no tools for solving them (other than > >> stabbing in the dark). Most of the time the error state itself isn't > >> even useful because it just shows a hang on a PIPE_CONTROL or similar. > >> > >> Until a time when the error state contains enough information to > >> actually solve a hang, stop telling users to file unsolvable bugs, and > >> instead rely on users who know where and how to file a good bug report > >> to find their own way there. > >> > >> Signed-off-by: Matt Turner > >> --- > >> Maybe now's a good time to discuss what *would* be useful to put in the > >> error state for debugging hangs. The currently executing shader program > >> would be a great place to start. > > > > I'm wondering why we're getting this patch now, and my guess is that > > it's because we have been reassigning the related bugs to Mesa more > > actively lately. Is that the case? > > No, it's simply because I spent a week going through Bugzilla and > realized how incomplete an unactionable the majority of GPU hang > reports are. > > Asking users to report bugs, but not telling them what actually > constitutes a bug report, is a recipe for a lot of wasted developer > time. > > I suspect we could improve the usefulness of the reports by directing > users to a webpage that gave a few suggestions (tell us what you were > doing when the hang occurred would be an obvious one) about filing a > bug and then provided a link to Bugzilla. Or even configured Bugzilla > to have a default template that requested various bits of information. I think dumping at least some of the aux buffers should make this tons more useful for mesa, since it would indicate stuff like "we always die on resolves on skl gt4" or stuff like that. Thus far error states have been mostly used by kernel folks to debug kernel issues, which is why none of that additional stuff gets dumped. But a bare-bones parser to hunt for indirect state base addresses and fish out the aux stuff shouldn't be that hard, and might make this fully useful. Like Chris said the goal is to at least be able to triage and classify bugs, and I'm perfectly fine with merging additional code to the dumper to get there for mesa folks. We z-compress the state, so size isn't really an issue. And Ben has commit rights, so shouldn't be a problem to get this all merged. > > IIUC the bug reports are useful for us when it's a kernel bug, but less > > useful for you when it's a Mesa bug. And you'd rather have fewer > > incoming bugs that you think are unsolvable with the information at > > hand. > > > > Sounds like a bug workflow issue between drm/i915 and Mesa to be ironed > > out. > > Indeed. I'd rather have the information provided in a bug report to > actually solve it. I hope having access to the shader program will > make many more reports useful. > > I am also happy to see that there's now a sunset to the GPU hang message. The other option is that mesa folks don't want error states that we triage to mesa. We could definitely update the process in that area. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt] intel-ci: Do module loads first + last
Do the module reload test first, so that it has the best chance of succeeding without outside influence (broken driver). And then do it last, so that it has the best chance of catching some missing finalisation (e.g. memleak) over the lifetime of the testing. Signed-off-by: Chris Wilson Cc: Petri Latvala --- tests/intel-ci/fast-feedback.testlist | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist index e25facf3..b79b0c14 100644 --- a/tests/intel-ci/fast-feedback.testlist +++ b/tests/intel-ci/fast-feedback.testlist @@ -1,11 +1,10 @@ +igt@drv_module_reload@basic-reload +igt@drv_module_reload@basic-reload-inject igt@core_auth@basic-auth igt@core_prop_blob@basic igt@drv_getparams_basic@basic-eu-total igt@drv_getparams_basic@basic-subslice-total igt@drv_hangman@error-state-basic -igt@drv_module_reload@basic-reload -igt@drv_module_reload@basic-reload-inject -igt@drv_module_reload@basic-reload-final igt@gem_basic@bad-close igt@gem_basic@create-close igt@gem_basic@create-fd-close @@ -245,3 +244,4 @@ igt@vgem_basic@mmap igt@vgem_basic@second-client igt@vgem_basic@sysfs igt@vgem_basic@unload +igt@drv_module_reload@basic-reload-final # keep last -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: make Pineview a platform of its own
== Series Details == Series: drm/i915: make Pineview a platform of its own URL : https://patchwork.freedesktop.org/series/16490/ State : warning == Summary == Series 16490v1 drm/i915: make Pineview a platform of its own https://patchwork.freedesktop.org/api/1.0/series/16490/revisions/1/mbox/ Test kms_force_connector_basic: Subgroup force-connector-state: pass -> DMESG-WARN (fi-snb-2520m) fi-bdw-5557u total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28 fi-bsw-n3050 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-byt-j1900 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-byt-n2820 total:247 pass:202 dwarn:0 dfail:0 fail:0 skip:45 fi-hsw-4770 total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-hsw-4770r total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-ilk-650 total:247 pass:181 dwarn:0 dfail:0 fail:0 skip:66 fi-ivb-3520m total:247 pass:213 dwarn:0 dfail:0 fail:0 skip:34 fi-ivb-3770 total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-kbl-7500u total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-skl-6260u total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-skl-6700hqtotal:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-skl-6700k total:247 pass:210 dwarn:3 dfail:0 fail:0 skip:34 fi-skl-6770hqtotal:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-snb-2520m total:247 pass:201 dwarn:1 dfail:0 fail:0 skip:45 fi-snb-2600 total:247 pass:201 dwarn:0 dfail:0 fail:0 skip:46 a20a4a6107256124d136c8320fc3f7adef93bb8a drm-tip: 2016y-12m-07d-15h-56m-01s UTC integration manifest 8b8c20d drm/i915: make Pineview a platform of its own == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3218/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] dim: Try git-merge --ff-only before git-rebase -i when updating branches.
Op 07-12-16 om 10:33 schreef Jani Nikula: > On Tue, 06 Dec 2016, Maarten Lankhorst > wrote: >> When a branch can be fast-forwarded, try it first before rebasing. >> This will prevent a whole lot of editor windows opening with 'noop' >> when running dim ub. >> >> Signed-off-by: Maarten Lankhorst >> --- >> dim | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/dim b/dim >> index fa63ae8c8a79..b2e6841e23d7 100755 >> --- a/dim >> +++ b/dim >> @@ -1286,9 +1286,7 @@ function dim_update_branches >> repo=`branch_to_repo $branch` >> remote=`repo_to_remote $repo` >> >> -if git diff --quiet $remote/$branch; then >> -$DRY git rebase >> -else >> +if ! $DRY git merge --ff-only ; then > What does it assume for branches to merge when none are provided? Would > it be better to be explicit? > > Thanks for this, I've been meaning to do this forever... Hm I guess explicit might be better, what about this? --8<--- When a branch can be fast-forwarded, try it first before rebasing. This will prevent a whole lot of editor windows opening with 'noop' when running dim ub. Signed-off-by: Maarten Lankhorst --- diff --git a/dim b/dim index fa63ae8c8a79..e0551ace54e4 100755 --- a/dim +++ b/dim @@ -1286,9 +1286,7 @@ function dim_update_branches repo=`branch_to_repo $branch` remote=`repo_to_remote $repo` - if git diff --quiet $remote/$branch; then - $DRY git rebase - else + if ! $DRY git merge --ff-only $remote/$branch; then $DRY git rebase -i fi done ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/11] drm/i915: Overlay fixes
From: Ville Syrjälä Fixes for some recentish regressions in the overlay support (kernel would oops as soon as you try to use the overlay), and I decided to dig up a few older cleanups and fixes I had lying around in my gen2 dungeon. Entire series (with Chris's phys obj fix) here: git://github.com/vsyrjala/linux.git overlay_fixes Ville Syrjälä (11): drm/i915: Fix oops in overlay due to frontbuffer trakcing drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff drm/i915: Restore dev_priv->mm.interruptible for overlay drm/i915: Fix the overlay frontbuffer tracking drm/i915: Use pipe_src_w in overlay code drm/i915: Kill intel_panel_fitter_pipe() drm/i915: Simplify SWIDTHSW calculation drm/i915: Reorganize overlay filter coeffs into a nicer form drm/i915: Use primary plane->state for overlay ckey setup drm/i915: Disable L2 cache clock gating on 830 when using the overlay drm/i915: Kill the 830 MI_OVERLAY_OFF workaround drivers/gpu/drm/i915/i915_gem_request.c | 36 + drivers/gpu/drm/i915/i915_gem_request.h | 35 +--- drivers/gpu/drm/i915/i915_reg.h | 4 + drivers/gpu/drm/i915/intel_display.c| 2 +- drivers/gpu/drm/i915/intel_overlay.c| 278 +--- drivers/gpu/drm/i915/intel_pm.c | 2 - 6 files changed, 193 insertions(+), 164 deletions(-) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/11] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff
From: Ville Syrjälä
The i915_gem_active stuff doesn't like a NULL ->retire hook, but
the overlay code can set it to NULL. That obviously ends up oopsing.
Fix it by setting the ->retire hook using init_request_active()
so that it'll do the NULL->i915_gem_retire_noop conversion for us.
Cc: Chris Wilson
Cc: Joonas Lahtinen
Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 1cc963814224..786389dd5175 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -216,7 +216,7 @@ static void intel_overlay_submit_request(struct
intel_overlay *overlay,
{
GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
&overlay->i915->drm.struct_mutex));
- overlay->last_flip.retire = retire;
+ init_request_active(&overlay->last_flip, retire);
i915_gem_active_set(&overlay->last_flip, req);
i915_add_request(req);
}
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay
From: Ville Syrjälä
We need an uninterruptible wait for the overlay off request during
modeset. Restore the operation of dev_priv->mm.interruptible
sufficiently for that.
Toss in a WARN_ON() to make sure the request succeeds.
Fixes: 7da844c5c6fc ("drm/i915: Move the special case wait-request handling to
its one caller")
Cc: Chris Wilson
Cc: Joonas Lahtinen
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_gem_request.c | 36 +
drivers/gpu/drm/i915/i915_gem_request.h | 35 ++--
drivers/gpu/drm/i915/intel_display.c| 2 +-
3 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c
b/drivers/gpu/drm/i915/i915_gem_request.c
index fcf22b0e2967..1a7b88166c51 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -1199,3 +1199,39 @@ void i915_gem_retire_requests(struct drm_i915_private
*dev_priv)
for_each_engine(engine, dev_priv, id)
engine_retire_requests(engine);
}
+
+/**
+ * i915_gem_active_retire - waits until the request is retired
+ * @active - the active request on which to wait
+ *
+ * i915_gem_active_retire() waits until the request is completed,
+ * and then ensures that at least the retirement handler for this
+ * @active tracker is called before returning. If the @active
+ * tracker is idle, the function returns immediately.
+ */
+int __must_check
+i915_gem_active_retire(struct i915_gem_active *active,
+ struct mutex *mutex)
+{
+ struct drm_i915_gem_request *request;
+ long ret;
+
+ request = i915_gem_active_raw(active, mutex);
+ if (!request)
+ return 0;
+
+ ret = i915_wait_request(request,
+ (request->i915->mm.interruptible ?
+I915_WAIT_INTERRUPTIBLE : 0) |
+ I915_WAIT_LOCKED,
+ MAX_SCHEDULE_TIMEOUT);
+ if (ret < 0)
+ return ret;
+
+ list_del_init(&active->link);
+ RCU_INIT_POINTER(active->request, NULL);
+
+ active->retire(active, request);
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h
b/drivers/gpu/drm/i915/i915_gem_request.h
index e2b077df2da0..09add6b9cfc7 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -657,39 +657,8 @@ i915_gem_active_wait(const struct i915_gem_active *active,
unsigned int flags)
return ret < 0 ? ret : 0;
}
-/**
- * i915_gem_active_retire - waits until the request is retired
- * @active - the active request on which to wait
- *
- * i915_gem_active_retire() waits until the request is completed,
- * and then ensures that at least the retirement handler for this
- * @active tracker is called before returning. If the @active
- * tracker is idle, the function returns immediately.
- */
-static inline int __must_check
-i915_gem_active_retire(struct i915_gem_active *active,
- struct mutex *mutex)
-{
- struct drm_i915_gem_request *request;
- long ret;
-
- request = i915_gem_active_raw(active, mutex);
- if (!request)
- return 0;
-
- ret = i915_wait_request(request,
- I915_WAIT_INTERRUPTIBLE | I915_WAIT_LOCKED,
- MAX_SCHEDULE_TIMEOUT);
- if (ret < 0)
- return ret;
-
- list_del_init(&active->link);
- RCU_INIT_POINTER(active->request, NULL);
-
- active->retire(active, request);
-
- return 0;
-}
+int __must_check i915_gem_active_retire(struct i915_gem_active *active,
+ struct mutex *mutex);
#define for_each_active(mask, idx) \
for (; mask ? idx = ffs(mask) - 1, 1 : 0; mask &= ~BIT(idx))
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a41082e2750e..5a74991854e3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4923,7 +4923,7 @@ static void intel_crtc_dpms_overlay_disable(struct
intel_crtc *intel_crtc)
mutex_lock(&dev->struct_mutex);
dev_priv->mm.interruptible = false;
- (void) intel_overlay_switch_off(intel_crtc->overlay);
+ WARN_ON(intel_overlay_switch_off(intel_crtc->overlay) != 0);
dev_priv->mm.interruptible = true;
mutex_unlock(&dev->struct_mutex);
}
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/11] drm/i915: Kill intel_panel_fitter_pipe()
From: Ville Syrjälä
Check pipe config gmch_pfit.control instead of using intel_panel_fitter_pipe()
to figure out if the pipe for the overlay is using the panel fitter.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 29 +
1 file changed, 1 insertion(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 638933bfd8a0..10e8d3b9a0c4 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1075,33 +1075,6 @@ static int check_overlay_src(struct drm_i915_private
*dev_priv,
return 0;
}
-/**
- * Return the pipe currently connected to the panel fitter,
- * or -1 if the panel fitter is not present or not in use
- */
-static int intel_panel_fitter_pipe(struct drm_i915_private *dev_priv)
-{
- u32 pfit_control;
-
- /* i830 doesn't have a panel fitter */
- if (INTEL_GEN(dev_priv) <= 3 &&
- (IS_I830(dev_priv) || !IS_MOBILE(dev_priv)))
- return -1;
-
- pfit_control = I915_READ(PFIT_CONTROL);
-
- /* See if the panel fitter is in use */
- if ((pfit_control & PFIT_ENABLE) == 0)
- return -1;
-
- /* 965 can place panel fitter on either pipe */
- if (IS_GEN4(dev_priv))
- return (pfit_control >> 29) & 0x3;
-
- /* older chips can only use pipe 1 */
- return 1;
-}
-
int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
@@ -1176,7 +1149,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev,
void *data,
/* line too wide, i.e. one-line-mode */
if (crtc->config->pipe_src_w > 1024 &&
- intel_panel_fitter_pipe(dev_priv) == crtc->pipe) {
+ crtc->config->gmch_pfit.control & PFIT_ENABLE) {
overlay->pfit_active = true;
update_pfit_vscale_ratio(overlay);
} else
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/11] drm/i915: Use pipe_src_w in overlay code
From: Ville Syrjälä
Replace the use of crtc->mode.h/vdisplay with the more appropriate
config->pipe_src_w/h.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 0c5e82beda4a..638933bfd8a0 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -937,12 +937,13 @@ static void update_pfit_vscale_ratio(struct intel_overlay
*overlay)
static int check_overlay_dst(struct intel_overlay *overlay,
struct drm_intel_overlay_put_image *rec)
{
- struct drm_display_mode *mode = &overlay->crtc->base.mode;
+ const struct intel_crtc_state *pipe_config =
+ overlay->crtc->config;
- if (rec->dst_x < mode->hdisplay &&
- rec->dst_x + rec->dst_width <= mode->hdisplay &&
- rec->dst_y < mode->vdisplay &&
- rec->dst_y + rec->dst_height <= mode->vdisplay)
+ if (rec->dst_x < pipe_config->pipe_src_w &&
+ rec->dst_x + rec->dst_width <= pipe_config->pipe_src_w &&
+ rec->dst_y < pipe_config->pipe_src_h &&
+ rec->dst_y + rec->dst_height <= pipe_config->pipe_src_h)
return 0;
else
return -EINVAL;
@@ -1162,7 +1163,6 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev,
void *data,
goto out_unlock;
if (overlay->crtc != crtc) {
- struct drm_display_mode *mode = &crtc->base.mode;
ret = intel_overlay_switch_off(overlay);
if (ret != 0)
goto out_unlock;
@@ -1175,7 +1175,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev,
void *data,
crtc->overlay = overlay;
/* line too wide, i.e. one-line-mode */
- if (mode->hdisplay > 1024 &&
+ if (crtc->config->pipe_src_w > 1024 &&
intel_panel_fitter_pipe(dev_priv) == crtc->pipe) {
overlay->pfit_active = true;
update_pfit_vscale_ratio(overlay);
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/11] drm/i915: Fix the overlay frontbuffer tracking
From: Ville Syrjälä
Do the overlay frontbuffer tracking properly so that it matches
the state of the overlay on/off/continue requests.
One slight problem is that intel_frontbuffer_flip_complete()
may get delayed by an arbitrarily liong time due to the fact that
the overlay code likes to bail out when a signal occurs. So the
flip may not get completed until the ioctl is restarted. But fixing
that would require bigger surgery, so I decided to ignore it for now.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 64 +++-
1 file changed, 41 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 786389dd5175..0c5e82beda4a 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -271,8 +271,30 @@ static int intel_overlay_on(struct intel_overlay *overlay)
return intel_overlay_do_wait_request(overlay, req, NULL);
}
+static void intel_overlay_flip_prepare(struct intel_overlay *overlay,
+ struct i915_vma *vma)
+{
+ enum pipe pipe = overlay->crtc->pipe;
+
+ WARN_ON(overlay->old_vma);
+
+ i915_gem_track_fb(overlay->vma ? overlay->vma->obj : NULL,
+ vma ? vma->obj : NULL,
+ INTEL_FRONTBUFFER_OVERLAY(pipe));
+
+ intel_frontbuffer_flip_prepare(overlay->i915,
+ INTEL_FRONTBUFFER_OVERLAY(pipe));
+
+ overlay->old_vma = overlay->vma;
+ if (vma)
+ overlay->vma = i915_vma_get(vma);
+ else
+ overlay->vma = NULL;
+}
+
/* overlay needs to be enabled in OCMD reg */
static int intel_overlay_continue(struct intel_overlay *overlay,
+ struct i915_vma *vma,
bool load_polyphase_filter)
{
struct drm_i915_private *dev_priv = overlay->i915;
@@ -307,43 +329,44 @@ static int intel_overlay_continue(struct intel_overlay
*overlay,
intel_ring_emit(ring, flip_addr);
intel_ring_advance(ring);
+ intel_overlay_flip_prepare(overlay, vma);
+
intel_overlay_submit_request(overlay, req, NULL);
return 0;
}
-static void intel_overlay_release_old_vid_tail(struct i915_gem_active *active,
- struct drm_i915_gem_request *req)
+static void intel_overlay_release_old_vma(struct intel_overlay *overlay)
{
- struct intel_overlay *overlay =
- container_of(active, typeof(*overlay), last_flip);
struct i915_vma *vma;
vma = fetch_and_zero(&overlay->old_vma);
if (WARN_ON(!vma))
return;
- i915_gem_track_fb(vma->obj, NULL,
- INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
+ intel_frontbuffer_flip_complete(overlay->i915,
+
INTEL_FRONTBUFFER_OVERLAY(overlay->crtc->pipe));
i915_gem_object_unpin_from_display_plane(vma);
i915_vma_put(vma);
}
+static void intel_overlay_release_old_vid_tail(struct i915_gem_active *active,
+ struct drm_i915_gem_request *req)
+{
+ struct intel_overlay *overlay =
+ container_of(active, typeof(*overlay), last_flip);
+
+ intel_overlay_release_old_vma(overlay);
+}
+
static void intel_overlay_off_tail(struct i915_gem_active *active,
struct drm_i915_gem_request *req)
{
struct intel_overlay *overlay =
container_of(active, typeof(*overlay), last_flip);
- struct i915_vma *vma;
- /* never have the overlay hw on without showing a frame */
- vma = fetch_and_zero(&overlay->vma);
- if (WARN_ON(!vma))
- return;
-
- i915_gem_object_unpin_from_display_plane(vma);
- i915_vma_put(vma);
+ intel_overlay_release_old_vma(overlay);
overlay->crtc->overlay = NULL;
overlay->crtc = NULL;
@@ -397,6 +420,8 @@ static int intel_overlay_off(struct intel_overlay *overlay)
}
intel_ring_advance(ring);
+ intel_overlay_flip_prepare(overlay, NULL);
+
return intel_overlay_do_wait_request(overlay, req,
intel_overlay_off_tail);
}
@@ -835,18 +860,10 @@ static int intel_overlay_do_put_image(struct
intel_overlay *overlay,
intel_overlay_unmap_regs(overlay, regs);
- ret = intel_overlay_continue(overlay, scale_changed);
+ ret = intel_overlay_continue(overlay, vma, scale_changed);
if (ret)
goto out_unpin;
- i915_gem_track_fb(overlay->vma ? overlay->vma->obj : NULL,
- vma->obj, INTEL_FRONTBUFFER_OVERLAY(pipe));
-
- overlay->old_vma = overlay->vma;
- overlay->vma = vma;
-
- intel_frontbuffer_flip(dev_priv, INTEL_FRONTBUFFER_OVERLAY(
[Intel-gfx] [PATCH 09/11] drm/i915: Use primary plane->state for overlay ckey setup
From: Ville Syrjälä
Extract the primary plane pixel format via plane state when setting up
the overlay colorkey.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index e4420b08f3ab..bc113ebc475f 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -689,31 +689,32 @@ static bool update_scaling_factors(struct intel_overlay
*overlay,
static void update_colorkey(struct intel_overlay *overlay,
struct overlay_registers __iomem *regs)
{
+ const struct intel_plane_state *state =
+ to_intel_plane_state(overlay->crtc->base.primary->state);
u32 key = overlay->color_key;
- u32 flags;
+ u32 format = 0;
+ u32 flags = 0;
- flags = 0;
if (overlay->color_key_enabled)
flags |= DST_KEY_ENABLE;
- switch (overlay->crtc->base.primary->fb->bits_per_pixel) {
- case 8:
+ if (state->base.visible)
+ format = state->base.fb->pixel_format;
+
+ switch (format) {
+ case DRM_FORMAT_C8:
key = 0;
flags |= CLK_RGB8I_MASK;
break;
-
- case 16:
- if (overlay->crtc->base.primary->fb->depth == 15) {
- key = RGB15_TO_COLORKEY(key);
- flags |= CLK_RGB15_MASK;
- } else {
- key = RGB16_TO_COLORKEY(key);
- flags |= CLK_RGB16_MASK;
- }
+ case DRM_FORMAT_XRGB1555:
+ key = RGB15_TO_COLORKEY(key);
+ flags |= CLK_RGB15_MASK;
break;
-
- case 24:
- case 32:
+ case DRM_FORMAT_RGB565:
+ key = RGB16_TO_COLORKEY(key);
+ flags |= CLK_RGB16_MASK;
+ break;
+ default:
flags |= CLK_RGB24_MASK;
break;
}
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/11] drm/i915: Reorganize overlay filter coeffs into a nicer form
From: Ville Syrjälä
Use two-dimensional arrays and named initializers to make the
overlay filter coefficient tables easier to parse for humans.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 64
1 file changed, 36 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 19ff5d86f9f9..e4420b08f3ab 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -579,36 +579,44 @@ static u32 calc_swidthsw(struct drm_i915_private
*dev_priv, u32 offset, u32 widt
return (sw - 32) >> 3;
}
-static const u16 y_static_hcoeffs[N_HORIZ_Y_TAPS * N_PHASES] = {
- 0x3000, 0xb4a0, 0x1930, 0x1920, 0xb4a0,
- 0x3000, 0xb500, 0x19d0, 0x1880, 0xb440,
- 0x3000, 0xb540, 0x1a88, 0x2f80, 0xb3e0,
- 0x3000, 0xb580, 0x1b30, 0x2e20, 0xb380,
- 0x3000, 0xb5c0, 0x1bd8, 0x2cc0, 0xb320,
- 0x3020, 0xb5e0, 0x1c60, 0x2b80, 0xb2c0,
- 0x3020, 0xb5e0, 0x1cf8, 0x2a20, 0xb260,
- 0x3020, 0xb5e0, 0x1d80, 0x28e0, 0xb200,
- 0x3020, 0xb5c0, 0x1e08, 0x3f40, 0xb1c0,
- 0x3020, 0xb580, 0x1e78, 0x3ce0, 0xb160,
- 0x3040, 0xb520, 0x1ed8, 0x3aa0, 0xb120,
- 0x3040, 0xb4a0, 0x1f30, 0x3880, 0xb0e0,
- 0x3040, 0xb400, 0x1f78, 0x3680, 0xb0a0,
- 0x3020, 0xb340, 0x1fb8, 0x34a0, 0xb060,
- 0x3020, 0xb240, 0x1fe0, 0x32e0, 0xb040,
- 0x3020, 0xb140, 0x1ff8, 0x3160, 0xb020,
- 0xb000, 0x3000, 0x0800, 0x3000, 0xb000
+static const u16 y_static_hcoeffs[N_PHASES][N_HORIZ_Y_TAPS] = {
+ [ 0] = { 0x3000, 0xb4a0, 0x1930, 0x1920, 0xb4a0, },
+ [ 1] = { 0x3000, 0xb500, 0x19d0, 0x1880, 0xb440, },
+ [ 2] = { 0x3000, 0xb540, 0x1a88, 0x2f80, 0xb3e0, },
+ [ 3] = { 0x3000, 0xb580, 0x1b30, 0x2e20, 0xb380, },
+ [ 4] = { 0x3000, 0xb5c0, 0x1bd8, 0x2cc0, 0xb320, },
+ [ 5] = { 0x3020, 0xb5e0, 0x1c60, 0x2b80, 0xb2c0, },
+ [ 6] = { 0x3020, 0xb5e0, 0x1cf8, 0x2a20, 0xb260, },
+ [ 7] = { 0x3020, 0xb5e0, 0x1d80, 0x28e0, 0xb200, },
+ [ 8] = { 0x3020, 0xb5c0, 0x1e08, 0x3f40, 0xb1c0, },
+ [ 9] = { 0x3020, 0xb580, 0x1e78, 0x3ce0, 0xb160, },
+ [10] = { 0x3040, 0xb520, 0x1ed8, 0x3aa0, 0xb120, },
+ [11] = { 0x3040, 0xb4a0, 0x1f30, 0x3880, 0xb0e0, },
+ [12] = { 0x3040, 0xb400, 0x1f78, 0x3680, 0xb0a0, },
+ [13] = { 0x3020, 0xb340, 0x1fb8, 0x34a0, 0xb060, },
+ [14] = { 0x3020, 0xb240, 0x1fe0, 0x32e0, 0xb040, },
+ [15] = { 0x3020, 0xb140, 0x1ff8, 0x3160, 0xb020, },
+ [16] = { 0xb000, 0x3000, 0x0800, 0x3000, 0xb000, },
};
-static const u16 uv_static_hcoeffs[N_HORIZ_UV_TAPS * N_PHASES] = {
- 0x3000, 0x1800, 0x1800, 0xb000, 0x18d0, 0x2e60,
- 0xb000, 0x1990, 0x2ce0, 0xb020, 0x1a68, 0x2b40,
- 0xb040, 0x1b20, 0x29e0, 0xb060, 0x1bd8, 0x2880,
- 0xb080, 0x1c88, 0x3e60, 0xb0a0, 0x1d28, 0x3c00,
- 0xb0c0, 0x1db8, 0x39e0, 0xb0e0, 0x1e40, 0x37e0,
- 0xb100, 0x1eb8, 0x3620, 0xb100, 0x1f18, 0x34a0,
- 0xb100, 0x1f68, 0x3360, 0xb0e0, 0x1fa8, 0x3240,
- 0xb0c0, 0x1fe0, 0x3140, 0xb060, 0x1ff0, 0x30a0,
- 0x3000, 0x0800, 0x3000
+static const u16 uv_static_hcoeffs[N_PHASES][N_HORIZ_UV_TAPS] = {
+ [ 0] = { 0x3000, 0x1800, 0x1800, },
+ [ 1] = { 0xb000, 0x18d0, 0x2e60, },
+ [ 2] = { 0xb000, 0x1990, 0x2ce0, },
+ [ 3] = { 0xb020, 0x1a68, 0x2b40, },
+ [ 4] = { 0xb040, 0x1b20, 0x29e0, },
+ [ 5] = { 0xb060, 0x1bd8, 0x2880, },
+ [ 6] = { 0xb080, 0x1c88, 0x3e60, },
+ [ 7] = { 0xb0a0, 0x1d28, 0x3c00, },
+ [ 8] = { 0xb0c0, 0x1db8, 0x39e0, },
+ [ 9] = { 0xb0e0, 0x1e40, 0x37e0, },
+ [10] = { 0xb100, 0x1eb8, 0x3620, },
+ [11] = { 0xb100, 0x1f18, 0x34a0, },
+ [12] = { 0xb100, 0x1f68, 0x3360, },
+ [13] = { 0xb0e0, 0x1fa8, 0x3240, },
+ [14] = { 0xb0c0, 0x1fe0, 0x3140, },
+ [15] = { 0xb060, 0x1ff0, 0x30a0, },
+ [16] = { 0x3000, 0x0800, 0x3000, },
};
static void update_polyphase_filter(struct overlay_registers __iomem *regs)
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/11] drm/i915: Disable L2 cache clock gating on 830 when using the overlay
From: Ville Syrjälä
BSpec says:
"Overlay Clock Gating Must be Disabled: Overlay & L2 Cache clock gating
must be disabled in order to prevent device hangs when turning off overlay.SW
must turn off Ovrunit clock gating (6200h) and L2 Cache clock gating (C8h)."
We only turned off the overlay clock gating (due to lack of docs I
presume). After a bit of experimentation it looks like the the magic
C8h register lives in the PCI config space of device 0, and the magic
bit appears to be bit 2. Or at the very least this eliminates the GPU
death after MI_OVERLAY_OFF.
L2 clock gating seems to save ~80mW, so let's keep it on unless we need
to actually use the overlay.
Also let's move the OVRUNIT clock gating to the same place since we can,
and 845 supposedly doesn't need it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 4
drivers/gpu/drm/i915/intel_overlay.c | 30 ++
drivers/gpu/drm/i915/intel_pm.c | 2 --
3 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 90685d235410..e077e40f5005 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -110,6 +110,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
#define GRDOM_RESET_STATUS (1 << 1)
#define GRDOM_RESET_ENABLE (1 << 0)
+/* BSpec only has register offset, PCI device and bit found empirically */
+#define I830_CLOCK_GATE0xc8 /* device 0 */
+#define I830_L2_CACHE_CLOCK_GATE_DISABLE (1 << 2)
+
#define GCDGMBUS 0xcc
#define GCFGC2 0xda
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index bc113ebc475f..9e8bbaa53a22 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -187,6 +187,29 @@ struct intel_overlay {
struct i915_gem_active last_flip;
};
+static void i830_overlay_clock_gating(struct drm_i915_private *dev_priv,
+ bool enable)
+{
+ struct pci_dev *pdev = dev_priv->drm.pdev;
+ u8 val;
+
+ /* WA_OVERLAY_CLKGATE:alm */
+ if (enable)
+ I915_WRITE(DSPCLK_GATE_D, 0);
+ else
+ I915_WRITE(DSPCLK_GATE_D, OVRUNIT_CLOCK_GATE_DISABLE);
+
+ /* WA_DISABLE_L2CACHE_CLOCK_GATING:alm */
+ pci_bus_read_config_byte(pdev->bus,
+PCI_DEVFN(0, 0), I830_CLOCK_GATE, &val);
+ if (enable)
+ val &= ~I830_L2_CACHE_CLOCK_GATE_DISABLE;
+ else
+ val |= I830_L2_CACHE_CLOCK_GATE_DISABLE;
+ pci_bus_write_config_byte(pdev->bus,
+ PCI_DEVFN(0, 0), I830_CLOCK_GATE, val);
+}
+
static struct overlay_registers __iomem *
intel_overlay_map_regs(struct intel_overlay *overlay)
{
@@ -261,6 +284,9 @@ static int intel_overlay_on(struct intel_overlay *overlay)
overlay->active = true;
+ if (IS_I830(dev_priv))
+ i830_overlay_clock_gating(dev_priv, false);
+
ring = req->ring;
intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_ON);
intel_ring_emit(ring, overlay->flip_addr | OFC_UPDATE);
@@ -365,12 +391,16 @@ static void intel_overlay_off_tail(struct i915_gem_active
*active,
{
struct intel_overlay *overlay =
container_of(active, typeof(*overlay), last_flip);
+ struct drm_i915_private *dev_priv = overlay->i915;
intel_overlay_release_old_vma(overlay);
overlay->crtc->overlay = NULL;
overlay->crtc = NULL;
overlay->active = false;
+
+ if (IS_I830(dev_priv))
+ i830_overlay_clock_gating(dev_priv, true);
}
/* overlay needs to be disabled in OCMD reg */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3ea7cf275be0..fd09ae08d86f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7593,8 +7593,6 @@ static void i85x_init_clock_gating(struct
drm_i915_private *dev_priv)
static void i830_init_clock_gating(struct drm_i915_private *dev_priv)
{
- I915_WRITE(DSPCLK_GATE_D, OVRUNIT_CLOCK_GATE_DISABLE);
-
I915_WRITE(MEM_MODE,
_MASKED_BIT_ENABLE(MEM_DISPLAY_A_TRICKLE_FEED_DISABLE) |
_MASKED_BIT_ENABLE(MEM_DISPLAY_B_TRICKLE_FEED_DISABLE));
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/11] drm/i915: Simplify SWIDTHSW calculation
From: Ville Syrjälä
The formula in Bspec for computing the overlay SWIDTHSW is overly
obfuscated. Simplify the formula to something that's easily parsed by
humans.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 10e8d3b9a0c4..19ff5d86f9f9 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -566,19 +566,17 @@ static int uv_vsubsampling(u32 format)
static u32 calc_swidthsw(struct drm_i915_private *dev_priv, u32 offset, u32
width)
{
- u32 mask, shift, ret;
- if (IS_GEN2(dev_priv)) {
- mask = 0x1f;
- shift = 5;
- } else {
- mask = 0x3f;
- shift = 6;
- }
- ret = ((offset + width + mask) >> shift) - (offset >> shift);
- if (!IS_GEN2(dev_priv))
- ret <<= 1;
- ret -= 1;
- return ret << 2;
+ u32 sw;
+
+ if (IS_GEN2(dev_priv))
+ sw = ALIGN((offset & 31) + width, 32);
+ else
+ sw = ALIGN((offset & 63) + width, 64);
+
+ if (sw == 0)
+ return 0;
+
+ return (sw - 32) >> 3;
}
static const u16 y_static_hcoeffs[N_HORIZ_Y_TAPS * N_PHASES] = {
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/11] drm/i915: Kill the 830 MI_OVERLAY_OFF workaround
From: Ville Syrjälä
Now that we're disabling L2 clock gating MI_OVERLAY_OFF actually works
on 830, so let's use it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 9e8bbaa53a22..bfc9fa41c04e 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -431,23 +431,17 @@ static int intel_overlay_off(struct intel_overlay
*overlay)
}
ring = req->ring;
+
/* wait for overlay to go idle */
intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_CONTINUE);
intel_ring_emit(ring, flip_addr);
intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+
/* turn overlay off */
- if (IS_I830(dev_priv)) {
- /* Workaround: Don't disable the overlay fully, since otherwise
-* it dies on the next OVERLAY_ON cmd. */
- intel_ring_emit(ring, MI_NOOP);
- intel_ring_emit(ring, MI_NOOP);
- intel_ring_emit(ring, MI_NOOP);
- } else {
- intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
- intel_ring_emit(ring, flip_addr);
- intel_ring_emit(ring,
- MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
- }
+ intel_ring_emit(ring, MI_OVERLAY_FLIP | MI_OVERLAY_OFF);
+ intel_ring_emit(ring, flip_addr);
+ intel_ring_emit(ring, MI_WAIT_FOR_EVENT | MI_WAIT_FOR_OVERLAY_FLIP);
+
intel_ring_advance(ring);
intel_overlay_flip_prepare(overlay, NULL);
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/11] drm/i915: Fix oops in overlay due to frontbuffer trakcing
From: Ville Syrjälä
The vma will be NULL if the overlay was previously off, so
dereferencing it will oops. Check for NULL before doing that.
Cc: Chris Wilson
Cc: Joonas Lahtinen
Fixes: 9b3b7841b86d ("drm/i915/overlay: Use VMA as the primary tracker for
images")
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_overlay.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 354f8cec96bb..1cc963814224 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -839,8 +839,8 @@ static int intel_overlay_do_put_image(struct intel_overlay
*overlay,
if (ret)
goto out_unpin;
- i915_gem_track_fb(overlay->vma->obj, new_bo,
- INTEL_FRONTBUFFER_OVERLAY(pipe));
+ i915_gem_track_fb(overlay->vma ? overlay->vma->obj : NULL,
+ vma->obj, INTEL_FRONTBUFFER_OVERLAY(pipe));
overlay->old_vma = overlay->vma;
overlay->vma = vma;
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay
On Wed, Dec 07, 2016 at 07:28:05PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> We need an uninterruptible wait for the overlay off request during
> modeset. Restore the operation of dev_priv->mm.interruptible
> sufficiently for that.
>
> Toss in a WARN_ON() to make sure the request succeeds.
>
> Fixes: 7da844c5c6fc ("drm/i915: Move the special case wait-request handling
> to its one caller")
> Cc: Chris Wilson
> Cc: Joonas Lahtinen
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_gem_request.c | 36
> +
> drivers/gpu/drm/i915/i915_gem_request.h | 35 ++--
> drivers/gpu/drm/i915/intel_display.c| 2 +-
> 3 files changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c
> b/drivers/gpu/drm/i915/i915_gem_request.c
> index fcf22b0e2967..1a7b88166c51 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -1199,3 +1199,39 @@ void i915_gem_retire_requests(struct drm_i915_private
> *dev_priv)
> for_each_engine(engine, dev_priv, id)
> engine_retire_requests(engine);
> }
> +
> +/**
> + * i915_gem_active_retire - waits until the request is retired
> + * @active - the active request on which to wait
> + *
> + * i915_gem_active_retire() waits until the request is completed,
> + * and then ensures that at least the retirement handler for this
> + * @active tracker is called before returning. If the @active
> + * tracker is idle, the function returns immediately.
> + */
> +int __must_check
> +i915_gem_active_retire(struct i915_gem_active *active,
> +struct mutex *mutex)
> +{
> + struct drm_i915_gem_request *request;
> + long ret;
> +
> + request = i915_gem_active_raw(active, mutex);
> + if (!request)
> + return 0;
> +
> + ret = i915_wait_request(request,
> + (request->i915->mm.interruptible ?
> + I915_WAIT_INTERRUPTIBLE : 0) |
> + I915_WAIT_LOCKED,
> + MAX_SCHEDULE_TIMEOUT);
At least pass in the flags.
> + if (ret < 0)
> + return ret;
> +
> + list_del_init(&active->link);
> + RCU_INIT_POINTER(active->request, NULL);
> +
> + active->retire(active, request);
> +
> + return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.h
> b/drivers/gpu/drm/i915/i915_gem_request.h
> index e2b077df2da0..09add6b9cfc7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.h
> +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> @@ -657,39 +657,8 @@ i915_gem_active_wait(const struct i915_gem_active
> *active, unsigned int flags)
> return ret < 0 ? ret : 0;
> }
>
> -/**
> - * i915_gem_active_retire - waits until the request is retired
> - * @active - the active request on which to wait
> - *
> - * i915_gem_active_retire() waits until the request is completed,
> - * and then ensures that at least the retirement handler for this
> - * @active tracker is called before returning. If the @active
> - * tracker is idle, the function returns immediately.
> - */
> -static inline int __must_check
> -i915_gem_active_retire(struct i915_gem_active *active,
> -struct mutex *mutex)
> -{
> - struct drm_i915_gem_request *request;
> - long ret;
> -
> - request = i915_gem_active_raw(active, mutex);
> - if (!request)
> - return 0;
> -
> - ret = i915_wait_request(request,
> - I915_WAIT_INTERRUPTIBLE | I915_WAIT_LOCKED,
> - MAX_SCHEDULE_TIMEOUT);
> - if (ret < 0)
> - return ret;
> -
> - list_del_init(&active->link);
> - RCU_INIT_POINTER(active->request, NULL);
> -
> - active->retire(active, request);
> -
> - return 0;
> -}
> +int __must_check i915_gem_active_retire(struct i915_gem_active *active,
> + struct mutex *mutex);
>
> #define for_each_active(mask, idx) \
> for (; mask ? idx = ffs(mask) - 1, 1 : 0; mask &= ~BIT(idx))
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index a41082e2750e..5a74991854e3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4923,7 +4923,7 @@ static void intel_crtc_dpms_overlay_disable(struct
> intel_crtc *intel_crtc)
>
> mutex_lock(&dev->struct_mutex);
> dev_priv->mm.interruptible = false;
> - (void) intel_overlay_switch_off(intel_crtc->overlay);
> + WARN_ON(intel_overlay_switch_off(intel_crtc->overlay) != 0);
> dev_priv->mm.interruptible = true;
> mutex_unlock(&dev->struct_mutex);
Or we can push this wait to where it can interrupt, such as prepare_planes_fb.
(For intel_atomic_commit_tail, intel_crtc_disable_noatomic() should alway
Re: [Intel-gfx] [PATCH 02/11] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff
On Wed, Dec 07, 2016 at 07:28:04PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The i915_gem_active stuff doesn't like a NULL ->retire hook, but
> the overlay code can set it to NULL. That obviously ends up oopsing.
> Fix it by setting the ->retire hook using init_request_active()
> so that it'll do the NULL->i915_gem_retire_noop conversion for us.
>
> Cc: Chris Wilson
> Cc: Joonas Lahtinen
> Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_overlay.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c
> b/drivers/gpu/drm/i915/intel_overlay.c
> index 1cc963814224..786389dd5175 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -216,7 +216,7 @@ static void intel_overlay_submit_request(struct
> intel_overlay *overlay,
> {
> GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
> &overlay->i915->drm.struct_mutex));
> - overlay->last_flip.retire = retire;
> + init_request_active(&overlay->last_flip, retire);
Hmm, init, not reinit. The alternative is to use i915_gem_retire_noop
instead of NULL. (And sorry, it did used to allow NULL.)
overlay->last_flip.retire = retire ?= i915_gem_retire_noop;
Or i915_gem_active_set_retire_fn(&overlay->last_flip, retire);
That's seems like it should make everyone a bit happier.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/dsi: Fix swapping of MIPI_SEQ_DEASSERT_RESET / MIPI_SEQ_ASSERT_RESET
On Fri, Dec 02, 2016 at 04:01:28PM +0100, Hans de Goede wrote:
> Looking at the ADF code from the Android kernel sources for a
> cherrytrail tablet I noticed that it is calling the
> MIPI_SEQ_ASSERT_RESET sequence from the panel prepare hook.
>
> Until commit b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences
> in panel prepare/unprepare hooks") the mainline i915 code was doing the
> same. That commits effectively swaps the calling of MIPI_SEQ_ASSERT_RESET /
> MIPI_SEQ_DEASSERT_RESET.
>
> Looking at the naming of the sequences that is the right thing to do,
> but the problem is, that the old mainline code and the ADF code was
> actually calling the right sequence (tested on a cube iwork8 air tablet),
> and the swapping of the calling breaks things.
>
> This breakage was likely not noticed in testing because on cherrytrail,
> currently chv_exec_gpio ends up disabling the gpio pins rather then
> setting them (this is fixed in the next patch in this patch-set).
>
> This commit fixes the swapping by fixing MIPI_SEQ_ASSERT/DEASSERT_RESET's
> places in the enum defining them, so that their (new) names match their
> actual use.
>
> Fixes: b1cb1bd29189 ("drm/i915/dsi: update reset and power sequences...")
> Cc: Jani Nikula
> Cc: Ville Syrjälä
> Signed-off-by: Hans de Goede
> Acked-by: Jani Nikula
> ---
> Changes in v2:
> -Add a comment to the enum explaining that the assert/reassert names
> are swapped in the spec
Pushed to dinq. Thanks for the patch.
I sucked in the changelog again. In the future please include it in
the actual commit message, cause that's how we like it.
> ---
> drivers/gpu/drm/i915/intel_bios.h | 12 +---
> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 4 ++--
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> b/drivers/gpu/drm/i915/intel_bios.h
> index 8405b5a..7e3545f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -46,14 +46,20 @@ struct edp_power_seq {
> u16 t11_t12;
> } __packed;
>
> -/* MIPI Sequence Block definitions */
> +/*
> + * MIPI Sequence Block definitions
> + *
> + * Note the VBT spec has AssertReset / DeassertReset swapped from their
> + * usual naming, we use the proper names here to avoid confusion when
> + * reading the code.
> + */
> enum mipi_seq {
> MIPI_SEQ_END = 0,
> - MIPI_SEQ_ASSERT_RESET,
> + MIPI_SEQ_DEASSERT_RESET,/* Spec says MipiAssertResetPin */
> MIPI_SEQ_INIT_OTP,
> MIPI_SEQ_DISPLAY_ON,
> MIPI_SEQ_DISPLAY_OFF,
> - MIPI_SEQ_DEASSERT_RESET,
> + MIPI_SEQ_ASSERT_RESET, /* Spec says MipiDeassertResetPin */
> MIPI_SEQ_BACKLIGHT_ON, /* sequence block v2+ */
> MIPI_SEQ_BACKLIGHT_OFF, /* sequence block v2+ */
> MIPI_SEQ_TEAR_ON, /* sequence block v2+ */
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 0d8ff00..579d2f5 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -376,11 +376,11 @@ static const fn_mipi_elem_exec exec_elem[] = {
> */
>
> static const char * const seq_name[] = {
> - [MIPI_SEQ_ASSERT_RESET] = "MIPI_SEQ_ASSERT_RESET",
> + [MIPI_SEQ_DEASSERT_RESET] = "MIPI_SEQ_DEASSERT_RESET",
> [MIPI_SEQ_INIT_OTP] = "MIPI_SEQ_INIT_OTP",
> [MIPI_SEQ_DISPLAY_ON] = "MIPI_SEQ_DISPLAY_ON",
> [MIPI_SEQ_DISPLAY_OFF] = "MIPI_SEQ_DISPLAY_OFF",
> - [MIPI_SEQ_DEASSERT_RESET] = "MIPI_SEQ_DEASSERT_RESET",
> + [MIPI_SEQ_ASSERT_RESET] = "MIPI_SEQ_ASSERT_RESET",
> [MIPI_SEQ_BACKLIGHT_ON] = "MIPI_SEQ_BACKLIGHT_ON",
> [MIPI_SEQ_BACKLIGHT_OFF] = "MIPI_SEQ_BACKLIGHT_OFF",
> [MIPI_SEQ_TEAR_ON] = "MIPI_SEQ_TEAR_ON",
> --
> 2.9.3
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 0/5] DRM logging tidy
On Tue, Dec 6, 2016 at 6:57 PM, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin > > I wasn't here at the beginnings of DRM so I might have gotten this wrong, > however the existance of DRM_NAME suggested to me that the intention was to > allow individual drivers to override it and get appropriate prefixes in > their > log messages. > > I can't see that any driver is using it like that but I still thought it > would > be neat to do that. That way we could have our log messages look more > obviously ours. For example after this series we have: > > [i915] Memory usable by graphics device = 4096M > [i915] VT-d active for gfx access > [i915] Replacing VGA console driver > [i915] ACPI BIOS requests an excessive sleep of 2 ms, using 1500 ms > instead > [i915] Finished loading DMC firmware i915/skl_dmc_ver1_26.bin (v1.26) > [i915] Disabling framebuffer compression (FBC) to prevent screen flicker > with VT-d enabled > [i915] GuC firmware load skipped > [i915] Initialized i915 1.6.0 20161205 for :00:02.0 on minor 0 > [i915] DRM_I915_DEBUG enabled > [i915] DRM_I915_DEBUG_GEM enabled > [i915] RC6 on > > Previously all that was prefixed with "[drm]" which was OK but I think the > above is even better. > > Also to consider is that recent drm_printk work has removed (it hardcoded) > DRM_NAME from DRM_ERROR and DRM_DEBUG macros, while leaving it with the > rest > (DRM_INFO, NOTE and WARNING) creating a bit of a inconsistency. > I wonder if I can maybe fold some of this idea into my related DRM_DEBUG [RFC] sent out recently: https://lists.freedesktop.org/archives/dri-devel/2016-December/126094.html Instead of using DRM_NAME, I've experimented with updating my changes adding support for dynamic debug to add a prefix based on module_name(THIS_MODULE) for a similar result One thing to consider here is that with the addition of dynamic debug support this prefix arguably becomes redundant because the dynamic_debug/control interface lets you choose to add a module name or function prefix to messages, e.g. like: echo "module i915 +mfp" > dynamic_debug/control I've ignored the redundancy because my change still allows enabling messages with the drm.drm_debug parameter and in that case the prefix is still useful. Br, - Robert > This series also makes all the logging macros use drm_printk, but also > makes DRM_NAME passed in from the macro wrappers in all cases. So drivers > can override it regardless of the log level. > > And finally, the series also removes a bit of redundant data from the debug > messages effectively converting this: > > [drm:edp_panel_off [i915]] Wait for panel power off time > > Into this: > > [edp_panel_off [i915]] Wait for panel power off time > > Which still has all the data in it. > > Tvrtko Ursulin (5): > drm/i915: Give our log messages our name > drm: Respect driver set DRM_NAME in drm_printk > drm: Respect driver set DRM_NAME in drm_dev_printk > drm: Use drm_printk for all logging macros > drm: Do not log driver prefix in debug messages > > drivers/gpu/drm/drm_drv.c | 39 +++-- > drivers/gpu/drm/i915/i915_drv.c | 3 +- > include/drm/drmP.h | 94 -- > --- > include/drm/drm_drv.h | 11 ++--- > include/uapi/drm/i915_drm.h | 3 ++ > 5 files changed, 92 insertions(+), 58 deletions(-) > > -- > 2.7.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Parse panel BL controller from VBT
I currently have this same patch in my tree (well with the two changes
below) and have been testing it so with the changes.
Reviewed-by: Bob Paauwe
Tested-by: Bob Paauwe
On Wed, 7 Dec 2016 20:32:18 +0530 Vidya Srinivas
wrote:
> Currently the backlight controller is taken as 0. It needs to derive
> value from the VBT. Adding the necessary changes.
>
> v2: Updated the commit header
>
> Signed-off-by: Uma Shankar
> Signed-off-by: Vidya Srinivas
> ---
> drivers/gpu/drm/i915/i915_drv.h| 1 +
> drivers/gpu/drm/i915/intel_bios.c | 5 +
> drivers/gpu/drm/i915/intel_panel.c | 2 +-
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8daa4fb..6a85fdf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1633,6 +1633,7 @@ struct intel_vbt_data {
> bool present;
> bool active_low_pwm;
> u8 min_brightness; /* min_brightness/255 of max */
> + u8 controller; /* brightness controller number */
> enum intel_backlight_type type;
> } backlight;
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index eaade27..130db0f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -330,6 +330,8 @@ static u32 get_blocksize(const void *block_data)
>
> method = &backlight_data->backlight_control[panel_type];
> dev_priv->vbt.backlight.type = method->type;
> + dev_priv->vbt.backlight.controller = 0;
Setting it to zero first seems redundant. Is there a reason you do
this?
> + dev_priv->vbt.backlight.controller = method->controller;
> }
>
> dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
> @@ -341,6 +343,9 @@ static u32 get_blocksize(const void *block_data)
> dev_priv->vbt.backlight.active_low_pwm ? "low" : "high",
> dev_priv->vbt.backlight.min_brightness,
> backlight_data->level[panel_type]);
> +
> + DRM_DEBUG_KMS("VBT BL controller %u\n",
> + dev_priv->vbt.backlight.controller);
> }
>
> /* Try to find sdvo panel data */
> diff --git a/drivers/gpu/drm/i915/intel_panel.c
> b/drivers/gpu/drm/i915/intel_panel.c
> index 3578b40..6a7d4c3 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1612,7 +1612,7 @@ static int vlv_setup_backlight(struct intel_connector
> *connector, enum pipe pipe
>* For BXT hard coding the Backlight controller to 0.
>* TODO : Read the controller value from VBT and generalize
>*/
> - panel->backlight.controller = 0;
> + panel->backlight.controller = dev_priv->vbt.backlight.controller;
You should remove the comment above now that it's not hard coded.
>
> pwm_ctl = I915_READ(BXT_BLC_PWM_CTL(panel->backlight.controller));
>
--
--
Bob Paauwe
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp. Folsom, CA
(916) 356-6193
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Catch non-existent registers in find_fw_domain (rev2)
== Series Details == Series: drm/i915: Catch non-existent registers in find_fw_domain (rev2) URL : https://patchwork.freedesktop.org/series/16487/ State : success == Summary == Series 16487v2 drm/i915: Catch non-existent registers in find_fw_domain https://patchwork.freedesktop.org/api/1.0/series/16487/revisions/2/mbox/ fi-bdw-5557u total:247 pass:219 dwarn:0 dfail:0 fail:0 skip:28 fi-bsw-n3050 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-byt-j1900 total:247 pass:206 dwarn:0 dfail:0 fail:0 skip:41 fi-byt-n2820 total:247 pass:202 dwarn:0 dfail:0 fail:0 skip:45 fi-hsw-4770 total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-hsw-4770r total:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-ilk-650 total:247 pass:181 dwarn:0 dfail:0 fail:0 skip:66 fi-ivb-3520m total:247 pass:213 dwarn:0 dfail:0 fail:0 skip:34 fi-ivb-3770 total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-kbl-7500u total:247 pass:212 dwarn:0 dfail:0 fail:0 skip:35 fi-skl-6260u total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-skl-6700hqtotal:247 pass:214 dwarn:0 dfail:0 fail:0 skip:33 fi-skl-6700k total:247 pass:210 dwarn:3 dfail:0 fail:0 skip:34 fi-skl-6770hqtotal:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-snb-2520m total:247 pass:202 dwarn:0 dfail:0 fail:0 skip:45 fi-snb-2600 total:247 pass:201 dwarn:0 dfail:0 fail:0 skip:46 722bab6dc6ca5b0fd0c667e5dd65f7734c550f94 drm-tip: 2016y-12m-07d-16h-41m-46s UTC integration manifest ee9b8cb drm/i915: Catch non-existent registers in find_fw_domain == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3219/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/18] drm/i915/dsi: Fix chv_exec_gpio disabling the GPIOs it is setting
On Thu, Dec 01, 2016 at 09:29:09PM +0100, Hans de Goede wrote:
> Set the CHV_GPIO_GPIOEN bit when updating GPIOs from chv_exec_gpio.
>
> Fixes: a0a6d4ffd2ad ("drm/i915/dsi: add support for gpio elements on CHV")
> Cc: sta...@vger.kernel.org
> Cc: Jani Nikula
> Cc: Ville Syrjälä
> Signed-off-by: Hans de Goede
> Reviewed-by: Ville Syrjälä
Pushed to dinq. Thanks for the patch.
> ---
> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> index 579d2f5..47cd1b2 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
> @@ -300,7 +300,8 @@ static void chv_exec_gpio(struct drm_i915_private
> *dev_priv,
> mutex_lock(&dev_priv->sb_lock);
> vlv_iosf_sb_write(dev_priv, port, cfg1, 0);
> vlv_iosf_sb_write(dev_priv, port, cfg0,
> - CHV_GPIO_GPIOCFG_GPO | CHV_GPIO_GPIOTXSTATE(value));
> + CHV_GPIO_GPIOEN | CHV_GPIO_GPIOCFG_GPO |
> + CHV_GPIO_GPIOTXSTATE(value));
> mutex_unlock(&dev_priv->sb_lock);
> }
>
> --
> 2.9.3
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/11] drm/i915: Fix oops in overlay due to frontbuffer trakcing
On Wed, Dec 07, 2016 at 07:28:03PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The vma will be NULL if the overlay was previously off, so
> dereferencing it will oops. Check for NULL before doing that.
>
> Cc: Chris Wilson
> Cc: Joonas Lahtinen
> Fixes: 9b3b7841b86d ("drm/i915/overlay: Use VMA as the primary tracker for
> images")
> Signed-off-by: Ville Syrjälä
Oops,
Reviewed-by: Chris Wilson
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay
On Wed, Dec 07, 2016 at 05:41:39PM +, Chris Wilson wrote:
> On Wed, Dec 07, 2016 at 07:28:05PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > We need an uninterruptible wait for the overlay off request during
> > modeset. Restore the operation of dev_priv->mm.interruptible
> > sufficiently for that.
> >
> > Toss in a WARN_ON() to make sure the request succeeds.
> >
> > Fixes: 7da844c5c6fc ("drm/i915: Move the special case wait-request handling
> > to its one caller")
> > Cc: Chris Wilson
> > Cc: Joonas Lahtinen
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_gem_request.c | 36
> > +
> > drivers/gpu/drm/i915/i915_gem_request.h | 35
> > ++--
> > drivers/gpu/drm/i915/intel_display.c| 2 +-
> > 3 files changed, 39 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_request.c
> > b/drivers/gpu/drm/i915/i915_gem_request.c
> > index fcf22b0e2967..1a7b88166c51 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_request.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> > @@ -1199,3 +1199,39 @@ void i915_gem_retire_requests(struct
> > drm_i915_private *dev_priv)
> > for_each_engine(engine, dev_priv, id)
> > engine_retire_requests(engine);
> > }
> > +
> > +/**
> > + * i915_gem_active_retire - waits until the request is retired
> > + * @active - the active request on which to wait
> > + *
> > + * i915_gem_active_retire() waits until the request is completed,
> > + * and then ensures that at least the retirement handler for this
> > + * @active tracker is called before returning. If the @active
> > + * tracker is idle, the function returns immediately.
> > + */
> > +int __must_check
> > +i915_gem_active_retire(struct i915_gem_active *active,
> > + struct mutex *mutex)
> > +{
> > + struct drm_i915_gem_request *request;
> > + long ret;
> > +
> > + request = i915_gem_active_raw(active, mutex);
> > + if (!request)
> > + return 0;
> > +
> > + ret = i915_wait_request(request,
> > + (request->i915->mm.interruptible ?
> > +I915_WAIT_INTERRUPTIBLE : 0) |
> > + I915_WAIT_LOCKED,
> > + MAX_SCHEDULE_TIMEOUT);
>
> At least pass in the flags.
>
> > + if (ret < 0)
> > + return ret;
> > +
> > + list_del_init(&active->link);
> > + RCU_INIT_POINTER(active->request, NULL);
> > +
> > + active->retire(active, request);
> > +
> > + return 0;
> > +}
> > diff --git a/drivers/gpu/drm/i915/i915_gem_request.h
> > b/drivers/gpu/drm/i915/i915_gem_request.h
> > index e2b077df2da0..09add6b9cfc7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_request.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> > @@ -657,39 +657,8 @@ i915_gem_active_wait(const struct i915_gem_active
> > *active, unsigned int flags)
> > return ret < 0 ? ret : 0;
> > }
> >
> > -/**
> > - * i915_gem_active_retire - waits until the request is retired
> > - * @active - the active request on which to wait
> > - *
> > - * i915_gem_active_retire() waits until the request is completed,
> > - * and then ensures that at least the retirement handler for this
> > - * @active tracker is called before returning. If the @active
> > - * tracker is idle, the function returns immediately.
> > - */
> > -static inline int __must_check
> > -i915_gem_active_retire(struct i915_gem_active *active,
> > - struct mutex *mutex)
> > -{
> > - struct drm_i915_gem_request *request;
> > - long ret;
> > -
> > - request = i915_gem_active_raw(active, mutex);
> > - if (!request)
> > - return 0;
> > -
> > - ret = i915_wait_request(request,
> > - I915_WAIT_INTERRUPTIBLE | I915_WAIT_LOCKED,
> > - MAX_SCHEDULE_TIMEOUT);
> > - if (ret < 0)
> > - return ret;
> > -
> > - list_del_init(&active->link);
> > - RCU_INIT_POINTER(active->request, NULL);
> > -
> > - active->retire(active, request);
> > -
> > - return 0;
> > -}
> > +int __must_check i915_gem_active_retire(struct i915_gem_active *active,
> > + struct mutex *mutex);
> >
> > #define for_each_active(mask, idx) \
> > for (; mask ? idx = ffs(mask) - 1, 1 : 0; mask &= ~BIT(idx))
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index a41082e2750e..5a74991854e3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4923,7 +4923,7 @@ static void intel_crtc_dpms_overlay_disable(struct
> > intel_crtc *intel_crtc)
> >
> > mutex_lock(&dev->struct_mutex);
> > dev_priv->mm.interruptible = false;
> > - (void) intel_overlay_switch_off(intel_crtc->overlay);
> > + WARN_ON(intel_overlay_switch_off(intel_crtc->overlay) != 0);
> >
[Intel-gfx] [PATCH] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff
From: Ville Syrjälä
The i915_gem_active stuff doesn't like a NULL ->retire hook, but
the overlay code can set it to NULL. That obviously ends up oopsing.
Fix it by introducing a new helper to assign the retirement callback
that will switch out the NULL function pointer with
i915_gem_retire_noop.
Cc: Chris Wilson
Cc: Joonas Lahtinen
Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
Signed-off-by: Ville Syrjälä
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem_request.h | 19 +++
drivers/gpu/drm/i915/intel_overlay.c| 3 ++-
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.h
b/drivers/gpu/drm/i915/i915_gem_request.h
index 2fc6b8b3f580..b0d50aa81acb 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -440,6 +440,25 @@ i915_gem_active_set(struct i915_gem_active *active,
rcu_assign_pointer(active->request, request);
}
+/**
+ * i915_gem_active_set_retire_fn - updates the retirement callback
+ * @active - the active tracker
+ * @fn - the routine called when the request is retired
+ * @mutex - struct_mutex used to guard retirements
+ *
+ * i915_gem_active_set_retire_fn() updates the function pointer that
+ * is called when the final request associated with the @active tracker
+ * is retired.
+ */
+static inline void
+i915_gem_active_set_retire_fn(struct i915_gem_active *active,
+ i915_gem_retire_fn fn,
+ struct mutex *mutex)
+{
+ lockdep_assert_held(mutex);
+ active->retire = fn ?: i915_gem_retire_noop;
+}
+
static inline struct drm_i915_gem_request *
__i915_gem_active_peek(const struct i915_gem_active *active)
{
diff --git a/drivers/gpu/drm/i915/intel_overlay.c
b/drivers/gpu/drm/i915/intel_overlay.c
index 354f8cec96bb..29509f3055c8 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -216,7 +216,8 @@ static void intel_overlay_submit_request(struct
intel_overlay *overlay,
{
GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
&overlay->i915->drm.struct_mutex));
- overlay->last_flip.retire = retire;
+ i915_gem_active_set_retire_fn(&overlay->last_flip, retire,
+ &overlay->i915->drm.struct_mutex);
i915_gem_active_set(&overlay->last_flip, req);
i915_add_request(req);
}
--
2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/11] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff
On Wed, Dec 07, 2016 at 05:44:15PM +, Chris Wilson wrote:
> On Wed, Dec 07, 2016 at 07:28:04PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > The i915_gem_active stuff doesn't like a NULL ->retire hook, but
> > the overlay code can set it to NULL. That obviously ends up oopsing.
> > Fix it by setting the ->retire hook using init_request_active()
> > so that it'll do the NULL->i915_gem_retire_noop conversion for us.
> >
> > Cc: Chris Wilson
> > Cc: Joonas Lahtinen
> > Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_overlay.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_overlay.c
> > b/drivers/gpu/drm/i915/intel_overlay.c
> > index 1cc963814224..786389dd5175 100644
> > --- a/drivers/gpu/drm/i915/intel_overlay.c
> > +++ b/drivers/gpu/drm/i915/intel_overlay.c
> > @@ -216,7 +216,7 @@ static void intel_overlay_submit_request(struct
> > intel_overlay *overlay,
> > {
> > GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
> > &overlay->i915->drm.struct_mutex));
> > - overlay->last_flip.retire = retire;
> > + init_request_active(&overlay->last_flip, retire);
>
> Hmm, init, not reinit.
Does it matter? The list head is the other thing in there I guess, but
at this point it shouldn't be on any list anymore AFAICS.
> The alternative is to use i915_gem_retire_noop
> instead of NULL. (And sorry, it did used to allow NULL.)
>
> overlay->last_flip.retire = retire ?= i915_gem_retire_noop;
>
> Or i915_gem_active_set_retire_fn(&overlay->last_flip, retire);
> That's seems like it should make everyone a bit happier.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] dim: Try git-merge --ff-only before git-rebase -i when updating branches.
On Wed, 07 Dec 2016, Maarten Lankhorst wrote: > Op 07-12-16 om 10:33 schreef Jani Nikula: >> On Tue, 06 Dec 2016, Maarten Lankhorst >> wrote: >>> When a branch can be fast-forwarded, try it first before rebasing. >>> This will prevent a whole lot of editor windows opening with 'noop' >>> when running dim ub. >>> >>> Signed-off-by: Maarten Lankhorst >>> --- >>> dim | 4 +--- >>> 1 file changed, 1 insertion(+), 3 deletions(-) >>> >>> diff --git a/dim b/dim >>> index fa63ae8c8a79..b2e6841e23d7 100755 >>> --- a/dim >>> +++ b/dim >>> @@ -1286,9 +1286,7 @@ function dim_update_branches >>> repo=`branch_to_repo $branch` >>> remote=`repo_to_remote $repo` >>> >>> - if git diff --quiet $remote/$branch; then >>> - $DRY git rebase >>> - else >>> + if ! $DRY git merge --ff-only ; then >> What does it assume for branches to merge when none are provided? Would >> it be better to be explicit? >> >> Thanks for this, I've been meaning to do this forever... > > Hm I guess explicit might be better, what about this? > --8<--- > When a branch can be fast-forwarded, try it first before rebasing. > This will prevent a whole lot of editor windows opening with 'noop' > when running dim ub. LGTM. > > Signed-off-by: Maarten Lankhorst > --- > diff --git a/dim b/dim > index fa63ae8c8a79..e0551ace54e4 100755 > --- a/dim > +++ b/dim > @@ -1286,9 +1286,7 @@ function dim_update_branches > repo=`branch_to_repo $branch` > remote=`repo_to_remote $repo` > > - if git diff --quiet $remote/$branch; then > - $DRY git rebase > - else > + if ! $DRY git merge --ff-only $remote/$branch; then > $DRY git rebase -i > fi > done > -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff
On Wed, Dec 07, 2016 at 05:56:47PM +, Chris Wilson wrote:
> From: Ville Syrjälä
>
> The i915_gem_active stuff doesn't like a NULL ->retire hook, but
> the overlay code can set it to NULL. That obviously ends up oopsing.
> Fix it by introducing a new helper to assign the retirement callback
> that will switch out the NULL function pointer with
> i915_gem_retire_noop.
>
> Cc: Chris Wilson
> Cc: Joonas Lahtinen
> Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
> Signed-off-by: Ville Syrjälä
> Signed-off-by: Chris Wilson
lgtm, feel free to put yourself as the author is you want.
BTW I don't think we ever call the init_foo() thing on last_flip.
Should be do that in overlay_init() or some such place?
> ---
> drivers/gpu/drm/i915/i915_gem_request.h | 19 +++
> drivers/gpu/drm/i915/intel_overlay.c| 3 ++-
> 2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.h
> b/drivers/gpu/drm/i915/i915_gem_request.h
> index 2fc6b8b3f580..b0d50aa81acb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.h
> +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> @@ -440,6 +440,25 @@ i915_gem_active_set(struct i915_gem_active *active,
> rcu_assign_pointer(active->request, request);
> }
>
> +/**
> + * i915_gem_active_set_retire_fn - updates the retirement callback
> + * @active - the active tracker
> + * @fn - the routine called when the request is retired
> + * @mutex - struct_mutex used to guard retirements
> + *
> + * i915_gem_active_set_retire_fn() updates the function pointer that
> + * is called when the final request associated with the @active tracker
> + * is retired.
> + */
> +static inline void
> +i915_gem_active_set_retire_fn(struct i915_gem_active *active,
> + i915_gem_retire_fn fn,
> + struct mutex *mutex)
> +{
> + lockdep_assert_held(mutex);
> + active->retire = fn ?: i915_gem_retire_noop;
> +}
> +
> static inline struct drm_i915_gem_request *
> __i915_gem_active_peek(const struct i915_gem_active *active)
> {
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c
> b/drivers/gpu/drm/i915/intel_overlay.c
> index 354f8cec96bb..29509f3055c8 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -216,7 +216,8 @@ static void intel_overlay_submit_request(struct
> intel_overlay *overlay,
> {
> GEM_BUG_ON(i915_gem_active_peek(&overlay->last_flip,
> &overlay->i915->drm.struct_mutex));
> - overlay->last_flip.retire = retire;
> + i915_gem_active_set_retire_fn(&overlay->last_flip, retire,
> + &overlay->i915->drm.struct_mutex);
> i915_gem_active_set(&overlay->last_flip, req);
> i915_add_request(req);
> }
> --
> 2.11.0
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Catch non-existent registers in find_fw_domain
On 07/12/2016 14:22, Joonas Lahtinen wrote: Add WARN_ON to find_fw_domain to registers related to uninitialized hardware. v2: - Print the uninitialized domains and register (Chris) Cc: Imre Deak Cc: Wang Elaine Cc: Chris Wilson Signed-off-by: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_uncore.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 07779d0..88f3611 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -625,7 +625,14 @@ find_fw_domain(struct drm_i915_private *dev_priv, u32 offset) dev_priv->uncore.fw_domains_table_entries, fw_range_cmp); - return entry ? entry->domains : 0; + if (!entry) + return 0; + + WARN(entry->domains & ~dev_priv->uncore.fw_domains, +"Uninitialized forcewake domain(s) 0x%x accessed at 0x%x\n", +entry->domains & ~dev_priv->uncore.fw_domains, offset); + + return entry->domains; } static void Only slight issues I can spot is that for some platforms, should it trigger, it would trigger twice since intel_uncore_forcewake_for_read/write functions have the same WARN. If we wanted to avoid that then this WARN would work better in fwtable_read/write I think. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix oopses in the overlay code due to i915_gem_active stuff
On Wed, Dec 07, 2016 at 08:11:33PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 07, 2016 at 05:56:47PM +, Chris Wilson wrote:
> > From: Ville Syrjälä
> >
> > The i915_gem_active stuff doesn't like a NULL ->retire hook, but
> > the overlay code can set it to NULL. That obviously ends up oopsing.
> > Fix it by introducing a new helper to assign the retirement callback
> > that will switch out the NULL function pointer with
> > i915_gem_retire_noop.
> >
> > Cc: Chris Wilson
> > Cc: Joonas Lahtinen
> > Fixes: 0d9bdd886f29 ("drm/i915: Convert intel_overlay to request tracking")
> > Signed-off-by: Ville Syrjälä
> > Signed-off-by: Chris Wilson
>
> lgtm, feel free to put yourself as the author is you want.
>
> BTW I don't think we ever call the init_foo() thing on last_flip.
> Should be do that in overlay_init() or some such place?
Eeek. yes, we are missing the init_request_active() as well. A trivial
patch to add it to overlay_init, gratefully r-b'ed.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/11] drm/i915: Restore dev_priv->mm.interruptible for overlay
On Wed, Dec 07, 2016 at 07:51:16PM +0200, Ville Syrjälä wrote: > On Wed, Dec 07, 2016 at 05:41:39PM +, Chris Wilson wrote: > > Or we can push this wait to where it can interrupt, such as > > prepare_planes_fb. > > (For intel_atomic_commit_tail, intel_crtc_disable_noatomic() should always > > be a no-op.) And then we are down to just the shrinker running > > uninterruptibly, just one last place to fix. > > Hmm. Yeah I guess we could try to do this in a different place. If we do the intel_overlay_off() via mmio (rather than CS) that makes it simpler, as we just have to wait for the prior operation. I'm not imagining that we can just use mmio, right? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
