Re: [Intel-gfx] [PATCH 15/36] drm/i915: Mark up Ironlake ips with rpm wakerefs
On 3/14/2018 3:07 PM, Chris Wilson wrote: Currently Ironlake operates under the assumption that rpm awake (and its error checking is disabled). As such, we have missed a few places where we access registers without taking the rpm wakeref and thus trigger warnings. intel_ips being one culprit. As this involved adding a potentially sleeping rpm_get, we have to rearrange the spinlocks slightly and so switch to acquiring a device-ref under the spinlock rather than hold the spinlock for the whole operation. To be consistent, we make the change in pattern common to the intel_ips interface even though this adds a few more atomic operations than necessary in a few cases. Signed-off-by: Chris Wilson--- drivers/gpu/drm/i915/i915_drv.c | 3 + drivers/gpu/drm/i915/intel_pm.c | 138 2 files changed, 73 insertions(+), 68 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3d0b7353fb09..5c28990aab7f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1440,6 +1440,9 @@ void i915_driver_unload(struct drm_device *dev) i915_driver_unregister(dev_priv); + /* Flush any external code that still may be under the RCU lock */ + synchronize_rcu(); + Hi Chris, Will this rcu change be equivalent to rcu_assign_pointer(i915_mch_dev, dev_priv) in gpu_ips_init rcu_assign_pointer(i915_mch_dev, NULL) in gpu_ips_teardown eliminating smp_store_mb from init/teardown and synchronize_rcu here. Thanks, Sagar if (i915_gem_suspend(dev_priv)) DRM_ERROR("failed to idle hardware; continuing to unload!\n"); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 447811c5be35..a2ebf66ff9ed 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5930,10 +5930,6 @@ void intel_init_ipc(struct drm_i915_private *dev_priv) */ DEFINE_SPINLOCK(mchdev_lock); -/* Global for IPS driver to get at the current i915 device. Protected by - * mchdev_lock. */ -static struct drm_i915_private *i915_mch_dev; - bool ironlake_set_drps(struct drm_i915_private *dev_priv, u8 val) { u16 rgvswctl; @@ -7577,11 +7573,13 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv) if (!IS_GEN5(dev_priv)) return 0; + intel_runtime_pm_get(dev_priv); spin_lock_irq(_lock); val = __i915_chipset_val(dev_priv); spin_unlock_irq(_lock); + intel_runtime_pm_put(dev_priv); return val; } @@ -7661,11 +7659,13 @@ void i915_update_gfx_val(struct drm_i915_private *dev_priv) if (!IS_GEN5(dev_priv)) return; + intel_runtime_pm_get(dev_priv); spin_lock_irq(_lock); __i915_update_gfx_val(dev_priv); spin_unlock_irq(_lock); + intel_runtime_pm_put(dev_priv); } static unsigned long __i915_gfx_val(struct drm_i915_private *dev_priv) @@ -7712,15 +7712,32 @@ unsigned long i915_gfx_val(struct drm_i915_private *dev_priv) if (!IS_GEN5(dev_priv)) return 0; + intel_runtime_pm_get(dev_priv); spin_lock_irq(_lock); val = __i915_gfx_val(dev_priv); spin_unlock_irq(_lock); + intel_runtime_pm_put(dev_priv); return val; } +static struct drm_i915_private *i915_mch_dev; + +static struct drm_i915_private *mchdev_get(void) +{ + struct drm_i915_private *i915; + + rcu_read_lock(); + i915 = i915_mch_dev; + if (!kref_get_unless_zero(>drm.ref)) + i915 = NULL; + rcu_read_unlock(); + + return i915; +} + /** * i915_read_mch_val - return value for IPS use * @@ -7729,23 +7746,22 @@ unsigned long i915_gfx_val(struct drm_i915_private *dev_priv) */ unsigned long i915_read_mch_val(void) { - struct drm_i915_private *dev_priv; - unsigned long chipset_val, graphics_val, ret = 0; - - spin_lock_irq(_lock); - if (!i915_mch_dev) - goto out_unlock; - dev_priv = i915_mch_dev; - - chipset_val = __i915_chipset_val(dev_priv); - graphics_val = __i915_gfx_val(dev_priv); + struct drm_i915_private *i915; + unsigned long chipset_val, graphics_val; - ret = chipset_val + graphics_val; + i915 = mchdev_get(); + if (!i915) + return 0; -out_unlock: + intel_runtime_pm_get(i915); + spin_lock_irq(_lock); + chipset_val = __i915_chipset_val(i915); + graphics_val = __i915_gfx_val(i915); spin_unlock_irq(_lock); + intel_runtime_pm_put(i915); - return ret; + drm_dev_put(>drm); + return chipset_val + graphics_val; } EXPORT_SYMBOL_GPL(i915_read_mch_val); @@ -7756,23 +7772,19 @@ EXPORT_SYMBOL_GPL(i915_read_mch_val); */ bool i915_gpu_raise(void) { - struct drm_i915_private *dev_priv; - bool ret = true; - - spin_lock_irq(_lock); -
Re: [Intel-gfx] [PATCH 12/36] drm/i915: Merge sbi read/write into a single accessor
On 3/14/2018 3:07 PM, Chris Wilson wrote: Since intel_sideband_read and intel_sideband_write differ by only a couple of lines (depending on whether we feed the value in or out), merge the two into a single common accessor. Signed-off-by: Chris Wilson-u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, u32 reg) vlv_flisdsi_read declaration can be removed from sideband.h +void intel_sbi_write(struct drm_i915_private *dev_priv, u16 reg, u32 value, +enum intel_sbi_destination destination) { - u32 val = 0; - vlv_sideband_rw(dev_priv, DPIO_DEVFN, IOSF_PORT_FLISDSI, SB_CRRDDA_NP, - reg, ); - return val; + intel_sbi_rw(dev_priv, reg, destination, , false); } void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val) -- Thanks, Sagar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH xf86-video-intel 1/2] Add Coffeelake PCI IDs for H Skus
Add the Coffeelake PCI IDs based on the following kernel patches: commit ccfd13215fd25a0e8c28221f3acc0dcaec11cd15 Author: Anusha SrivatsaDate: Thu Jun 8 16:41:06 2017 -0700 drm/i915/cfl: Add Coffee Lake PCI IDs for H Sku. Signed-off-by: Liwei Song --- src/i915_pciids.h | 4 src/intel_module.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/i915_pciids.h b/src/i915_pciids.h index 77b200f4a844..31f94f86d984 100644 --- a/src/i915_pciids.h +++ b/src/i915_pciids.h @@ -333,4 +333,8 @@ INTEL_VGA_DEVICE(0x3E92, info), /* SRV GT2 */ \ INTEL_VGA_DEVICE(0x3E96, info) /* SRV GT2 */ +#define INTEL_CFL_H_IDS(info) \ + INTEL_VGA_DEVICE(0x3E9B, info), /* Halo GT2 */ \ + INTEL_VGA_DEVICE(0x3E94, info) /* Halo GT2 */ + #endif /* _I915_PCIIDS_H */ diff --git a/src/intel_module.c b/src/intel_module.c index ffbd1923d688..1691bea37c2f 100644 --- a/src/intel_module.c +++ b/src/intel_module.c @@ -313,6 +313,8 @@ static const SymTabRec intel_chipsets[] = { {0x3E91, "HD Graphics"}, {0x3E92, "HD Graphics"}, {0x3E96, "HD Graphics"}, + {0x3E9B, "HD Graphics"}, + {0x3E94, "HD Graphics"}, /* When adding new identifiers, also update: * 1. intel_identify() @@ -370,6 +372,7 @@ static const struct pci_id_match intel_device_match[] = { INTEL_GLK_IDS(_geminilake_info), INTEL_CFL_S_IDS(_coffeelake_info), + INTEL_CFL_H_IDS(_coffeelake_info), INTEL_VGA_DEVICE(PCI_MATCH_ANY, _generic_info), #endif -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/i915/psr: Rename intel_crtc_state has_psr to can_psr
On Wed, 2018-03-14 at 15:36 -0700, José Roberto de Souza wrote: > This value is a match of hardware and sink has PSR + if it can be > enabled by the requested state, see intel_psr_compute_config(). > > Cc: Dhinakaran Pandiyan> Cc: Rodrigo Vivi > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/intel_drv.h | 4 ++-- > drivers/gpu/drm/i915/intel_psr.c | 12 ++-- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index a215aa78b0be..cccaf84415ab 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -807,8 +807,8 @@ struct intel_crtc_state { > struct intel_link_m_n dp_m2_n2; > bool has_drrs; > > - bool has_psr; > - bool has_psr2; > + bool can_psr; > + bool can_psr2; I am not convinced by this change, the computed state either has PSR1 or PSR2, "can" connotes ambiguity in my opinion. I was thinking of converting this to an u8 psr = [0,1,2] to mean no PSR, PSR1 and PSR2 respectively. We can do away with the has/can confusion :) Using bool does save us 6 bits though depending on how the structure is packed. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915/psr: Enable aux frame sync in source
On Wed, 2018-03-14 at 15:36 -0700, José Roberto de Souza wrote: > Even with GTC not enabled lets send the aux frame sync. If this was never enabled on the source side, why do we place a requirement on the sink to support aux frame sync? > Hardware is going to send dummy values but this way we can get rid of > this workarround in PSR exit: 'drm/i915/psr: disable aux_frame_sync > on psr2 exit'. > Also moving the line disabling aux frame sync in sink to after report > that PSR2 has exit to avoid. > > Cc: Dhinakaran Pandiyan> Cc: Rodrigo Vivi > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_psr.c | 15 ++- > 2 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index e9fc1722c0fb..5a2364656aa5 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4132,6 +4132,7 @@ enum { > #define EDP_PSR2_CTL _MMIO(0x6f900) > #define EDP_PSR2_ENABLE(1<<31) > #define EDP_SU_TRACK_ENABLE(1<<30) > +#define EDP_AUX_FRAME_SYNC_ENABLE (1<<27) > #define EDP_Y_COORDINATE_VALID (1<<26) > #define EDP_Y_COORDINATE_ENABLE(1<<25) > #define EDP_MAX_SU_DISABLE_TIME(t) ((t)<<20) > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index d622e37894d4..7aab66b5bc91 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -418,6 +418,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) >* good enough. */ > val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; > val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE; > + val |= EDP_AUX_FRAME_SYNC_ENABLE; > > if (drm_dp_dpcd_readb(_dp->aux, > DP_SYNCHRONIZATION_LATENCY_IN_SINK, > @@ -715,11 +716,6 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > i915_reg_t psr_status; > u32 psr_status_mask; > > - if (dev_priv->psr.psr2_enabled) > - drm_dp_dpcd_writeb(_dp->aux, > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF, > - 0); > - > if (dev_priv->psr.psr2_enabled) { > psr_status = EDP_PSR2_STATUS; > psr_status_mask = EDP_PSR2_STATUS_STATE_MASK; > @@ -742,6 +738,11 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > 2000)) > DRM_ERROR("Timed out waiting for PSR Idle State\n"); > > + if (dev_priv->psr.psr2_enabled) > + drm_dp_dpcd_writeb(_dp->aux, > +DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF, > +0); > + > dev_priv->psr.active = false; > } else { > if (dev_priv->psr.psr2_enabled) > @@ -863,10 +864,6 @@ static void intel_psr_exit(struct drm_i915_private > *dev_priv) > return; > > if (HAS_DDI(dev_priv)) { > - if (dev_priv->psr.psr2_enabled) > - drm_dp_dpcd_writeb(_dp->aux, > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF, > - 0); > if (dev_priv->psr.psr2_enabled) { > val = I915_READ(EDP_PSR2_CTL); > WARN_ON(!(val & EDP_PSR2_ENABLE)); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915/psr: Enable Y-coordinate support in source
On Thu, 2018-03-15 at 17:28 -0700, Rodrigo Vivi wrote: > drm/i915/cnl: > > On Wed, Mar 14, 2018 at 03:36:14PM -0700, José Roberto de Souza wrote: > > We are requiring that sink requires Y-coordinate but we are not > > sending it in the main-link. > > Also add on CNL here > > > Even if hardware tracking isn't good enough it will not cause > > any more issues enabling it. > > > > Cc: Dhinakaran Pandiyan> > Cc: Rodrigo Vivi > > Signed-off-by: José Roberto de Souza > > --- > > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > > drivers/gpu/drm/i915/intel_psr.c | 4 ++-- > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index a15db41a208a..e9fc1722c0fb 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -4132,6 +4132,8 @@ enum { > > #define EDP_PSR2_CTL _MMIO(0x6f900) > > #define EDP_PSR2_ENABLE (1<<31) > > #define EDP_SU_TRACK_ENABLE (1<<30) > > +#define EDP_Y_COORDINATE_VALID (1<<26) > > +#define EDP_Y_COORDINATE_ENABLE (1<<25) > > probably good add CNL_ prefix on these bits... > > > #define EDP_MAX_SU_DISABLE_TIME(t) ((t)<<20) > > #define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f<<20) > > #define EDP_PSR2_TP2_TIME_500(0<<8) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index 62d97d5576d1..c9da1390a727 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -416,8 +416,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) > > /* FIXME: selective update is probably totally broken because it doesn't > > * mesh at all with our frontbuffer tracking. And the hw alone isn't > > * good enough. */ > > - val |= EDP_PSR2_ENABLE | > > - EDP_SU_TRACK_ENABLE; > > + val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; > > + val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE; > > if (INTEL_GEN(dev_priv) >= 10) > val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE; > > since those bits were reserved before CNL. > How does this work on pre-CNL platforms without the enable bit? > With those changes: > > Reviewed-by: Rodrigo Vivi > > > > > if (drm_dp_dpcd_readb(_dp->aux, > > DP_SYNCHRONIZATION_LATENCY_IN_SINK, > > -- > > 2.16.2 > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm 2/5] intel: reorganize internal function
From: "Xiong, James"split drm_intel_gem_bo_alloc_internal, and add a function to search for a suitable buffer for given size for reuse. Signed-off-by: Xiong, James --- intel/intel_bufmgr_gem.c | 141 --- 1 file changed, 73 insertions(+), 68 deletions(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 386da30..2fcb0a0 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -704,6 +704,71 @@ drm_intel_gem_bo_cache_purge_bucket(drm_intel_bufmgr_gem *bufmgr_gem, } } +static drm_intel_bo_gem * +drm_intel_gem_bo_cached_for_size(drm_intel_bufmgr_gem *bufmgr_gem, +unsigned long size, +uint32_t tiling_mode, +unsigned long stride, +unsigned long alignment, +bool for_render) +{ + struct drm_intel_gem_bo_bucket *bucket = + drm_intel_gem_bo_bucket_for_size(bufmgr_gem, size); + +if (bucket != NULL) { + drm_intel_bo_gem *bo_gem, *temp_bo_gem; +retry: + bo_gem = NULL; + if (for_render) { + /* Allocate new render-target BOs from the tail (MRU) +* of the list, as it will likely be hot in the GPU +* cache and in the aperture for us. +*/ + bo_gem = DRMLISTENTRY(drm_intel_bo_gem, + bucket->head.prev, head); + DRMLISTDEL(_gem->head); + bo_gem->bo.align = alignment; + } else { + assert(alignment == 0); +/* For non-render-target BOs (where we're probably +* going to map it first thing in order to fill it +* with data), check if the last BO in the cache is +* unbusy, and only reuse in that case. Otherwise, +* allocating a new buffer is probably faster than +* waiting for the GPU to finish. +*/ + bo_gem = DRMLISTENTRY(drm_intel_bo_gem, + bucket->head.next, head); + if (!drm_intel_gem_bo_busy(_gem->bo)) { + DRMLISTDEL(_gem->head); + } else { + bo_gem = NULL; + } + } + + if (bo_gem) { + if (!drm_intel_gem_bo_madvise_internal + (bufmgr_gem, bo_gem, I915_MADV_WILLNEED)) { + drm_intel_gem_bo_free(_gem->bo); + drm_intel_gem_bo_cache_purge_bucket(bufmgr_gem, + bucket); + return NULL; + } + + if (drm_intel_gem_bo_set_tiling_internal(_gem->bo, +tiling_mode, +stride)) { + drm_intel_gem_bo_free(_gem->bo); + goto retry; + } + } + + return bo_gem; + } + + return NULL; +} + static drm_intel_bo * drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, const char *name, @@ -715,81 +780,21 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr, { drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr; drm_intel_bo_gem *bo_gem; - unsigned int page_size = getpagesize(); int ret; - struct drm_intel_gem_bo_bucket *bucket; - bool alloc_from_cache; - unsigned long bo_size; bool for_render = false; if (flags & BO_ALLOC_FOR_RENDER) for_render = true; - /* Round the allocated size up to a power of two number of pages. */ - bucket = drm_intel_gem_bo_bucket_for_size(bufmgr_gem, size); - - /* If we don't have caching at this size, don't actually round the -* allocation up. -*/ - if (bucket == NULL) { - bo_size = size; - if (bo_size < page_size) - bo_size = page_size; - } else { - bo_size = bucket->size; - } + /* first align the size on page boundary */ + size = ALIGN(size, getpagesize()); pthread_mutex_lock(_gem->lock); /* Get a buffer out of the cache if available */ -retry: - alloc_from_cache = false; - if (bucket != NULL && !DRMLISTEMPTY(>head)) { - if (for_render) { -
[Intel-gfx] [PATCH libdrm 1/5] drm: add a macro DRMLISTFOREACHENTRYREVERSE
From: "Xiong, James"it goes through DRMLIST in a reverse order Signed-off-by: Xiong, James --- libdrm_lists.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/libdrm_lists.h b/libdrm_lists.h index 8926d8d..400c731 100644 --- a/libdrm_lists.h +++ b/libdrm_lists.h @@ -101,6 +101,12 @@ typedef struct _drmMMListHead (__item) = DRMLISTENTRY(typeof(*__item), \ (__item)->__head.next, __head)) +#define DRMLISTFOREACHENTRYREVERSE(__item, __list, __head) \ + for ((__item) = DRMLISTENTRY(typeof(*__item), (__list)->prev, __head); \ +&(__item)->__head != (__list);\ +(__item) = DRMLISTENTRY(typeof(*__item), \ +(__item)->__head.prev, __head)) + #define DRMLISTFOREACHENTRYSAFE(__item, __temp, __list, __head) \ for ((__item) = DRMLISTENTRY(typeof(*__item), (__list)->next, __head), \ (__temp) = DRMLISTENTRY(typeof(*__item), \ -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm 0/5] improve reuse implementation
From: "Xiong, James"With gem_reuse enabled, when a buffer size is different than the sizes of buckets, it is aligned to the next bucket's size, which means about 25% more memory than the requested is allocated in the worst senario. For example: Orignal sizeActual 32KB+1Byte 40KB . . . 8MB+1Byte 10MB . . . 96MB+1Byte 112MB This is very memory expensive and make the reuse feature less favorable than it deserves to be. This series aligns the reuse buffer size on page size instead to save memory. Performed gfxbench tests on Gen9 without LLC, the performances and reuse ratioes (reuse count/allocation count) were same as before, saved memory usage by 1% ~ 7%(gl_manhattan: peak allocated memory size was reduced from 448401408 to 419078144). v2: split the patch to a series of small functional changes (Chris) Xiong, James (5): drm: add a macro DRMLISTFOREACHENTRYREVERSE intel: reorganize internal function intel: get a cached bucket by size intel: get a cached buffer by size for reuse intel: purge cached bucket when its cached buffer is evicted intel/intel_bufmgr_gem.c | 180 +-- libdrm_lists.h | 6 ++ 2 files changed, 100 insertions(+), 86 deletions(-) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm 4/5] intel: get a cached buffer by size for reuse
From: "Xiong, James"Previously all cached buffers in a given bucket were same sized, when reusing, the MRU buffer at the tail was poped out. With the new implementation, we go through the buffer list in a reverse order to search for a MRU buffer with a suitable size. Signed-off-by: Xiong, James --- intel/intel_bufmgr_gem.c | 25 +++-- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index f8317a4..e3d5a8d 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -723,10 +723,14 @@ retry: * of the list, as it will likely be hot in the GPU * cache and in the aperture for us. */ - bo_gem = DRMLISTENTRY(drm_intel_bo_gem, - bucket->head.prev, head); - DRMLISTDEL(_gem->head); - bo_gem->bo.align = alignment; + DRMLISTFOREACHENTRYREVERSE(temp_bo_gem, >head, head) { + if (temp_bo_gem->bo.size >= size) { + bo_gem = temp_bo_gem; + DRMLISTDEL(_gem->head); + bo_gem->bo.align = alignment; + break; + } + } } else { assert(alignment == 0); /* For non-render-target BOs (where we're probably @@ -736,12 +740,13 @@ retry: * allocating a new buffer is probably faster than * waiting for the GPU to finish. */ - bo_gem = DRMLISTENTRY(drm_intel_bo_gem, - bucket->head.next, head); - if (!drm_intel_gem_bo_busy(_gem->bo)) { - DRMLISTDEL(_gem->head); - } else { - bo_gem = NULL; + DRMLISTFOREACHENTRY(temp_bo_gem, >head, head) { + if (temp_bo_gem->bo.size >= size && + !drm_intel_gem_bo_busy(_bo_gem->bo)) { + bo_gem = temp_bo_gem; + DRMLISTDEL(_gem->head); + break; + } } } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm 5/5] intel: purge cached bucket when its cached buffer is evicted
From: "Xiong, James"Previously when a cached MRU buffer was found to be evicted by kernel, the bucket was emptied. The new implementation purged these buffers that were freed before the evicted one. Signed-off-by: Xiong, James --- intel/intel_bufmgr_gem.c | 29 ++--- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index e3d5a8d..69b361b 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -684,22 +684,20 @@ drm_intel_gem_bo_madvise(drm_intel_bo *bo, int madv) madv); } -/* drop the oldest entries that have been purged by the kernel */ +/* drop the entries that are older than the given time */ static void drm_intel_gem_bo_cache_purge_bucket(drm_intel_bufmgr_gem *bufmgr_gem, - struct drm_intel_gem_bo_bucket *bucket) + struct drm_intel_gem_bo_bucket *bucket, + time_t time) { - while (!DRMLISTEMPTY(>head)) { - drm_intel_bo_gem *bo_gem; - - bo_gem = DRMLISTENTRY(drm_intel_bo_gem, - bucket->head.next, head); - if (drm_intel_gem_bo_madvise_internal - (bufmgr_gem, bo_gem, I915_MADV_DONTNEED)) - break; - - DRMLISTDEL(_gem->head); - drm_intel_gem_bo_free(_gem->bo); + drm_intel_bo_gem *bo_gem, *temp; + DRMLISTFOREACHENTRYSAFE(bo_gem, temp, >head, head) { + if (bo_gem->free_time >= time) { + drm_intel_gem_bo_madvise_internal + (bufmgr_gem, bo_gem, I915_MADV_DONTNEED); + DRMLISTDEL(_gem->head); + drm_intel_gem_bo_free(_gem->bo); + } } } @@ -753,9 +751,10 @@ retry: if (bo_gem) { if (!drm_intel_gem_bo_madvise_internal (bufmgr_gem, bo_gem, I915_MADV_WILLNEED)) { - drm_intel_gem_bo_free(_gem->bo); drm_intel_gem_bo_cache_purge_bucket(bufmgr_gem, - bucket); + bucket, + bo_gem->free_time); + drm_intel_gem_bo_free(_gem->bo); return NULL; } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm 3/5] intel: get a cached bucket by size
From: "Xiong, James"cached buckets are sorted by size in increasing order, each now contains cached buffers with different sizes. A buffer with size >= buckets[n].size and < buckets[n+1].size is put in bucket n for future reuse. Signed-off-by: Xiong, James --- intel/intel_bufmgr_gem.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 2fcb0a0..f8317a4 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -402,11 +402,10 @@ drm_intel_gem_bo_bucket_for_size(drm_intel_bufmgr_gem *bufmgr_gem, { int i; - for (i = 0; i < bufmgr_gem->num_buckets; i++) { - struct drm_intel_gem_bo_bucket *bucket = - _gem->cache_bucket[i]; - if (bucket->size >= size) { - return bucket; + for (i = 0; i < bufmgr_gem->num_buckets - 1; i++) { + if (size >= bufmgr_gem->cache_bucket[i].size && + size < bufmgr_gem->cache_bucket[i+1].size) { + return _gem->cache_bucket[i]; } } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
On Thu, Mar 15, 2018 at 06:04:27PM -0700, Souza, Jose wrote: > On Thu, 2018-03-15 at 17:35 -0700, Rodrigo Vivi wrote: > > On Wed, Mar 14, 2018 at 03:36:13PM -0700, José Roberto de Souza > > wrote: > > > Move to only one place the sink requirements that the actual driver > > > needs to enable PSR2. > > > > > > Also intel_psr2_config_valid() is called every time the crtc config > > > is computed, wasting some time every time it was checking for > > > Y coordinate requirement. > > > > > > This allow us to nuke y_cord_support and some of VSC setup code > > > that > > > was handling a scenario that would never happen(PSR2 without Y > > > coordinate). > > > > > > Cc: Dhinakaran Pandiyan> > > Cc: Rodrigo Vivi > > > Signed-off-by: José Roberto de Souza > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 1 - > > > drivers/gpu/drm/i915/intel_psr.c | 55 -- > > > -- > > > 2 files changed, 28 insertions(+), 28 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h > > > index 8a584273f897..d4bc8d18f56c 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -603,7 +603,6 @@ struct i915_psr { > > > unsigned busy_frontbuffer_bits; > > > bool psr2_support; > > > bool link_standby; > > > - bool y_cord_support; > > > bool colorimetry_support; > > > bool alpm; > > > bool has_hw_tracking; > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > > b/drivers/gpu/drm/i915/intel_psr.c > > > index 9811f5f0bc75..62d97d5576d1 100644 > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > @@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp > > > *intel_dp) > > > intel_display_power_put(dev_priv, > > > psr_aux_domain(intel_dp)); > > > } > > > > > > -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp) > > > +static bool intel_dp_get_y_coord_required(struct intel_dp > > > *intel_dp) > > > { > > > uint8_t psr_caps = 0; > > > > > > @@ -137,22 +137,38 @@ void intel_psr_init_dpcd(struct intel_dp > > > *intel_dp) > > > > > > if (INTEL_GEN(dev_priv) >= 9 && > > > (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) { > > > - uint8_t frame_sync_cap; > > > + uint8_t frame_sync_cap, y_coord_req; > > > > > > dev_priv->psr.sink_support = true; > > > + > > > + /* PSR2 needs frame sync to do selective updates > > > */ > > > if (drm_dp_dpcd_readb(_dp->aux, > > > DP_SINK_DEVICE_AUX_FRAME_SYN > > > C_CAP, > > > _sync_cap) != 1) > > > frame_sync_cap = 0; > > > frame_sync_cap = (frame_sync_cap & > > > DP_AUX_FRAME_SYNC_CAP); > > > - /* PSR2 needs frame sync as well */ > > > - dev_priv->psr.psr2_support = frame_sync_cap; > > > - DRM_DEBUG_KMS("PSR2 %s on sink", > > > - dev_priv->psr.psr2_support ? > > > "supported" : "not supported"); > > > + > > > + /* > > > + * FIXME: Enable PSR2 only for y-coordinate PSR2 > > > panels > > > + * After GTC implementation, remove this > > > restriction. > > > > I believe we should remove the FIXME and just comment out that non y- > > coordinate > > would need GTC that we currently don't implement. > > > > later if we change our minds and decided to add that support we > > revisit here > > but without a FIXME poluting code for something we don't plan to > > support for now. > > > > > + * > > > + * All panels that supports PSR version 3 that is > > > > PSRv3? is there really something formal about that? > > Yes, from eDP spec: > > PSR Support and Version > > 00h = Panel Self Refresh Capability is not supported (default). > If PSR/PSR2 is supported, the SET_POWER_CAPABLE bit in the > EDP_GENERAL_CAPABILITY_1 register (DPCD Address 00701h, bit 7) must > be set to 1. > 01h = Panel Self Refresh capability is supported, PSR version is 01h > (PSR). > 02h = Panel Self Refresh with Selective Update (SU) capability is > supported, > PSR version is 02h (PSR2). Y-coordinates for SU are not supported. > Supported > by eDP v1.4 (and higher). > 03h = Panel Self Refresh with SU capability and Y-coordinate supported, > PSR > version is 03h (PSR2 + Y-coordinate). Supported by eDP v1.4a (and > higher). Oh cool! now I understand the comment. Could we add the comment like in spec ie s/PSR version 3 that is PSR2 + Y-coordinate/PSR version 03h (PSR2 + Y-coordinate)/ ? just to avoid giving impression of a PSR3 support and someone freaking out thinking that we would need much more changes ;) > > > > > > + * PSR2 + Y-coordinate support can handle Y > > > coordinates in VSC > > > + * but we are only sure that it is going to be > > > used when > > > + * required by the
Re: [Intel-gfx] [PATCH 2/6] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
On Thu, 2018-03-15 at 17:35 -0700, Rodrigo Vivi wrote: > On Wed, Mar 14, 2018 at 03:36:13PM -0700, José Roberto de Souza > wrote: > > Move to only one place the sink requirements that the actual driver > > needs to enable PSR2. > > > > Also intel_psr2_config_valid() is called every time the crtc config > > is computed, wasting some time every time it was checking for > > Y coordinate requirement. > > > > This allow us to nuke y_cord_support and some of VSC setup code > > that > > was handling a scenario that would never happen(PSR2 without Y > > coordinate). > > > > Cc: Dhinakaran Pandiyan> > Cc: Rodrigo Vivi > > Signed-off-by: José Roberto de Souza > > --- > > drivers/gpu/drm/i915/i915_drv.h | 1 - > > drivers/gpu/drm/i915/intel_psr.c | 55 -- > > -- > > 2 files changed, 28 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index 8a584273f897..d4bc8d18f56c 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -603,7 +603,6 @@ struct i915_psr { > > unsigned busy_frontbuffer_bits; > > bool psr2_support; > > bool link_standby; > > - bool y_cord_support; > > bool colorimetry_support; > > bool alpm; > > bool has_hw_tracking; > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index 9811f5f0bc75..62d97d5576d1 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp > > *intel_dp) > > intel_display_power_put(dev_priv, > > psr_aux_domain(intel_dp)); > > } > > > > -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp) > > +static bool intel_dp_get_y_coord_required(struct intel_dp > > *intel_dp) > > { > > uint8_t psr_caps = 0; > > > > @@ -137,22 +137,38 @@ void intel_psr_init_dpcd(struct intel_dp > > *intel_dp) > > > > if (INTEL_GEN(dev_priv) >= 9 && > > (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) { > > - uint8_t frame_sync_cap; > > + uint8_t frame_sync_cap, y_coord_req; > > > > dev_priv->psr.sink_support = true; > > + > > + /* PSR2 needs frame sync to do selective updates > > */ > > if (drm_dp_dpcd_readb(_dp->aux, > > DP_SINK_DEVICE_AUX_FRAME_SYN > > C_CAP, > > _sync_cap) != 1) > > frame_sync_cap = 0; > > frame_sync_cap = (frame_sync_cap & > > DP_AUX_FRAME_SYNC_CAP); > > - /* PSR2 needs frame sync as well */ > > - dev_priv->psr.psr2_support = frame_sync_cap; > > - DRM_DEBUG_KMS("PSR2 %s on sink", > > - dev_priv->psr.psr2_support ? > > "supported" : "not supported"); > > + > > + /* > > +* FIXME: Enable PSR2 only for y-coordinate PSR2 > > panels > > +* After GTC implementation, remove this > > restriction. > > I believe we should remove the FIXME and just comment out that non y- > coordinate > would need GTC that we currently don't implement. > > later if we change our minds and decided to add that support we > revisit here > but without a FIXME poluting code for something we don't plan to > support for now. > > > +* > > +* All panels that supports PSR version 3 that is > > PSRv3? is there really something formal about that? Yes, from eDP spec: PSR Support and Version 00h = Panel Self Refresh Capability is not supported (default). If PSR/PSR2 is supported, the SET_POWER_CAPABLE bit in the EDP_GENERAL_CAPABILITY_1 register (DPCD Address 00701h, bit 7) must be set to 1. 01h = Panel Self Refresh capability is supported, PSR version is 01h (PSR). 02h = Panel Self Refresh with Selective Update (SU) capability is supported, PSR version is 02h (PSR2). Y-coordinates for SU are not supported. Supported by eDP v1.4 (and higher). 03h = Panel Self Refresh with SU capability and Y-coordinate supported, PSR version is 03h (PSR2 + Y-coordinate). Supported by eDP v1.4a (and higher). > > > +* PSR2 + Y-coordinate support can handle Y > > coordinates in VSC > > +* but we are only sure that it is going to be > > used when > > +* required by the panel. This way panel is > > capable to do > > +* selective update even without a valid aux frame > > sync. > > +*/ > > + y_coord_req = > > intel_dp_get_y_coord_required(intel_dp); > > + > > + dev_priv->psr.psr2_support = frame_sync_cap && > > y_coord_req; > > + if (dev_priv->psr.psr2_support) > > + DRM_DEBUG_KMS("PSR2 supported on sink\n"); > > + else > > + DRM_DEBUG_KMS("PSR2 not supported on sink" > > +
[Intel-gfx] ✓ Fi.CI.IGT: success for Guc and HuC for Cannonlake (rev2)
== Series Details == Series: Guc and HuC for Cannonlake (rev2) URL : https://patchwork.freedesktop.org/series/40064/ State : success == Summary == Known issues: Test gem_eio: Subgroup in-flight-external: pass -> INCOMPLETE (shard-apl) fdo#105341 Test kms_cursor_crc: Subgroup cursor-256x256-suspend: skip -> PASS (shard-snb) fdo#103375 Test kms_flip: Subgroup 2x-plain-flip-ts-check: pass -> FAIL (shard-hsw) fdo#100368 Test kms_rotation_crc: Subgroup primary-rotation-90: pass -> FAIL (shard-apl) fdo#105185 Test kms_setmode: Subgroup basic: fail -> PASS (shard-apl) fdo#99912 Test kms_sysfs_edid_timing: warn -> PASS (shard-apl) fdo#100047 fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047 shard-apltotal:3441 pass:1813 dwarn:1 dfail:0 fail:7 skip:1619 time:12700s shard-hswtotal:3442 pass:1767 dwarn:1 dfail:0 fail:2 skip:1671 time:11995s shard-snbtotal:3442 pass:1357 dwarn:1 dfail:0 fail:3 skip:2081 time:7247s Blacklisted hosts: shard-kbltotal:2045 pass:1163 dwarn:1 dfail:1 fail:4 skip:875 time:5724s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8372/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Show dmc debug registers on CFL and GLK
On Thu, Mar 15, 2018 at 03:35:02PM +0200, David Weinehall wrote: > Since Coffee Lake uses the Kaby Lake DMC it's a safe > bet that the debug registers are the same. I haven't > double-checked that the GLK DMC uses the same registers > as BXT, but it seems as good of a guess as any. It would be good to check. Last time that I checked we were getting deep PC residencies but these registers were zeroed on CFL. Also this bit confuses many people since it is not a proper residency counter :/ > > Signed-off-by: David Weinehall> --- > drivers/gpu/drm/i915/i915_debugfs.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index c4cc8fef11a0..dad0776d58b4 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2813,13 +2813,14 @@ static int i915_dmc_info(struct seq_file *m, void > *unused) > seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version), > CSR_VERSION_MINOR(csr->version)); > > - if (IS_KABYLAKE(dev_priv) || > + if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) || > (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) { > seq_printf(m, "DC3 -> DC5 count: %d\n", > I915_READ(SKL_CSR_DC3_DC5_COUNT)); > seq_printf(m, "DC5 -> DC6 count: %d\n", > I915_READ(SKL_CSR_DC5_DC6_COUNT)); > - } else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4)) { > + } else if (IS_GEMINILAKE(dev_priv) || > +IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4)) { > seq_printf(m, "DC3 -> DC5 count: %d\n", > I915_READ(BXT_CSR_DC3_DC5_COUNT)); > } > -- > 2.16.2 > > ___ > 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/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
On Thu, Mar 15, 2018 at 02:08:51PM -0700, matthew.s.atw...@intel.com wrote: > From: Matt Atwood> > DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheeme from 8 > bits to 7 in DPCD 0x000e. The 8th bit is used to identify extended > receiver capabilities. For panels that use this new feature wait interval > would be increased by 512 ms, when spec is max 16 ms. This behavior is > described in table 2-158 of DP 1.4 spec address eh. > > With the introduction of DP 1.4 spec main link clock recovery was > standardized to 100 us regardless of TRAINING_AUX_RD_INTERVAL value. > > To avoid breaking panels that are not spec compiant we now warn on > invalid values. > > V2: commit title/message, masking all 7 bits, warn on out of spec values. > V3: commit message, make link train clock recovery follow DP 1.4 spec. > V4: style changes > V5: typo > V6: print statement revisions, DP_REV to DPCD_REV, comment correction > V7: typo > V8: Style thanks :) > > Signed-off-by: Matt Atwood Reviewed-by: Rodrigo Vivi > --- > drivers/gpu/drm/drm_dp_helper.c | 22 ++ > include/drm/drm_dp_helper.h | 6 ++ > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c > index adf79be..6bee2df 100644 > --- a/drivers/gpu/drm/drm_dp_helper.c > +++ b/drivers/gpu/drm/drm_dp_helper.c > @@ -119,18 +119,32 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 > link_status[DP_LINK_STATUS_SI > EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis); > > void drm_dp_link_train_clock_recovery_delay(const u8 > dpcd[DP_RECEIVER_CAP_SIZE]) { > - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > + int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & > + DP_TRAINING_AUX_RD_MASK; > + > + if (rd_interval > 4) > + DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n", > + rd_interval); > + > + if (rd_interval == 0 || dpcd[DP_DPCD_REV] >= DPCD_REV_14) > udelay(100); > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + mdelay(rd_interval * 4); > } > EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); > > void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) > { > - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) > + int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & > + DP_TRAINING_AUX_RD_MASK; > + > + if (rd_interval > 4) > + DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n", > + rd_interval); > + > + if (rd_interval == 0) > udelay(400); > else > - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); > + mdelay(rd_interval * 4); > } > EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index da58a42..8c59ce4 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -64,6 +64,11 @@ > /* AUX CH addresses */ > /* DPCD */ > #define DP_DPCD_REV 0x000 > +# define DPCD_REV_100x10 > +# define DPCD_REV_110x11 > +# define DPCD_REV_120x12 > +# define DPCD_REV_130x13 > +# define DPCD_REV_140x14 > > #define DP_MAX_LINK_RATE0x001 > > @@ -118,6 +123,7 @@ > # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher > */ > > #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ > +# define DP_TRAINING_AUX_RD_MASK0x7F/* XXX 1.2? */ > > #define DP_ADAPTER_CAP 0x00f /* 1.2 */ > # define DP_FORCE_LOAD_SENSE_CAP (1 << 0) > -- > 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 2/6] drm/i915/psr: Tie PSR2 support to Y coordinate requirement in intel_psr_init_dpcd()
On Wed, Mar 14, 2018 at 03:36:13PM -0700, José Roberto de Souza wrote: > Move to only one place the sink requirements that the actual driver > needs to enable PSR2. > > Also intel_psr2_config_valid() is called every time the crtc config > is computed, wasting some time every time it was checking for > Y coordinate requirement. > > This allow us to nuke y_cord_support and some of VSC setup code that > was handling a scenario that would never happen(PSR2 without Y > coordinate). > > Cc: Dhinakaran Pandiyan> Cc: Rodrigo Vivi > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/i915_drv.h | 1 - > drivers/gpu/drm/i915/intel_psr.c | 55 > > 2 files changed, 28 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8a584273f897..d4bc8d18f56c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -603,7 +603,6 @@ struct i915_psr { > unsigned busy_frontbuffer_bits; > bool psr2_support; > bool link_standby; > - bool y_cord_support; > bool colorimetry_support; > bool alpm; > bool has_hw_tracking; > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index 9811f5f0bc75..62d97d5576d1 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp *intel_dp) > intel_display_power_put(dev_priv, psr_aux_domain(intel_dp)); > } > > -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp) > +static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp) > { > uint8_t psr_caps = 0; > > @@ -137,22 +137,38 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) > > if (INTEL_GEN(dev_priv) >= 9 && > (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) { > - uint8_t frame_sync_cap; > + uint8_t frame_sync_cap, y_coord_req; > > dev_priv->psr.sink_support = true; > + > + /* PSR2 needs frame sync to do selective updates */ > if (drm_dp_dpcd_readb(_dp->aux, > DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > _sync_cap) != 1) > frame_sync_cap = 0; > frame_sync_cap = (frame_sync_cap & DP_AUX_FRAME_SYNC_CAP); > - /* PSR2 needs frame sync as well */ > - dev_priv->psr.psr2_support = frame_sync_cap; > - DRM_DEBUG_KMS("PSR2 %s on sink", > - dev_priv->psr.psr2_support ? "supported" : "not > supported"); > + > + /* > + * FIXME: Enable PSR2 only for y-coordinate PSR2 panels > + * After GTC implementation, remove this restriction. I believe we should remove the FIXME and just comment out that non y-coordinate would need GTC that we currently don't implement. later if we change our minds and decided to add that support we revisit here but without a FIXME poluting code for something we don't plan to support for now. > + * > + * All panels that supports PSR version 3 that is PSRv3? is there really something formal about that? > + * PSR2 + Y-coordinate support can handle Y coordinates in VSC > + * but we are only sure that it is going to be used when > + * required by the panel. This way panel is capable to do > + * selective update even without a valid aux frame sync. > + */ > + y_coord_req = intel_dp_get_y_coord_required(intel_dp); > + > + dev_priv->psr.psr2_support = frame_sync_cap && y_coord_req; > + if (dev_priv->psr.psr2_support) > + DRM_DEBUG_KMS("PSR2 supported on sink\n"); > + else > + DRM_DEBUG_KMS("PSR2 not supported on sink" > + "(frame sync: %d Y-coord required: %d)\n", > + frame_sync_cap, y_coord_req); > > if (dev_priv->psr.psr2_support) { > - dev_priv->psr.y_cord_support = > - intel_dp_get_y_cord_status(intel_dp); > dev_priv->psr.colorimetry_support = > intel_dp_get_colorimetry_status(intel_dp); > dev_priv->psr.alpm = > @@ -198,16 +214,12 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp, > memset(_vsc, 0, sizeof(psr_vsc)); > psr_vsc.sdp_header.HB0 = 0; > psr_vsc.sdp_header.HB1 = 0x7; > - if (dev_priv->psr.colorimetry_support && > - dev_priv->psr.y_cord_support) { > + if (dev_priv->psr.colorimetry_support) { >
Re: [Intel-gfx] [PATCH 6/6] drm/i915/psr: Enable aux frame sync in source
On Wed, Mar 14, 2018 at 03:36:17PM -0700, José Roberto de Souza wrote: > Even with GTC not enabled lets send the aux frame sync. > Hardware is going to send dummy values but this way we can get rid of > this workarround in PSR exit: 'drm/i915/psr: disable aux_frame_sync > on psr2 exit'. > Also moving the line disabling aux frame sync in sink to after report > that PSR2 has exit to avoid. > > Cc: Dhinakaran Pandiyan> Cc: Rodrigo Vivi > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/intel_psr.c | 15 ++- > 2 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index e9fc1722c0fb..5a2364656aa5 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4132,6 +4132,7 @@ enum { > #define EDP_PSR2_CTL _MMIO(0x6f900) > #define EDP_PSR2_ENABLE(1<<31) > #define EDP_SU_TRACK_ENABLE(1<<30) > +#define EDP_AUX_FRAME_SYNC_ENABLE (1<<27) This shows reserved here for me. I believe we should just drop this patch. > #define EDP_Y_COORDINATE_VALID (1<<26) > #define EDP_Y_COORDINATE_ENABLE(1<<25) > #define EDP_MAX_SU_DISABLE_TIME(t) ((t)<<20) > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index d622e37894d4..7aab66b5bc91 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -418,6 +418,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) >* good enough. */ > val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; > val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE; > + val |= EDP_AUX_FRAME_SYNC_ENABLE; > > if (drm_dp_dpcd_readb(_dp->aux, > DP_SYNCHRONIZATION_LATENCY_IN_SINK, > @@ -715,11 +716,6 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > i915_reg_t psr_status; > u32 psr_status_mask; > > - if (dev_priv->psr.psr2_enabled) > - drm_dp_dpcd_writeb(_dp->aux, > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF, > - 0); > - > if (dev_priv->psr.psr2_enabled) { > psr_status = EDP_PSR2_STATUS; > psr_status_mask = EDP_PSR2_STATUS_STATE_MASK; > @@ -742,6 +738,11 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > 2000)) > DRM_ERROR("Timed out waiting for PSR Idle State\n"); > > + if (dev_priv->psr.psr2_enabled) > + drm_dp_dpcd_writeb(_dp->aux, > +DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF, > +0); > + > dev_priv->psr.active = false; > } else { > if (dev_priv->psr.psr2_enabled) > @@ -863,10 +864,6 @@ static void intel_psr_exit(struct drm_i915_private > *dev_priv) > return; > > if (HAS_DDI(dev_priv)) { > - if (dev_priv->psr.psr2_enabled) > - drm_dp_dpcd_writeb(_dp->aux, > - DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF, > - 0); > if (dev_priv->psr.psr2_enabled) { > val = I915_READ(EDP_PSR2_CTL); > WARN_ON(!(val & EDP_PSR2_ENABLE)); > -- > 2.16.2 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/6] drm/i915/psr: Nuke aux_frame_sync
On Wed, Mar 14, 2018 at 03:36:12PM -0700, José Roberto de Souza wrote: > PSR2 selective update requires aux frame sync(even though we don't > support it in i915) and do not makes sense active PSR2 to only do > full screen updates aka PSR1. > Having aux_frame_sync flag could cause it be set to true even when > the PSR1 is being used, see intel_psr2_config_valid(). > > Cc: Dhinakaran Pandiyan> Cc: Rodrigo Vivi > Signed-off-by: José Roberto de Souza Reviewed-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/i915_drv.h | 1 - > drivers/gpu/drm/i915/intel_psr.c | 10 +- > 2 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index e27ba8fb64e6..8a584273f897 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -602,7 +602,6 @@ struct i915_psr { > struct delayed_work work; > unsigned busy_frontbuffer_bits; > bool psr2_support; > - bool aux_frame_sync; > bool link_standby; > bool y_cord_support; > bool colorimetry_support; > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index 317cb4a12693..9811f5f0bc75 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -144,9 +144,9 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp) > DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, > _sync_cap) != 1) > frame_sync_cap = 0; > - dev_priv->psr.aux_frame_sync = frame_sync_cap & > DP_AUX_FRAME_SYNC_CAP; > + frame_sync_cap = (frame_sync_cap & DP_AUX_FRAME_SYNC_CAP); > /* PSR2 needs frame sync as well */ > - dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync; > + dev_priv->psr.psr2_support = frame_sync_cap; > DRM_DEBUG_KMS("PSR2 %s on sink", > dev_priv->psr.psr2_support ? "supported" : "not > supported"); > > @@ -269,7 +269,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp) > aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0); > > /* Enable AUX frame sync at sink */ > - if (dev_priv->psr.aux_frame_sync) > + if (dev_priv->psr.psr2_support) > drm_dp_dpcd_writeb(_dp->aux, > DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF, > DP_AUX_FRAME_SYNC_ENABLE); > @@ -714,7 +714,7 @@ static void hsw_psr_disable(struct intel_dp *intel_dp, > i915_reg_t psr_status; > u32 psr_status_mask; > > - if (dev_priv->psr.aux_frame_sync) > + if (dev_priv->psr.psr2_support) > drm_dp_dpcd_writeb(_dp->aux, > DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF, > 0); > @@ -862,7 +862,7 @@ static void intel_psr_exit(struct drm_i915_private > *dev_priv) > return; > > if (HAS_DDI(dev_priv)) { > - if (dev_priv->psr.aux_frame_sync) > + if (dev_priv->psr.psr2_support) > drm_dp_dpcd_writeb(_dp->aux, > DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF, > 0); > -- > 2.16.2 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915/psr: Enable Y-coordinate support in source
drm/i915/cnl: On Wed, Mar 14, 2018 at 03:36:14PM -0700, José Roberto de Souza wrote: > We are requiring that sink requires Y-coordinate but we are not > sending it in the main-link. Also add on CNL here > Even if hardware tracking isn't good enough it will not cause > any more issues enabling it. > > Cc: Dhinakaran Pandiyan> Cc: Rodrigo Vivi > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > drivers/gpu/drm/i915/intel_psr.c | 4 ++-- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index a15db41a208a..e9fc1722c0fb 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4132,6 +4132,8 @@ enum { > #define EDP_PSR2_CTL _MMIO(0x6f900) > #define EDP_PSR2_ENABLE(1<<31) > #define EDP_SU_TRACK_ENABLE(1<<30) > +#define EDP_Y_COORDINATE_VALID (1<<26) > +#define EDP_Y_COORDINATE_ENABLE(1<<25) probably good add CNL_ prefix on these bits... > #define EDP_MAX_SU_DISABLE_TIME(t) ((t)<<20) > #define EDP_MAX_SU_DISABLE_TIME_MASK (0x1f<<20) > #define EDP_PSR2_TP2_TIME_500 (0<<8) > diff --git a/drivers/gpu/drm/i915/intel_psr.c > b/drivers/gpu/drm/i915/intel_psr.c > index 62d97d5576d1..c9da1390a727 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -416,8 +416,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) > /* FIXME: selective update is probably totally broken because it doesn't >* mesh at all with our frontbuffer tracking. And the hw alone isn't >* good enough. */ > - val |= EDP_PSR2_ENABLE | > - EDP_SU_TRACK_ENABLE; > + val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE; > + val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE; if (INTEL_GEN(dev_priv) >= 10) val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE; since those bits were reserved before CNL. With those changes: Reviewed-by: Rodrigo Vivi > > if (drm_dp_dpcd_readb(_dp->aux, > DP_SYNCHRONIZATION_LATENCY_IN_SINK, > -- > 2.16.2 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] PSR lag fixes
On Wed, Mar 14, 2018 at 04:35:55PM -0700, Pandiyan, Dhinakaran wrote: > > > > On Wed, 2018-03-14 at 23:09 +0100, Hans de Goede wrote: > > Hi, > > > > On 14-03-18 21:49, Pandiyan, Dhinakaran wrote: > > > > > > On Wed, 2018-02-14 at 09:25 +0100, Hans de Goede wrote: > > >> Hi, > > >> > > >> On 12-02-18 18:42, Pandiyan, Dhinakaran wrote: > > >>> On Mon, 2018-02-12 at 09:45 +0100, Hans de Goede wrote: > > Hi, > > > > On 12-02-18 07:08, Dhinakaran Pandiyan wrote: > > > PSR currently when enabled results in semi-permanent freezes or > > > noticeable > > > cursor lags. > > > > > > https://patchwork.freedesktop.org/series/37598/ will fix long freezes > > > due > > > to frame counter resets. > > > > > > This series has three more fixes - > > > Patch 1 eliminates PSR exit for flips and makes us rely on the HW to > > > do it. > > > Patch 2 fixes cusor move lag by relying on HW to exit PSR. > > > Patch 3 fixes temporary freeze seen with fbdev. > > > > > > With both the series applied, PSR on my SKL ThinkPad feels pretty > > > good. > > > > Thank you for your great work on this. > > > > Are there any more PSR fixes in the pipeline? > > >>> > > >>> Yeah, there are a few more fixes that I hope will appear on the list in > > >>> the next two weeks or so. > > >> > > >> Ok, can you send a mail when you're done (in sofar any software is ever > > >> "done") and you would like me to ask all people who have been kind enough > > >> to test PSR to retest ? > > >> > > > > > > Hi Hans, > > > > > > Thanks for your patience and help. I believe the current drm-tip is in a > > > decent shape to retest PSR. Booting with i915.enable_psr = 1 is still > > > needed. The fixes have been mostly developed/tested on gen-9 hardware > > > but they apply to other platforms too. > > > > Cool, thank you. Current drm-tip will all be merged into 4.17-rc1, right? Unfortunately no. All changes current in dinq and not on drm-intel-next are only going to 4.18-rc1. > > Rodrigo, > > Can you help me answer that? > > > > > > Then I think I will just wait for that, most distros already provide rc > > builds > > for testing, so that way it will be easy for people to test. > > > > Regards, > > > > Hans > > > > > > > > > > > > > > -DK > > > > > If not I think I should do > > a custom Fedora kernel build based on 4.15 + recent fixes and ask all > > my > > testers to retest with that. > > > > I do have some questions before I do this: > > > > 1) I believe that only testers with skylake (normal or LP) or newer > > should > > re-test, correct? > > >>> > > >>> These fixes do apply for HSW/BDW, so essentially all the big cores > > >>> supporting PSR. But, HSW/BDW need fixes for AUX channel-PSR interaction > > >>> also. I haven't looked into CHV/VLV. > > >>> > > > > 2) I know there are 2 series (including this one), can someone provide > > a link > > to the latest patchwork version of those 2 series, or even better a git > > branch with 4.15 + those patches? Any patches I'm missing if I pick up > > these > > 2 series? > > >>> > > >>> https://patchwork.freedesktop.org/series/37598/ > > >>> https://patchwork.freedesktop.org/series/38067/ > > >>> > > >>> > > 3) I'm thinking 4.15 atm, but I could also do a 4.16-rc1 test kernel > > instead > > if that would be better, would that be better ? > > >>> > > >>> I can't think of any diff that would affect PSR, but the latest is > > >>> better I suppose. > > >> > > >> Ok. > > >> > > >> Regards, > > >> > > >> Hans > > >> ___ > > >> 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't initialize plane_to_crtc_mapping[] on SKL+
On Mon, 2018-03-05 at 19:41 +0200, Ville Syrjala wrote: > From: Ville Syrjälä> > We don't use the enum i9xx_plane_id namespace on SKL+ anymore, so > do not initialize the related plane_to_crtc_mapping[] table either. > > Actually the only remaining user of that table is the pre-g4x > watermark code, but no harm in initializing the table on all > pre-SKL platforms. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 15 +++ > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index fb08590b4d40..435462bfc719 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13572,10 +13572,17 @@ static int intel_crtc_init(struct drm_i915_private > *dev_priv, enum pipe pipe) > /* initialize shared scalers */ > intel_crtc_init_scalers(intel_crtc, crtc_state); > > - BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) || > -dev_priv->plane_to_crtc_mapping[primary->i9xx_plane] != NULL); > - dev_priv->plane_to_crtc_mapping[primary->i9xx_plane] = intel_crtc; > - dev_priv->pipe_to_crtc_mapping[intel_crtc->pipe] = intel_crtc; > + BUG_ON(pipe >= ARRAY_SIZE(dev_priv->pipe_to_crtc_mapping) || > +dev_priv->pipe_to_crtc_mapping[pipe] != NULL); > + dev_priv->pipe_to_crtc_mapping[pipe] = intel_crtc; > + > + if (INTEL_GEN(dev_priv) < 9) { > + enum i9xx_plane_id i9xx_plane = primary->i9xx_plane; > + > + BUG_ON(i9xx_plane >= > ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) || > +dev_priv->plane_to_crtc_mapping[i9xx_plane] != NULL); > + dev_priv->plane_to_crtc_mapping[i9xx_plane] = intel_crtc; Verified that plane_to_crtc_mapping[i9xx_plane] is used only in pre-gen9 code. Patch looks harmless Reviewed-by: Dhinakaran Pandiyan Should intel_get_crtc_for_plane be renamed to i9xx_get_crtc_for_plane()? and the table as i9xx_plane_to_crtc_mapping[]? > + } > > drm_crtc_helper_add(_crtc->base, _helper_funcs); > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave, Sorry for the last minute and for sending 2 pull requests in a short time, but we just got a pull request from GVT. It passes our CI-fast-feedback and the full run is still running. Please if we still have time please consider pulling it, otherwise this will be part of next regular one. Here goes drm-intel-fixes-2018-03-15: Only GVT fixes: - Two warnings fix for runtime pm and usr copy (Xiong, Zhenyu) - OA context fix for vGPU profiling (Min) - privilege batch buffer reloc fix (Fred) Thanks, Rodrigo. The following changes since commit 0c8efd610b58cb23cefdfa12015799079aef94ae: Linux 4.16-rc5 (2018-03-11 17:25:09 -0700) are available in the git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2018-03-15 for you to fetch changes up to 05b429a8eed83084a8a7fc04c97120a5ba07b5be: Merge tag 'gvt-fixes-2018-03-15' of https://github.com/intel/gvt-linux into drm-intel-fixes (2018-03-15 15:37:57 -0700) Only GVT fixes: - Two warnings fix for runtime pm and usr copy (Xiong, Zhenyu) - OA context fix for vGPU profiling (Min) - privilege batch buffer reloc fix (Fred) Chris Wilson (2): drm/i915: Only prune fences after wait-for-all drm/i915: Kick the rps worker when changing the boost frequency Min He (1): drm/i915/gvt: keep oa config in shadow ctx Mustamin B Mustaffa (1): drm/i915: Enable VBT based BL control for DP Rodrigo Vivi (1): Merge tag 'gvt-fixes-2018-03-15' of https://github.com/intel/gvt-linux into drm-intel-fixes Xiong Zhang (1): drm/i915/gvt: Add runtime_pm_get/put into gvt_switch_mmio Zhenyu Wang (1): drm/i915/gvt: fix user copy warning by whitelist workload rb_tail field fred gao (1): drm/i915/gvt: Correct the privilege shadow batch buffer address drivers/gpu/drm/i915/gvt/cmd_parser.c | 8 drivers/gpu/drm/i915/gvt/mmio_context.c | 2 + drivers/gpu/drm/i915/gvt/scheduler.c| 71 +++-- drivers/gpu/drm/i915/gvt/scheduler.h| 5 +++ drivers/gpu/drm/i915/i915_gem.c | 16 ++-- drivers/gpu/drm/i915/i915_sysfs.c | 10 - drivers/gpu/drm/i915/intel_dp.c | 10 ++--- 7 files changed, 105 insertions(+), 17 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/6] Revert "drm: Use a flexible array member for blob property data" (rev4)
== Series Details == Series: series starting with [1/6] Revert "drm: Use a flexible array member for blob property data" (rev4) URL : https://patchwork.freedesktop.org/series/38886/ State : failure == Summary == Possible new issues: Test kms_cursor_crc: Subgroup cursor-128x128-suspend: pass -> FAIL (shard-apl) Known issues: Test kms_frontbuffer_tracking: Subgroup fbc-1p-primscrn-cur-indfb-draw-mmap-gtt: pass -> FAIL (shard-apl) fdo#101623 Test kms_vblank: Subgroup pipe-b-ts-continuation-suspend: pass -> SKIP (shard-snb) fdo#105411 fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623 fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411 shard-apltotal:3442 pass:1812 dwarn:1 dfail:0 fail:9 skip:1619 time:12935s shard-hswtotal:3442 pass:1768 dwarn:1 dfail:0 fail:1 skip:1671 time:11957s shard-snbtotal:3442 pass:1355 dwarn:1 dfail:0 fail:3 skip:2083 time:7154s Blacklisted hosts: shard-kbltotal:2091 pass:1139 dwarn:17 dfail:1 fail:4 skip:929 time:5806s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8371/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] sna/uxa: Fix colormap handling at screen depth 30.
Oops, didn't reply yet, sorry! On Thu, Mar 15, 2018 at 5:14 PM, Chris Wilsonwrote: > Quoting Ville Syrjälä (2018-03-15 16:02:42) >> On Thu, Mar 15, 2018 at 03:28:18PM +, Chris Wilson wrote: >> > Quoting Ville Syrjälä (2018-03-01 11:12:53) >> > > On Thu, Mar 01, 2018 at 02:20:48AM +0100, Mario Kleiner wrote: >> > > > The various clut handling functions like a setup >> > > > consistent with the x-screen color depth. Otherwise >> > > > we observe improper sampling in the gamma tables >> > > > at depth 30. >> > > > >> > > > Therefore replace hard-coded bitsPerRGB = 8 by actual >> > > > bits per channel scrn->rgbBits. Also use this for call >> > > > to xf86HandleColormaps(). >> > > > >> > > > Tested for uxa and sna at depths 8, 16, 24 and 30 on >> > > > IvyBridge, and tested at depth 24 and 30 that xgamma >> > > > and gamma table animations work, and with measurement >> > > > equipment to make sure identity gamma ramps actually >> > > > are identity mappings at the output. >> > > >> > > You mean identity mapping at 8bpc? We don't support higher precision >> > > gamma on pre-bdw atm, and the ddx doesn't use the higher precision >> > > stuff even on bdw+. I'm working on fixing both, but it turned out to >> > > be a bit more work than I anticipated so will take a while. >> > > >> > > > >> > > > Signed-off-by: Mario Kleiner >> > > > --- >> > > > src/sna/sna_driver.c | 5 +++-- >> > > > src/uxa/intel_driver.c | 3 ++- >> > > > 2 files changed, 5 insertions(+), 3 deletions(-) >> > > > >> > > > diff --git a/src/sna/sna_driver.c b/src/sna/sna_driver.c >> > > > index 2643e6c..9c4bcd4 100644 >> > > > --- a/src/sna/sna_driver.c >> > > > +++ b/src/sna/sna_driver.c >> > > > @@ -1145,7 +1145,7 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL) >> > > > if (!miInitVisuals(, , , , >> > > > , >> > > > , >> > > > ((unsigned long)1 << (scrn->bitsPerPixel - >> > > > 1)), >> > > > -8, -1)) >> > > > +scrn->rgbBits, -1)) >> > > > return FALSE; >> > > > >> > > > if (!miScreenInit(screen, NULL, >> > > > @@ -1217,7 +1217,8 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL) >> > > > return FALSE; >> > > > >> > > > if (sna->mode.num_real_crtc && >> > > > - !xf86HandleColormaps(screen, 256, 8, sna_load_palette, NULL, >> > > > + !xf86HandleColormaps(screen, 1 << scrn->rgbBits, >> > > > scrn->rgbBits, >> > > > + sna_load_palette, NULL, >> > > >CMAP_RELOAD_ON_MODE_SWITCH | >> > > >CMAP_PALETTED_TRUECOLOR)) >> > > >> > > I already forgot what this does prior to your randr fix. IIRC bumping >> > > the 8 alone would cause the thing to segfault, but I guess bumping both >> > > was fine? >> > > We always need this fix for X-Screen depth 30, even on older servers. With the current maxColors=256, bitsPerPixel=8 setting and color depth 30, the server does some out of bounds reads in its gamma handling code for maxColors=256, and we get crash at server startup. It's a bit a matter of luck to reproduce. I had it running for months without problems, then after some Ubuntu system upgrade i got crashes at server startup under sna and at server shutdown under uxa. Without raising the bitsPerPixel to 10, we get some bottleneck in the way the server mushes together the old XF86VidMode gamma ramps (per-x-screen) and the new RandR per-crtc gamma ramps, so there are artifacts in the gamma table finally uploaded to the hw. For DefaultDeph=24 this patch doesn't change anything. On X-Server < 1.20 however, without my fix, at color depth 30 this will get us stuck on a identity gamma ramp, as the update code in the server effectively no-ops. Or so i think, because i tested so many permutations of so many things on intel,amd,nouveau with different mesa,server,ddx branches lately that i may misremember something. Ilia reported some odd behavior on 1.19 with the corresponding fix to nouveau-ddx... I'll give it another try in testing. I actually want to get that fix cherry-picked into server 1.19.7, so depth 30 would be usable on the 1.19 series. >> > > Hmm. So the server always initializes crtc->gamma_size to 256 >> > > (which does match the normal hw LUT size), and so before your >> > > fix we will always get gamma_slots==0 at >8bpc and so the hw LUT >> > > is never actually updated? >> > Yes. The kernel kms reports true hw gamma size, but no ddx ever made use of that info on any driver, so changing it to anything but 256 on the kernel side would break all existing userspace drivers, at least for the old gammaset ioctl. >> > Was there any conclusion to this? Aiui, these bits should be set based >> > on the underlying HW gamma table depth which is not the same as the >> > screen colour depth. It stuck at 256 for ages as that is all anyone >> > ever expected... >>
Re: [Intel-gfx] [PULL] more gvt-fixes for 4.16
On Thu, Mar 15, 2018 at 06:00:23PM +0800, Zhenyu Wang wrote: > > Hi, > > Here's more gvt-fixes for final 4.16. Sorry it's a little late, > as Zhi had two days off this week and more regression tests > have been done for this. pulled to drm-intel-fixes and leaving CI run on it right now... I hope to send it soon... or next week... > > Thanks. > > -- > The following changes since commit 88d3dfb6a69042381161290c7ce19e1f53fc2a66: > > drm/i915: Suspend submission tasklets around wedging (2018-03-05 16:08:31 > -0800) > > are available in the Git repository at: > > https://github.com/intel/gvt-linux.git tags/gvt-fixes-2018-03-15 > > for you to fetch changes up to 850555d1d31e45fc3e9a2982f81717387e8d5e1b: > > drm/i915/gvt: fix user copy warning by whitelist workload rb_tail field > (2018-03-15 15:07:22 +0800) > > > gvt-fixes-2018-03-15 > > - Two warnings fix for runtime pm and usr copy (Xiong, Zhenyu) > - OA context fix for vGPU profiling (Min) > - privilege batch buffer reloc fix (Fred) > > > Min He (1): > drm/i915/gvt: keep oa config in shadow ctx > > Xiong Zhang (1): > drm/i915/gvt: Add runtime_pm_get/put into gvt_switch_mmio > > Zhenyu Wang (1): > drm/i915/gvt: fix user copy warning by whitelist workload rb_tail field > > fred gao (1): > drm/i915/gvt: Correct the privilege shadow batch buffer address > > drivers/gpu/drm/i915/gvt/cmd_parser.c | 8 > drivers/gpu/drm/i915/gvt/mmio_context.c | 2 + > drivers/gpu/drm/i915/gvt/scheduler.c| 71 > +++-- > drivers/gpu/drm/i915/gvt/scheduler.h| 5 +++ > 4 files changed, 82 insertions(+), 4 deletions(-) > > > -- > Open Source Technology Center, Intel ltd. > > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Use correct reST syntax for WOPCM and GuC kernel-doc diagrams (rev3)
== Series Details == Series: drm/i915: Use correct reST syntax for WOPCM and GuC kernel-doc diagrams (rev3) URL : https://patchwork.freedesktop.org/series/39979/ State : success == Summary == Known issues: Test kms_cursor_crc: Subgroup cursor-256x256-suspend: skip -> PASS (shard-snb) fdo#103375 Test kms_flip: Subgroup 2x-flip-vs-expired-vblank: pass -> FAIL (shard-hsw) fdo#102887 Subgroup 2x-plain-flip-ts-check: pass -> FAIL (shard-hsw) fdo#100368 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 shard-apltotal:3442 pass:1814 dwarn:1 dfail:0 fail:7 skip:1619 time:13064s shard-hswtotal:3442 pass:1766 dwarn:1 dfail:0 fail:3 skip:1671 time:11848s shard-snbtotal:3442 pass:1357 dwarn:1 dfail:0 fail:3 skip:2081 time:7250s Blacklisted hosts: shard-kbltotal:2040 pass:1140 dwarn:0 dfail:0 fail:5 skip:892 time:5410s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8369/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Guc and HuC for Cannonlake (rev2)
== Series Details == Series: Guc and HuC for Cannonlake (rev2) URL : https://patchwork.freedesktop.org/series/40064/ State : success == Summary == Series 40064v2 Guc and HuC for Cannonlake https://patchwork.freedesktop.org/api/1.0/series/40064/revisions/2/mbox/ Known issues: Test debugfs_test: Subgroup read_all_entries: incomplete -> PASS (fi-snb-2520m) fdo#103713 Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> FAIL (fi-skl-6770hq) fdo#100368 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:433s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:438s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:382s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:539s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:298s fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:516s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:512s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:517s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:505s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:411s fi-cfl-s2total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:579s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:511s fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:515s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:585s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:425s fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:319s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:536s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:404s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:417s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:482s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:429s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:474s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:465s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:514s fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:657s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:446s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:542s fi-skl-6700hqtotal:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:541s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:509s fi-skl-6770hqtotal:285 pass:264 dwarn:0 dfail:0 fail:1 skip:20 time:496s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:429s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:444s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:576s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:405s 3b4800f0237f9422817a1a9695d742598d2fb868 drm-tip: 2018y-03m-15d-15h-48m-56s UTC integration manifest b348c080f72d linux-firmware/huc/cnl: Load HuC on Cannonlake 46df09841213 linux-firmware/guc/cnl: Load GuC on Cannonlake == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8372/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4
From: Matt AtwoodDP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheeme from 8 bits to 7 in DPCD 0x000e. The 8th bit is used to identify extended receiver capabilities. For panels that use this new feature wait interval would be increased by 512 ms, when spec is max 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address eh. With the introduction of DP 1.4 spec main link clock recovery was standardized to 100 us regardless of TRAINING_AUX_RD_INTERVAL value. To avoid breaking panels that are not spec compiant we now warn on invalid values. V2: commit title/message, masking all 7 bits, warn on out of spec values. V3: commit message, make link train clock recovery follow DP 1.4 spec. V4: style changes V5: typo V6: print statement revisions, DP_REV to DPCD_REV, comment correction V7: typo V8: Style Signed-off-by: Matt Atwood --- drivers/gpu/drm/drm_dp_helper.c | 22 ++ include/drm/drm_dp_helper.h | 6 ++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..6bee2df 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -119,18 +119,32 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SI EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis); void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) + int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & + DP_TRAINING_AUX_RD_MASK; + + if (rd_interval > 4) + DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n", + rd_interval); + + if (rd_interval == 0 || dpcd[DP_DPCD_REV] >= DPCD_REV_14) udelay(100); else - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); + mdelay(rd_interval * 4); } EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { - if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) + int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & + DP_TRAINING_AUX_RD_MASK; + + if (rd_interval > 4) + DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n", + rd_interval); + + if (rd_interval == 0) udelay(400); else - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); + mdelay(rd_interval * 4); } EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay); diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index da58a42..8c59ce4 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -64,6 +64,11 @@ /* AUX CH addresses */ /* DPCD */ #define DP_DPCD_REV 0x000 +# define DPCD_REV_100x10 +# define DPCD_REV_110x11 +# define DPCD_REV_120x12 +# define DPCD_REV_130x13 +# define DPCD_REV_140x14 #define DP_MAX_LINK_RATE0x001 @@ -118,6 +123,7 @@ # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher */ #define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */ +# define DP_TRAINING_AUX_RD_MASK0x7F/* XXX 1.2? */ #define DP_ADAPTER_CAP 0x00f /* 1.2 */ # define DP_FORCE_LOAD_SENSE_CAP (1 << 0) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] linux-firmware/guc/cnl: Load GuC on Cannonlake
GuC is now available for Cannonlake. Load GuC v11.102 on Cannonlake. Cc: Tomi SarvelaCc: Jani Saarinen Cc: Rodrigo vivi Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/intel_guc_fw.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c index 978668c..e03a1ec 100644 --- a/drivers/gpu/drm/i915/intel_guc_fw.c +++ b/drivers/gpu/drm/i915/intel_guc_fw.c @@ -39,6 +39,9 @@ #define KBL_FW_MAJOR 9 #define KBL_FW_MINOR 39 +#define CNL_FW_MAJOR 11 +#define CNL_FW_MINOR 102 + #define GUC_FW_PATH(platform, major, minor) \ "i915/" __stringify(platform) "_guc_ver" __stringify(major) "_" __stringify(minor) ".bin" @@ -51,6 +54,9 @@ MODULE_FIRMWARE(I915_BXT_GUC_UCODE); #define I915_KBL_GUC_UCODE GUC_FW_PATH(kbl, KBL_FW_MAJOR, KBL_FW_MINOR) MODULE_FIRMWARE(I915_KBL_GUC_UCODE); +#define I915_CNL_GUC_UCODE GUC_FW_PATH(cnl, CNL_FW_MAJOR, CNL_FW_MINOR) +MODULE_FIRMWARE(I915_CNL_GUC_UCODE); + static void guc_fw_select(struct intel_uc_fw *guc_fw) { struct intel_guc *guc = container_of(guc_fw, struct intel_guc, fw); @@ -77,6 +83,10 @@ static void guc_fw_select(struct intel_uc_fw *guc_fw) guc_fw->path = I915_KBL_GUC_UCODE; guc_fw->major_ver_wanted = KBL_FW_MAJOR; guc_fw->minor_ver_wanted = KBL_FW_MINOR; + } else if (IS_CANNONLAKE(dev_priv)) { + guc_fw->path = I915_CNL_GUC_UCODE; + guc_fw->major_ver_wanted = CNL_FW_MAJOR; + guc_fw->minor_ver_wanted = CNL_FW_MINOR; } else { DRM_WARN("%s: No firmware known for this platform!\n", intel_uc_fw_type_repr(guc_fw->type)); -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] linux-firmware/huc/cnl: Load HuC on Cannonlake
Huc is available now for cannonlake. Load v9.01.2678 on Cannonlake Cc: Tomi SarvelaCc: Jani Saarinen Cc: Rodrigo Vivi Signed-off-by: Anusha Srivatsa --- drivers/gpu/drm/i915/intel_huc_fw.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c b/drivers/gpu/drm/i915/intel_huc_fw.c index bb0f8b7..57e8816 100644 --- a/drivers/gpu/drm/i915/intel_huc_fw.c +++ b/drivers/gpu/drm/i915/intel_huc_fw.c @@ -34,6 +34,10 @@ #define KBL_HUC_FW_MINOR 00 #define KBL_BLD_NUM 1810 +#define CNL_HUC_FW_MAJOR 9 +#define CNL_HUC_FW_MINOR 01 +#define CNL_BLD_NUM 2678 + #define HUC_FW_PATH(platform, major, minor, bld_num) \ "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \ __stringify(minor) "_" __stringify(bld_num) ".bin" @@ -50,6 +54,10 @@ MODULE_FIRMWARE(I915_BXT_HUC_UCODE); KBL_HUC_FW_MINOR, KBL_BLD_NUM) MODULE_FIRMWARE(I915_KBL_HUC_UCODE); +#define I915_CNL_HUC_UCODE HUC_FW_PATH(cnl, CNL_HUC_FW_MAJOR, \ + CNL_HUC_FW_MINOR, CNL_BLD_NUM) +MODULE_FIRMWARE(I915_CNL_HUC_UCODE); + static void huc_fw_select(struct intel_uc_fw *huc_fw) { struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw); @@ -76,6 +84,10 @@ static void huc_fw_select(struct intel_uc_fw *huc_fw) huc_fw->path = I915_KBL_HUC_UCODE; huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR; huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR; + } else if (IS_CANNONLAKE(dev_priv)) { + huc_fw->path = I915_CNL_HUC_UCODE; + huc_fw->major_ver_wanted = CNL_HUC_FW_MAJOR; + huc_fw->minor_ver_wanted = CNL_HUC_FW_MINOR; } else { DRM_WARN("%s: No firmware known for this platform!\n", intel_uc_fw_type_repr(huc_fw->type)); -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/2] Guc and HuC for Cannonlake
Adding the Pull request: The following changes since commit 4c0bf113a55975d702673e57c5542f150807ad66: linux-firmware: intel: Update Kabylake audio firmware (2018-03-14 16:23:08 +0530) are available in the git repository at: ssh://git.freedesktop.org/git/drm/drm-firmware master for you to fetch changes up to 2c66d104c2c3d18260539ae68a72e439dea2df20: linux-firmware/i915: HuC firmware for Cannonlake v9.01.2678 (2018-03-14 16:28:37 -0700) Anusha Srivatsa (2): linux-firmware/i915: GuC firmware for Cannonlake v11.102 linux-firmware/i915: HuC firmware for Cannonlake v9.01.2678 WHENCE| 7 +++ i915/cnl_guc_ver11_102.bin| Bin 0 -> 215936 bytes i915/cnl_huc_ver9_01_2678.bin | Bin 0 -> 430336 bytes 3 files changed, 7 insertions(+) create mode 100644 i915/cnl_guc_ver11_102.bin create mode 100644 i915/cnl_huc_ver9_01_2678.bin Anusha Srivatsa (2): linux-firmware/guc/cnl: Load GuC on Cannonlake linux-firmware/huc/cnl: Load HuC on Cannonlake drivers/gpu/drm/i915/intel_guc_fw.c | 10 ++ drivers/gpu/drm/i915/intel_huc_fw.c | 12 2 files changed, 22 insertions(+) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/guc: Fix build break on config without DEBUG_FS
== Series Details == Series: drm/i915/guc: Fix build break on config without DEBUG_FS URL : https://patchwork.freedesktop.org/series/40041/ State : success == Summary == Known issues: Test gem_eio: Subgroup in-flight-external: incomplete -> PASS (shard-apl) fdo#105341 fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341 shard-apltotal:3407 pass:1803 dwarn:1 dfail:0 fail:7 skip:1595 time:12660s shard-hswtotal:3442 pass:1768 dwarn:1 dfail:0 fail:1 skip:1671 time:11876s shard-snbtotal:3442 pass:1358 dwarn:1 dfail:0 fail:2 skip:2081 time:7176s Blacklisted hosts: shard-kbltotal:2097 pass:1127 dwarn:52 dfail:0 fail:8 skip:907 time:5186s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8368/shards.html ___ 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 [1/6] Revert "drm: Use a flexible array member for blob property data" (rev4)
== Series Details == Series: series starting with [1/6] Revert "drm: Use a flexible array member for blob property data" (rev4) URL : https://patchwork.freedesktop.org/series/38886/ State : success == Summary == Series 38886v4 series starting with [1/6] Revert "drm: Use a flexible array member for blob property data" https://patchwork.freedesktop.org/api/1.0/series/38886/revisions/4/mbox/ Known issues: Test debugfs_test: Subgroup read_all_entries: incomplete -> PASS (fi-snb-2520m) fdo#103713 Test gem_mmap_gtt: Subgroup basic-small-bo-tiledx: pass -> FAIL (fi-gdg-551) fdo#102575 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:430s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:447s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:379s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:539s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:296s fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:513s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:513s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:526s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:503s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:410s fi-cfl-s2total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:579s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:509s fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:522s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:589s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:427s fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:316s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:536s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:402s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:424s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:477s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:428s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:475s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:472s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:511s fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:663s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:444s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:531s fi-skl-6700hqtotal:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:542s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:504s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:489s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:426s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:447s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:572s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:398s 3b4800f0237f9422817a1a9695d742598d2fb868 drm-tip: 2018y-03m-15d-15h-48m-56s UTC integration manifest 68f3ee590850 drm/i915: Use drm_color_lut_size() 1fa90c007ea3 drm/i915: Remove the blob->data casts 398a140ba5c6 drm: Introduce drm_color_lut_size() d7495261fd50 drm: Verify gamma/degamma LUT size c232fc1aed2d drm: Remove now pointelss blob->data casts 9acadb8402c7 Revert "drm: Use a flexible array member for blob property data" == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8371/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Trace GEM steps between submit and wedging (rev3)
== Series Details == Series: series starting with [1/2] drm/i915: Trace GEM steps between submit and wedging (rev3) URL : https://patchwork.freedesktop.org/series/40029/ State : success == Summary == Known issues: Test gem_eio: Subgroup in-flight-contexts: incomplete -> PASS (shard-apl) fdo#105341 +1 Test kms_flip: Subgroup 2x-plain-flip-ts-check-interruptible: pass -> FAIL (shard-hsw) fdo#100368 Subgroup flip-vs-panning-vs-hang: pass -> DMESG-WARN (shard-snb) fdo#103821 Test kms_sysfs_edid_timing: pass -> WARN (shard-apl) fdo#100047 fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103821 https://bugs.freedesktop.org/show_bug.cgi?id=103821 fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047 shard-apltotal:3442 pass:1814 dwarn:1 dfail:0 fail:7 skip:1619 time:12933s shard-hswtotal:3442 pass:1767 dwarn:1 dfail:0 fail:2 skip:1671 time:11820s shard-snbtotal:3442 pass:1357 dwarn:2 dfail:0 fail:2 skip:2081 time:7237s Blacklisted hosts: shard-kbltotal:2141 pass:1185 dwarn:26 dfail:0 fail:8 skip:921 time:5833s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8366/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm: Make sure at least one plane supports the fb format
On Thu, Mar 15, 2018 at 08:03:44PM +0200, Ville Syrjälä wrote: > On Thu, Mar 15, 2018 at 07:48:02PM +0200, Ville Syrjälä wrote: > > On Thu, Mar 15, 2018 at 10:42:17AM -0700, Eric Anholt wrote: > > > Ville Syrjalawrites: > > > > > > > From: Ville Syrjälä > > > > > > > > To make life easier for drivers, let's have the core check that the > > > > requested pixel format is supported by at least one plane when creating > > > > a new framebuffer. > > > > > > > > This eases the burden on drivers by making sure they'll never get > > > > requests to create fbs with unsupported pixel formats. Thanks to the > > > > new .fb_modifier() hook this check can now be done whether the request > > > > specifies the modifier directly or driver has to deduce it from the > > > > gem bo tiling (or via any other method really). > > > > > > > > v0: Accept anyformat if the driver doesn't do planes (Eric) > > > > s/planes_have_format/any_plane_has_format/ (Eric) > > > > Check the modifier as well since we already have a function > > > > that does both > > > > v3: Don't do the check in the core since we may not know the > > > > modifier yet, instead export the function and let drivers > > > > call it themselves > > > > v4: Unexport the functiona and put the format_default check back > > > > since this will again be called by the core, ie. undo v3 ;) > > > > > > > > Cc: Eric Anholt > > > > Testcase: igt/kms_addfb_basic/expected-formats > > > > Signed-off-by: Ville Syrjälä > > > > --- > > > > drivers/gpu/drm/drm_framebuffer.c | 30 ++ > > > > 1 file changed, 30 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/drm_framebuffer.c > > > > b/drivers/gpu/drm/drm_framebuffer.c > > > > index 21d3d51eb261..e618a6b728d4 100644 > > > > --- a/drivers/gpu/drm/drm_framebuffer.c > > > > +++ b/drivers/gpu/drm/drm_framebuffer.c > > > > @@ -152,6 +152,26 @@ static int fb_plane_height(int height, > > > > return DIV_ROUND_UP(height, format->vsub); > > > > } > > > > > > > > +static bool any_plane_has_format(struct drm_device *dev, > > > > +u32 format, u64 modifier) > > > > +{ > > > > + struct drm_plane *plane; > > > > + > > > > + drm_for_each_plane(plane, dev) { > > > > + /* > > > > +* In case the driver doesn't really do > > > > +* planes we have to accept any format here. > > > > +*/ > > > > + if (plane->format_default) > > > > + return true; > > > > + > > > > + if (drm_plane_check_pixel_format(plane, format, > > > > modifier) == 0) > > > > + return true; > > > > + } > > > > + > > > > + return false; > > > > +} > > > > > > This drm_plane_check_pixel_format() will always fail for VC4's SAND > > > modifiers or VC5's UIF modifiers, where we're using the middle 48 bits > > > as a bit of metadata (like how we have horizontal stride passed outside > > > of the modifier) and you can't list all of the possible values in an > > > array on the plane. > > > > Hmm. drm_atomic_plane_check() etc. call this thing as well. How does > > that manage to work currently? > > Maybe it doesn't. I added the modifier checks in > > commit 23163a7d4b032489d375099d56571371c0456980 > Author: Ville Syrjälä > AuthorDate: Fri Dec 22 21:22:30 2017 +0200 > Commit: Ville Syrjälä > CommitDate: Mon Feb 26 16:29:47 2018 +0200 > > drm: Check that the plane supports the request format+modifier combo > > Maybe that broke vc4? > > Hmm. So either we need to stop checking against the modifiers array and > rely purely or .format_mod_supported(), or we need to somehow get the > driver to reduce the modifier to its base form. I guess we could make > .fb_modifier() do that and call it also for addfb with modifiers. And > I'd need to undo some of the modifier[0] vs. deduced modifier changes > I did to framebuffer_check(), and we'd need to preserve the original > modifier in the request for .fb_create(). Oh, but that wouldn't allow > returning a non-base modifier from .fb_modifuer() for the !modifiers > case. This is turning slightly more tricky than I had hoped... > > I guess relying on .format_mod_supported() might be what we need to > do. Unfortunately it does mean that the .format_mod_supported() > implementations must be prepared for modifiers that were not > registered with the plane. Which does feel quite a bit more > fragile. And that last apporach would be annoying for i915. So I think the other apporach is better. Fortunately it seems your SAND modifiers aren't upstream yet so I didn't break them :) I had a quick look at your github and it looks like the first apporach would work. Something like this is what I had in mind. Loosk
Re: [Intel-gfx] [maintainer-tools PATCH] doc: how to become a drm-intel committer
On Thu, Mar 15, 2018 at 4:32 PM, Jani Nikulawrote: > On Thu, 15 Mar 2018, Daniel Vetter wrote: >> On Wed, Mar 14, 2018 at 05:11:02PM +0200, Jani Nikula wrote: >>> Until now, the drm-intel commit access have been handed out ad hoc, >>> without transparency, consistency, or fairness. With pressure to add >>> more committers, this is no longer tenable, if it ever was. Document the >>> requirements and expectations around becoming a drm-intel committer. >>> >>> The Linux kernel operates in a model where, by and large, only >>> maintainers commit patches. Maintainer teams are no longer rare, but the >>> drm-intel and drm-misc maintainer/committer model is definitely an >>> outlier. >>> >>> The drm-intel maintainers believe that a reasonable level of experience >>> and track record of working on the driver, as well as actively engaging >>> in the community upstream, are necessary before becoming a >>> committer. While the requirements outlined here may seem strict in >>> contrast with many projects, they are extremely liberal by kernel >>> standards. >>> >>> Finally, no rules are carved in stone. We fully expect the requirements >>> to be adjusted later. However, it will be much easier to start strict >>> and relax the requirements later than the other way round. >>> >>> Cc: Gustavo Padovan >>> Cc: Maarten Lankhorst >>> Cc: Sean Paul >>> Cc: Dave Airlie >>> Cc: dim-to...@lists.freedesktop.org >>> Cc: dri-de...@lists.freedesktop.org >>> Cc: intel-gfx@lists.freedesktop.org >>> Signed-off-by: Jani Nikula >>> Signed-off-by: Joonas Lahtinen >>> Signed-off-by: Rodrigo Vivi >> >> I've chatted for a few hours with Joonas, and I think before we can >> discuss the proposed document here itself, we first need to reach some >> agreement on why we even have commit rights. I think there's a very wide >> range of answers to that questions, and of course with different goals you >> end up with completely different rules about how to handle commit rights. >> >> Joonas suggested we first discuss this internally, perhaps with the >> maintainers, Kimmo and me. > > Fine. I would rather have discussed this transparently out in the > open. That was, after all, the purpose of sending this out. This was more or less Joonas' requests after our long discussion (he did take a quick look at my reply before I sent it out). We can also have the discussion here, I do have the draft still lying around somewhere. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove hole and padding from intel_shared_dpll
On Thu, Mar 15, 2018 at 11:07:04AM -0700, Lucas De Marchi wrote: > On Thu, Mar 15, 2018 at 11:54:19AM +0200, Ville Syrjälä wrote: > > > We can even (or alternatively) make dpll_info part of intel_shared_dpll. > > > > You mean something like? > > > > struct intel_shared_dpll { > > ... > > - id; > > - name; > > - flags; > > + const struct dpll_info *info; > > ... > > }; > > yep, that. > > > That would make sense to me since the info seems to be all read-only > > data. Oh and then we wouldn't even need the extra 'funcs' pointer. > > Some extra indirection there but this isn't performance sensitive or > > anything. > > > > Even if it wouldn't make things smaller I'd still like it just for > > the clarity of having all the read-only data being const. > > > > > > > > Below is the diff to make funcs a pointer on top of previous patch. > > > > This one is > > Reviewed-by: Ville Syrjälä> > Humn... do you mean the initial patch or the diff below? The diff. > If I'm going to > embed dpll_info inside intel_shared_dpll, this patch would be pointless. Yeah. But I gave the r-b anyway in case you were going to stop here. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/6] Revert "drm: Use a flexible array member for blob property data" (rev4)
== Series Details == Series: series starting with [1/6] Revert "drm: Use a flexible array member for blob property data" (rev4) URL : https://patchwork.freedesktop.org/series/38886/ State : failure == Summary == Series 38886v4 series starting with [1/6] Revert "drm: Use a flexible array member for blob property data" https://patchwork.freedesktop.org/api/1.0/series/38886/revisions/4/mbox/ Possible new issues: Test drv_module_reload: Subgroup basic-reload: pass -> DMESG-WARN (fi-bdw-gvtdvm) Test gem_exec_suspend: Subgroup basic-s4-devices: pass -> INCOMPLETE (fi-skl-guc) Known issues: Test gem_mmap_gtt: Subgroup basic-small-bo-tiledx: pass -> FAIL (fi-gdg-551) fdo#102575 fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:437s fi-bdw-gvtdvmtotal:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:443s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:381s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:533s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:298s fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:513s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:511s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:517s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:504s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:413s fi-cfl-s2total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:578s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:514s fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:536s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:593s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:424s fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:317s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:540s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:401s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:421s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:474s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:432s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:482s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:465s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:515s fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:655s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:455s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:534s fi-skl-6700hqtotal:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:541s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:504s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:496s fi-skl-guc total:109 pass:97 dwarn:0 dfail:0 fail:0 skip:11 fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:443s fi-snb-2520m total:3pass:2dwarn:0 dfail:0 fail:0 skip:0 fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:397s 3b4800f0237f9422817a1a9695d742598d2fb868 drm-tip: 2018y-03m-15d-15h-48m-56s UTC integration manifest 0b0cb08e3a43 drm/i915: Use drm_color_lut_size() 7844af96bfb7 drm/i915: Remove the blob->data casts 5ba3d49dfdda drm: Introduce drm_color_lut_size() 09561443137b drm: Verify gamma/degamma LUT size 7a8038be701b drm: Remove now pointelss blob->data casts 73972c43e891 Revert "drm: Use a flexible array member for blob property data" == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8370/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Remove hole and padding from intel_shared_dpll
On Thu, Mar 15, 2018 at 11:54:19AM +0200, Ville Syrjälä wrote: > > We can even (or alternatively) make dpll_info part of intel_shared_dpll. > > You mean something like? > > struct intel_shared_dpll { > ... > - id; > - name; > - flags; > + const struct dpll_info *info; > ... > }; yep, that. > That would make sense to me since the info seems to be all read-only > data. Oh and then we wouldn't even need the extra 'funcs' pointer. > Some extra indirection there but this isn't performance sensitive or > anything. > > Even if it wouldn't make things smaller I'd still like it just for > the clarity of having all the read-only data being const. > > > > > Below is the diff to make funcs a pointer on top of previous patch. > > This one is > Reviewed-by: Ville SyrjäläHumn... do you mean the initial patch or the diff below? If I'm going to embed dpll_info inside intel_shared_dpll, this patch would be pointless. Lucas De Marchi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm: Make sure at least one plane supports the fb format
On Thu, Mar 15, 2018 at 07:48:02PM +0200, Ville Syrjälä wrote: > On Thu, Mar 15, 2018 at 10:42:17AM -0700, Eric Anholt wrote: > > Ville Syrjalawrites: > > > > > From: Ville Syrjälä > > > > > > To make life easier for drivers, let's have the core check that the > > > requested pixel format is supported by at least one plane when creating > > > a new framebuffer. > > > > > > This eases the burden on drivers by making sure they'll never get > > > requests to create fbs with unsupported pixel formats. Thanks to the > > > new .fb_modifier() hook this check can now be done whether the request > > > specifies the modifier directly or driver has to deduce it from the > > > gem bo tiling (or via any other method really). > > > > > > v0: Accept anyformat if the driver doesn't do planes (Eric) > > > s/planes_have_format/any_plane_has_format/ (Eric) > > > Check the modifier as well since we already have a function > > > that does both > > > v3: Don't do the check in the core since we may not know the > > > modifier yet, instead export the function and let drivers > > > call it themselves > > > v4: Unexport the functiona and put the format_default check back > > > since this will again be called by the core, ie. undo v3 ;) > > > > > > Cc: Eric Anholt > > > Testcase: igt/kms_addfb_basic/expected-formats > > > Signed-off-by: Ville Syrjälä > > > --- > > > drivers/gpu/drm/drm_framebuffer.c | 30 ++ > > > 1 file changed, 30 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_framebuffer.c > > > b/drivers/gpu/drm/drm_framebuffer.c > > > index 21d3d51eb261..e618a6b728d4 100644 > > > --- a/drivers/gpu/drm/drm_framebuffer.c > > > +++ b/drivers/gpu/drm/drm_framebuffer.c > > > @@ -152,6 +152,26 @@ static int fb_plane_height(int height, > > > return DIV_ROUND_UP(height, format->vsub); > > > } > > > > > > +static bool any_plane_has_format(struct drm_device *dev, > > > + u32 format, u64 modifier) > > > +{ > > > + struct drm_plane *plane; > > > + > > > + drm_for_each_plane(plane, dev) { > > > + /* > > > + * In case the driver doesn't really do > > > + * planes we have to accept any format here. > > > + */ > > > + if (plane->format_default) > > > + return true; > > > + > > > + if (drm_plane_check_pixel_format(plane, format, modifier) == 0) > > > + return true; > > > + } > > > + > > > + return false; > > > +} > > > > This drm_plane_check_pixel_format() will always fail for VC4's SAND > > modifiers or VC5's UIF modifiers, where we're using the middle 48 bits > > as a bit of metadata (like how we have horizontal stride passed outside > > of the modifier) and you can't list all of the possible values in an > > array on the plane. > > Hmm. drm_atomic_plane_check() etc. call this thing as well. How does > that manage to work currently? Maybe it doesn't. I added the modifier checks in commit 23163a7d4b032489d375099d56571371c0456980 Author: Ville Syrjälä AuthorDate: Fri Dec 22 21:22:30 2017 +0200 Commit: Ville Syrjälä CommitDate: Mon Feb 26 16:29:47 2018 +0200 drm: Check that the plane supports the request format+modifier combo Maybe that broke vc4? Hmm. So either we need to stop checking against the modifiers array and rely purely or .format_mod_supported(), or we need to somehow get the driver to reduce the modifier to its base form. I guess we could make .fb_modifier() do that and call it also for addfb with modifiers. And I'd need to undo some of the modifier[0] vs. deduced modifier changes I did to framebuffer_check(), and we'd need to preserve the original modifier in the request for .fb_create(). Oh, but that wouldn't allow returning a non-base modifier from .fb_modifuer() for the !modifiers case. This is turning slightly more tricky than I had hoped... I guess relying on .format_mod_supported() might be what we need to do. Unfortunately it does mean that the .format_mod_supported() implementations must be prepared for modifiers that were not registered with the plane. Which does feel quite a bit more fragile. -- 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: Remove hole and padding from intel_shared_dpll
On Thu, Mar 15, 2018 at 08:16:32AM -0300, Jani Nikula wrote: > On Wed, 14 Mar 2018, Lucas De Marchiwrote: > > Reorder fields so we save 8 bytes per instance: this removes a 4-bytes > > hole after enum intel_dpll_id and a 4-bytes padding. > > Does GCC have anything like the Clang -Wpadded option to warn us about Humn.. I never saw one. > alignment holes and padding? Certainly it would be too noisy to run all > the time anyway, but it seems a bit tedious to have to do this by > hand. We could run it every once in a while to catch the worst > offenders. oh, no. I didn't do this manually. I used pahole (https://git.kernel.org/pub/scm/devel/pahole/pahole.git/) It has even a --reorganize option that should be used with care as some times data locality is more important on big structs. I always use it passing the struct I'm checking, but you can give it just the .o/.ko to analyze all the structs there (possibly with the -H option to filter by number of holes. Doing this on drivers/gpu/drm/i915/i915_debugfs.o for example I find a big offender: struct intel_dp { ... /* size: 2688, cachelines: 42, members: 55 */ /* sum members: 2662, holes: 7, sum holes: 26 */ /* paddings: 2, sum paddings: 8 */ }; Entire output of drm_i915_private for reference: $ pahole -C drm_i915_private drivers/gpu/drm/i915/i915.ko struct drm_i915_private { struct drm_device drm; /* 0 1576 */ /* --- cacheline 24 boundary (1536 bytes) was 40 bytes ago --- */ struct kmem_cache *objects; /* 1576 8 */ struct kmem_cache *vmas; /* 1584 8 */ struct kmem_cache *luts; /* 1592 8 */ /* --- cacheline 25 boundary (1600 bytes) --- */ struct kmem_cache *requests; /* 1600 8 */ struct kmem_cache *dependencies; /* 1608 8 */ struct kmem_cache *priorities; /* 1616 8 */ struct intel_device_infoconst info; /* 1624 184 */ /* --- cacheline 28 boundary (1792 bytes) was 16 bytes ago --- */ struct intel_driver_caps caps; /* 1808 4 */ /* XXX 4 bytes hole, try to pack */ struct resourcedsm; /* 181664 */ /* --- cacheline 29 boundary (1856 bytes) was 24 bytes ago --- */ struct resourcedsm_reserved; /* 188064 */ /* --- cacheline 30 boundary (1920 bytes) was 24 bytes ago --- */ resource_size_tstolen_usable_size; /* 1944 8 */ void * regs; /* 1952 8 */ struct intel_uncoreuncore; /* 1960 952 */ /* --- cacheline 45 boundary (2880 bytes) was 32 bytes ago --- */ struct i915_virtual_gpuvgpu; /* 2912 8 */ struct intel_gvt * gvt; /* 2920 8 */ struct intel_wopcm wopcm;/* 292812 */ /* XXX 4 bytes hole, try to pack */ /* --- cacheline 46 boundary (2944 bytes) --- */ struct intel_huc huc; /* 294472 */ /* --- cacheline 47 boundary (3008 bytes) was 8 bytes ago --- */ struct intel_guc guc; /* 3016 792 */ /* --- cacheline 59 boundary (3776 bytes) was 32 bytes ago --- */ struct intel_csr csr; /* 3808 136 */ /* --- cacheline 61 boundary (3904 bytes) was 40 bytes ago --- */ struct intel_gmbus gmbus[13];/* 3944 14144 */ /* --- cacheline 282 boundary (18048 bytes) was 40 bytes ago --- */ struct mutex gmbus_mutex; /* 1808832 */ /* --- cacheline 283 boundary (18112 bytes) was 8 bytes ago --- */ uint32_t gpio_mmio_base; /* 18120 4 */ uint32_t mipi_mmio_base; /* 18124 4 */ uint32_t psr_mmio_base;/* 18128 4 */ uint32_t pps_mmio_base;/* 18132 4 */ wait_queue_head_t gmbus_wait_queue; /* 1813624 */ struct pci_dev * bridge_dev; /* 18160 8 */ struct intel_engine_cs * engine[8];/* 1816864 */ /* --- cacheline 284 boundary (18176 bytes) was 56 bytes ago --- */ struct i915_gem_context * kernel_context; /* 18232 8 */ /* --- cacheline 285 boundary (18240 bytes) --- */ struct i915_gem_context * preempt_context; /* 18240 8 */ struct intel_engine_cs * engine_class[5][4]; /* 18248 160 */ /* --- cacheline 287 boundary (18368 bytes) was 40 bytes ago --- */ struct drm_dma_handle *
Re: [Intel-gfx] [PATCH 2/3] drm: Make sure at least one plane supports the fb format
On Thu, Mar 15, 2018 at 10:42:17AM -0700, Eric Anholt wrote: > Ville Syrjalawrites: > > > From: Ville Syrjälä > > > > To make life easier for drivers, let's have the core check that the > > requested pixel format is supported by at least one plane when creating > > a new framebuffer. > > > > This eases the burden on drivers by making sure they'll never get > > requests to create fbs with unsupported pixel formats. Thanks to the > > new .fb_modifier() hook this check can now be done whether the request > > specifies the modifier directly or driver has to deduce it from the > > gem bo tiling (or via any other method really). > > > > v0: Accept anyformat if the driver doesn't do planes (Eric) > > s/planes_have_format/any_plane_has_format/ (Eric) > > Check the modifier as well since we already have a function > > that does both > > v3: Don't do the check in the core since we may not know the > > modifier yet, instead export the function and let drivers > > call it themselves > > v4: Unexport the functiona and put the format_default check back > > since this will again be called by the core, ie. undo v3 ;) > > > > Cc: Eric Anholt > > Testcase: igt/kms_addfb_basic/expected-formats > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/drm_framebuffer.c | 30 ++ > > 1 file changed, 30 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_framebuffer.c > > b/drivers/gpu/drm/drm_framebuffer.c > > index 21d3d51eb261..e618a6b728d4 100644 > > --- a/drivers/gpu/drm/drm_framebuffer.c > > +++ b/drivers/gpu/drm/drm_framebuffer.c > > @@ -152,6 +152,26 @@ static int fb_plane_height(int height, > > return DIV_ROUND_UP(height, format->vsub); > > } > > > > +static bool any_plane_has_format(struct drm_device *dev, > > +u32 format, u64 modifier) > > +{ > > + struct drm_plane *plane; > > + > > + drm_for_each_plane(plane, dev) { > > + /* > > +* In case the driver doesn't really do > > +* planes we have to accept any format here. > > +*/ > > + if (plane->format_default) > > + return true; > > + > > + if (drm_plane_check_pixel_format(plane, format, modifier) == 0) > > + return true; > > + } > > + > > + return false; > > +} > > This drm_plane_check_pixel_format() will always fail for VC4's SAND > modifiers or VC5's UIF modifiers, where we're using the middle 48 bits > as a bit of metadata (like how we have horizontal stride passed outside > of the modifier) and you can't list all of the possible values in an > array on the plane. Hmm. drm_atomic_plane_check() etc. call this thing as well. How does that manage to work currently? -- 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 igt 6/8] lib/igt_pm: turn absence of autosuspend_delay_ms from fail to skip
On Thu, Mar 15, 2018 at 03:45:42PM +0100, Ulrich Hecht wrote: > Fixes false negatives on everything that doesn't happen to be at a > specific hard-coded sysfs path... > > Signed-off-by: Ulrich Hecht> --- > lib/igt_pm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/igt_pm.c b/lib/igt_pm.c > index 5bf5b2e..641157b 100644 > --- a/lib/igt_pm.c > +++ b/lib/igt_pm.c > @@ -262,7 +262,7 @@ bool igt_setup_runtime_pm(void) >* suite goes faster and we have a higher probability of triggering race >* conditions. */ > fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); The hardocded path should probably go then. igt_sysfs_path() + "/device/power" looks like it should dtrt. Would need to plumb the fd down though. Hopefully every caller has it handy. > - igt_assert_f(fd >= 0, > + igt_require_f(fd >= 0, >"Can't open " POWER_DIR "/autosuspend_delay_ms\n"); > > /* If we fail to write to the file, it means this system doesn't support > -- > 2.7.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 2/3] drm: Make sure at least one plane supports the fb format
Ville Syrjalawrites: > From: Ville Syrjälä > > To make life easier for drivers, let's have the core check that the > requested pixel format is supported by at least one plane when creating > a new framebuffer. > > This eases the burden on drivers by making sure they'll never get > requests to create fbs with unsupported pixel formats. Thanks to the > new .fb_modifier() hook this check can now be done whether the request > specifies the modifier directly or driver has to deduce it from the > gem bo tiling (or via any other method really). > > v0: Accept anyformat if the driver doesn't do planes (Eric) > s/planes_have_format/any_plane_has_format/ (Eric) > Check the modifier as well since we already have a function > that does both > v3: Don't do the check in the core since we may not know the > modifier yet, instead export the function and let drivers > call it themselves > v4: Unexport the functiona and put the format_default check back > since this will again be called by the core, ie. undo v3 ;) > > Cc: Eric Anholt > Testcase: igt/kms_addfb_basic/expected-formats > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_framebuffer.c | 30 ++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/drm_framebuffer.c > b/drivers/gpu/drm/drm_framebuffer.c > index 21d3d51eb261..e618a6b728d4 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -152,6 +152,26 @@ static int fb_plane_height(int height, > return DIV_ROUND_UP(height, format->vsub); > } > > +static bool any_plane_has_format(struct drm_device *dev, > + u32 format, u64 modifier) > +{ > + struct drm_plane *plane; > + > + drm_for_each_plane(plane, dev) { > + /* > + * In case the driver doesn't really do > + * planes we have to accept any format here. > + */ > + if (plane->format_default) > + return true; > + > + if (drm_plane_check_pixel_format(plane, format, modifier) == 0) > + return true; > + } > + > + return false; > +} This drm_plane_check_pixel_format() will always fail for VC4's SAND modifiers or VC5's UIF modifiers, where we're using the middle 48 bits as a bit of metadata (like how we have horizontal stride passed outside of the modifier) and you can't list all of the possible values in an array on the plane. signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt 5/8] tests/kms_plane_lowres: skip i915-specific tests on other platforms
On Thu, Mar 15, 2018 at 03:45:41PM +0100, Ulrich Hecht wrote: > Add is_i915_device() requirement to tests using Intel-specific APIs. > > Signed-off-by: Ulrich Hecht> --- > tests/kms_plane_lowres.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c > index d1e4b3c..8fc7654 100644 > --- a/tests/kms_plane_lowres.c > +++ b/tests/kms_plane_lowres.c > @@ -270,6 +270,7 @@ run_tests_for_pipe(data_t *data, enum pipe pipe) > igt_skip_on(pipe >= data->display.n_pipes); > > igt_display_require_output_on_pipe(>display, pipe); > + igt_require(is_i915_device(data->drm_fd)); What's the i915 specific thing here? The tiling formats? You should still be able to do the linear tests. We probably want to utilize https://patchwork.freedesktop.org/patch/210341/ to skip any test that is trying to use an unsupported modifier. Looks like I didn't account for drivers that don't support blobifiers. Shouldn't be too difficult to fix that up though. > } > > igt_subtest_f("pipe-%s-tiling-none", > -- > 2.7.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 igt 3/8] lib/igt_gt: has_gpu_reset(): fix failed assertion on non-i915 platforms
On Thu, Mar 15, 2018 at 03:45:39PM +0100, Ulrich Hecht wrote: > Checks if we have an i915 device before using intel_get_drm_devid(). > > Signed-off-by: Ulrich Hecht> --- > lib/igt_gt.c | 19 +++ > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/lib/igt_gt.c b/lib/igt_gt.c > index e630550..9cb07c2 100644 > --- a/lib/igt_gt.c > +++ b/lib/igt_gt.c I would think igt_gt as a whole is pretty much i915 specific. So feels to me like we should not have gotten this deep when using another driver. > @@ -59,14 +59,17 @@ static bool has_gpu_reset(int fd) > struct drm_i915_getparam gp; > int val = 0; > > - memset(, 0, sizeof(gp)); > - gp.param = 35; /* HAS_GPU_RESET */ > - gp.value = > - > - if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, )) > - once = intel_gen(intel_get_drm_devid(fd)) >= 5; > - else > - once = val > 0; > + if (is_i915_device(fd)) { > + memset(, 0, sizeof(gp)); > + gp.param = 35; /* HAS_GPU_RESET */ > + gp.value = > + > + if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, )) > + once = intel_gen(intel_get_drm_devid(fd)) >= 5; > + else > + once = val > 0; > + } else > + once = 0; > } > return once; > } > -- > 2.7.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ 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: Use correct reST syntax for WOPCM and GuC kernel-doc diagrams (rev3)
== Series Details == Series: drm/i915: Use correct reST syntax for WOPCM and GuC kernel-doc diagrams (rev3) URL : https://patchwork.freedesktop.org/series/39979/ State : success == Summary == Series 39979v3 drm/i915: Use correct reST syntax for WOPCM and GuC kernel-doc diagrams https://patchwork.freedesktop.org/api/1.0/series/39979/revisions/3/mbox/ Known issues: Test debugfs_test: Subgroup read_all_entries: incomplete -> PASS (fi-snb-2520m) fdo#103713 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-c: pass -> DMESG-WARN (fi-cnl-y3) fdo#104951 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:433s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:441s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:381s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:536s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:300s fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:513s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:512s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:523s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:507s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:411s fi-cfl-s2total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:576s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:512s fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:530s fi-cnl-y3total:285 pass:258 dwarn:1 dfail:0 fail:0 skip:26 time:588s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:425s fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:316s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:537s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:403s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:418s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:478s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:428s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:477s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:468s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:520s fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:655s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:447s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:532s fi-skl-6700hqtotal:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:539s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:507s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:494s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:432s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:447s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:590s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:400s 3b4800f0237f9422817a1a9695d742598d2fb868 drm-tip: 2018y-03m-15d-15h-48m-56s UTC integration manifest 7ac0518f687e drm/i915: Use correct reST syntax for WOPCM and GuC kernel-doc diagrams == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8369/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 5/6] drm/i915: Remove the blob->data casts
From: Ville SyrjäläNow that blob->data is void* again we don't need to cast it. v2: Rebase Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_color.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 89ab0f70aa22..92f148832a89 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -153,8 +153,7 @@ static void ilk_load_csc_matrix(struct drm_crtc_state *crtc_state) ilk_load_ycbcr_conversion_matrix(intel_crtc); return; } else if (crtc_state->ctm) { - struct drm_color_ctm *ctm = - (struct drm_color_ctm *)crtc_state->ctm->data; + struct drm_color_ctm *ctm = crtc_state->ctm->data; const u64 *input; u64 temp[9]; @@ -262,8 +261,7 @@ static void cherryview_load_csc_matrix(struct drm_crtc_state *state) uint32_t mode; if (state->ctm) { - struct drm_color_ctm *ctm = - (struct drm_color_ctm *) state->ctm->data; + struct drm_color_ctm *ctm = state->ctm->data; uint16_t coeffs[9] = { 0, }; int i; @@ -330,7 +328,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc, } if (blob) { - struct drm_color_lut *lut = (struct drm_color_lut *) blob->data; + struct drm_color_lut *lut = blob->data; for (i = 0; i < 256; i++) { uint32_t word = (drm_color_lut_extract(lut[i].red, 8) << 16) | @@ -400,8 +398,7 @@ static void bdw_load_degamma_lut(struct drm_crtc_state *state) PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT); if (state->degamma_lut) { - struct drm_color_lut *lut = - (struct drm_color_lut *) state->degamma_lut->data; + struct drm_color_lut *lut = state->degamma_lut->data; for (i = 0; i < lut_size; i++) { uint32_t word = @@ -435,8 +432,7 @@ static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset) offset); if (state->gamma_lut) { - struct drm_color_lut *lut = - (struct drm_color_lut *) state->gamma_lut->data; + struct drm_color_lut *lut = state->gamma_lut->data; for (i = 0; i < lut_size; i++) { uint32_t word = @@ -568,7 +564,7 @@ static void cherryview_load_luts(struct drm_crtc_state *state) } if (state->degamma_lut) { - lut = (struct drm_color_lut *) state->degamma_lut->data; + lut = state->degamma_lut->data; lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size; for (i = 0; i < lut_size; i++) { /* Write LUT in U0.14 format. */ @@ -583,7 +579,7 @@ static void cherryview_load_luts(struct drm_crtc_state *state) } if (state->gamma_lut) { - lut = (struct drm_color_lut *) state->gamma_lut->data; + lut = state->gamma_lut->data; lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size; for (i = 0; i < lut_size; i++) { /* Write LUT in U0.10 format. */ -- 2.16.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Show dmc debug registers on CFL and GLK (rev2)
== Series Details == Series: drm/i915: Show dmc debug registers on CFL and GLK (rev2) URL : https://patchwork.freedesktop.org/series/40031/ State : failure == Summary == Possible new issues: Test kms_frontbuffer_tracking: Subgroup fbc-modesetfrombusy: pass -> FAIL (shard-apl) Known issues: Test gem_eio: Subgroup in-flight-external: incomplete -> PASS (shard-apl) fdo#105341 +1 Test kms_flip: Subgroup 2x-plain-flip-fb-recreate: pass -> FAIL (shard-hsw) fdo#100368 Test kms_sysfs_edid_timing: pass -> WARN (shard-apl) fdo#100047 fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341 fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047 shard-apltotal:3399 pass:1795 dwarn:1 dfail:0 fail:8 skip:1592 time:12040s shard-hswtotal:3442 pass:1767 dwarn:1 dfail:0 fail:2 skip:1671 time:11814s shard-snbtotal:3442 pass:1358 dwarn:1 dfail:0 fail:2 skip:2081 time:7201s Blacklisted hosts: shard-kbltotal:2129 pass:1187 dwarn:4 dfail:0 fail:5 skip:932 time:5546s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8365/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: Use correct reST syntax for WOPCM and GuC kernel-doc diagrams
GuC Address Space and WOPCM Layout diagrams won't be generated correctly by sphinx build if not using proper reST syntax. This patch uses reST literal blocks to make sure GuC Address Space and WOPCM Layout diagrams to be generated correctly, and it also corrects some errors in the diagram description. v2: - Fixed errors in diagram description v3: - Updated GuC Address Space kernel-doc based on Michal's suggestion Signed-off-by: Jackie LiCc: Michal Wajdeczko Cc: Sagar Arun Kamble Cc: Joonas Lahtinen --- drivers/gpu/drm/i915/intel_guc.c | 56 -- drivers/gpu/drm/i915/intel_wopcm.c | 44 -- 2 files changed, 52 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c index 3eb516e..bcbdf15 100644 --- a/drivers/gpu/drm/i915/intel_guc.c +++ b/drivers/gpu/drm/i915/intel_guc.c @@ -495,35 +495,37 @@ int intel_guc_resume(struct intel_guc *guc) /** * DOC: GuC Address Space * - * The layout of GuC address space is shown as below: + * The layout of GuC address space is shown below: * - *+==> ++ <== GUC_GGTT_TOP - *^|| - *||| - *||DRAM| - *|| Memory | - *||| - * GuC || - * Address +> ++ <== WOPCM Top - * Space ^ | HW contexts RSVD | - *| | |WOPCM | - *| | +==> ++ <== GuC WOPCM Top - *|GuC^|| - *|GGTT ||| - *|Pin GuC |GuC | - *|Bias WOPCM | WOPCM| - *| |Size || - *| | ||| - *v v v|| - *+=+=+==> ++ <== GuC WOPCM Base - * | Non-GuC WOPCM| - * | (HuC/Reserved) | - * ++ <== WOPCM Base + * :: * - * The lower part [0, GuC ggtt_pin_bias) is mapped to WOPCM which consists of - * GuC WOPCM and WOPCM reserved for other usage (e.g.RC6 context). The value of - * the GuC ggtt_pin_bias is determined by the actually GuC WOPCM size which is - * set in GUC_WOPCM_SIZE register. + * +==> ++ <== GUC_GGTT_TOP + * ^|| + * ||| + * ||DRAM| + * || Memory | + * ||| + *GuC || + * Address +> ++ <== WOPCM Top + * Space ^ | HW contexts RSVD | + * | | |WOPCM | + * | | +==> ++ <== GuC WOPCM Top + * |GuC^|| + * |GGTT ||| + * |Pin GuC |GuC | + * |Bias WOPCM | WOPCM| + * | |Size || + * | | ||| + * v v v|| + * +=+=+==> ++ <== GuC WOPCM Base + * | Non-GuC WOPCM| + * | (HuC/Reserved) | + * ++ <== WOPCM Base + * + * The lower part of GuC Address Space [0, ggtt_pin_bias) is mapped to WOPCM + * while upper part of GuC Address Space [ggtt_pin_bias, GUC_GGTT_TOP) is mapped + * to DRAM. The value of the GuC ggtt_pin_bias is determined by WOPCM size and + * actual GuC WOPCM size. */ /** diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c index 4117886..74bf76f 100644 --- a/drivers/gpu/drm/i915/intel_wopcm.c +++ b/drivers/gpu/drm/i915/intel_wopcm.c @@ -11,28 +11,30 @@ * DOC: WOPCM Layout * * The layout of the WOPCM will be fixed after writing to GuC WOPCM size and - * offset registers whose are calculated are determined by size of HuC/GuC - * firmware size and set of hw requirements/restrictions as shown below: + * offset registers whose values are calculated and determined by HuC/GuC + * firmware size and set of hardware requirements/restrictions as shown below: * - * +=> ++ <== WOPCM Top - * ^ | HW contexts RSVD | - * | +===> ++ <== GuC WOPCM Top - * | ^ || - * | | || - * | | || - * |GuC|
Re: [Intel-gfx] [PATCH] drm/i915/guc: Unify naming of private GuC action functions
On Thu, Mar 15, 2018 at 05:19:27PM +0100, Michal Wajdeczko wrote: > On Thu, 15 Mar 2018 16:57:26 +0100, Michał Winiarski >wrote: > > > On Wed, Mar 14, 2018 at 06:37:15PM +, Michal Wajdeczko wrote: > > > We should avoid using guc_log prefix for functions that don't > > > operate on GuC log, but rather request action from the GuC. > > > Better to use guc_action prefix. > > > > > > Signed-off-by: Michal Wajdeczko > > > Cc: Michal Winiarski > > > Cc: Sagar Arun Kamble > > > --- > > > drivers/gpu/drm/i915/intel_guc_log.c | 16 +--- > > > 1 file changed, 9 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_guc_log.c > > > b/drivers/gpu/drm/i915/intel_guc_log.c > > > index b9c7bd7..457168a 100644 > > > --- a/drivers/gpu/drm/i915/intel_guc_log.c > > > +++ b/drivers/gpu/drm/i915/intel_guc_log.c > > > @@ -39,7 +39,7 @@ > > > * registers value. > > > */ > > > > > > -static int guc_log_flush_complete(struct intel_guc *guc) > > > +static int guc_action_flush_log_complete(struct intel_guc *guc) > > > { > > > u32 action[] = { > > > INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE > > > @@ -48,7 +48,7 @@ static int guc_log_flush_complete(struct intel_guc > > > *guc) > > > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > > > } > > > > > > -static int guc_log_flush(struct intel_guc *guc) > > > +static int guc_action_flush_log(struct intel_guc *guc) > > > { > > > u32 action[] = { > > > INTEL_GUC_ACTION_FORCE_LOG_BUFFER_FLUSH, > > > @@ -58,7 +58,8 @@ static int guc_log_flush(struct intel_guc *guc) > > > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > > > } > > > > > > -static int guc_log_control(struct intel_guc *guc, bool enable, u32 > > > verbosity) > > > +static int guc_action_enable_log(struct intel_guc *guc, bool enable, > > > + u32 verbosity) > > > > Let's hide the fact that the actual action is called "ENABLE_LOGGING", > > and stick > > with guc_action_log_control, especially since we're using > > guc_log_control union, > > and the action itself is also used for verbosity (and default log... > > more than > > just enable/disable switch). > > Hmm, I think that using action name as base for function is right thing. > If in your opinion action name is not correct, we should start with action > rename first. Nooo, then we're going to have i915/GuC mismatch :/ > And I would rather prefer to drop definition of union guc_log_control > and replace it with set of SHIFT/MASK macros as we do for other bitfields. Sure - why not. > Also using actual action name as base for new function name, we could > avoid having yet another [log|control|log] function name permutation. We're not consistent with maching action/function name, and I think 4 arguments "enable" function is going to be really confusing. But I don't have a strong opinion here, it's going to be used in a single place, and it has "guc_action_*" warning sign now ;) -Michał > But I'm flexible ;) > > > > > With that: > > > > Reviewed-by: Michał Winiarski > > > > -Michał > > > > > > > > /* GuC would have updated log buffer by now, so capture it */ > > > @@ -639,10 +640,11 @@ int intel_guc_log_control_set(struct intel_guc > > > *guc, u64 val) > > > } > > > > > > intel_runtime_pm_get(dev_priv); > > > - ret = guc_log_control(guc, enabled, LOG_LEVEL_TO_VERBOSITY(val)); > > > + ret = guc_action_enable_log(guc, enabled, > > > LOG_LEVEL_TO_VERBOSITY(val)); > > > intel_runtime_pm_put(dev_priv); > > > if (ret) { > > > - DRM_DEBUG_DRIVER("guc_log_control action failed %d\n", ret); > > > + DRM_DEBUG_DRIVER("GuC action to %s log failed (%d)\n", > > > + enabled ? "enable" : "disable", ret); > > > goto out_unlock; > > > } > > > > > > -- > > > 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Use correct reST syntax for WOPCM and GuC kernel-doc diagrams
On 03/14/2018 11:54 PM, Sagar Arun Kamble wrote: Are we required to add reference to intel_guc.c and intel_wopcm.c in Documentation/gpu/i915.rst? hmm, I have the same question too:-). Can I modify the i915.rst to include kernel-doc from WOPCM and GuC WOPCM related changes? or someone would decide what contents should be included in i915 kernel doc? From my point of view, I think it would make the kernel-doc more comprehensible (at least for the wopcm part) if these diagrams were exported as a part of kernel-doc. Thanks, -Jackie ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
On Thu, 2018-03-15 at 18:14 +0200, Ville Syrjälä wrote: > > There's no trade-off in this patch for faster/larger. > > This patch is simply smaller. Smaller is better. > > This feels a bit like saying pink is better than red because it's > more pink. Silly. If you can't say smaller total object code that performs the same task identically is better, I think we can't discuss much of anything about code together. Any printk related mechanism is not fast-path so any icache dilution isn't an issue. > That said, I'm not arguing against this patch as such. Making things > smaller "just because" usually doesn't cause problems. It seems more like you haven't read the patch. > But I was > hoping that we might be after some more tangible gains here, and > thus pointed out that there may be a better way to achieve even > bigger gains. Sure, it's just any such a discussion should not affect this patch being applied. This patch reduces the argument count of the drm_printk (now drm_dbg) call and so is faster to execute even if the emit test is internal to the drm_dbg function. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] intel: align reuse buffer's size on page size instead
Thanks for the review, Chris. Sorry for the late response. >-Original Message- >From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of >Chris Wilson >Sent: Saturday, March 3, 2018 1:46 AM >To: Xiong, James; dri-de...@lists.freedesktop.org; >intel-gfx@lists.freedesktop.org >Cc: Xiong, James >Subject: Re: [Intel-gfx] [PATCH 1/1] intel: align reuse buffer's size on page >size instead > >Quoting James Xiong (2018-03-02 17:53:04) >> From: "Xiong, James" >> >> With gem_reuse enabled, when a buffer size is different than the sizes >> of buckets, it is aligned to the next bucket's size, which means about >> 25% more memory than the requested is allocated in the worst senario. >> For example: >> >> Orignal sizeActual >> 32KB+1Byte 40KB >>. >>. >>. >>8MB+1Byte 10MB >>. >>. >>. >>96MB+1Byte 112MB >> >> This is very memory expensive and make the reuse feature less >> favorable than it deserves to be. >> >> This commit aligns the reuse buffer size on page size instead, the >> buffer whose size falls between bucket[n] and bucket[n+1] is put in >> bucket[n] when it's done; And when searching for a cached buffer for >> reuse, it goes through the cached buffers list in the bucket until a >> cached buffer, whose size is larger than or equal to the requested >> size, is found. >> >> Performed gfxbench tests, the performances and reuse ratioes (reuse >> count/allocation count) were same as before, saved memory usage by 1% >> ~ 7%(gl_manhattan: peak allocated memory size was reduced from >> 448401408 to 419078144). > >Apart from GL doesn't use this... And what is the impact on !llc, where buffer >reuse is more important? (Every new buffer requires clflushing.) The test was run against a Gen9 that doesn't support LLC. Please let me know if you have some performance tests for me to run. > > > >> Signed-off-by: Xiong, James >> --- >> intel/intel_bufmgr_gem.c | 180 >> +-- >> libdrm_lists.h | 6 ++ >> 2 files changed, 101 insertions(+), 85 deletions(-) >> >> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index >> 386da30..5b2d0d0 100644 >> --- a/intel/intel_bufmgr_gem.c >> +++ b/intel/intel_bufmgr_gem.c >> @@ -402,11 +402,10 @@ >> drm_intel_gem_bo_bucket_for_size(drm_intel_bufmgr_gem *bufmgr_gem, { >> int i; >> >> - for (i = 0; i < bufmgr_gem->num_buckets; i++) { >> - struct drm_intel_gem_bo_bucket *bucket = >> - _gem->cache_bucket[i]; >> - if (bucket->size >= size) { >> - return bucket; >> + for (i = 0; i < bufmgr_gem->num_buckets - 1; i++) { >> + if (size >= bufmgr_gem->cache_bucket[i].size && >> + size < bufmgr_gem->cache_bucket[i+1].size) { >> + return _gem->cache_bucket[i]; > >Are your buckets not ordered correctly? The order is the same as before, so are the buckets' sizes. > >Please reduce this patch to a series of small functional changes required to >bring into effect having mixed, page-aligned bo->size within a bucket. Will do. >-Chris ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Unify naming of private GuC action functions
On Thu, 15 Mar 2018 16:57:26 +0100, Michał Winiarskiwrote: On Wed, Mar 14, 2018 at 06:37:15PM +, Michal Wajdeczko wrote: We should avoid using guc_log prefix for functions that don't operate on GuC log, but rather request action from the GuC. Better to use guc_action prefix. Signed-off-by: Michal Wajdeczko Cc: Michal Winiarski Cc: Sagar Arun Kamble --- drivers/gpu/drm/i915/intel_guc_log.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index b9c7bd7..457168a 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -39,7 +39,7 @@ * registers value. */ -static int guc_log_flush_complete(struct intel_guc *guc) +static int guc_action_flush_log_complete(struct intel_guc *guc) { u32 action[] = { INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE @@ -48,7 +48,7 @@ static int guc_log_flush_complete(struct intel_guc *guc) return intel_guc_send(guc, action, ARRAY_SIZE(action)); } -static int guc_log_flush(struct intel_guc *guc) +static int guc_action_flush_log(struct intel_guc *guc) { u32 action[] = { INTEL_GUC_ACTION_FORCE_LOG_BUFFER_FLUSH, @@ -58,7 +58,8 @@ static int guc_log_flush(struct intel_guc *guc) return intel_guc_send(guc, action, ARRAY_SIZE(action)); } -static int guc_log_control(struct intel_guc *guc, bool enable, u32 verbosity) +static int guc_action_enable_log(struct intel_guc *guc, bool enable, +u32 verbosity) Let's hide the fact that the actual action is called "ENABLE_LOGGING", and stick with guc_action_log_control, especially since we're using guc_log_control union, and the action itself is also used for verbosity (and default log... more than just enable/disable switch). Hmm, I think that using action name as base for function is right thing. If in your opinion action name is not correct, we should start with action rename first. And I would rather prefer to drop definition of union guc_log_control and replace it with set of SHIFT/MASK macros as we do for other bitfields. Also using actual action name as base for new function name, we could avoid having yet another [log|control|log] function name permutation. But I'm flexible ;) With that: Reviewed-by: Michał Winiarski -Michał { union guc_log_control control_val = { { @@ -525,7 +526,7 @@ static void guc_log_capture_logs(struct intel_guc *guc) * time, so get/put should be really quick. */ intel_runtime_pm_get(dev_priv); - guc_log_flush_complete(guc); + guc_action_flush_log_complete(guc); intel_runtime_pm_put(dev_priv); } @@ -541,7 +542,7 @@ static void guc_flush_logs(struct intel_guc *guc) /* Ask GuC to update the log buffer state */ intel_runtime_pm_get(dev_priv); - guc_log_flush(guc); + guc_action_flush_log(guc); intel_runtime_pm_put(dev_priv); /* GuC would have updated log buffer by now, so capture it */ @@ -639,10 +640,11 @@ int intel_guc_log_control_set(struct intel_guc *guc, u64 val) } intel_runtime_pm_get(dev_priv); - ret = guc_log_control(guc, enabled, LOG_LEVEL_TO_VERBOSITY(val)); + ret = guc_action_enable_log(guc, enabled, LOG_LEVEL_TO_VERBOSITY(val)); intel_runtime_pm_put(dev_priv); if (ret) { - DRM_DEBUG_DRIVER("guc_log_control action failed %d\n", ret); + DRM_DEBUG_DRIVER("GuC action to %s log failed (%d)\n", +enabled ? "enable" : "disable", ret); goto out_unlock; } -- 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: Reduce object size of DRM_ERROR and DRM_DEBUG uses
On Thu, Mar 15, 2018 at 08:44:05AM -0700, Joe Perches wrote: > On Thu, 2018-03-15 at 17:37 +0200, Ville Syrjälä wrote: > > On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote: > > > On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote: > > > > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote: > > > > > Op 15-03-18 om 14:30 schreef Ville Syrjälä: > > > > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote: > > > > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with > > > > > > > unnecessary > > > > > > > arguments that can be removed by creating separate functins. > > > > > > > > > > > > > > Create specific functions for these calls to reduce x86/64 > > > > > > > defconfig > > > > > > > size by ~20k. > > > > > > > > > > > > > > Modify the existing macros to use the specific calls. > > > > > > > > > > > > > > new: > > > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > > > > > > 1876562 44542 995 1922099 1d5433 (TOTALS) > > > > > > > > > > > > > > old: > > > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > > > > > > 1897565 44542 995 1943102 1da63e (TOTALS) > > > > > > > > > > > > > > Miscellanea: > > > > > > > > > > > > > > o intel_display requires a change to use the specific calls. > > > > > > > > > > > > How much would we lose if we move the (drm_debug) outside the > > > > > > functions again? > > > > > > again? > > > > We used to do that. Someone changed it a while back, unintentially > > I believe. > > > > > > > > > > > I'm somewhat concerned about all the function call > > > > > > overhead when debugs aren't even enabled. > > > > > > Perhaps better to have compilation elimination > > > of the entire debug output instead. > > > > That would require every bug reporter to recompile the kernel first. > > So this is not a solution we would ever seriously consider. > > > > Not sure if it would be possible to use the alternatives thing to > > eliminate the function calls unless the user boots wih drm.debug!=0? > > > > > > > > I think you are discussing a different issue and > > > this discussion should not block this patch as > > > this patch has no impact other than code size > > > reduction. > > > > But what is the goal of the code size reduction? > > Smaller code. > > > I assume the main > > goal is to make better use of the instruction cache to make the > > code faster. If there's a tradeoff between smaller and slightly > > faster vs. larger and a singificantly faster I tend to think we > > should go for the latter option. > > There's no trade-off in this patch for faster/larger. > This patch is simply smaller. Smaller is better. This feels a bit like saying pink is better than red because it's more pink. That said, I'm not arguing against this patch as such. Making things smaller "just because" usually doesn't cause problems. But I was hoping that we might be after some more tangible gains here, and thus pointed out that there may be a better way to achieve even bigger gains. -- 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] sna/uxa: Fix colormap handling at screen depth 30.
Quoting Ville Syrjälä (2018-03-15 16:02:42) > On Thu, Mar 15, 2018 at 03:28:18PM +, Chris Wilson wrote: > > Quoting Ville Syrjälä (2018-03-01 11:12:53) > > > On Thu, Mar 01, 2018 at 02:20:48AM +0100, Mario Kleiner wrote: > > > > The various clut handling functions like a setup > > > > consistent with the x-screen color depth. Otherwise > > > > we observe improper sampling in the gamma tables > > > > at depth 30. > > > > > > > > Therefore replace hard-coded bitsPerRGB = 8 by actual > > > > bits per channel scrn->rgbBits. Also use this for call > > > > to xf86HandleColormaps(). > > > > > > > > Tested for uxa and sna at depths 8, 16, 24 and 30 on > > > > IvyBridge, and tested at depth 24 and 30 that xgamma > > > > and gamma table animations work, and with measurement > > > > equipment to make sure identity gamma ramps actually > > > > are identity mappings at the output. > > > > > > You mean identity mapping at 8bpc? We don't support higher precision > > > gamma on pre-bdw atm, and the ddx doesn't use the higher precision > > > stuff even on bdw+. I'm working on fixing both, but it turned out to > > > be a bit more work than I anticipated so will take a while. > > > > > > > > > > > Signed-off-by: Mario Kleiner> > > > --- > > > > src/sna/sna_driver.c | 5 +++-- > > > > src/uxa/intel_driver.c | 3 ++- > > > > 2 files changed, 5 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/src/sna/sna_driver.c b/src/sna/sna_driver.c > > > > index 2643e6c..9c4bcd4 100644 > > > > --- a/src/sna/sna_driver.c > > > > +++ b/src/sna/sna_driver.c > > > > @@ -1145,7 +1145,7 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL) > > > > if (!miInitVisuals(, , , , > > > > , > > > > , > > > > ((unsigned long)1 << (scrn->bitsPerPixel - 1)), > > > > -8, -1)) > > > > +scrn->rgbBits, -1)) > > > > return FALSE; > > > > > > > > if (!miScreenInit(screen, NULL, > > > > @@ -1217,7 +1217,8 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL) > > > > return FALSE; > > > > > > > > if (sna->mode.num_real_crtc && > > > > - !xf86HandleColormaps(screen, 256, 8, sna_load_palette, NULL, > > > > + !xf86HandleColormaps(screen, 1 << scrn->rgbBits, > > > > scrn->rgbBits, > > > > + sna_load_palette, NULL, > > > >CMAP_RELOAD_ON_MODE_SWITCH | > > > >CMAP_PALETTED_TRUECOLOR)) > > > > > > I already forgot what this does prior to your randr fix. IIRC bumping > > > the 8 alone would cause the thing to segfault, but I guess bumping both > > > was fine? > > > > > > Hmm. So the server always initializes crtc->gamma_size to 256 > > > (which does match the normal hw LUT size), and so before your > > > fix we will always get gamma_slots==0 at >8bpc and so the hw LUT > > > is never actually updated? > > > > Was there any conclusion to this? Aiui, these bits should be set based > > on the underlying HW gamma table depth which is not the same as the > > screen colour depth. It stuck at 256 for ages as that is all anyone > > ever expected... > > IIRC there was some kind of agreement that Mario's approach here is > generally what we want to do. The server randr code will simply throw > away the LUT entries we can't use and driver will still get the > expected 256 entry LUT via the randr hooks. Obviously that means the > output may not be exactly what the user wanted, but for normal > gamma curve type of stuff it's pretty close. > > I'm mostly concerned what happens when the server doesn't have > Mario's gamma fixes. IIRC something will either crash, or simply > not update the gamma LUT ever. I guess we just make it conditional on xorg_version >= 1.20 to be on the safe side. > Once I manage to pimp up the i915 gamma LUT stuff I plan to tackle > the ddx side to actually make use of the higher precision gamma modes > in the hardware. Actually I already started on the ddx side a bit just > didn't get the kernel quite ready yet. Once it's all done we should > have 10bit gamma to match the screen depth on all platforms. On gen4 > it will be an interpolated curve though, so if the user programs an > arbitrary LUT we'll not be able to replicate it correctly. But again, > for normal gamma curve type of stuff it'll work just fine. For ilk+ > we will get a full 1024 entry LUT. Sounds good. -Chris ___ 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/guc: Fix build break on config without DEBUG_FS
== Series Details == Series: drm/i915/guc: Fix build break on config without DEBUG_FS URL : https://patchwork.freedesktop.org/series/40041/ State : success == Summary == Series 40041v1 drm/i915/guc: Fix build break on config without DEBUG_FS https://patchwork.freedesktop.org/api/1.0/series/40041/revisions/1/mbox/ Known issues: Test kms_pipe_crc_basic: Subgroup nonblocking-crc-pipe-b-frame-sequence: pass -> FAIL (fi-cfl-s2) fdo#103481 fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:430s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:437s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:378s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:538s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:301s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:508s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:514s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:505s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:409s fi-cfl-s2total:285 pass:258 dwarn:0 dfail:0 fail:1 skip:26 time:579s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:511s fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:527s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:585s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:427s fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:315s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:536s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:402s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:423s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:478s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:427s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:471s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:464s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:513s fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:651s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:442s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:526s fi-skl-6700hqtotal:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:539s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:505s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:493s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:429s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:446s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:597s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:400s 95626f201526d97b6f284b397fd73c8a2d514ff4 drm-tip: 2018y-03m-15d-11h-01m-14s UTC integration manifest 1621199df03e drm/i915/guc: Fix build break on config without DEBUG_FS == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8368/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] sna/uxa: Fix colormap handling at screen depth 30.
On Thu, Mar 15, 2018 at 03:28:18PM +, Chris Wilson wrote: > Quoting Ville Syrjälä (2018-03-01 11:12:53) > > On Thu, Mar 01, 2018 at 02:20:48AM +0100, Mario Kleiner wrote: > > > The various clut handling functions like a setup > > > consistent with the x-screen color depth. Otherwise > > > we observe improper sampling in the gamma tables > > > at depth 30. > > > > > > Therefore replace hard-coded bitsPerRGB = 8 by actual > > > bits per channel scrn->rgbBits. Also use this for call > > > to xf86HandleColormaps(). > > > > > > Tested for uxa and sna at depths 8, 16, 24 and 30 on > > > IvyBridge, and tested at depth 24 and 30 that xgamma > > > and gamma table animations work, and with measurement > > > equipment to make sure identity gamma ramps actually > > > are identity mappings at the output. > > > > You mean identity mapping at 8bpc? We don't support higher precision > > gamma on pre-bdw atm, and the ddx doesn't use the higher precision > > stuff even on bdw+. I'm working on fixing both, but it turned out to > > be a bit more work than I anticipated so will take a while. > > > > > > > > Signed-off-by: Mario Kleiner> > > --- > > > src/sna/sna_driver.c | 5 +++-- > > > src/uxa/intel_driver.c | 3 ++- > > > 2 files changed, 5 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/sna/sna_driver.c b/src/sna/sna_driver.c > > > index 2643e6c..9c4bcd4 100644 > > > --- a/src/sna/sna_driver.c > > > +++ b/src/sna/sna_driver.c > > > @@ -1145,7 +1145,7 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL) > > > if (!miInitVisuals(, , , , > > > , > > > , > > > ((unsigned long)1 << (scrn->bitsPerPixel - 1)), > > > -8, -1)) > > > +scrn->rgbBits, -1)) > > > return FALSE; > > > > > > if (!miScreenInit(screen, NULL, > > > @@ -1217,7 +1217,8 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL) > > > return FALSE; > > > > > > if (sna->mode.num_real_crtc && > > > - !xf86HandleColormaps(screen, 256, 8, sna_load_palette, NULL, > > > + !xf86HandleColormaps(screen, 1 << scrn->rgbBits, scrn->rgbBits, > > > + sna_load_palette, NULL, > > >CMAP_RELOAD_ON_MODE_SWITCH | > > >CMAP_PALETTED_TRUECOLOR)) > > > > I already forgot what this does prior to your randr fix. IIRC bumping > > the 8 alone would cause the thing to segfault, but I guess bumping both > > was fine? > > > > Hmm. So the server always initializes crtc->gamma_size to 256 > > (which does match the normal hw LUT size), and so before your > > fix we will always get gamma_slots==0 at >8bpc and so the hw LUT > > is never actually updated? > > Was there any conclusion to this? Aiui, these bits should be set based > on the underlying HW gamma table depth which is not the same as the > screen colour depth. It stuck at 256 for ages as that is all anyone > ever expected... IIRC there was some kind of agreement that Mario's approach here is generally what we want to do. The server randr code will simply throw away the LUT entries we can't use and driver will still get the expected 256 entry LUT via the randr hooks. Obviously that means the output may not be exactly what the user wanted, but for normal gamma curve type of stuff it's pretty close. I'm mostly concerned what happens when the server doesn't have Mario's gamma fixes. IIRC something will either crash, or simply not update the gamma LUT ever. Once I manage to pimp up the i915 gamma LUT stuff I plan to tackle the ddx side to actually make use of the higher precision gamma modes in the hardware. Actually I already started on the ddx side a bit just didn't get the kernel quite ready yet. Once it's all done we should have 10bit gamma to match the screen depth on all platforms. On gen4 it will be an interpolated curve though, so if the user programs an arbitrary LUT we'll not be able to replicate it correctly. But again, for normal gamma curve type of stuff it'll work just fine. For ilk+ we will get a full 1024 entry LUT. -- 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] [igt-dev] [PATCH i-g-t v2] tests/perf_pmu: Improve accuracy by waiting on spinner to start
Quoting Tvrtko Ursulin (2018-03-15 15:46:00) > static void > -__submit_spin_batch(int gem_fd, > - struct drm_i915_gem_exec_object2 *obj, > +__submit_spin_batch(int gem_fd, igt_spin_t *spin, > const struct intel_execution_engine2 *e) > { > - struct drm_i915_gem_execbuffer2 eb = { > - .buffer_count = 1, > - .buffers_ptr = to_user_pointer(obj), > - .flags = e2ring(gem_fd, e), > - }; > + struct drm_i915_gem_execbuffer2 eb = spin->execbuf; > + > + eb.flags &= ~(0x3f | I915_EXEC_BSD_MASK); > + eb.flags |= e2ring(gem_fd, e); I'm dubious about keeping spin->execbuf for precisely this reason. Almost all the time I want to specify a different execution, and not use the same context, random fences, etc. However, it does give a convenient way to get buffers_count and buffers_ptr, but that is all that is valid (imo) or at least should be judiciously copied from rather than wholesale. eb = { .buffers_ptr = spin->execbuf.buffers_ptr, .buffer_count = spin->execbuf.buffer_count, .flags = e2ring(gem_fd, e) | I915_EXEC_NORELOC, }; > @@ -1545,24 +1575,30 @@ accuracy(int gem_fd, const struct > intel_execution_engine2 *e, > > igt_nsec_elapsed(_start); > do { > - unsigned int target_idle_us, t_busy; > + unsigned int target_idle_us; > + struct timespec start = { }; > + unsigned long prep_delay_ns; > > /* Restart the spinbatch. */ > + igt_nsec_elapsed(); > __rearm_spin_batch(spin); > - __submit_spin_batch(gem_fd, , e); > + __submit_spin_batch(gem_fd, spin, e); > > - /* > -* Note that the submission may be delayed to > a > -* tasklet (ksoftirqd) which cannot run until > we > -* sleep as we hog the cpu (we are RT). > -*/ > +/* Wait for batch to start executing. */ > + __spin_wait(gem_fd, spin); > + prep_delay_ns = igt_nsec_elapsed(); > > - t_busy = measured_usleep(busy_us); > + /* PWM busy sleep. */ > + memset(, 0, sizeof(start)); > + igt_nsec_elapsed(); Can just keep using start, it's already has the current time from calculating prep_delay_ns. > + measured_usleep(busy_us); > igt_spin_batch_end(spin); > - gem_sync(gem_fd, obj.handle); > + gem_sync(gem_fd, spin->handle); > > - total_busy_ns += t_busy; > + total_busy_ns += igt_nsec_elapsed(); > + total_idle_ns += prep_delay_ns; Ok. Reviewed-by: Chris Wilson-Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Trace GEM steps between submit and wedging (rev2)
== Series Details == Series: series starting with [1/2] drm/i915: Trace GEM steps between submit and wedging (rev2) URL : https://patchwork.freedesktop.org/series/40029/ State : success == Summary == Known issues: Test gem_eio: Subgroup in-flight-external: incomplete -> PASS (shard-apl) fdo#105341 +1 Test kms_rotation_crc: Subgroup cursor-rotation-180: pass -> FAIL (shard-snb) fdo#105185 Test kms_sysfs_edid_timing: pass -> WARN (shard-apl) fdo#100047 fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341 fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185 fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047 shard-apltotal:3442 pass:1814 dwarn:1 dfail:0 fail:7 skip:1619 time:12992s shard-hswtotal:3442 pass:1768 dwarn:1 dfail:0 fail:1 skip:1671 time:11796s shard-snbtotal:3442 pass:1357 dwarn:1 dfail:0 fail:3 skip:2081 time:7204s Blacklisted hosts: shard-kbltotal:2097 pass:1170 dwarn:0 dfail:0 fail:7 skip:919 time:5852s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8363/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/guc: Unify naming of private GuC action functions
On Wed, Mar 14, 2018 at 06:37:15PM +, Michal Wajdeczko wrote: > We should avoid using guc_log prefix for functions that don't > operate on GuC log, but rather request action from the GuC. > Better to use guc_action prefix. > > Signed-off-by: Michal Wajdeczko> Cc: Michal Winiarski > Cc: Sagar Arun Kamble > --- > drivers/gpu/drm/i915/intel_guc_log.c | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_log.c > b/drivers/gpu/drm/i915/intel_guc_log.c > index b9c7bd7..457168a 100644 > --- a/drivers/gpu/drm/i915/intel_guc_log.c > +++ b/drivers/gpu/drm/i915/intel_guc_log.c > @@ -39,7 +39,7 @@ > * registers value. > */ > > -static int guc_log_flush_complete(struct intel_guc *guc) > +static int guc_action_flush_log_complete(struct intel_guc *guc) > { > u32 action[] = { > INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE > @@ -48,7 +48,7 @@ static int guc_log_flush_complete(struct intel_guc *guc) > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > > -static int guc_log_flush(struct intel_guc *guc) > +static int guc_action_flush_log(struct intel_guc *guc) > { > u32 action[] = { > INTEL_GUC_ACTION_FORCE_LOG_BUFFER_FLUSH, > @@ -58,7 +58,8 @@ static int guc_log_flush(struct intel_guc *guc) > return intel_guc_send(guc, action, ARRAY_SIZE(action)); > } > > -static int guc_log_control(struct intel_guc *guc, bool enable, u32 verbosity) > +static int guc_action_enable_log(struct intel_guc *guc, bool enable, > + u32 verbosity) Let's hide the fact that the actual action is called "ENABLE_LOGGING", and stick with guc_action_log_control, especially since we're using guc_log_control union, and the action itself is also used for verbosity (and default log... more than just enable/disable switch). With that: Reviewed-by: Michał Winiarski -Michał > { > union guc_log_control control_val = { > { > @@ -525,7 +526,7 @@ static void guc_log_capture_logs(struct intel_guc *guc) >* time, so get/put should be really quick. >*/ > intel_runtime_pm_get(dev_priv); > - guc_log_flush_complete(guc); > + guc_action_flush_log_complete(guc); > intel_runtime_pm_put(dev_priv); > } > > @@ -541,7 +542,7 @@ static void guc_flush_logs(struct intel_guc *guc) > > /* Ask GuC to update the log buffer state */ > intel_runtime_pm_get(dev_priv); > - guc_log_flush(guc); > + guc_action_flush_log(guc); > intel_runtime_pm_put(dev_priv); > > /* GuC would have updated log buffer by now, so capture it */ > @@ -639,10 +640,11 @@ int intel_guc_log_control_set(struct intel_guc *guc, > u64 val) > } > > intel_runtime_pm_get(dev_priv); > - ret = guc_log_control(guc, enabled, LOG_LEVEL_TO_VERBOSITY(val)); > + ret = guc_action_enable_log(guc, enabled, LOG_LEVEL_TO_VERBOSITY(val)); > intel_runtime_pm_put(dev_priv); > if (ret) { > - DRM_DEBUG_DRIVER("guc_log_control action failed %d\n", ret); > + DRM_DEBUG_DRIVER("GuC action to %s log failed (%d)\n", > + enabled ? "enable" : "disable", ret); > goto out_unlock; > } > > -- > 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: Stop engines when declaring the machine wedged
Chris Wilsonwrites: > If we fail to reset the GPU, we declare the machine wedged. However, the > GPU may well still be running in the background with an in-flight > request. So despite our efforts in cleaning up the request queue and > faking the breadcrumb in the HWSP, the GPU may eventually write the > in-flght seqno there breaking all of our assumptions and throwing the > driver into a deep turmoil, wedging beyond wedged. > > To avoid this we ideally want to reset the GPU. Since that has already > failed, make sure the rings have the stop bit set instead. This is part > of the normal GPU reset sequence, but that is actually disabled by > igt/gem_eio to force the wedged state. If we assume the worst, we must > poke at the bit again before we give up. I am pondering if you would save so much trouble by just adding int intel_get_gpu_reset(struct drm_i915_private *dev_priv, bool ignore_modparam) and then forcing a stop engine and the last reset straight in wedging -Mika > > Testcase: igt/gem_eio > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Cc: Michał Winiarski > Cc: Michal Wajdeczko > Cc: Michel Thierry > --- > git add fail > We want to stop the tasklets before hitting stop-engines or else we > may trigger an early CS completion and more asserts. > -Chris > --- > drivers/gpu/drm/i915/i915_gem.c | 2 ++ > drivers/gpu/drm/i915/intel_engine_cs.c | 43 ++ > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++ > drivers/gpu/drm/i915/intel_uncore.c | 46 > + > 4 files changed, 48 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 2fbd622bba30..208619981ea1 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3246,6 +3246,8 @@ void i915_gem_set_wedged(struct drm_i915_private *i915) > } > i915->caps.scheduler = 0; > > + intel_engines_stop(i915, ALL_ENGINES); > + > /* >* Make sure no one is running the old callback before we proceed with >* cancelling requests and resetting the completion tracking. Otherwise > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c > b/drivers/gpu/drm/i915/intel_engine_cs.c > index 337dfa56a738..44eddf10f4d1 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -1727,6 +1727,49 @@ unsigned int > intel_engines_has_context_isolation(struct drm_i915_private *i915) > return which; > } > > +static void gen3_stop_engine(struct intel_engine_cs *engine) > +{ > + struct drm_i915_private *dev_priv = engine->i915; > + const u32 base = engine->mmio_base; > + const i915_reg_t mode = RING_MI_MODE(base); > + > + I915_WRITE_FW(mode, _MASKED_BIT_ENABLE(STOP_RING)); > + if (intel_wait_for_register_fw(dev_priv, > +mode, > +MODE_IDLE, > +MODE_IDLE, > +500)) > + DRM_DEBUG_DRIVER("%s: timed out on STOP_RING\n", > + engine->name); > + > + I915_WRITE_FW(RING_HEAD(base), I915_READ_FW(RING_TAIL(base))); > + POSTING_READ_FW(RING_HEAD(base)); /* paranoia */ > + > + I915_WRITE_FW(RING_HEAD(base), 0); > + I915_WRITE_FW(RING_TAIL(base), 0); > + POSTING_READ_FW(RING_TAIL(base)); > + > + /* The ring must be empty before it is disabled */ > + I915_WRITE_FW(RING_CTL(base), 0); > + > + /* Check acts as a post */ > + if (I915_READ_FW(RING_HEAD(base)) != 0) > + DRM_DEBUG_DRIVER("%s: ring head not parked\n", > + engine->name); > +} > + > +void intel_engines_stop(struct drm_i915_private *i915, unsigned engine_mask) > +{ > + struct intel_engine_cs *engine; > + enum intel_engine_id id; > + > + if (INTEL_GEN(i915) < 3) > + return; > + > + for_each_engine_masked(engine, i915, engine_mask, id) > + gen3_stop_engine(engine); > +} > + > static void print_request(struct drm_printer *m, > struct i915_request *rq, > const char *prefix) > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 1f50727a5ddb..1369f7c5b57e 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -1018,6 +1018,8 @@ bool intel_engine_has_kernel_context(const struct > intel_engine_cs *engine); > void intel_engines_park(struct drm_i915_private *i915); > void intel_engines_unpark(struct drm_i915_private *i915); > > +void intel_engines_stop(struct drm_i915_private *i915, unsigned engine_mask); > + > void
Re: [Intel-gfx] [PATCH] drm/i915/guc: Fix build break on config without DEBUG_FS
On Thu, Mar 15, 2018 at 03:28:47PM +, Michal Wajdeczko wrote: > In commit 56b9a8b08387 ("drm/i915/guc: Update syntax of GuC > log functions") we accidentally removed debugfs.h header > where needed stub functions were defined. > > Reported-by: Mike Lothian> Signed-off-by: Michal Wajdeczko > Cc: Mike Lothian > Cc: Chris Wilson Acked-by: Rodrigo Vivi and pushed. thanks for the patch. > --- > drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_guc_log.c > b/drivers/gpu/drm/i915/intel_guc_log.c > index bfb9a68..1c2127b 100644 > --- a/drivers/gpu/drm/i915/intel_guc_log.c > +++ b/drivers/gpu/drm/i915/intel_guc_log.c > @@ -22,6 +22,8 @@ > * > */ > > +#include > + > #include "intel_guc_log.h" > #include "i915_drv.h" > > -- > 1.9.1 > > ___ > 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
[Intel-gfx] [PATCH i-g-t v2] tests/perf_pmu: Improve accuracy by waiting on spinner to start
From: Tvrtko UrsulinMore than one test assumes that the spinner is running pretty much immediately after we have create or submitted it. In actuality there is a variable delay, especially on execlists platforms, between submission and spin batch starting to run on the hardware. To enable tests which care about this level of timing to account for this, we add a new spin batch constructor which provides an output field which can be polled to determine when the batch actually started running. This is implemented via MI_STOREDW_IMM from the spin batch, writing into memory mapped page shared with userspace. Using this facility from perf_pmu, where applicable, should improve very occasional test fails across the set and platforms. v2: Chris Wilson: * Use caching mapping if available. * Handle old gens better. * Use gem_can_store_dword. * Cache exec obj array in spin_batch_t for easier resubmit. Signed-off-by: Tvrtko Ursulin Suggested-by: Chris Wilson --- lib/igt_dummyload.c | 149 +++ lib/igt_dummyload.h | 11 lib/ioctl_wrappers.c | 2 +- lib/ioctl_wrappers.h | 1 + tests/perf_pmu.c | 144 ++--- 5 files changed, 230 insertions(+), 77 deletions(-) diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c index 4b20f23dfe26..83b9727499ff 100644 --- a/lib/igt_dummyload.c +++ b/lib/igt_dummyload.c @@ -74,16 +74,19 @@ fill_reloc(struct drm_i915_gem_relocation_entry *reloc, reloc->write_domain = write_domains; } -static int emit_recursive_batch(igt_spin_t *spin, - int fd, uint32_t ctx, unsigned engine, - uint32_t dep, bool out_fence) +#define OUT_FENCE (1 << 0) +#define POLL_RUN (1 << 1) + +static int +emit_recursive_batch(igt_spin_t *spin, int fd, uint32_t ctx, unsigned engine, +uint32_t dep, unsigned int flags) { #define SCRATCH 0 #define BATCH 1 const int gen = intel_gen(intel_get_drm_devid(fd)); - struct drm_i915_gem_exec_object2 obj[2]; struct drm_i915_gem_relocation_entry relocs[2]; - struct drm_i915_gem_execbuffer2 execbuf; + struct drm_i915_gem_execbuffer2 *execbuf; + struct drm_i915_gem_exec_object2 *obj; unsigned int engines[16]; unsigned int nengine; int fence_fd = -1; @@ -101,8 +104,10 @@ static int emit_recursive_batch(igt_spin_t *spin, } igt_require(nengine); - memset(, 0, sizeof(execbuf)); - memset(obj, 0, sizeof(obj)); + memset(>execbuf, 0, sizeof(spin->execbuf)); + execbuf = >execbuf; + memset(spin->obj, 0, sizeof(spin->obj)); + obj = spin->obj; memset(relocs, 0, sizeof(relocs)); obj[BATCH].handle = gem_create(fd, BATCH_SIZE); @@ -113,16 +118,62 @@ static int emit_recursive_batch(igt_spin_t *spin, BATCH_SIZE, PROT_WRITE); gem_set_domain(fd, obj[BATCH].handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT); - execbuf.buffer_count++; + execbuf->buffer_count++; if (dep) { + igt_assert(!(flags & POLL_RUN)); + /* dummy write to dependency */ obj[SCRATCH].handle = dep; fill_reloc([obj[BATCH].relocation_count++], dep, 1020, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER); - execbuf.buffer_count++; + execbuf->buffer_count++; + } else if (flags & POLL_RUN) { + unsigned int offset; + + igt_assert(!dep); + + if (gen == 4 || gen == 5) + execbuf->flags |= I915_EXEC_SECURE; + + spin->poll_handle = gem_create(fd, 4096); + + if (__gem_set_caching(fd, spin->poll_handle, + I915_CACHING_CACHED) == 0) + spin->running = __gem_mmap__cpu(fd, spin->poll_handle, + 0, 4096, + PROT_READ | PROT_WRITE); + else + spin->running = __gem_mmap__wc(fd, spin->poll_handle, + 0, 4096, + PROT_READ | PROT_WRITE); + igt_assert(spin->running); + igt_assert_eq(*spin->running, 0); + + *batch++ = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0); + + if (gen >= 8) { + offset = sizeof(uint32_t); + *batch++ = 0; + *batch++ = 0; + } else if (gen >= 4) { + offset = 2 * sizeof(uint32_t); +
[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/6] Revert "drm: Use a flexible array member for blob property data" (rev3)
== Series Details == Series: series starting with [1/6] Revert "drm: Use a flexible array member for blob property data" (rev3) URL : https://patchwork.freedesktop.org/series/38886/ State : failure == Summary == Applying: Revert "drm: Use a flexible array member for blob property data" Applying: drm: Remove now pointelss blob->data casts Applying: drm: Verify gamma/degamma LUT size Applying: drm: Introduce drm_color_lut_size() Applying: drm/i915: Remove the blob->data casts error: Failed to merge in the changes. Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/intel_color.c Falling back to patching base and 3-way merge... Auto-merging drivers/gpu/drm/i915/intel_color.c CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_color.c Patch failed at 0005 drm/i915: Remove the blob->data casts The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ 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 [1/2] drm/i915: Trace GEM steps between submit and wedging (rev3)
== Series Details == Series: series starting with [1/2] drm/i915: Trace GEM steps between submit and wedging (rev3) URL : https://patchwork.freedesktop.org/series/40029/ State : success == Summary == Series 40029v3 series starting with [1/2] drm/i915: Trace GEM steps between submit and wedging https://patchwork.freedesktop.org/api/1.0/series/40029/revisions/3/mbox/ Known issues: Test gem_exec_suspend: Subgroup basic-s3: pass -> DMESG-WARN (fi-elk-e7500) fdo#103989 Subgroup basic-s4-devices: pass -> INCOMPLETE (fi-bdw-5557u) fdo#104162 fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989 fdo#104162 https://bugs.freedesktop.org/show_bug.cgi?id=104162 fi-bdw-5557u total:109 pass:105 dwarn:0 dfail:0 fail:0 skip:3 fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:441s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:381s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:540s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:299s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:506s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:515s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:502s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:409s fi-cfl-s2total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:578s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:510s fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:544s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:581s fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:426s fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:316s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:541s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:402s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:427s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:464s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:426s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:482s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:473s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:513s fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:654s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:440s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:527s fi-skl-6700hqtotal:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:549s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:498s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:500s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:427s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:443s fi-snb-2520m total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:579s fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:404s 95626f201526d97b6f284b397fd73c8a2d514ff4 drm-tip: 2018y-03m-15d-11h-01m-14s UTC integration manifest 54a90b07 drm/i915: Stop engines when declaring the machine wedged 62012cc290af drm/i915: Trace GEM steps between submit and wedging == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8366/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
On Thu, 2018-03-15 at 17:37 +0200, Ville Syrjälä wrote: > On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote: > > On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote: > > > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote: > > > > Op 15-03-18 om 14:30 schreef Ville Syrjälä: > > > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote: > > > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary > > > > > > arguments that can be removed by creating separate functins. > > > > > > > > > > > > Create specific functions for these calls to reduce x86/64 defconfig > > > > > > size by ~20k. > > > > > > > > > > > > Modify the existing macros to use the specific calls. > > > > > > > > > > > > new: > > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > > > > > 1876562 44542 995 1922099 1d5433 (TOTALS) > > > > > > > > > > > > old: > > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > > > > > 1897565 44542 995 1943102 1da63e (TOTALS) > > > > > > > > > > > > Miscellanea: > > > > > > > > > > > > o intel_display requires a change to use the specific calls. > > > > > > > > > > How much would we lose if we move the (drm_debug) outside the > > > > > functions again? > > > > again? > > We used to do that. Someone changed it a while back, unintentially > I believe. > > > > > > > > I'm somewhat concerned about all the function call > > > > > overhead when debugs aren't even enabled. > > > > Perhaps better to have compilation elimination > > of the entire debug output instead. > > That would require every bug reporter to recompile the kernel first. > So this is not a solution we would ever seriously consider. > > Not sure if it would be possible to use the alternatives thing to > eliminate the function calls unless the user boots wih drm.debug!=0? > > > > > I think you are discussing a different issue and > > this discussion should not block this patch as > > this patch has no impact other than code size > > reduction. > > But what is the goal of the code size reduction? Smaller code. > I assume the main > goal is to make better use of the instruction cache to make the > code faster. If there's a tradeoff between smaller and slightly > faster vs. larger and a singificantly faster I tend to think we > should go for the latter option. There's no trade-off in this patch for faster/larger. This patch is simply smaller. Smaller is better. Your faster/larger should be a different patch proposal. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote: > On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote: > > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote: > > > Op 15-03-18 om 14:30 schreef Ville Syrjälä: > > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote: > > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary > > > > > arguments that can be removed by creating separate functins. > > > > > > > > > > Create specific functions for these calls to reduce x86/64 defconfig > > > > > size by ~20k. > > > > > > > > > > Modify the existing macros to use the specific calls. > > > > > > > > > > new: > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > > > > 1876562 44542 995 1922099 1d5433 (TOTALS) > > > > > > > > > > old: > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > > > > 1897565 44542 995 1943102 1da63e (TOTALS) > > > > > > > > > > Miscellanea: > > > > > > > > > > o intel_display requires a change to use the specific calls. > > > > > > > > How much would we lose if we move the (drm_debug) outside the > > > > functions again? > > again? We used to do that. Someone changed it a while back, unintentially I believe. > > > > > I'm somewhat concerned about all the function call > > > > overhead when debugs aren't even enabled. > > Perhaps better to have compilation elimination > of the entire debug output instead. That would require every bug reporter to recompile the kernel first. So this is not a solution we would ever seriously consider. Not sure if it would be possible to use the alternatives thing to eliminate the function calls unless the user boots wih drm.debug!=0? > > I think you are discussing a different issue and > this discussion should not block this patch as > this patch has no impact other than code size > reduction. But what is the goal of the code size reduction? I assume the main goal is to make better use of the instruction cache to make the code faster. If there's a tradeoff between smaller and slightly faster vs. larger and a singificantly faster I tend to think we should go for the latter option. -- 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] [maintainer-tools PATCH] doc: how to become a drm-intel committer
On Thu, 15 Mar 2018, Daniel Vetterwrote: > On Wed, Mar 14, 2018 at 05:11:02PM +0200, Jani Nikula wrote: >> Until now, the drm-intel commit access have been handed out ad hoc, >> without transparency, consistency, or fairness. With pressure to add >> more committers, this is no longer tenable, if it ever was. Document the >> requirements and expectations around becoming a drm-intel committer. >> >> The Linux kernel operates in a model where, by and large, only >> maintainers commit patches. Maintainer teams are no longer rare, but the >> drm-intel and drm-misc maintainer/committer model is definitely an >> outlier. >> >> The drm-intel maintainers believe that a reasonable level of experience >> and track record of working on the driver, as well as actively engaging >> in the community upstream, are necessary before becoming a >> committer. While the requirements outlined here may seem strict in >> contrast with many projects, they are extremely liberal by kernel >> standards. >> >> Finally, no rules are carved in stone. We fully expect the requirements >> to be adjusted later. However, it will be much easier to start strict >> and relax the requirements later than the other way round. >> >> Cc: Gustavo Padovan >> Cc: Maarten Lankhorst >> Cc: Sean Paul >> Cc: Dave Airlie >> Cc: dim-to...@lists.freedesktop.org >> Cc: dri-de...@lists.freedesktop.org >> Cc: intel-gfx@lists.freedesktop.org >> Signed-off-by: Jani Nikula >> Signed-off-by: Joonas Lahtinen >> Signed-off-by: Rodrigo Vivi > > I've chatted for a few hours with Joonas, and I think before we can > discuss the proposed document here itself, we first need to reach some > agreement on why we even have commit rights. I think there's a very wide > range of answers to that questions, and of course with different goals you > end up with completely different rules about how to handle commit rights. > > Joonas suggested we first discuss this internally, perhaps with the > maintainers, Kimmo and me. Fine. I would rather have discussed this transparently out in the open. That was, after all, the purpose of sending this out. BR, Jani. -- 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] drm/i915/guc: Fix build break on config without DEBUG_FS
In commit 56b9a8b08387 ("drm/i915/guc: Update syntax of GuC log functions") we accidentally removed debugfs.h header where needed stub functions were defined. Reported-by: Mike LothianSigned-off-by: Michal Wajdeczko Cc: Mike Lothian Cc: Chris Wilson --- drivers/gpu/drm/i915/intel_guc_log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c index bfb9a68..1c2127b 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -22,6 +22,8 @@ * */ +#include + #include "intel_guc_log.h" #include "i915_drv.h" -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] sna/uxa: Fix colormap handling at screen depth 30.
Quoting Ville Syrjälä (2018-03-01 11:12:53) > On Thu, Mar 01, 2018 at 02:20:48AM +0100, Mario Kleiner wrote: > > The various clut handling functions like a setup > > consistent with the x-screen color depth. Otherwise > > we observe improper sampling in the gamma tables > > at depth 30. > > > > Therefore replace hard-coded bitsPerRGB = 8 by actual > > bits per channel scrn->rgbBits. Also use this for call > > to xf86HandleColormaps(). > > > > Tested for uxa and sna at depths 8, 16, 24 and 30 on > > IvyBridge, and tested at depth 24 and 30 that xgamma > > and gamma table animations work, and with measurement > > equipment to make sure identity gamma ramps actually > > are identity mappings at the output. > > You mean identity mapping at 8bpc? We don't support higher precision > gamma on pre-bdw atm, and the ddx doesn't use the higher precision > stuff even on bdw+. I'm working on fixing both, but it turned out to > be a bit more work than I anticipated so will take a while. > > > > > Signed-off-by: Mario Kleiner> > --- > > src/sna/sna_driver.c | 5 +++-- > > src/uxa/intel_driver.c | 3 ++- > > 2 files changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/src/sna/sna_driver.c b/src/sna/sna_driver.c > > index 2643e6c..9c4bcd4 100644 > > --- a/src/sna/sna_driver.c > > +++ b/src/sna/sna_driver.c > > @@ -1145,7 +1145,7 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL) > > if (!miInitVisuals(, , , , , > > , > > ((unsigned long)1 << (scrn->bitsPerPixel - 1)), > > -8, -1)) > > +scrn->rgbBits, -1)) > > return FALSE; > > > > if (!miScreenInit(screen, NULL, > > @@ -1217,7 +1217,8 @@ sna_screen_init(SCREEN_INIT_ARGS_DECL) > > return FALSE; > > > > if (sna->mode.num_real_crtc && > > - !xf86HandleColormaps(screen, 256, 8, sna_load_palette, NULL, > > + !xf86HandleColormaps(screen, 1 << scrn->rgbBits, scrn->rgbBits, > > + sna_load_palette, NULL, > >CMAP_RELOAD_ON_MODE_SWITCH | > >CMAP_PALETTED_TRUECOLOR)) > > I already forgot what this does prior to your randr fix. IIRC bumping > the 8 alone would cause the thing to segfault, but I guess bumping both > was fine? > > Hmm. So the server always initializes crtc->gamma_size to 256 > (which does match the normal hw LUT size), and so before your > fix we will always get gamma_slots==0 at >8bpc and so the hw LUT > is never actually updated? Was there any conclusion to this? Aiui, these bits should be set based on the underlying HW gamma table depth which is not the same as the screen colour depth. It stuck at 256 for ages as that is all anyone ever expected... -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/6] drm: Introduce drm_color_lut_size()
From: Ville SyrjäläProvide a small helper to convert the blob length in bytes to the number of LUT entries. v2: Add kerneldoc (Daniel) Cc: Daniel Vetter Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter --- include/drm/drm_color_mgmt.h | 12 1 file changed, 12 insertions(+) diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index b3b6d302ca8c..44f04233e3db 100644 --- a/include/drm/drm_color_mgmt.h +++ b/include/drm/drm_color_mgmt.h @@ -38,6 +38,18 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc, int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc, int gamma_size); +/** + * drm_color_lut_size - calculate the number of entries in the LUT + * @blob: blob containing the LUT + * + * Returns: + * The number of entries in the color LUT stored in @blob. + */ +static inline int drm_color_lut_size(const struct drm_property_blob *blob) +{ + return blob->length / sizeof(struct drm_color_lut); +} + enum drm_color_encoding { DRM_COLOR_YCBCR_BT601, DRM_COLOR_YCBCR_BT709, -- 2.16.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/6] drm: Verify gamma/degamma LUT size
From: Ville SyrjäläWhile we want to potentially support multiple different gamma/degamma LUT sizes we can (and should) at least check that the blob length is a multiple of the LUT entry size. v2: s/expected_size_mod/expected_elem_size/ (Daniel) Add kernel doc (Daniel) Cc: Daniel Vetter Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter --- Documentation/gpu/drm-kms.rst | 3 +++ drivers/gpu/drm/drm_atomic.c | 39 +++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 56a3780e39b8..1dffd1ac4cd4 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -286,6 +286,9 @@ Atomic Mode Setting Function Reference .. kernel-doc:: drivers/gpu/drm/drm_atomic.c :export: +.. kernel-doc:: drivers/gpu/drm/drm_atomic.c + :internal: + CRTC Abstraction diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 01060669dd49..a55f611d2a6f 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -408,11 +408,36 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, } EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc); +/** + * drm_atomic_replace_property_blob_from_id - lookup the new blob and replace the old one with it + * @dev: DRM device + * @blob: a pointer to the member blob to be replaced + * @blob_id: ID of the new blob + * @expected_size: total expected size of the blob data (in bytes) + * @expected_elem_size: expected element size of the blob data (in bytes) + * @replaced: did the blob get replaced? + * + * Replace @blob with another blob with the ID @blob_id. If @blob_id is zero + * @blob becomes NULL. + * + * If @expected_size is positive the new blob length is expected to be equal + * to @expected_size bytes. If @expected_elem_size is positive the new blob + * length is expected to be a multiple of @expected_elem_size bytes. Otherwise + * an error is returned. + * + * @replaced will indicate to the caller whether the blob was replaced or not. + * If the old and new blobs we in fact the same blob @replaced will be false + * otherwise it will be true. + * + * RETURNS: + * Zero on success, error code on failure. + */ static int drm_atomic_replace_property_blob_from_id(struct drm_device *dev, struct drm_property_blob **blob, uint64_t blob_id, ssize_t expected_size, +ssize_t expected_elem_size, bool *replaced) { struct drm_property_blob *new_blob = NULL; @@ -422,7 +447,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev, if (new_blob == NULL) return -EINVAL; - if (expected_size > 0 && expected_size != new_blob->length) { + if (expected_size > 0 && + new_blob->length != expected_size) { + drm_property_blob_put(new_blob); + return -EINVAL; + } + if (expected_elem_size > 0 && + new_blob->length % expected_elem_size != 0) { drm_property_blob_put(new_blob); return -EINVAL; } @@ -470,7 +501,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, ret = drm_atomic_replace_property_blob_from_id(dev, >degamma_lut, val, - -1, + -1, sizeof(struct drm_color_lut), ); state->color_mgmt_changed |= replaced; return ret; @@ -478,7 +509,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, ret = drm_atomic_replace_property_blob_from_id(dev, >ctm, val, - sizeof(struct drm_color_ctm), + sizeof(struct drm_color_ctm), -1, ); state->color_mgmt_changed |= replaced; return ret; @@ -486,7 +517,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, ret = drm_atomic_replace_property_blob_from_id(dev, >gamma_lut, val, - -1, + -1, sizeof(struct drm_color_lut), ); state->color_mgmt_changed |= replaced;
Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote: > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote: > > Op 15-03-18 om 14:30 schreef Ville Syrjälä: > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote: > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary > > > > arguments that can be removed by creating separate functins. > > > > > > > > Create specific functions for these calls to reduce x86/64 defconfig > > > > size by ~20k. > > > > > > > > Modify the existing macros to use the specific calls. > > > > > > > > new: > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > > > 1876562 44542 995 1922099 1d5433 (TOTALS) > > > > > > > > old: > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > > > 1897565 44542 995 1943102 1da63e (TOTALS) > > > > > > > > Miscellanea: > > > > > > > > o intel_display requires a change to use the specific calls. > > > > > > How much would we lose if we move the (drm_debug) outside the > > > functions again? again? > > > I'm somewhat concerned about all the function call > > > overhead when debugs aren't even enabled. Perhaps better to have compilation elimination of the entire debug output instead. I think you are discussing a different issue and this discussion should not block this patch as this patch has no impact other than code size reduction. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Stop engines when declaring the machine wedged
If we fail to reset the GPU, we declare the machine wedged. However, the GPU may well still be running in the background with an in-flight request. So despite our efforts in cleaning up the request queue and faking the breadcrumb in the HWSP, the GPU may eventually write the in-flght seqno there breaking all of our assumptions and throwing the driver into a deep turmoil, wedging beyond wedged. To avoid this we ideally want to reset the GPU. Since that has already failed, make sure the rings have the stop bit set instead. This is part of the normal GPU reset sequence, but that is actually disabled by igt/gem_eio to force the wedged state. If we assume the worst, we must poke at the bit again before we give up. v2: Move the intel_gpu_reset() from set-wedged in the reset error path into i915_gem_set_wedged() itself. Even if the reset fails (e.g. if it is disabled by gem_eio), it still tries to make sure the engines are stopped. For i915_gem_set_wedged() callers from outside of i915_reset(), this should make sure the GPU is disabled while the driver is marked as being wedged. Testcase: igt/gem_eio Signed-off-by: Chris WilsonCc: Mika Kuoppala Cc: Michał Winiarski Cc: Michal Wajdeczko Cc: Michel Thierry --- drivers/gpu/drm/i915/i915_drv.c | 1 - drivers/gpu/drm/i915/i915_gem.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f03555efc520..3df5193487f3 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1995,7 +1995,6 @@ void i915_reset(struct drm_i915_private *i915, unsigned int flags) error: i915_gem_set_wedged(i915); i915_retire_requests(i915); - intel_gpu_reset(i915, ALL_ENGINES); goto finish; } diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 2fbd622bba30..802df8e1a544 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3246,6 +3246,9 @@ void i915_gem_set_wedged(struct drm_i915_private *i915) } i915->caps.scheduler = 0; + /* Even if the GPU reset fails, it should still stop the engines */ + intel_gpu_reset(i915, ALL_ENGINES); + /* * Make sure no one is running the old callback before we proceed with * cancelling requests and resetting the completion tracking. Otherwise -- 2.16.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote: > Op 15-03-18 om 14:30 schreef Ville Syrjälä: > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote: > >> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary > >> arguments that can be removed by creating separate functins. > >> > >> Create specific functions for these calls to reduce x86/64 defconfig > >> size by ~20k. > >> > >> Modify the existing macros to use the specific calls. > >> > >> new: > >> $ size -t drivers/gpu/drm/built-in.a | tail -1 > >> 1876562 44542 995 1922099 1d5433 (TOTALS) > >> > >> old: > >> $ size -t drivers/gpu/drm/built-in.a | tail -1 > >> 1897565 44542 995 1943102 1da63e (TOTALS) > >> > >> Miscellanea: > >> > >> o intel_display requires a change to use the specific calls. > > How much would we lose if we move the (drm_debug) outside the > > functions again? I'm somewhat concerned about all the function call > > overhead when debugs aren't even enabled. > > Upstream: >textdata bss dec hex filename > 37714356894352 387184 5e870 drivers/gpu/drm/drm.ko > > With this patch: > 37383156894352 383872 5db80 drivers/gpu/drm/drm.ko > > Moving the if outside (below): > 37762956894352 387670 5ea56 drivers/gpu/drm/drm.ko > > Bye savings.. > > I don't think there are any places in which the debug output is performance > sensitive, > so I'm ok with not inlining. Not performance sensitive as such perhaps. But pointlessly wasting cpu cycles for nop function calls isn't particularly great. Would be nice to actually measure how much overhead there is on some weaker systems. IIRC older Atoms were particularly bad at this stuff. > --- > diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c > index 79abf6d5b4db..928822403a59 100644 > --- a/drivers/gpu/drm/drm_print.c > +++ b/drivers/gpu/drm/drm_print.c > @@ -89,14 +89,11 @@ void drm_dev_printk(const struct device *dev, const char > *level, > } > EXPORT_SYMBOL(drm_dev_printk); > > -void drm_dbg(unsigned int category, const char *format, ...) > +void __drm_dbg(const char *format, ...) > { > struct va_format vaf; > va_list args; > > - if (!(drm_debug & category)) > - return; > - > va_start(args, format); > vaf.fmt = format; > vaf.va = > @@ -106,7 +103,7 @@ void drm_dbg(unsigned int category, const char *format, > ...) > > va_end(args); > } > -EXPORT_SYMBOL(drm_dbg); > +EXPORT_SYMBOL(__drm_dbg); > > void drm_err(const char *format, ...) > { > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h > index 3a40c5a3a5fa..2a145b97bdfc 100644 > --- a/include/drm/drm_print.h > +++ b/include/drm/drm_print.h > @@ -200,8 +200,17 @@ __printf(6, 7) > void drm_dev_printk(const struct device *dev, const char *level, > unsigned int category, const char *function_name, > const char *prefix, const char *format, ...); > -__printf(2, 3) > -void drm_dbg(unsigned int category, const char *format, ...); > + > +__printf(1, 2) > +void __drm_dbg(const char *format, ...); > + > + > +#define drm_dbg(category, format, ...) \ > + do {\ > + if (drm_debug & category) \ > + __drm_dbg(format, ## __VA_ARGS__); \ > + } while (0) > + > __printf(1, 2) > void drm_err(const char *format, ...); > -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for Non-Intel test suite fixes
== Series Details == Series: Non-Intel test suite fixes URL : https://patchwork.freedesktop.org/series/40038/ State : failure == Summary == Applying: tests/kms_addfb_basic: skip i915-specific tests on other platforms Applying: tests/kms_panel_fitting: check for i915 before checking version Applying: lib/igt_gt: has_gpu_reset(): fix failed assertion on non-i915 platforms Applying: lib/igt_gt: check for presence of GPU reset before using it Patch failed at 0004 lib/igt_gt: check for presence of GPU reset before using it The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/perf_pmu: Improve accuracy by waiting on spinner to start
Quoting Tvrtko Ursulin (2018-03-15 14:53:08) > > On 15/03/2018 14:46, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-03-15 14:37:59) > >> > >> On 15/03/2018 13:45, Chris Wilson wrote: > >>> As we are making changes to igt_spin_t, one of the ideas was that we put > >>> the obj[] array there (with the offsets and flags setup correctly) so > >>> that we could just feed that in again later without having to worry > >>> about the relocations. > >> > >> I tried that before but we couldn't agree on resubmit semantics. > >> > >> My patch had igt_spin_batch_restart(fd, spin) - so emitting the exact > >> same batch, including the dependency. That would actually work well for > >> this use case. > >> > >> So if you are happy with that, I can resurrect that patch, add one more > >> to implement stuff from this patch, and rebase perf_pmu changes to follow. > > > > Honestly, best to do here first, as we will probably take forever to come > > up with something we both like and applies to more test cases. > > Don't quite get what you mean by "best to do here first" - where is > here? Fix perf_pmu, then worry about API. We're still waiting for kasan results, we may have more work ahead of us yet. > Still locally to perf_pmu so no generic spin batch resubmit yet? > But I can cache the obj array under spin_batch_t as a shortcut for time > being? I'd take igt_spin_t.obj[] :) But I don't insist on it, I'd like to get the wait-for-spin working before tackling the resubmit API. There's a few other places that either have a open-coded wait-for-submit, or need one. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/perf_pmu: Improve accuracy by waiting on spinner to start
On 15/03/2018 14:46, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-03-15 14:37:59) On 15/03/2018 13:45, Chris Wilson wrote: As we are making changes to igt_spin_t, one of the ideas was that we put the obj[] array there (with the offsets and flags setup correctly) so that we could just feed that in again later without having to worry about the relocations. I tried that before but we couldn't agree on resubmit semantics. My patch had igt_spin_batch_restart(fd, spin) - so emitting the exact same batch, including the dependency. That would actually work well for this use case. So if you are happy with that, I can resurrect that patch, add one more to implement stuff from this patch, and rebase perf_pmu changes to follow. Honestly, best to do here first, as we will probably take forever to come up with something we both like and applies to more test cases. Don't quite get what you mean by "best to do here first" - where is here? Still locally to perf_pmu so no generic spin batch resubmit yet? But I can cache the obj array under spin_batch_t as a shortcut for time being? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
On Thu, 2018-03-15 at 14:22 +0100, Maarten Lankhorst wrote: > Op 13-03-18 om 23:02 schreef Joe Perches: > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary > > arguments that can be removed by creating separate functins. > > > > Create specific functions for these calls to reduce x86/64 defconfig > > size by ~20k. > > > > Modify the existing macros to use the specific calls. > > > > new: > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > 1876562 44542 995 1922099 1d5433 (TOTALS) > > > > old: > > $ size -t drivers/gpu/drm/built-in.a | tail -1 > > 1897565 44542 995 1943102 1da63e (TOTALS) [] > I guess this adds up. Nice reduction. :) Yup. 1% of all drm object code. > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c [] > > > > - drm_printk(level, category, "mismatch in %s %pV", name, ); > > + if (adjust) > > + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, ); > > + else > > + drm_err("mismatch in %s %pV", name, ); > > Could this use DRM_DEBUG_KMS/DRM_ERROR? > > Rest looks good, so I can fix up if you want. If want you change something like that, it should be separate patch. btw: There was separate patch that also reduced object size of the drm_dev_printk calls several months ago. Never applied. https://lkml.org/lkml/2017/9/25/247 cheers, Joe ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt 8/8] test/kms_addfb_basic: tolerate absence of 8-bit format
Ignores failure to add DRM_FORMAT_C8 frame buffer; some devices do not support any 8-bit format. Signed-off-by: Ulrich Hecht--- tests/kms_addfb_basic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c index d1da718..db79827 100644 --- a/tests/kms_addfb_basic.c +++ b/tests/kms_addfb_basic.c @@ -273,8 +273,8 @@ static void size_tests(int fd) igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, _16) == 0); igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, _16.fb_id) == 0); f.fb_id = 0; - igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, _8) == 0); - igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, _8.fb_id) == 0); + if (drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, _8) == 0) + igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, _8.fb_id) == 0); f.fb_id = 0; } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt 1/8] tests/kms_addfb_basic: skip i915-specific tests on other platforms
Add is_i915_device() requirement to tests using Intel-specific APIs. Signed-off-by: Ulrich Hecht--- tests/kms_addfb_basic.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c index 7d8852f..cf9ba37 100644 --- a/tests/kms_addfb_basic.c +++ b/tests/kms_addfb_basic.c @@ -104,6 +104,7 @@ static void invalid_tests(int fd) } igt_subtest("clobberred-modifier") { + igt_require(is_i915_device(fd)); f.flags = 0; f.modifier[0] = 0; gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4); @@ -318,6 +319,7 @@ static void size_tests(int fd) } igt_subtest("bo-too-small-due-to-tiling") { + igt_require(is_i915_device(fd)); gem_set_tiling(fd, gem_bo_small, I915_TILING_X, 1024*4); igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, ) == -1 && errno == EINVAL); @@ -369,6 +371,7 @@ static void addfb25_tests(int fd) igt_subtest_group { igt_fixture { + igt_require(is_i915_device(fd)); gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4); igt_require_fb_modifiers(fd); } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt 3/8] lib/igt_gt: has_gpu_reset(): fix failed assertion on non-i915 platforms
Checks if we have an i915 device before using intel_get_drm_devid(). Signed-off-by: Ulrich Hecht--- lib/igt_gt.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/igt_gt.c b/lib/igt_gt.c index e630550..9cb07c2 100644 --- a/lib/igt_gt.c +++ b/lib/igt_gt.c @@ -59,14 +59,17 @@ static bool has_gpu_reset(int fd) struct drm_i915_getparam gp; int val = 0; - memset(, 0, sizeof(gp)); - gp.param = 35; /* HAS_GPU_RESET */ - gp.value = - - if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, )) - once = intel_gen(intel_get_drm_devid(fd)) >= 5; - else - once = val > 0; + if (is_i915_device(fd)) { + memset(, 0, sizeof(gp)); + gp.param = 35; /* HAS_GPU_RESET */ + gp.value = + + if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, )) + once = intel_gen(intel_get_drm_devid(fd)) >= 5; + else + once = val > 0; + } else + once = 0; } return once; } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt 0/8] Non-Intel test suite fixes
Hi! I have run the tests on a Renesas R-Car M3-W's DU device, and have found a number of false negatives that mostly stem from use of Intel-specifics without checking if that makes sense first. So here's a bunch of fixes for those, hope they are generic enough for upstreaming. CU Uli Ulrich Hecht (8): tests/kms_addfb_basic: skip i915-specific tests on other platforms tests/kms_panel_fitting: check for i915 before checking version lib/igt_gt: has_gpu_reset(): fix failed assertion on non-i915 platforms lib/igt_gt: check for presence of GPU reset before using it tests/kms_plane_lowres: skip i915-specific tests on other platforms lib/igt_pm: turn absence of autosuspend_delay_ms from fail to skip tests/kms_addfb_basic: size_tests(): reduce test buffer size test/kms_addfb_basic: tolerate absence of 8-bit format lib/igt_gt.c | 24 ++-- lib/igt_pm.c | 2 +- tests/kms_addfb_basic.c | 33 ++--- tests/kms_panel_fitting.c | 1 + tests/kms_plane_lowres.c | 1 + 5 files changed, 35 insertions(+), 26 deletions(-) -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt 7/8] tests/kms_addfb_basic: size_tests(): reduce test buffer size
Fixes fails on low-memory devices. Signed-off-by: Ulrich Hecht--- tests/kms_addfb_basic.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c index cf9ba37..d1da718 100644 --- a/tests/kms_addfb_basic.c +++ b/tests/kms_addfb_basic.c @@ -238,26 +238,26 @@ static void size_tests(int fd) struct drm_mode_fb_cmd2 f_16 = {}; struct drm_mode_fb_cmd2 f_8 = {}; - f.width = 1024; - f.height = 1024; + f.width = 512; + f.height = 512; f.pixel_format = DRM_FORMAT_XRGB; - f.pitches[0] = 1024*4; + f.pitches[0] = 512*4; - f_16.width = 1024; - f_16.height = 1024*2; + f_16.width = 512; + f_16.height = 512*2; f_16.pixel_format = DRM_FORMAT_RGB565; - f_16.pitches[0] = 1024*2; + f_16.pitches[0] = 512*2; - f_8.width = 1024*2; - f_8.height = 1024*2; + f_8.width = 512*2; + f_8.height = 512*2; f_8.pixel_format = DRM_FORMAT_C8; - f_8.pitches[0] = 1024*2; + f_8.pitches[0] = 512*2; igt_fixture { - gem_bo = igt_create_bo_with_dimensions(fd, 1024, 1024, + gem_bo = igt_create_bo_with_dimensions(fd, 512, 512, DRM_FORMAT_XRGB, 0, 0, NULL, NULL, NULL); igt_assert(gem_bo); - gem_bo_small = igt_create_bo_with_dimensions(fd, 1024, 1023, + gem_bo_small = igt_create_bo_with_dimensions(fd, 512, 511, DRM_FORMAT_XRGB, 0, 0, NULL, NULL, NULL); igt_assert(gem_bo_small); } @@ -311,7 +311,7 @@ static void size_tests(int fd) } /* Just to check that the parameters would work. */ - f.height = 1020; + f.height = 510; igt_subtest("small-bo") { igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, ) == 0); igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, _id) == 0); @@ -320,7 +320,7 @@ static void size_tests(int fd) igt_subtest("bo-too-small-due-to-tiling") { igt_require(is_i915_device(fd)); - gem_set_tiling(fd, gem_bo_small, I915_TILING_X, 1024*4); + gem_set_tiling(fd, gem_bo_small, I915_TILING_X, 512*4); igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, ) == -1 && errno == EINVAL); } -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt 5/8] tests/kms_plane_lowres: skip i915-specific tests on other platforms
Add is_i915_device() requirement to tests using Intel-specific APIs. Signed-off-by: Ulrich Hecht--- tests/kms_plane_lowres.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/kms_plane_lowres.c b/tests/kms_plane_lowres.c index d1e4b3c..8fc7654 100644 --- a/tests/kms_plane_lowres.c +++ b/tests/kms_plane_lowres.c @@ -270,6 +270,7 @@ run_tests_for_pipe(data_t *data, enum pipe pipe) igt_skip_on(pipe >= data->display.n_pipes); igt_display_require_output_on_pipe(>display, pipe); + igt_require(is_i915_device(data->drm_fd)); } igt_subtest_f("pipe-%s-tiling-none", -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/perf_pmu: Improve accuracy by waiting on spinner to start
Quoting Tvrtko Ursulin (2018-03-15 14:37:59) > > On 15/03/2018 13:45, Chris Wilson wrote: > > As we are making changes to igt_spin_t, one of the ideas was that we put > > the obj[] array there (with the offsets and flags setup correctly) so > > that we could just feed that in again later without having to worry > > about the relocations. > > I tried that before but we couldn't agree on resubmit semantics. > > My patch had igt_spin_batch_restart(fd, spin) - so emitting the exact > same batch, including the dependency. That would actually work well for > this use case. > > So if you are happy with that, I can resurrect that patch, add one more > to implement stuff from this patch, and rebase perf_pmu changes to follow. Honestly, best to do here first, as we will probably take forever to come up with something we both like and applies to more test cases. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt 2/8] tests/kms_panel_fitting: check for i915 before checking version
Fixes false negatives on non-i915 platforms. Signed-off-by: Ulrich Hecht--- tests/kms_panel_fitting.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c index b3cee22..6d0be50 100644 --- a/tests/kms_panel_fitting.c +++ b/tests/kms_panel_fitting.c @@ -243,6 +243,7 @@ static void test_atomic_fastset(igt_display_t *display) igt_set_module_param_int("fastboot", 1); igt_require(display->is_atomic); + igt_require(is_i915_device(display->drm_fd)); igt_require(intel_gen(intel_get_drm_devid(display->drm_fd)) >= 5); for_each_pipe_with_valid_output(display, pipe, output) { -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt 4/8] lib/igt_gt: check for presence of GPU reset before using it
Fixes failed assertion on non-i915 devices. Signed-off-by: Ulrich Hecht--- lib/igt_gt.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/igt_gt.c b/lib/igt_gt.c index 9cb07c2..825ea52 100644 --- a/lib/igt_gt.c +++ b/lib/igt_gt.c @@ -166,14 +166,15 @@ igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags) struct drm_i915_gem_context_param param; unsigned ban; + if (!igt_check_boolean_env_var("IGT_HANG_WITHOUT_RESET", false)) + igt_require(has_gpu_reset(fd)); + igt_assert(igt_sysfs_set_parameter (fd, "reset", "%d", INT_MAX /* any reset method */)); if (!igt_check_boolean_env_var("IGT_HANG", true)) igt_skip("hang injection disabled by user"); gem_context_require_bannable(fd); - if (!igt_check_boolean_env_var("IGT_HANG_WITHOUT_RESET", false)) - igt_require(has_gpu_reset(fd)); param.ctx_id = ctx; param.size = 0; -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt 6/8] lib/igt_pm: turn absence of autosuspend_delay_ms from fail to skip
Fixes false negatives on everything that doesn't happen to be at a specific hard-coded sysfs path... Signed-off-by: Ulrich Hecht--- lib/igt_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/igt_pm.c b/lib/igt_pm.c index 5bf5b2e..641157b 100644 --- a/lib/igt_pm.c +++ b/lib/igt_pm.c @@ -262,7 +262,7 @@ bool igt_setup_runtime_pm(void) * suite goes faster and we have a higher probability of triggering race * conditions. */ fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY); - igt_assert_f(fd >= 0, + igt_require_f(fd >= 0, "Can't open " POWER_DIR "/autosuspend_delay_ms\n"); /* If we fail to write to the file, it means this system doesn't support -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/perf_pmu: Improve accuracy by waiting on spinner to start
On 15/03/2018 13:45, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-03-15 13:36:26) On 15/03/2018 13:14, Chris Wilson wrote: Quoting Tvrtko Ursulin (2018-03-15 12:56:17) From: Tvrtko UrsulinMore than one test assumes that the spinner is running pretty much immediately after we have create or submitted it. In actuality there is a variable delay, especially on execlists platforms, between submission and spin batch starting to run on the hardware. To enable tests which care about this level of timing to account for this, we add a new spin batch constructor which provides an output field which can be polled to determine when the batch actually started running. This is implemented via MI_STOREDW_IMM from the spin batch, writing into memory mapped page shared with userspace. Using this facility from perf_pmu, where applicable, should improve very occasional test fails across the set and platforms. Signed-off-by: Tvrtko Ursulin Suggested-by: Chris Wilson --- lib/igt_dummyload.c | 99 +++ lib/igt_dummyload.h | 9 tests/perf_pmu.c| 145 +++- 3 files changed, 196 insertions(+), 57 deletions(-) diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c index 4b20f23dfe26..0447d2f14d57 100644 --- a/lib/igt_dummyload.c +++ b/lib/igt_dummyload.c @@ -74,9 +74,12 @@ fill_reloc(struct drm_i915_gem_relocation_entry *reloc, reloc->write_domain = write_domains; } -static int emit_recursive_batch(igt_spin_t *spin, - int fd, uint32_t ctx, unsigned engine, - uint32_t dep, bool out_fence) +#define OUT_FENCE (1 << 0) +#define POLL_RUN (1 << 1) + +static int +emit_recursive_batch(igt_spin_t *spin, int fd, uint32_t ctx, unsigned engine, +uint32_t dep, unsigned int flags) { #define SCRATCH 0 #define BATCH 1 @@ -116,6 +119,8 @@ static int emit_recursive_batch(igt_spin_t *spin, execbuf.buffer_count++; if (dep) { + igt_assert(!(flags & POLL_RUN)); + Challenge left to the reader :) Well not the reader, whoever gets to need both. :) /* dummy write to dependency */ obj[SCRATCH].handle = dep; fill_reloc([obj[BATCH].relocation_count++], @@ -123,6 +128,41 @@ static int emit_recursive_batch(igt_spin_t *spin, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER); execbuf.buffer_count++; + } else if (flags & POLL_RUN) { + unsigned int offset; + + igt_assert(!dep); + + spin->poll_handle = gem_create(fd, 4096); + spin->running = __gem_mmap__wc(fd, spin->poll_handle, + 0, 4096, PROT_READ | PROT_WRITE); Use mmap_cpu and gem_set_caching(). Wouldn't that get us into coherency issues on some platforms? I keep the page mapped for API users to poll on. bxt-a? The point of using gem_set_caching() is that it is coherent with the CPU cache even on !llc via snooping. It's then essentially the same as how we handle breadcrumbs. Now admittedly, we should do if (__gem_set_caching() == 0) running = __gem_mmap__wb(); else running = __gem_mmap__wc(); The caller need be known the wiser; except having to assume the worst and so __sync_synchronize() if they do *running = x themselves. Ok. + igt_assert(spin->running); + igt_assert_eq(*spin->running, 0); + + *batch++ = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0); Hmm, have we forgot the (len-2) or is this an unusual command that knows its own length? I lifted the code from elsewhere. I checked, we have the same bug everywhere or nowhere. :| +/** + * igt_spin_batch_new_poll: + * @fd: open i915 drm file descriptor + * @engine: Ring to execute batch OR'd with execbuf flags. If value is less + * than 0, execute on all available rings. + * + * Start a recursive batch on a ring. Immediately returns a #igt_spin_t that + * contains the batch's handle that can be waited upon. The returned structure + * must be passed to igt_spin_batch_free() for post-processing. + * + * igt_spin_t->running will containt a pointer which target will change from + * zero to one once the spinner actually starts executing on the GPU. + * + * Returns: + * Structure with helper internal state for igt_spin_batch_free(). + */ +igt_spin_t * +igt_spin_batch_new_poll(int fd, uint32_t ctx, unsigned engine) +{ + igt_spin_t *spin; + + igt_require_gem(fd); + igt_require(gem_mmap__has_wc(fd)); igt_require(gem_can_store_dword(fd, engine)); Not all platforms have a MI_STORE_DWORD/DATA_IMM (with virtual addresses at least) and some platforms will die (*cough* snb *cough*).
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Show dmc debug registers on CFL and GLK (rev2)
== Series Details == Series: drm/i915: Show dmc debug registers on CFL and GLK (rev2) URL : https://patchwork.freedesktop.org/series/40031/ State : success == Summary == Series 40031v2 drm/i915: Show dmc debug registers on CFL and GLK https://patchwork.freedesktop.org/api/1.0/series/40031/revisions/2/mbox/ Known issues: Test debugfs_test: Subgroup read_all_entries: pass -> INCOMPLETE (fi-snb-2520m) fdo#103713 Test gem_mmap_gtt: Subgroup basic-small-bo-tiledx: fail -> PASS (fi-gdg-551) fdo#102575 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575 fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:435s fi-bdw-gvtdvmtotal:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:444s fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:385s fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:536s fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:298s fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:514s fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:517s fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:503s fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:413s fi-cfl-s2total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:578s fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:513s fi-cnl-drrs total:285 pass:254 dwarn:3 dfail:0 fail:0 skip:28 time:524s fi-cnl-y3total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:588s fi-elk-e7500 total:285 pass:226 dwarn:0 dfail:0 fail:0 skip:59 time:421s fi-gdg-551 total:285 pass:177 dwarn:0 dfail:0 fail:0 skip:108 time:318s fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:538s fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:404s fi-ilk-650 total:285 pass:225 dwarn:0 dfail:0 fail:0 skip:60 time:421s fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:475s fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:428s fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:477s fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:472s fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:514s fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:656s fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:444s fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:532s fi-skl-6700hqtotal:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:541s fi-skl-6700k2total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:507s fi-skl-6770hqtotal:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:506s fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:428s fi-skl-gvtdvmtotal:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:454s fi-snb-2520m total:3pass:2dwarn:0 dfail:0 fail:0 skip:0 fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:405s 95626f201526d97b6f284b397fd73c8a2d514ff4 drm-tip: 2018y-03m-15d-11h-01m-14s UTC integration manifest 7800a68bfb39 drm/i915: Show dmc debug registers on CFL and GLK == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8365/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.IGT: failure for Add NV12 support (rev2)
== Series Details == Series: Add NV12 support (rev2) URL : https://patchwork.freedesktop.org/series/39670/ State : failure == Summary == Possible new issues: Test gem_exec_capture: Subgroup capture-vebox: pass -> FAIL (shard-apl) Test kms_plane_scaling: Subgroup pipe-a-scaler-with-pixel-format: pass -> FAIL (shard-apl) Subgroup pipe-a-scaler-with-rotation: pass -> FAIL (shard-apl) Subgroup pipe-b-scaler-with-pixel-format: pass -> FAIL (shard-apl) Subgroup pipe-b-scaler-with-rotation: pass -> FAIL (shard-apl) Known issues: Test gem_eio: Subgroup in-flight-external: incomplete -> PASS (shard-apl) fdo#105341 Test kms_sysfs_edid_timing: pass -> WARN (shard-apl) fdo#100047 fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341 fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047 shard-apltotal:3407 pass:1797 dwarn:1 dfail:0 fail:12 skip:1595 time:12625s shard-hswtotal:3442 pass:1768 dwarn:1 dfail:0 fail:1 skip:1671 time:11815s shard-snbtotal:3442 pass:1358 dwarn:1 dfail:0 fail:2 skip:2081 time:7186s Blacklisted hosts: shard-kbltotal:2035 pass:1158 dwarn:1 dfail:0 fail:8 skip:866 time:5156s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8362/shards.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
Op 15-03-18 om 14:30 schreef Ville Syrjälä: > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote: >> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary >> arguments that can be removed by creating separate functins. >> >> Create specific functions for these calls to reduce x86/64 defconfig >> size by ~20k. >> >> Modify the existing macros to use the specific calls. >> >> new: >> $ size -t drivers/gpu/drm/built-in.a | tail -1 >> 187656244542 995 1922099 1d5433 (TOTALS) >> >> old: >> $ size -t drivers/gpu/drm/built-in.a | tail -1 >> 189756544542 995 1943102 1da63e (TOTALS) >> >> Miscellanea: >> >> o intel_display requires a change to use the specific calls. > How much would we lose if we move the (drm_debug) outside the > functions again? I'm somewhat concerned about all the function call > overhead when debugs aren't even enabled. Upstream: textdata bss dec hex filename 37714356894352 387184 5e870 drivers/gpu/drm/drm.ko With this patch: 37383156894352 383872 5db80 drivers/gpu/drm/drm.ko Moving the if outside (below): 37762956894352 387670 5ea56 drivers/gpu/drm/drm.ko Bye savings.. I don't think there are any places in which the debug output is performance sensitive, so I'm ok with not inlining. --- diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c index 79abf6d5b4db..928822403a59 100644 --- a/drivers/gpu/drm/drm_print.c +++ b/drivers/gpu/drm/drm_print.c @@ -89,14 +89,11 @@ void drm_dev_printk(const struct device *dev, const char *level, } EXPORT_SYMBOL(drm_dev_printk); -void drm_dbg(unsigned int category, const char *format, ...) +void __drm_dbg(const char *format, ...) { struct va_format vaf; va_list args; - if (!(drm_debug & category)) - return; - va_start(args, format); vaf.fmt = format; vaf.va = @@ -106,7 +103,7 @@ void drm_dbg(unsigned int category, const char *format, ...) va_end(args); } -EXPORT_SYMBOL(drm_dbg); +EXPORT_SYMBOL(__drm_dbg); void drm_err(const char *format, ...) { diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h index 3a40c5a3a5fa..2a145b97bdfc 100644 --- a/include/drm/drm_print.h +++ b/include/drm/drm_print.h @@ -200,8 +200,17 @@ __printf(6, 7) void drm_dev_printk(const struct device *dev, const char *level, unsigned int category, const char *function_name, const char *prefix, const char *format, ...); -__printf(2, 3) -void drm_dbg(unsigned int category, const char *format, ...); + +__printf(1, 2) +void __drm_dbg(const char *format, ...); + + +#define drm_dbg(category, format, ...) \ + do {\ + if (drm_debug & category) \ + __drm_dbg(format, ## __VA_ARGS__); \ + } while (0) + __printf(1, 2) void drm_err(const char *format, ...); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [maintainer-tools PATCH] doc: how to become a drm-intel committer
On Wed, Mar 14, 2018 at 05:11:02PM +0200, Jani Nikula wrote: > Until now, the drm-intel commit access have been handed out ad hoc, > without transparency, consistency, or fairness. With pressure to add > more committers, this is no longer tenable, if it ever was. Document the > requirements and expectations around becoming a drm-intel committer. > > The Linux kernel operates in a model where, by and large, only > maintainers commit patches. Maintainer teams are no longer rare, but the > drm-intel and drm-misc maintainer/committer model is definitely an > outlier. > > The drm-intel maintainers believe that a reasonable level of experience > and track record of working on the driver, as well as actively engaging > in the community upstream, are necessary before becoming a > committer. While the requirements outlined here may seem strict in > contrast with many projects, they are extremely liberal by kernel > standards. > > Finally, no rules are carved in stone. We fully expect the requirements > to be adjusted later. However, it will be much easier to start strict > and relax the requirements later than the other way round. > > Cc: Gustavo Padovan> Cc: Maarten Lankhorst > Cc: Sean Paul > Cc: Dave Airlie > Cc: dim-to...@lists.freedesktop.org > Cc: dri-de...@lists.freedesktop.org > Cc: intel-gfx@lists.freedesktop.org > Signed-off-by: Jani Nikula > Signed-off-by: Joonas Lahtinen > Signed-off-by: Rodrigo Vivi I've chatted for a few hours with Joonas, and I think before we can discuss the proposed document here itself, we first need to reach some agreement on why we even have commit rights. I think there's a very wide range of answers to that questions, and of course with different goals you end up with completely different rules about how to handle commit rights. Joonas suggested we first discuss this internally, perhaps with the maintainers, Kimmo and me. -Daniel > > --- > > We encourage the drm-misc maintainers to consider and document your > requirements for committers. While there's certain appeal to aligning on > the rules between drm-misc and drm-intel, we don't think they > necessarily have to be the same. > --- > commit-access.rst | 143 > ++ > index.rst | 1 + > 2 files changed, 144 insertions(+) > create mode 100644 commit-access.rst > > diff --git a/commit-access.rst b/commit-access.rst > new file mode 100644 > index ..dbc50f2b5bd3 > --- /dev/null > +++ b/commit-access.rst > @@ -0,0 +1,143 @@ > +=== > + Commit Access > +=== > + > +The drm-misc and drm-intel repositories operate in a maintainer/committer > model > +with a large pool committers who can push patches in accordance with the > stated > +merge criteria, and maintainers handling pull requests, topic branches, > merges, > +and so on. > + > +This document outlines the requirements for becoming a committer. > + > +drm-misc > + > + > +TBD. > + > +drm-intel > +- > + > +Requirements > + > + > +The baseline requirements for becoming a drm-intel committer are: > + > +- Comfortable with the open collaboration model we have. > + > +- Enough experience to gauge reasonably well how much review a patch needs, > and > + when pushing a patch, whether it needs acks from domain experts or > + maintainers. > + > +- Strong presence in the project communication channels (intel-gfx mailing > list > + and #intel-gfx IRC channel) for topics in their area of expertise. > + > +Since everyone is different and has different focus in their work, there are > no > +hard and fast rules, but just indicators and some judgement. > + > +Positive indicators are: > + > +- Decent amount of non-trivial patches merged in the past year (25+ patches, > + excluding simple code movement, typo fixes and similar patches). > + > +- Decent amount of in-depth review, again 25+ in the past year as the > threshold. > + > +- Lots of experience and commit rights in related open-source projects, like > + Mesa, or kernel trees like drm-misc, since the processes are very similar. > 2+ > + years as the threshold here, and this is also fulfilled after 2+ years of > + working in the virtual upstream team (product tree hacking doesn't count). > + > +- Already merged a complex feature, e.g. spanning multiple subsystems or even > + involving userspace. > + > +- Active member on freedesktop.org bugzilla. A developer that besides > submitting > + patches to fix bugs is also engaged on bugs discussion giving constructive > + comments helping to drive the bug entry to a solution. Hence keeping bug > list > + active, clean, and under control. > + > +Contra-indicators are: > + > +- Not around on IRC (taking timezones into account of course). > + > +- Mostly simple
[Intel-gfx] [PATCH] drm/i915: Show dmc debug registers on CFL and GLK
Since Coffee Lake uses the Kaby Lake DMC it's a safe bet that the debug registers are the same. I haven't double-checked that the GLK DMC uses the same registers as BXT, but it seems as good of a guess as any. v2: Add parentheses to silence warning Signed-off-by: David Weinehall--- drivers/gpu/drm/i915/i915_debugfs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index c4cc8fef11a0..87be104347cd 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2813,13 +2813,14 @@ static int i915_dmc_info(struct seq_file *m, void *unused) seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version), CSR_VERSION_MINOR(csr->version)); - if (IS_KABYLAKE(dev_priv) || + if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) || (IS_SKYLAKE(dev_priv) && csr->version >= CSR_VERSION(1, 6))) { seq_printf(m, "DC3 -> DC5 count: %d\n", I915_READ(SKL_CSR_DC3_DC5_COUNT)); seq_printf(m, "DC5 -> DC6 count: %d\n", I915_READ(SKL_CSR_DC5_DC6_COUNT)); - } else if (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4)) { + } else if (IS_GEMINILAKE(dev_priv) || + (IS_BROXTON(dev_priv) && csr->version >= CSR_VERSION(1, 4))) { seq_printf(m, "DC3 -> DC5 count: %d\n", I915_READ(BXT_CSR_DC3_DC5_COUNT)); } -- 2.16.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/perf_pmu: Improve accuracy by waiting on spinner to start
Quoting Tvrtko Ursulin (2018-03-15 13:36:26) > > On 15/03/2018 13:14, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-03-15 12:56:17) > >> From: Tvrtko Ursulin> >> > >> More than one test assumes that the spinner is running pretty much > >> immediately after we have create or submitted it. > >> > >> In actuality there is a variable delay, especially on execlists platforms, > >> between submission and spin batch starting to run on the hardware. > >> > >> To enable tests which care about this level of timing to account for this, > >> we add a new spin batch constructor which provides an output field which > >> can be polled to determine when the batch actually started running. > >> > >> This is implemented via MI_STOREDW_IMM from the spin batch, writing into > >> memory mapped page shared with userspace. > >> > >> Using this facility from perf_pmu, where applicable, should improve very > >> occasional test fails across the set and platforms. > >> > >> Signed-off-by: Tvrtko Ursulin > >> Suggested-by: Chris Wilson > >> --- > >> lib/igt_dummyload.c | 99 +++ > >> lib/igt_dummyload.h | 9 > >> tests/perf_pmu.c| 145 > >> +++- > >> 3 files changed, 196 insertions(+), 57 deletions(-) > >> > >> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c > >> index 4b20f23dfe26..0447d2f14d57 100644 > >> --- a/lib/igt_dummyload.c > >> +++ b/lib/igt_dummyload.c > >> @@ -74,9 +74,12 @@ fill_reloc(struct drm_i915_gem_relocation_entry *reloc, > >> reloc->write_domain = write_domains; > >> } > >> > >> -static int emit_recursive_batch(igt_spin_t *spin, > >> - int fd, uint32_t ctx, unsigned engine, > >> - uint32_t dep, bool out_fence) > >> +#define OUT_FENCE (1 << 0) > >> +#define POLL_RUN (1 << 1) > >> + > >> +static int > >> +emit_recursive_batch(igt_spin_t *spin, int fd, uint32_t ctx, unsigned > >> engine, > >> +uint32_t dep, unsigned int flags) > >> { > >> #define SCRATCH 0 > >> #define BATCH 1 > >> @@ -116,6 +119,8 @@ static int emit_recursive_batch(igt_spin_t *spin, > >> execbuf.buffer_count++; > >> > >> if (dep) { > >> + igt_assert(!(flags & POLL_RUN)); > >> + > > > > Challenge left to the reader :) > > Well not the reader, whoever gets to need both. :) > > >> /* dummy write to dependency */ > >> obj[SCRATCH].handle = dep; > >> fill_reloc([obj[BATCH].relocation_count++], > >> @@ -123,6 +128,41 @@ static int emit_recursive_batch(igt_spin_t *spin, > >> I915_GEM_DOMAIN_RENDER, > >> I915_GEM_DOMAIN_RENDER); > >> execbuf.buffer_count++; > >> + } else if (flags & POLL_RUN) { > >> + unsigned int offset; > >> + > >> + igt_assert(!dep); > >> + > >> + spin->poll_handle = gem_create(fd, 4096); > >> + spin->running = __gem_mmap__wc(fd, spin->poll_handle, > >> + 0, 4096, PROT_READ | > >> PROT_WRITE); > > > > Use mmap_cpu and gem_set_caching(). > > Wouldn't that get us into coherency issues on some platforms? I keep the > page mapped for API users to poll on. bxt-a? The point of using gem_set_caching() is that it is coherent with the CPU cache even on !llc via snooping. It's then essentially the same as how we handle breadcrumbs. Now admittedly, we should do if (__gem_set_caching() == 0) running = __gem_mmap__wb(); else running = __gem_mmap__wc(); The caller need be known the wiser; except having to assume the worst and so __sync_synchronize() if they do *running = x themselves. > >> + igt_assert(spin->running); > >> + igt_assert_eq(*spin->running, 0); > >> + > >> + *batch++ = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0); > > > > Hmm, have we forgot the (len-2) or is this an unusual command that knows > > its own length? > > I lifted the code from elsewhere. I checked, we have the same bug everywhere or nowhere. :| > >> +/** > >> + * igt_spin_batch_new_poll: > >> + * @fd: open i915 drm file descriptor > >> + * @engine: Ring to execute batch OR'd with execbuf flags. If value is > >> less > >> + * than 0, execute on all available rings. > >> + * > >> + * Start a recursive batch on a ring. Immediately returns a #igt_spin_t > >> that > >> + * contains the batch's handle that can be waited upon. The returned > >> structure > >> + * must be passed to igt_spin_batch_free() for post-processing. > >> + * > >> + * igt_spin_t->running will containt a pointer which target will change > >> from > >> + * zero to one once the spinner actually starts executing on the GPU. > >> + * > >> + * Returns: > >> + *