date:20210816

On Mon, 16 Aug 2021, Matt Roper  wrote:
> On Fri, Aug 13, 2021 at 03:33:30PM +0300, Jani Nikula wrote:
>> DG2 has 243000 but not 648000.
>
> Am I looking in the wrong place?  When I check the bspec page I still
> see:
>
> eDP/DP link bit rates: 1.62, 2.16, 2.7, 3.24, 4.32, 5.4, 6.48,
> 8.1 GHz, SSC and Non-SSC
>
> which matches the ICL rates list (i.e., 648000 included, but no
> reference to 243000).

I see the same thing. I can see no recent edits on the page either, so I
can only assume I've screwed up something, somehow when I wrote the
patch.

Never mind, and sorry for the noise.

BR,
Jani.



>
>
> Matt
>
>> 
>> Bspec: 54034
>> Signed-off-by: Jani Nikula 
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 75d4ebc66941..e21de08fea32 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -247,6 +247,9 @@ static void
>>  intel_dp_set_source_rates(struct intel_dp *intel_dp)
>>  {
>>  /* The values must be in increasing order */
>> +static const int dg2_rates[] = {
>> +162000, 216000, 243000, 27, 324000, 432000, 54, 81,
>> +};
>>  static const int icl_rates[] = {
>>  162000, 216000, 27, 324000, 432000, 54, 648000, 81
>>  };
>> @@ -272,7 +275,10 @@ intel_dp_set_source_rates(struct intel_dp *intel_dp)
>>  drm_WARN_ON(_priv->drm,
>>  intel_dp->source_rates || intel_dp->num_source_rates);
>>  
>> -if (DISPLAY_VER(dev_priv) >= 11) {
>> +if (IS_DG2(dev_priv)) {
>> +source_rates = dg2_rates;
>> +size = ARRAY_SIZE(dg2_rates);
>> +} else if (DISPLAY_VER(dev_priv) >= 11) {
>>  source_rates = icl_rates;
>>  size = ARRAY_SIZE(icl_rates);
>>  if (IS_JSL_EHL(dev_priv))
>> -- 
>> 2.20.1
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] refactor the i915 GVT support

2021-08-16 Thread Zhenyu Wang

On 2021.08.16 19:34:58 +0200, Christoph Hellwig wrote:
> On Wed, Aug 04, 2021 at 01:26:06PM +0800, Zhenyu Wang wrote:
> > On 2021.08.03 11:30:58 -0300, Jason Gunthorpe wrote:
> > > On Tue, Aug 03, 2021 at 05:43:15PM +0800, Zhenyu Wang wrote:
> > > > Acked-by: Zhenyu Wang 
> > > > 
> > > > Thanks a lot for this effort!
> > > 
> > > Great, do we have a submission plan for this? how much does it clash
> > > with my open_device/etc patch? ie does the whole thing have to go
> > > through the vfio tree?
> > > 
> > 
> > I think Alex would determine when to merge open_device series, gvt part
> > can be through vfio tree without problem. For this refactor, I would first
> > merge for gvt staging to do more regression testing before sending through
> > i915 tree.
> 
> Any updates on this?  I'd really hate to miss this merge window.

I'm still waiting for our validation team's report on this. I'm afraid
it might be missing for next version as i915 merge window is mostly
till rc5...and for any change outside of gvt, it still needs to be
acked by i915 maintainers.


signature.asc
Description: PGP signature

Re: [Intel-gfx] [PATCH V2 5/5] drm/i95/adl: Define MOCS table for Alderlake

On Mon, Aug 16, 2021 at 10:22:29AM +0530, Ayaz A Siddiqui wrote:
> In order to program unused and reserved mocs entries to L3_WB,
> we need to create a separate mocs table for alderlake.
> 
> This patch will also covers wa_1608975824.
> 
> Cc: Chris P Wilson 
> Cc: Lucas De Marchi 
> 
> Signed-off-by: Ayaz A Siddiqui 

Reviewed-by: Matt Roper 

> ---
>  drivers/gpu/drm/i915/gt/intel_mocs.c | 41 +++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
> b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index cf00537ba4acc..f76e2a2b3ea82 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -323,6 +323,39 @@ static const struct drm_i915_mocs_entry dg1_mocs_table[] 
> = {
>   MOCS_ENTRY(63, 0, L3_1_UC),
>  };
>  
> +static const struct drm_i915_mocs_entry adl_mocs_table[] = {
> + /* wa_1608975824 */
> + MOCS_ENTRY(0,
> +LE_3_WB | LE_TC_1_LLC | LE_LRUM(3),
> +L3_3_WB),
> +
> + GEN11_MOCS_ENTRIES,
> + /* Implicitly enable L1 - HDC:L1 + L3 + LLC */
> + MOCS_ENTRY(48,
> +LE_3_WB | LE_TC_1_LLC | LE_LRUM(3),
> +L3_3_WB),
> + /* Implicitly enable L1 - HDC:L1 + L3 */
> + MOCS_ENTRY(49,
> +LE_1_UC | LE_TC_1_LLC,
> +L3_3_WB),
> + /* Implicitly enable L1 - HDC:L1 + LLC */
> + MOCS_ENTRY(50,
> +LE_3_WB | LE_TC_1_LLC | LE_LRUM(3),
> +L3_1_UC),
> + /* Implicitly enable L1 - HDC:L1 */
> + MOCS_ENTRY(51,
> +LE_1_UC | LE_TC_1_LLC,
> +L3_1_UC),
> + /* HW Special Case (CCS) */
> + MOCS_ENTRY(60,
> +LE_3_WB | LE_TC_1_LLC | LE_LRUM(3),
> +L3_1_UC),
> + /* HW Special Case (Displayable) */
> + MOCS_ENTRY(61,
> +LE_1_UC | LE_TC_1_LLC,
> +L3_3_WB),
> +};
> +
>  enum {
>   HAS_GLOBAL_MOCS = BIT(0),
>   HAS_ENGINE_MOCS = BIT(1),
> @@ -444,7 +477,13 @@ static unsigned int get_mocs_settings(const struct 
> drm_i915_private *i915,
>  
>   memset(table, 0, sizeof(struct drm_i915_mocs_table));
>  
> - if (IS_DG1(i915)) {
> + if (IS_ALDERLAKE_S(i915) || IS_ALDERLAKE_P(i915)) {
> + table->size = ARRAY_SIZE(adl_mocs_table);
> + table->table = adl_mocs_table;
> + table->n_entries = GEN9_NUM_MOCS_ENTRIES;
> + table->uc_index = 3;
> + table->unused_entries_index = 2;
> + } else if (IS_DG1(i915)) {
>   table->size = ARRAY_SIZE(dg1_mocs_table);
>   table->table = dg1_mocs_table;
>   table->n_entries = GEN9_NUM_MOCS_ENTRIES;
> -- 
> 2.26.2
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

Re: [Intel-gfx] [PATCH V2 4/5] drm/i915/gt: Initialize unused MOCS entries with device specific values

On Mon, Aug 16, 2021 at 10:22:28AM +0530, Ayaz A Siddiqui wrote:
> During to creation mocs table,used field of drm_i915_mocs_entry
> is being checked, if used field is 0, then it will check values
> of index 1. All the unspecified indexes of xxx_mocs_table[] will
> contain control value and l3cc value of index I915_MOCS_PTE if
> its initialized.

I think there might be some words missing in the description here; I'm
having a bit of trouble following what it's saying.  Maybe something
like this would be more clear:

Historically we've initialized all undefined/reserved entries in
a platform's MOCS table to the contents of table entry #1 (i.e.,
I915_MOCS_PTE).

> 
> This patch is intended to provide capability to program device
> specific control value and l3cc value index which can be used
> for all the unspecified indexes of MOCS table.

And maybe for this part

Going forward, we can't assume that table entry #1 will always
contain suitable values to use for undefined/reserved table
indices.  We'll allow a platform-specific table index to be
selected at table initialization time in these cases.

We should also make some mention about using this new mechanism to
select an L3 WB entry for DG1 and all new platforms going forward, but
note that we can't change our production gen12 platforms (TGL and RKL)
since that would be an ABI break.

> 
> Signed-off-by: Ayaz A Siddiqui 
> ---
>  drivers/gpu/drm/i915/gt/intel_mocs.c | 38 +++-
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
> b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index df3c5d550c46a..cf00537ba4acc 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -23,6 +23,7 @@ struct drm_i915_mocs_table {
>   unsigned int n_entries;
>   const struct drm_i915_mocs_entry *table;
>   u8 uc_index;
> + u8 unused_entries_index;
>  };
>  
>  struct drm_i915_aux_table {
> @@ -99,17 +100,23 @@ struct drm_i915_aux_table {
>   * Entries not part of the following tables are undefined as far as
>   * userspace is concerned and shouldn't be relied upon.  For Gen < 12
>   * they will be initialized to PTE. Gen >= 12 onwards don't have a setting 
> for
> - * PTE and will be initialized to an invalid value.
> + * PTE and will be initialized L3 WB to catch accidental use of reserved and
> + * unused mocs indexes.

This comment doesn't seem to be quite true for all graphics ver >= 12
platforms; TGL/RKL are still using an I915_MOCS_PTE setting (which is L3
uncached) since we can't change it now without breaking ABI.  Same for
the NOTE2 below.


Matt

>   *
>   * The last few entries are reserved by the hardware. For ICL+ they
>   * should be initialized according to bspec and never used, for older
>   * platforms they should never be written to.
>   *
> - * NOTE: These tables are part of bspec and defined as part of hardware
> + * NOTE1: These tables are part of bspec and defined as part of hardware
>   *   interface for ICL+. For older platforms, they are part of kernel
>   *   ABI. It is expected that, for specific hardware platform, existing
>   *   entries will remain constant and the table will only be updated by
>   *   adding new entries, filling unused positions.
> + *
> + * NOTE2: For GEN >= 12, reserved and unspecified MOCS indices have been
> + *set to L3 WB. These reserved entries should never be used, they
> + *may be changed to low performant variants with better coherency
> + *in the future if more entries are needed.
>   */
>  #define GEN9_MOCS_ENTRIES \
>   MOCS_ENTRY(I915_MOCS_UNCACHED, \
> @@ -292,17 +299,9 @@ static const struct drm_i915_mocs_entry icl_mocs_table[] 
> = {
>  };
>  
>  static const struct drm_i915_mocs_entry dg1_mocs_table[] = {
> - /* Error */
> - MOCS_ENTRY(0, 0, L3_0_DIRECT),
>  
>   /* UC */
>   MOCS_ENTRY(1, 0, L3_1_UC),
> -
> - /* Reserved */
> - MOCS_ENTRY(2, 0, L3_0_DIRECT),
> - MOCS_ENTRY(3, 0, L3_0_DIRECT),
> - MOCS_ENTRY(4, 0, L3_0_DIRECT),
> -
>   /* WB - L3 */
>   MOCS_ENTRY(5, 0, L3_3_WB),
>   /* WB - L3 50% */
> @@ -450,6 +449,7 @@ static unsigned int get_mocs_settings(const struct 
> drm_i915_private *i915,
>   table->table = dg1_mocs_table;
>   table->n_entries = GEN9_NUM_MOCS_ENTRIES;
>   table->uc_index = 1;
> + table->unused_entries_index = 5;
>   } else if (GRAPHICS_VER(i915) >= 12) {
>   table->size  = ARRAY_SIZE(tgl_mocs_table);
>   table->table = tgl_mocs_table;
> @@ -500,16 +500,17 @@ static unsigned int get_mocs_settings(const struct 
> drm_i915_private *i915,
>  }
>  
>  /*
> - * Get control_value from MOCS entry taking into account when it's not used:
> - * I915_MOCS_PTE's value is returned in this case.
> + * Get control_value from MOCS entry taking into

Re: [Intel-gfx] [PATCH V2 3/5] drm/i915/gt: Set BLIT_CCTL reg to un-cached

On Mon, Aug 16, 2021 at 10:22:27AM +0530, Ayaz A Siddiqui wrote:
> From: Apoorva Singh 
> 
> Blitter commands which does not have MOCS fields rely on
> cacheability of BlitterCacheControlRegister which was mapped
> to index 0 by default.Once we changed the MOCS value of
> index 0 to L3 WB, tests like gem_linear_blits started failing
> due to change in cacheability from UC to WB.
> 
> Program and place the BlitterCacheControlRegister in
> build_aux_regs().

As noted on the previous patch, I think it might be simpler to add this
register to the engine's context workaround list.  You'd need to do so
before the check for RENDER_CLASS in the __intel_engine_init_ctx_wa, but
I think overall it might be simpler than adding a separate table to add
these extra engine context settings.

Note that we also have some other "fake" workarounds coming that will do
something similar.  For example,
  https://patchwork.freedesktop.org/patch/448804/?series=92135=10


Matt

> 
> Signed-off-by: Apoorva Singh 
> Signed-off-by: Ayaz A Siddiqui 
> ---
>  drivers/gpu/drm/i915/gt/intel_mocs.c | 13 +
>  drivers/gpu/drm/i915/i915_reg.h  |  7 +++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
> b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index 92141cf6f9a79..df3c5d550c46a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -372,6 +372,17 @@ add_aux_reg(struct drm_i915_aux_table *aux,
>   return x;
>  }
>  
> +static struct drm_i915_aux_table *
> +add_blit_cctl_override(struct drm_i915_aux_table *aux, u8 idx)
> +{
> + return add_aux_reg(aux,
> +"BLIT_CCTL",
> +BLIT_CCTL(0),
> +BLIT_CCTL_MOCS(idx, idx),
> +BLIT_CCTL_DST_MOCS_MASK | BLIT_CCTL_SRC_MOCS_MASK,
> +true);
> +}
> +
>  static struct drm_i915_aux_table *
>  add_cmd_cctl_override(struct drm_i915_aux_table *aux, u8 idx)
>  {
> @@ -398,6 +409,8 @@ build_aux_regs(const struct intel_engine_cs *engine,
>* uncached index.
>*/
>   aux = add_cmd_cctl_override(aux, mocs->uc_index);
> + if (engine->class == COPY_ENGINE_CLASS && mocs->uc_index)
> + aux = add_blit_cctl_override(aux, mocs->uc_index);
>   }
>  
>   return aux;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c8e2ca1b20796..de3cc9d66ffaa 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2567,6 +2567,13 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> REG_FIELD_PREP(CMD_CCTL_WRITE_OVERRIDE_MASK, (write) << 
> 1) | \
> REG_FIELD_PREP(CMD_CCTL_READ_OVERRIDE_MASK, (read) << 1))
>  
> +#define BLIT_CCTL(base)  _MMIO((base) + 0x204)
> +#define   BLIT_CCTL_DST_MOCS_MASKREG_GENMASK(14, 8)
> +#define   BLIT_CCTL_SRC_MOCS_MASKREG_GENMASK(6, 0)
> +#define   BLIT_CCTL_DST_MOCS_SHIFT   8
> +#define   BLIT_CCTL_MOCS(dst, src)   
> \
> + dst) << 1) << BLIT_CCTL_DST_MOCS_SHIFT) | ((src) << 1))
> +
>  #define RING_RESET_CTL(base) _MMIO((base) + 0xd0)
>  #define   RESET_CTL_CAT_ERROR   REG_BIT(2)
>  #define   RESET_CTL_READY_TO_RESET REG_BIT(1)
> -- 
> 2.26.2
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

Re: [Intel-gfx] [PATCH V2 2/5] drm/i915/gt: Use cmd_cctl override for platforms >= gen12

On Mon, Aug 16, 2021 at 10:22:26AM +0530, Ayaz A Siddiqui wrote:
> From: Srinivasan Shanmugam 
> 
> Program CMD_CCTL to use a mocs entry for uncached access.
> This controls memory accesses by CS as it reads instructions
> from the ring and batch buffers.
> 
> v2: Added CMD_CCTL in guc_mmio_regset_init(), so that this
> register can restored after engine reset.
> 
> Signed-off-by: Srinivasan Shanmugam 
> Signed-off-by: Ayaz A Siddiqui 
> Cc: Chris Wilson 
> Cc: Matt Roper 
> ---
>  drivers/gpu/drm/i915/gt/intel_mocs.c   | 96 ++
>  drivers/gpu/drm/i915/gt/selftest_mocs.c| 49 +++
>  drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c |  1 +
>  drivers/gpu/drm/i915/i915_reg.h| 16 
>  4 files changed, 162 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c 
> b/drivers/gpu/drm/i915/gt/intel_mocs.c
> index 10cc508c1a4f6..92141cf6f9a79 100644
> --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> @@ -25,6 +25,15 @@ struct drm_i915_mocs_table {
>   u8 uc_index;
>  };
>  
> +struct drm_i915_aux_table {

It's not clear to me exactly what the term "aux table" refers to here.
I guess it's just extra context registers (that aren't associated with a
workaround) that we want to initialize before the point where the
default context gets recorded?  Maybe calling it something like
"ctx_init_table" would make it more clear what these are for?

However a possibly simpler approach would just be to add these registers
directly to the ctx workaround list with a comment noting that they're
"fake" workarounds and describing what they're for (we already have
other similar context programming for disabling fine-grained preemption,
disabling nested batchbuffer mode, etc.  The benefit of just tossing
these on the workaround list is that the settings get automatically
verified by the workaround checking that we already have without needing
to code up new table management, register readback, value verification,
etc.

> + const char *name;
> + i915_reg_t offset;
> + u32 value;
> + u32 readmask;
> + bool skip_check;
> + struct drm_i915_aux_table *next;
> +};
> +
>  /* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */
>  #define _LE_CACHEABILITY(value)  ((value) << 0)
>  #define _LE_TGT_CACHE(value) ((value) << 2)
> @@ -336,6 +345,86 @@ static bool has_mocs(const struct drm_i915_private *i915)
>   return !IS_DGFX(i915);
>  }
>  
> +static struct drm_i915_aux_table *
> +add_aux_reg(struct drm_i915_aux_table *aux,
> + const char *name,
> + i915_reg_t offset,
> + u32 value,
> + u32 read,
> + bool skip_check)
> +
> +{
> + struct drm_i915_aux_table *x;
> +
> + x = kmalloc(sizeof(*x), GFP_ATOMIC);
> + if (!x) {
> + DRM_ERROR("Failed to allocate aux reg '%s'\n", name);

Generic DRM_ERROR() and such are deprecated now; we want to use the
per-device functions like drm_err() now.


Matt

> + return aux;
> + }
> +
> + x->name = name;
> + x->offset = offset;
> + x->value = value;
> + x->readmask = read;
> + x->skip_check = skip_check;
> +
> + x->next = aux;
> + return x;
> +}
> +
> +static struct drm_i915_aux_table *
> +add_cmd_cctl_override(struct drm_i915_aux_table *aux, u8 idx)
> +{
> + return add_aux_reg(aux,
> +"CMD_CCTL",
> +RING_CMD_CCTL(0),
> +CMD_CCTL_MOCS_OVERRIDE(idx, idx),
> +CMD_CCTL_WRITE_OVERRIDE_MASK | 
> CMD_CCTL_READ_OVERRIDE_MASK,
> +false);
> +}
> +
> +static const struct drm_i915_aux_table *
> +build_aux_regs(const struct intel_engine_cs *engine,
> +const struct drm_i915_mocs_table *mocs)
> +{
> + struct drm_i915_aux_table *aux = NULL;
> +
> + if (GRAPHICS_VER(engine->i915) >= 12 &&
> + !drm_WARN_ONCE(>i915->drm, !mocs->uc_index,
> + "Platform that should have UC index defined and does not\n")) {
> + /*
> +  * Index-0 does not operate as an uncached value as believed,
> +  * but causes invalid write cycles. Steer CMD_CCTL to another
> +  * uncached index.
> +  */
> + aux = add_cmd_cctl_override(aux, mocs->uc_index);
> + }
> +
> + return aux;
> +}
> +
> +static void
> +free_aux_regs(const struct drm_i915_aux_table *aux)
> +{
> + while (aux) {
> + struct drm_i915_aux_table *next = aux->next;
> +
> + kfree(aux);
> + aux = next;
> + }
> +}
> +
> +static void apply_aux_regs(struct intel_engine_cs *engine,
> +const struct drm_i915_aux_table *aux)
> +{
> + while (aux) {
> + intel_uncore_write_fw(engine->uncore,
> +   _MMIO(engine->mmio_base + 
> i915_mmio_reg_offset(aux->offset)),
> +   aux->value);
> +

Re: [Intel-gfx] [PATCH] drm/i915/dg2: DG2 intermediate DP source rates are different from CNL+

On Fri, Aug 13, 2021 at 03:33:30PM +0300, Jani Nikula wrote:
> DG2 has 243000 but not 648000.

Am I looking in the wrong place?  When I check the bspec page I still
see:

eDP/DP link bit rates: 1.62, 2.16, 2.7, 3.24, 4.32, 5.4, 6.48,
8.1 GHz, SSC and Non-SSC

which matches the ICL rates list (i.e., 648000 included, but no
reference to 243000).


Matt

> 
> Bspec: 54034
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 75d4ebc66941..e21de08fea32 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -247,6 +247,9 @@ static void
>  intel_dp_set_source_rates(struct intel_dp *intel_dp)
>  {
>   /* The values must be in increasing order */
> + static const int dg2_rates[] = {
> + 162000, 216000, 243000, 27, 324000, 432000, 54, 81,
> + };
>   static const int icl_rates[] = {
>   162000, 216000, 27, 324000, 432000, 54, 648000, 81
>   };
> @@ -272,7 +275,10 @@ intel_dp_set_source_rates(struct intel_dp *intel_dp)
>   drm_WARN_ON(_priv->drm,
>   intel_dp->source_rates || intel_dp->num_source_rates);
>  
> - if (DISPLAY_VER(dev_priv) >= 11) {
> + if (IS_DG2(dev_priv)) {
> + source_rates = dg2_rates;
> + size = ARRAY_SIZE(dg2_rates);
> + } else if (DISPLAY_VER(dev_priv) >= 11) {
>   source_rates = icl_rates;
>   size = ARRAY_SIZE(icl_rates);
>   if (IS_JSL_EHL(dev_priv))
> -- 
> 2.20.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795

[Intel-gfx] [PATCH] drm/i915/adl_p: Also disable underrun recovery with MSO

One of the cases that the bspec lists for when underrun recovery must be
disabled is "COG;" that note actually refers to eDP multi-segmented
operation (MSO).  Let's ensure the this additional restriction is
honored by the driver.

Bspec: 50351
Cc: Ville Syrjälä 
Fixes: ba3b049f4774 ("drm/i915/adl_p: Allow underrun recovery when possible")
Signed-off-by: Matt Roper 
---
 drivers/gpu/drm/i915/display/intel_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index a257e5dc381c..134a6acbd8fb 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -2226,6 +2226,9 @@ static bool underrun_recovery_supported(const struct 
intel_crtc_state *crtc_stat
if (crtc_state->has_psr2)
return false;
 
+   if (crtc_state->splitter.enable)
+   return false;
+
return true;
 }
 
-- 
2.25.4

Re: [Intel-gfx] [PATCH 02/10] drm/i915/bios: Start to support two integrated panels

On Mon, Aug 16, 2021 at 01:09:27PM +0300, Jani Nikula wrote:
> On Wed, 21 Jul 2021, José Roberto de Souza  wrote:
> > VBT has support for up two integrated panels but i915 only supports one.
> >
> > So here stating to add the basic support for two integrated panels
> > and moving the DRRS to ddi_vbt_port_info instead of keeping a global
> > one.
> > Other VBT blocks will be converted in following patches.
> >
> > While at is also nucking lvds_dither as it is not used.
> 
> If you were to get a mysterious bisect result on this patch, would you
> know what's wrong?
> 
> I think there's too much going on in one patch, and this needs to be
> split. Small, incremental changes with functional and non-functional
> separated if possible. This is incredibly fragile stuff.
> 
> Also, I think we'll need to be able to figure out the panel type based
> on EDID, i.e. we need to parse some stuff first, and the panel stuff
> only after we've read EDID. Obviously you don't have to do that here,
> but this need to be done so that this doesn't become impossible.

My incredibly incomplete start at that:
git://github.com/vsyrjala/linux.git vbt_panel_type_pnp_id

Totally missing any and all restructuring of the VBT
parsing to happen after we have the EDID.

And apart from that we also have this horrible
chicken vs. egg issue:
1. need the panel type to get power sequencer delays
2. need to turn on the VDD to read the EDID
3. need the EDID to get the panel type

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 09/10] drm/i915/bios: Only use opregion panel index for display ver 8 and older

On Wed, Jul 21, 2021 at 10:43:37PM -0700, José Roberto de Souza wrote:
> On newer platform this opregion call always fails, also it do not
> support multiple panels so dropping it.

We only use the panel type from opregion on specific machines based on
aa DMI match. So this patch is basically a no-op.

> 
> Cc: Ville Syrjälä 
> Cc: Jani Nikula 
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index d1ad6d625e521..6c848384a2ada 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1924,7 +1924,7 @@ static void parse_integrated_panel(struct 
> drm_i915_private *i915,
>  {
>   const struct vbt_header *vbt = i915->opregion.vbt;
>   const struct bdb_header *bdb;
> - int lfp_inst = 0, panel_index, opregion_panel_index;
> + int lfp_inst = 0, panel_index;
>  
>   if (devdata->child.handle == HANDLE_LFP_1)
>   lfp_inst = 1;
> @@ -1937,17 +1937,12 @@ static void parse_integrated_panel(struct 
> drm_i915_private *i915,
>   bdb = get_bdb_header(vbt);
>   panel_index = get_lfp_panel_index(i915, bdb, lfp_inst);
>  
> - opregion_panel_index = intel_opregion_get_panel_type(i915);
> - /*
> -  * TODO: the current implementation always use the panel index from
> -  * opregion if available due to issues with old platforms.
> -  * But this do not supports two panels and in SKL or newer I never saw a
> -  * system were this call returns a valid value.
> -  * So will change this to only use opregion up to BDW in a separated
> -  * commit.
> -  */
> - if (opregion_panel_index >= 0)
> - panel_index = opregion_panel_index;
> + if (DISPLAY_VER(i915) < 9) {
> + int opregion_panel_index = intel_opregion_get_panel_type(i915);
> +
> + if (opregion_panel_index >= 0)
> + opregion_panel_index = panel_index;
> + }
>  
>   if (panel_index == -1)
>   return;
> -- 
> 2.32.0

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] refactor the i915 GVT support

2021-08-16 Thread Christoph Hellwig

On Wed, Aug 04, 2021 at 01:26:06PM +0800, Zhenyu Wang wrote:
> On 2021.08.03 11:30:58 -0300, Jason Gunthorpe wrote:
> > On Tue, Aug 03, 2021 at 05:43:15PM +0800, Zhenyu Wang wrote:
> > > Acked-by: Zhenyu Wang 
> > > 
> > > Thanks a lot for this effort!
> > 
> > Great, do we have a submission plan for this? how much does it clash
> > with my open_device/etc patch? ie does the whole thing have to go
> > through the vfio tree?
> > 
> 
> I think Alex would determine when to merge open_device series, gvt part
> can be through vfio tree without problem. For this refactor, I would first
> merge for gvt staging to do more regression testing before sending through
> i915 tree.

Any updates on this?  I'd really hate to miss this merge window.

[Intel-gfx] [PATCH v2] drm/i915: Ditch the i915_gem_ww_ctx loop member

It's only used by the for_i915_gem_ww() macro and we can use
the (typically) on-stack _err variable in its place.

v2:
- Don't clear the _err variable when entering the loop
  (Matthew Auld, Maarten Lankhorst).
- Use parentheses around the _err macro argument.
- Fix up comment.

Cc: Matthew Auld 
Suggested-by: Maarten Lankhorst 
Signed-off-by: Thomas Hellström 
---
 drivers/gpu/drm/i915/i915_gem_ww.h | 25 -
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h 
b/drivers/gpu/drm/i915/i915_gem_ww.h
index f6b1a796667b..86f0fe343de6 100644
--- a/drivers/gpu/drm/i915/i915_gem_ww.h
+++ b/drivers/gpu/drm/i915/i915_gem_ww.h
@@ -11,8 +11,7 @@ struct i915_gem_ww_ctx {
struct ww_acquire_ctx ctx;
struct list_head obj_list;
struct drm_i915_gem_object *contended;
-   unsigned short intr;
-   unsigned short loop;
+   bool intr;
 };
 
 void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr);
@@ -20,31 +19,23 @@ void i915_gem_ww_ctx_fini(struct i915_gem_ww_ctx *ctx);
 int __must_check i915_gem_ww_ctx_backoff(struct i915_gem_ww_ctx *ctx);
 void i915_gem_ww_unlock_single(struct drm_i915_gem_object *obj);
 
-/* Internal functions used by the inlines! Don't use. */
+/* Internal function used by the inlines! Don't use. */
 static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)
 {
-   ww->loop = 0;
if (err == -EDEADLK) {
err = i915_gem_ww_ctx_backoff(ww);
if (!err)
-   ww->loop = 1;
+   err = -EDEADLK;
}
 
-   if (!ww->loop)
+   if (err != -EDEADLK)
i915_gem_ww_ctx_fini(ww);
 
return err;
 }
 
-static inline void
-__i915_gem_ww_init(struct i915_gem_ww_ctx *ww, bool intr)
-{
-   i915_gem_ww_ctx_init(ww, intr);
-   ww->loop = 1;
-}
-
-#define for_i915_gem_ww(_ww, _err, _intr)  \
-   for (__i915_gem_ww_init(_ww, _intr); (_ww)->loop;   \
-_err = __i915_gem_ww_fini(_ww, _err))
-
+#define for_i915_gem_ww(_ww, _err, _intr)\
+   for (i915_gem_ww_ctx_init(_ww, _intr), (_err) = -EDEADLK; \
+(_err) == -EDEADLK;  \
+(_err) = __i915_gem_ww_fini(_ww, _err))
 #endif
-- 
2.31.1

Re: [Intel-gfx] [PATCH i-g-t 1/3] i915/gem_exec_schedule: Make gem_exec_schedule understand static priority mapping

On Fri, Aug 13, 2021 at 04:24:37PM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 8/3/2021 6:23 PM, Matthew Brost wrote:
> > The i915 currently has 2k visible priority levels which are currently
> > unique. This is changing to statically map these 2k levels into 3
> > buckets:
> > 
> > low: < 0
> > mid: 0
> > high: > 0
> > 
> > Update gem_exec_schedule to understand this. This entails updating
> > promotion test to use 3 levels that will map into different buckets and
> > also add bit of delay after releasing a cork beforing completing the
> > spinners.
> 
> This needs a line about why we add the delay, something like "to give time
> to the i915 scheduler to process the fence release and queue the requests"
> or something.

Will reword, have typo here too.

> BTW, any reason not to just add the delay unconditionally in
> unplug_show_queue, instead of only in one test? Other tests might suffer
> from the same problem even if they're not hitting it at the moment.
>

Yea, probably a better approach to future proof this as I could see
other sections randomly failing in CI and wasting our time. Will fix
this and subsequent patch too.

Matt
 
> Daniele
> 
> > 
> > Also skip any tests that rely on having more than 3 priority levels.
> > 
> > v2: Add a delay between starting releasing spinner and cork in
> > promotion, add local define for static mapping engine info
> > 
> > Signed-off-by: Matthew Brost 
> > ---
> >   lib/i915/gem_scheduler.c   | 14 
> >   lib/i915/gem_scheduler.h   |  1 +
> >   lib/i915/i915_drm_local.h  | 10 ++
> >   tests/i915/gem_exec_schedule.c | 62 +-
> >   4 files changed, 63 insertions(+), 24 deletions(-)
> > 
> > diff --git a/lib/i915/gem_scheduler.c b/lib/i915/gem_scheduler.c
> > index cdddf42ad..d006b8676 100644
> > --- a/lib/i915/gem_scheduler.c
> > +++ b/lib/i915/gem_scheduler.c
> > @@ -28,6 +28,7 @@
> >   #include "igt_core.h"
> >   #include "ioctl_wrappers.h"
> > +#include "i915/i915_drm_local.h"
> >   #include "i915/gem_scheduler.h"
> >   #include "i915/gem_submission.h"
> > @@ -90,6 +91,19 @@ bool gem_scheduler_has_ctx_priority(int fd)
> > I915_SCHEDULER_CAP_PRIORITY;
> >   }
> > +/**
> > + * gem_scheduler_has_static_priority:
> > + * @fd: open i915 drm file descriptor
> > + *
> > + * Feature test macro to query whether the driver supports priority 
> > assigned
> > + * from user space are statically mapping into 3 buckets.
> > + */
> > +bool gem_scheduler_has_static_priority(int fd)
> > +{
> > +   return gem_scheduler_capability(fd) &
> > +   I915_SCHEDULER_CAP_STATIC_PRIORITY_MAP;
> > +}
> > +
> >   /**
> >* gem_scheduler_has_preemption:
> >* @fd: open i915 drm file descriptor
> > diff --git a/lib/i915/gem_scheduler.h b/lib/i915/gem_scheduler.h
> > index d43e84bd2..b00804f70 100644
> > --- a/lib/i915/gem_scheduler.h
> > +++ b/lib/i915/gem_scheduler.h
> > @@ -29,6 +29,7 @@
> >   unsigned gem_scheduler_capability(int fd);
> >   bool gem_scheduler_enabled(int fd);
> >   bool gem_scheduler_has_ctx_priority(int fd);
> > +bool gem_scheduler_has_static_priority(int fd);
> >   bool gem_scheduler_has_preemption(int fd);
> >   bool gem_scheduler_has_semaphores(int fd);
> >   bool gem_scheduler_has_engine_busy_stats(int fd);
> > diff --git a/lib/i915/i915_drm_local.h b/lib/i915/i915_drm_local.h
> > index dd646aedf..a1527ff21 100644
> > --- a/lib/i915/i915_drm_local.h
> > +++ b/lib/i915/i915_drm_local.h
> > @@ -20,6 +20,16 @@ extern "C" {
> >* clean these up when kernel uapi headers are sync'd.
> >*/
> > +/*
> > + * Indicates the 2k user priority levels are statically mapped into 3 
> > buckets as
> > + * follows:
> > + *
> > + * -1k to -1   Low priority
> > + * 0   Normal priority
> > + * 1 to 1k Highest priority
> > + */
> > +#define   I915_SCHEDULER_CAP_STATIC_PRIORITY_MAP   (1ul << 5)
> > +
> >   #if defined(__cplusplus)
> >   }
> >   #endif
> > diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> > index e5fb45982..bb9fb6c14 100644
> > --- a/tests/i915/gem_exec_schedule.c
> > +++ b/tests/i915/gem_exec_schedule.c
> > @@ -199,7 +199,8 @@ create_highest_priority(int fd, const intel_ctx_cfg_t 
> > *cfg)
> >   static void unplug_show_queue(int fd, struct igt_cork *c,
> >   const intel_ctx_cfg_t *cfg,
> > - unsigned int engine)
> > + unsigned int engine,
> > + unsigned usec_delay)
> >   {
> > igt_spin_t *spin[MAX_ELSP_QLEN];
> > int max = MAX_ELSP_QLEN;
> > @@ -216,6 +217,7 @@ static void unplug_show_queue(int fd, struct igt_cork 
> > *c,
> > igt_cork_unplug(c); /* batches will now be queued on the engine */
> > igt_debugfs_dump(fd, "i915_engine_info");
> > +   usleep(usec_delay);
> > for (int n = 0; n < max; n++)
> > igt_spin_free(fd, spin[n]);
> > @@ -237,7 +239,7 @@ static void fifo(int fd, const intel_ctx_t *ctx, 
> > unsigned

Re: [Intel-gfx] [PATCH] drm/i915/dp: Use max params for older panels

On Wed, Aug 04, 2021 at 11:24:02PM +0800, Kai-Heng Feng wrote:
> Users reported that after commit 2bbd6dba84d4 ("drm/i915: Try to use
> fast+narrow link on eDP again and fall back to the old max strategy on
> failure"), the screen starts to have wobbly effect.
> 
> Commit a5c936add6a2 ("drm/i915/dp: Use slow and wide link training for
> everything") doesn't help either, that means the affected panels only
> work with max params.

Unfortunate that the link training apparently passes with the bad
params and thus the automagic use_max_params fallback doesn't kick in
:(

> 
> The panels are all DP 1.1 ones, so apply max params to them to resolve
> the issue.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3714
> Fixes: 2bbd6dba84d4 ("drm/i915: Try to use fast+narrow link on eDP again and 
> fall back to the old max strategy on failure")
> Fixes: a5c936add6a2 ("drm/i915/dp: Use slow and wide link training for 
> everything")
> Signed-off-by: Kai-Heng Feng 
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 75d4ebc669411..e64bab4b016e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1330,14 +1330,16 @@ intel_dp_compute_link_config(struct intel_encoder 
> *encoder,
>   limits.min_bpp = intel_dp_min_bpp(pipe_config->output_format);
>   limits.max_bpp = intel_dp_max_bpp(intel_dp, pipe_config);
>  
> - if (intel_dp->use_max_params) {
> + if (intel_dp->use_max_params ||
> + intel_dp->dpcd[DP_DPCD_REV] <= DP_DPCD_REV_11) {

IIRC Windows uses the optimal link rate only for EPD_REV>=1.4.
We should probably do the same the minimize future headaches.

>   /*
>* Use the maximum clock and number of lanes the eDP panel
>* advertizes being capable of in case the initial fast
> -  * optimal params failed us. The panels are generally
> -  * designed to support only a single clock and lane
> -  * configuration, and typically on older panels these
> -  * values correspond to the native resolution of the panel.
> +  * optimal params failed us or the panel is DP 1.1 or earlier.
> +  * The panels are generally designed to support only a single
> +  * clock and lane configuration, and typically on older panels
> +  * these values correspond to the native resolution of the
> +  * panel.
>*/
>   limits.min_lane_count = limits.max_lane_count;
>   limits.min_clock = limits.max_clock;
> -- 
> 2.31.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects

2021-08-16 Thread Daniele Ceraolo Spurio





On 8/16/2021 8:15 AM, Daniel Vetter wrote:

On Fri, Aug 13, 2021 at 08:18:02AM -0700, Daniele Ceraolo Spurio wrote:


On 8/13/2021 7:37 AM, Daniel Vetter wrote:

On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:

This api allow user mode to create protected buffers and to mark
contexts as making use of such objects. Only when using contexts
marked in such a way is the execution guaranteed to work as expected.

Contexts can only be marked as using protected content at creation time
(i.e. the parameter is immutable) and they must be both bannable and not
recoverable.

All protected objects and contexts that have backing storage will be
considered invalid when the PXP session is destroyed and all new
submissions using them will be rejected. All intel contexts within the
invalidated gem contexts will be marked banned. A new flag has been
added to the RESET_STATS ioctl to report the context invalidation to
userspace.

This patch was previously sent as 2 separate patches, which have been
squashed following a request to have all the uapi in a single patch.
I've retained the s-o-b from both.

v5: squash patches, rebase on proto_ctx, update kerneldoc

v6: rebase on obj create_ext changes

Signed-off-by: Daniele Ceraolo Spurio 
Signed-off-by: Bommu Krishnaiah 
Cc: Rodrigo Vivi 
Cc: Chris Wilson 
Cc: Lionel Landwerlin 
Cc: Jason Ekstrand 
Cc: Daniel Vetter 
Reviewed-by: Rodrigo Vivi  #v5
---
   drivers/gpu/drm/i915/gem/i915_gem_context.c   | 68 --
   drivers/gpu/drm/i915/gem/i915_gem_context.h   | 18 
   .../gpu/drm/i915/gem/i915_gem_context_types.h |  2 +
   drivers/gpu/drm/i915/gem/i915_gem_create.c| 75 
   .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
   drivers/gpu/drm/i915/gem/i915_gem_object.c|  6 ++
   drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
   .../gpu/drm/i915/gem/i915_gem_object_types.h  |  9 ++
   drivers/gpu/drm/i915/pxp/intel_pxp.c  | 89 +++
   drivers/gpu/drm/i915/pxp/intel_pxp.h  | 15 
   drivers/gpu/drm/i915/pxp/intel_pxp_session.c  |  3 +
   drivers/gpu/drm/i915/pxp/intel_pxp_types.h|  5 ++
   include/uapi/drm/i915_drm.h   | 55 +++-
   13 files changed, 371 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index cff72679ad7c..0cd3e2d06188 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -77,6 +77,8 @@
   #include "gt/intel_gpu_commands.h"
   #include "gt/intel_ring.h"
+#include "pxp/intel_pxp.h"
+
   #include "i915_gem_context.h"
   #include "i915_trace.h"
   #include "i915_user_extensions.h"
@@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct 
drm_i915_private *i915,
return 0;
   }
+static int proto_context_set_protected(struct drm_i915_private *i915,
+  struct i915_gem_proto_context *pc,
+  bool protected)
+{
+   int ret = 0;
+
+   if (!intel_pxp_is_enabled(>gt.pxp))
+   ret = -ENODEV;
+   else if (!protected)
+   pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
+   else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
+!(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
+   ret = -EPERM;
+   else
+   pc->user_flags |= BIT(UCONTEXT_PROTECTED);
+
+   return ret;
+}
+
   static struct i915_gem_proto_context *
   proto_context_create(struct drm_i915_private *i915, unsigned int flags)
   {
@@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct drm_i915_file_private 
*fpriv,
ret = -EPERM;
else if (args->value)
pc->user_flags |= BIT(UCONTEXT_BANNABLE);
+   else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+   ret = -EPERM;
else
pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
break;
@@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct 
drm_i915_file_private *fpriv,
case I915_CONTEXT_PARAM_RECOVERABLE:
if (args->size)
ret = -EINVAL;
-   else if (args->value)
-   pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
-   else
+   else if (!args->value)
pc->user_flags &= ~BIT(UCONTEXT_RECOVERABLE);
+   else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
+   ret = -EPERM;
+   else
+   pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
break;
case I915_CONTEXT_PARAM_PRIORITY:
@@ -724,6 +749,11 @@ static int set_proto_ctx_param(struct 
drm_i915_file_private *fpriv,
args->value);
break;
+   case

Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects

On Fri, Aug 13, 2021 at 08:24:44AM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 8/13/2021 7:42 AM, Daniel Vetter wrote:
> > On Fri, Aug 13, 2021 at 04:37:53PM +0200, Daniel Vetter wrote:
> > > On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
> > > > This api allow user mode to create protected buffers and to mark
> > > > contexts as making use of such objects. Only when using contexts
> > > > marked in such a way is the execution guaranteed to work as expected.
> > > > 
> > > > Contexts can only be marked as using protected content at creation time
> > > > (i.e. the parameter is immutable) and they must be both bannable and not
> > > > recoverable.
> > > > 
> > > > All protected objects and contexts that have backing storage will be
> > > > considered invalid when the PXP session is destroyed and all new
> > > > submissions using them will be rejected. All intel contexts within the
> > > > invalidated gem contexts will be marked banned. A new flag has been
> > > > added to the RESET_STATS ioctl to report the context invalidation to
> > > > userspace.
> > > > 
> > > > This patch was previously sent as 2 separate patches, which have been
> > > > squashed following a request to have all the uapi in a single patch.
> > > > I've retained the s-o-b from both.
> > > > 
> > > > v5: squash patches, rebase on proto_ctx, update kerneldoc
> > > > 
> > > > v6: rebase on obj create_ext changes
> > > > 
> > > > Signed-off-by: Daniele Ceraolo Spurio 
> > > > Signed-off-by: Bommu Krishnaiah 
> > > > Cc: Rodrigo Vivi 
> > > > Cc: Chris Wilson 
> > > > Cc: Lionel Landwerlin 
> > > > Cc: Jason Ekstrand 
> > > > Cc: Daniel Vetter 
> > > > Reviewed-by: Rodrigo Vivi  #v5
> > > > ---
> > > >   drivers/gpu/drm/i915/gem/i915_gem_context.c   | 68 --
> > > >   drivers/gpu/drm/i915/gem/i915_gem_context.h   | 18 
> > > >   .../gpu/drm/i915/gem/i915_gem_context_types.h |  2 +
> > > >   drivers/gpu/drm/i915/gem/i915_gem_create.c| 75 
> > > >   .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
> > > >   drivers/gpu/drm/i915/gem/i915_gem_object.c|  6 ++
> > > >   drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
> > > >   .../gpu/drm/i915/gem/i915_gem_object_types.h  |  9 ++
> > > >   drivers/gpu/drm/i915/pxp/intel_pxp.c  | 89 +++
> > > >   drivers/gpu/drm/i915/pxp/intel_pxp.h  | 15 
> > > >   drivers/gpu/drm/i915/pxp/intel_pxp_session.c  |  3 +
> > > >   drivers/gpu/drm/i915/pxp/intel_pxp_types.h|  5 ++
> > > >   include/uapi/drm/i915_drm.h   | 55 +++-
> > > >   13 files changed, 371 insertions(+), 26 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
> > > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > index cff72679ad7c..0cd3e2d06188 100644
> > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > > @@ -77,6 +77,8 @@
> > > >   #include "gt/intel_gpu_commands.h"
> > > >   #include "gt/intel_ring.h"
> > > > +#include "pxp/intel_pxp.h"
> > > > +
> > > >   #include "i915_gem_context.h"
> > > >   #include "i915_trace.h"
> > > >   #include "i915_user_extensions.h"
> > > > @@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct 
> > > > drm_i915_private *i915,
> > > > return 0;
> > > >   }
> > > > +static int proto_context_set_protected(struct drm_i915_private *i915,
> > > > +  struct i915_gem_proto_context 
> > > > *pc,
> > > > +  bool protected)
> > > > +{
> > > > +   int ret = 0;
> > > > +
> > > > +   if (!intel_pxp_is_enabled(>gt.pxp))
> > > > +   ret = -ENODEV;
> > > > +   else if (!protected)
> > > > +   pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
> > > > +   else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
> > > > +!(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
> > > > +   ret = -EPERM;
> > > > +   else
> > > > +   pc->user_flags |= BIT(UCONTEXT_PROTECTED);
> > > > +
> > > > +   return ret;
> > > > +}
> > > > +
> > > >   static struct i915_gem_proto_context *
> > > >   proto_context_create(struct drm_i915_private *i915, unsigned int 
> > > > flags)
> > > >   {
> > > > @@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct 
> > > > drm_i915_file_private *fpriv,
> > > > ret = -EPERM;
> > > > else if (args->value)
> > > > pc->user_flags |= BIT(UCONTEXT_BANNABLE);
> > > > +   else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> > > > +   ret = -EPERM;
> > > > else
> > > > pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
> > > > break;
> > > > @@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct 
> > > > drm_i915_file_private *fpriv,
> > > > case

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm: avoid races with modesetting rights

== Series Details ==

Series: drm: avoid races with modesetting rights
URL   : https://patchwork.freedesktop.org/series/93714/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10488_full -> Patchwork_20831_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_20831_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_20831_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_20831_full:

### IGT changes ###

 Possible regressions 

  * igt@kms_lease@cursor_implicit_plane:
- shard-snb:  [PASS][1] -> [DMESG-WARN][2] +6 similar issues
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/shard-snb2/igt@kms_lease@cursor_implicit_plane.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-snb5/igt@kms_lease@cursor_implicit_plane.html

  * igt@kms_lease@implicit-plane-lease:
- shard-tglb: [PASS][3] -> [DMESG-WARN][4] +13 similar issues
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/shard-tglb5/igt@kms_le...@implicit-plane-lease.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-tglb2/igt@kms_le...@implicit-plane-lease.html

  * igt@kms_lease@master-vs-lease:
- shard-iclb: [PASS][5] -> [DMESG-WARN][6] +13 similar issues
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/shard-iclb1/igt@kms_le...@master-vs-lease.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-iclb8/igt@kms_le...@master-vs-lease.html
- shard-snb:  NOTRUN -> [DMESG-WARN][7] +2 similar issues
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-snb6/igt@kms_le...@master-vs-lease.html

  * igt@perf_pmu@busy-double-start@vcs1:
- shard-tglb: [PASS][8] -> [INCOMPLETE][9]
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/shard-tglb5/igt@perf_pmu@busy-double-st...@vcs1.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-tglb8/igt@perf_pmu@busy-double-st...@vcs1.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@feature_discovery@psr2:
- shard-iclb: [PASS][10] -> [SKIP][11] ([i915#658])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/shard-iclb2/igt@feature_discov...@psr2.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-iclb7/igt@feature_discov...@psr2.html

  * igt@gem_ctx_persistence@legacy-engines-mixed-process:
- shard-snb:  NOTRUN -> [SKIP][12] ([fdo#109271] / [i915#1099]) +4 
similar issues
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-snb6/igt@gem_ctx_persiste...@legacy-engines-mixed-process.html

  * igt@gem_exec_fair@basic-none@vcs0:
- shard-kbl:  NOTRUN -> [FAIL][13] ([i915#2842]) +1 similar issue
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-kbl4/igt@gem_exec_fair@basic-n...@vcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
- shard-glk:  [PASS][14] -> [FAIL][15] ([i915#2842])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/shard-glk1/igt@gem_exec_fair@basic-pace-sh...@rcs0.html
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-glk3/igt@gem_exec_fair@basic-pace-sh...@rcs0.html

  * igt@gem_userptr_blits@vma-merge:
- shard-snb:  NOTRUN -> [FAIL][16] ([i915#2724])
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-snb7/igt@gem_userptr_bl...@vma-merge.html

  * igt@i915_suspend@forcewake:
- shard-apl:  [PASS][17] -> [DMESG-WARN][18] ([i915#180]) +1 
similar issue
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/shard-apl2/igt@i915_susp...@forcewake.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-apl6/igt@i915_susp...@forcewake.html

  * igt@kms_ccs@pipe-a-ccs-on-another-bo-yf_tiled_ccs:
- shard-tglb: NOTRUN -> [SKIP][19] ([i915#3689])
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-tglb7/igt@kms_ccs@pipe-a-ccs-on-another-bo-yf_tiled_ccs.html

  * igt@kms_ccs@pipe-a-crc-primary-basic-y_tiled_gen12_mc_ccs:
- shard-kbl:  NOTRUN -> [SKIP][20] ([fdo#109271] / [i915#3886]) +1 
similar issue
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/shard-kbl4/igt@kms_ccs@pipe-a-crc-primary-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc:
- shard-apl:  NOTRUN -> [SKIP][21] ([fdo#109271] / [i915#3886]) +4 
similar issues

Re: [Intel-gfx] [PATCH v6 10/15] drm/i915/pxp: interfaces for using protected objects

On Fri, Aug 13, 2021 at 08:18:02AM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 8/13/2021 7:37 AM, Daniel Vetter wrote:
> > On Wed, Jul 28, 2021 at 07:01:01PM -0700, Daniele Ceraolo Spurio wrote:
> > > This api allow user mode to create protected buffers and to mark
> > > contexts as making use of such objects. Only when using contexts
> > > marked in such a way is the execution guaranteed to work as expected.
> > > 
> > > Contexts can only be marked as using protected content at creation time
> > > (i.e. the parameter is immutable) and they must be both bannable and not
> > > recoverable.
> > > 
> > > All protected objects and contexts that have backing storage will be
> > > considered invalid when the PXP session is destroyed and all new
> > > submissions using them will be rejected. All intel contexts within the
> > > invalidated gem contexts will be marked banned. A new flag has been
> > > added to the RESET_STATS ioctl to report the context invalidation to
> > > userspace.
> > > 
> > > This patch was previously sent as 2 separate patches, which have been
> > > squashed following a request to have all the uapi in a single patch.
> > > I've retained the s-o-b from both.
> > > 
> > > v5: squash patches, rebase on proto_ctx, update kerneldoc
> > > 
> > > v6: rebase on obj create_ext changes
> > > 
> > > Signed-off-by: Daniele Ceraolo Spurio 
> > > Signed-off-by: Bommu Krishnaiah 
> > > Cc: Rodrigo Vivi 
> > > Cc: Chris Wilson 
> > > Cc: Lionel Landwerlin 
> > > Cc: Jason Ekstrand 
> > > Cc: Daniel Vetter 
> > > Reviewed-by: Rodrigo Vivi  #v5
> > > ---
> > >   drivers/gpu/drm/i915/gem/i915_gem_context.c   | 68 --
> > >   drivers/gpu/drm/i915/gem/i915_gem_context.h   | 18 
> > >   .../gpu/drm/i915/gem/i915_gem_context_types.h |  2 +
> > >   drivers/gpu/drm/i915/gem/i915_gem_create.c| 75 
> > >   .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 40 -
> > >   drivers/gpu/drm/i915/gem/i915_gem_object.c|  6 ++
> > >   drivers/gpu/drm/i915/gem/i915_gem_object.h| 12 +++
> > >   .../gpu/drm/i915/gem/i915_gem_object_types.h  |  9 ++
> > >   drivers/gpu/drm/i915/pxp/intel_pxp.c  | 89 +++
> > >   drivers/gpu/drm/i915/pxp/intel_pxp.h  | 15 
> > >   drivers/gpu/drm/i915/pxp/intel_pxp_session.c  |  3 +
> > >   drivers/gpu/drm/i915/pxp/intel_pxp_types.h|  5 ++
> > >   include/uapi/drm/i915_drm.h   | 55 +++-
> > >   13 files changed, 371 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c 
> > > b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > index cff72679ad7c..0cd3e2d06188 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > @@ -77,6 +77,8 @@
> > >   #include "gt/intel_gpu_commands.h"
> > >   #include "gt/intel_ring.h"
> > > +#include "pxp/intel_pxp.h"
> > > +
> > >   #include "i915_gem_context.h"
> > >   #include "i915_trace.h"
> > >   #include "i915_user_extensions.h"
> > > @@ -241,6 +243,25 @@ static int proto_context_set_persistence(struct 
> > > drm_i915_private *i915,
> > >   return 0;
> > >   }
> > > +static int proto_context_set_protected(struct drm_i915_private *i915,
> > > +struct i915_gem_proto_context *pc,
> > > +bool protected)
> > > +{
> > > + int ret = 0;
> > > +
> > > + if (!intel_pxp_is_enabled(>gt.pxp))
> > > + ret = -ENODEV;
> > > + else if (!protected)
> > > + pc->user_flags &= ~BIT(UCONTEXT_PROTECTED);
> > > + else if ((pc->user_flags & BIT(UCONTEXT_RECOVERABLE)) ||
> > > +  !(pc->user_flags & BIT(UCONTEXT_BANNABLE)))
> > > + ret = -EPERM;
> > > + else
> > > + pc->user_flags |= BIT(UCONTEXT_PROTECTED);
> > > +
> > > + return ret;
> > > +}
> > > +
> > >   static struct i915_gem_proto_context *
> > >   proto_context_create(struct drm_i915_private *i915, unsigned int flags)
> > >   {
> > > @@ -686,6 +707,8 @@ static int set_proto_ctx_param(struct 
> > > drm_i915_file_private *fpriv,
> > >   ret = -EPERM;
> > >   else if (args->value)
> > >   pc->user_flags |= BIT(UCONTEXT_BANNABLE);
> > > + else if (pc->user_flags & BIT(UCONTEXT_PROTECTED))
> > > + ret = -EPERM;
> > >   else
> > >   pc->user_flags &= ~BIT(UCONTEXT_BANNABLE);
> > >   break;
> > > @@ -693,10 +716,12 @@ static int set_proto_ctx_param(struct 
> > > drm_i915_file_private *fpriv,
> > >   case I915_CONTEXT_PARAM_RECOVERABLE:
> > >   if (args->size)
> > >   ret = -EINVAL;
> > > - else if (args->value)
> > > - pc->user_flags |= BIT(UCONTEXT_RECOVERABLE);
> > > - else
> > > + else if (!args->value)
> > >   pc->user_flags &=

Re: [Intel-gfx] [PATCH] drm/i915/adl_p: Allow underrun recovery when possible

On Mon, Jul 26, 2021 at 11:00:46PM -0700, Matt Roper wrote:
> ADL_P requires that we disable underrun recovery when downscaling

Does that mean plane downsclaing or pipe downscaling or both?

> (or
> using the scaler for YUV420 pipe output), using DSC, or using PSR2.
> Otherwise we should be able to enable the underrun recovery.
> 
> On DG2 we need to keep underrun recovery disabled at all times, but the
> chicken bit in PIPE_CHICKEN has an inverted meaning (it's an enable bit
> instead of disable).
> 
> Bspec: 50351
> Signed-off-by: Matt Roper 
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 50 +---
>  drivers/gpu/drm/i915/i915_reg.h  |  3 +-
>  2 files changed, 36 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
> b/drivers/gpu/drm/i915/display/intel_display.c
> index bd533813e5e2..b861b169fb82 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2211,8 +2211,26 @@ void intel_display_finish_reset(struct 
> drm_i915_private *dev_priv)
>   clear_bit_unlock(I915_RESET_MODESET, _priv->gt.reset.flags);
>  }
>  
> -static void icl_set_pipe_chicken(struct intel_crtc *crtc)
> +static bool underrun_recovery_supported(const struct intel_crtc_state 
> *crtc_state)
>  {
> + if (crtc_state->pch_pfit.enabled &&
> + (crtc_state->pipe_src_w > drm_rect_width(_state->pch_pfit.dst) 
> ||
> +  crtc_state->pipe_src_h > 
> drm_rect_height(_state->pch_pfit.dst) ||
> +  crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420))
> + return false;
> +
> + if (crtc_state->dsc.compression_enable)
> + return false;
> +
> + if (crtc_state->has_psr2)
> + return false;
> +
> + return true;
> +}
> +
> +static void icl_set_pipe_chicken(const struct intel_crtc_state *crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>   enum pipe pipe = crtc->pipe;
>   u32 tmp;
> @@ -2233,19 +2251,19 @@ static void icl_set_pipe_chicken(struct intel_crtc 
> *crtc)
>*/
>   tmp |= PIXEL_ROUNDING_TRUNC_FB_PASSTHRU;
>  
> - /*
> -  * "The underrun recovery mechanism should be disabled
> -  *  when the following is enabled for this pipe:
> -  *  WiDi
> -  *  Downscaling (this includes YUV420 fullblend)
> -  *  COG

Seems you forgot about this one?

> -  *  DSC
> -  *  PSR2"
> -  *
> -  * FIXME: enable whenever possible...
> -  */
> - if (IS_ALDERLAKE_P(dev_priv))
> - tmp |= UNDERRUN_RECOVERY_DISABLE;
> + if (IS_DG2(dev_priv)) {
> + /*
> +  * Underrun recovery must always be disabled on DG2.  However
> +  * the chicken bit meaning is inverted compared to other
> +  * platforms.
> +  */
> + tmp &= ~UNDERRUN_RECOVERY_ENABLE_DG2;
> + } else if (DISPLAY_VER(dev_priv) >= 13) {
> + if (underrun_recovery_supported(crtc_state))
> + tmp |= UNDERRUN_RECOVERY_DISABLE_ADLP;
> + else
> + tmp &= ~UNDERRUN_RECOVERY_DISABLE_ADLP;
> + }
>  
>   intel_de_write(dev_priv, PIPE_CHICKEN(pipe), tmp);
>  }
> @@ -3561,7 +3579,7 @@ static void hsw_crtc_enable(struct intel_atomic_state 
> *state,
>   hsw_set_linetime_wm(new_crtc_state);
>  
>   if (DISPLAY_VER(dev_priv) >= 11)
> - icl_set_pipe_chicken(crtc);
> + icl_set_pipe_chicken(new_crtc_state);
>  
>   if (dev_priv->display.initial_watermarks)
>   dev_priv->display.initial_watermarks(state, crtc);
> @@ -10193,7 +10211,7 @@ static void intel_pipe_fastset(const struct 
> intel_crtc_state *old_crtc_state,
>   hsw_set_linetime_wm(new_crtc_state);
>  
>   if (DISPLAY_VER(dev_priv) >= 11)
> - icl_set_pipe_chicken(crtc);
> + icl_set_pipe_chicken(new_crtc_state);
>  }
>  
>  static void commit_pipe_pre_planes(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d5c67fd5dc7d..a10cdd1a3001 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8416,7 +8416,8 @@ enum {
>  #define _PIPEC_CHICKEN   0x72038
>  #define PIPE_CHICKEN(pipe)   _MMIO_PIPE(pipe, 
> _PIPEA_CHICKEN,\
>  _PIPEB_CHICKEN)
> -#define   UNDERRUN_RECOVERY_DISABLE  REG_BIT(30)
> +#define   UNDERRUN_RECOVERY_DISABLE_ADLP REG_BIT(30)
> +#define   UNDERRUN_RECOVERY_ENABLE_DG2   REG_BIT(30)
>  #define   PIXEL_ROUNDING_TRUNC_FB_PASSTHRU   (1 << 15)
>  #define   PER_PIXEL_ALPHA_BYPASS_EN  (1 << 7)
>  
> -- 
> 2.25.4
> 
> ___
> Intel-gfx mailing list
>

Re: [Intel-gfx] [PATCH v2] drm: avoid races with modesetting rights

On Mon, Aug 16, 2021 at 12:31 PM Desmond Cheong Zhi Xi
 wrote:
>
> On 16/8/21 5:04 pm, Daniel Vetter wrote:
> > On Mon, Aug 16, 2021 at 10:53 AM Desmond Cheong Zhi Xi
> >  wrote:
> >> On 16/8/21 2:47 am, kernel test robot wrote:
> >>> Hi Desmond,
> >>>
> >>> Thank you for the patch! Yet something to improve:
> >>>
> >>> [auto build test ERROR on next-20210813]
> >>> [also build test ERROR on v5.14-rc5]
> >>> [cannot apply to linus/master v5.14-rc5 v5.14-rc4 v5.14-rc3]
> >>> [If your patch is applied to the wrong git tree, kindly drop us a note.
> >>> And when submitting patch, we suggest to use '--base' as documented in
> >>> https://git-scm.com/docs/git-format-patch]
> >>>
> >>> url:
> >>> https://github.com/0day-ci/linux/commits/Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
> >>> base:4b358aabb93a2c654cd1dcab1a25a589f6e2b153
> >>> config: i386-randconfig-a004-20210815 (attached as .config)
> >>> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> >>> reproduce (this is a W=1 build):
> >>>   # 
> >>> https://github.com/0day-ci/linux/commit/cf6d8354b7d7953cd866fad004cbb189adfa074f
> >>>   git remote add linux-review https://github.com/0day-ci/linux
> >>>   git fetch --no-tags linux-review 
> >>> Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
> >>>   git checkout cf6d8354b7d7953cd866fad004cbb189adfa074f
> >>>   # save the attached .config to linux build tree
> >>>   make W=1 ARCH=i386
> >>>
> >>> If you fix the issue, kindly add following tag as appropriate
> >>> Reported-by: kernel test robot 
> >>>
> >>> All errors (new ones prefixed by >>, old ones prefixed by <<):
> >>>
> > ERROR: modpost: "task_work_add" [drivers/gpu/drm/drm.ko] undefined!
> >>>
> >>
> >> I'm a bit uncertain about this. Looking into the .config used, this
> >> error seems to happen because task_work_add isn't an exported symbol,
> >> but DRM is being compiled as a loadable kernel module (CONFIG_DRM=m).
> >>
> >> One way to deal with this is to export the symbol, but there was a
> >> proposed patch to do this a few months back that wasn't picked up [1],
> >> so I'm not sure what to make of this.
> >>
> >> I'll export the symbol as part of a v3 series, and check in with the
> >> task-work maintainers.
> >>
> >> Link:
> >> https://lore.kernel.org/lkml/20210127150029.13766-3-josh...@samsung.com/ 
> >> [1]
> >
> > Yeah that sounds best. I have two more thoughts on the patch:
> > - drm_master_flush isn't used by any modules outside of drm.ko, so we
> > can unexport it and drop the kerneldoc (the comment is still good).
> > These kind of internal functions have their declaration in
> > drm-internal.h - there's already a few there from drm_auth.c
> >
>
> Sounds good, I'll do that and move the declaration from drm_auth.h to
> drm_internal.h.
>
> > - We know have 3 locks for master state, that feels a bit like
> > overkill. The spinlock I think we need to keep due to lock inversions,
> > but the master_mutex and master_rwsem look like we should be able to
> > merge them? I.e. anywhere we currently grab the master_mutex we could
> > instead grab the rwsem in either write mode (when we change stuff) or
> > read mode (when we just check, like in master_internal_acquire).
> >
> > Thoughts?
> > -Daniel
> >
>
> Using rwsem in the places where we currently hold the mutex seems pretty
> doable.
>
> There are some tricky bits once we add rwsem read locks to the ioctl
> handler. Some ioctl functions like drm_authmagic need a write lock.

Ah yes, I only looked at the dropmaster/setmaster ioctl, and those
don't have the DRM_MASTER bit set.

> In this particular case, it might make sense to break master_mutex down
> into finer-grained locks, since the function doesn't change master
> permissions. It just needs to prevent concurrent writes to the
> drm_master.magic_map idr.

Yeah for authmagic we could perhaps just reuse the spinlock to protect
->magic_map?

> For other ioctls, I'll take a closer look on a case-by-case basis.

If it's too much shuffling then I think totally fine to leave things
as-is. Just feels a bit silly to have 3 locks, on of which is an
rwlock itself, for this fairly small amount of state.
-Daniel

>
> >>
> >>> ---
> >>> 0-DAY CI Kernel Test Service, Intel Corporation
> >>> https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
> >>>
> >>
> >
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[Intel-gfx] [PATCH 18/22] drm/i915/guc: Rework and simplify locking

Rework and simplify the locking with GuC subission. Drop
sched_state_no_lock and move all fields under the guc_state.sched_state
and protect all these fields with guc_state.lock . This requires
changing the locking hierarchy from guc_state.lock -> sched_engine.lock
to sched_engine.lock -> guc_state.lock.

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/intel_context_types.h |   5 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 186 --
 drivers/gpu/drm/i915/i915_trace.h |   6 +-
 3 files changed, 89 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index c06171ee8792..d5d643b04d54 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -161,7 +161,7 @@ struct intel_context {
 * sched_state: scheduling state of this context using GuC
 * submission
 */
-   u16 sched_state;
+   u32 sched_state;
/*
 * fences: maintains of list of requests that have a submit
 * fence related to GuC submission
@@ -178,9 +178,6 @@ struct intel_context {
struct list_head requests;
} guc_active;
 
-   /* GuC scheduling state flags that do not require a lock. */
-   atomic_t guc_sched_state_no_lock;
-
/* GuC LRC descriptor ID */
u16 guc_id;
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 7aa16371908a..ba19b99173fc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -72,86 +72,23 @@ guc_create_virtual(struct intel_engine_cs **siblings, 
unsigned int count);
 
 #define GUC_REQUEST_SIZE 64 /* bytes */
 
-/*
- * Below is a set of functions which control the GuC scheduling state which do
- * not require a lock as all state transitions are mutually exclusive. i.e. It
- * is not possible for the context pinning code and submission, for the same
- * context, to be executing simultaneously. We still need an atomic as it is
- * possible for some of the bits to changing at the same time though.
- */
-#define SCHED_STATE_NO_LOCK_ENABLEDBIT(0)
-#define SCHED_STATE_NO_LOCK_PENDING_ENABLE BIT(1)
-#define SCHED_STATE_NO_LOCK_REGISTERED BIT(2)
-static inline bool context_enabled(struct intel_context *ce)
-{
-   return (atomic_read(>guc_sched_state_no_lock) &
-   SCHED_STATE_NO_LOCK_ENABLED);
-}
-
-static inline void set_context_enabled(struct intel_context *ce)
-{
-   atomic_or(SCHED_STATE_NO_LOCK_ENABLED, >guc_sched_state_no_lock);
-}
-
-static inline void clr_context_enabled(struct intel_context *ce)
-{
-   atomic_and((u32)~SCHED_STATE_NO_LOCK_ENABLED,
-  >guc_sched_state_no_lock);
-}
-
-static inline bool context_pending_enable(struct intel_context *ce)
-{
-   return (atomic_read(>guc_sched_state_no_lock) &
-   SCHED_STATE_NO_LOCK_PENDING_ENABLE);
-}
-
-static inline void set_context_pending_enable(struct intel_context *ce)
-{
-   atomic_or(SCHED_STATE_NO_LOCK_PENDING_ENABLE,
- >guc_sched_state_no_lock);
-}
-
-static inline void clr_context_pending_enable(struct intel_context *ce)
-{
-   atomic_and((u32)~SCHED_STATE_NO_LOCK_PENDING_ENABLE,
-  >guc_sched_state_no_lock);
-}
-
-static inline bool context_registered(struct intel_context *ce)
-{
-   return (atomic_read(>guc_sched_state_no_lock) &
-   SCHED_STATE_NO_LOCK_REGISTERED);
-}
-
-static inline void set_context_registered(struct intel_context *ce)
-{
-   atomic_or(SCHED_STATE_NO_LOCK_REGISTERED,
- >guc_sched_state_no_lock);
-}
-
-static inline void clr_context_registered(struct intel_context *ce)
-{
-   atomic_and((u32)~SCHED_STATE_NO_LOCK_REGISTERED,
-  >guc_sched_state_no_lock);
-}
-
 /*
  * Below is a set of functions which control the GuC scheduling state which
- * require a lock, aside from the special case where the functions are called
- * from guc_lrc_desc_pin(). In that case it isn't possible for any other code
- * path to be executing on the context.
+ * require a lock.
  */
 #define SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTERBIT(0)
 #define SCHED_STATE_DESTROYED  BIT(1)
 #define SCHED_STATE_PENDING_DISABLEBIT(2)
 #define SCHED_STATE_BANNED BIT(3)
-#define SCHED_STATE_BLOCKED_SHIFT  4
+#define SCHED_STATE_ENABLEDBIT(4)
+#define SCHED_STATE_PENDING_ENABLE BIT(5)
+#define SCHED_STATE_REGISTERED BIT(6)
+#define SCHED_STATE_BLOCKED_SHIFT  7
 #define SCHED_STATE_BLOCKEDBIT(SCHED_STATE_BLOCKED_SHIFT)

[Intel-gfx] [PATCH 16/22] drm/i915/guc: Release submit fence from an IRQ

A subsequent patch will flip the locking hierarchy from
ce->guc_state.lock -> sched_engine->lock to sched_engine->lock ->
ce->guc_state.lock. As such we need to release the submit fence for a
request from an IRQ to break a lock inversion - i.e. the fence must be
release went holding ce->guc_state.lock and the releasing of the can
acquire sched_engine->lock.

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 ++-
 drivers/gpu/drm/i915/i915_request.h   |  5 +
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 8c560ed14976..9ae4633aa7cb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -2017,6 +2017,14 @@ static const struct intel_context_ops guc_context_ops = {
.create_virtual = guc_create_virtual,
 };
 
+static void submit_work_cb(struct irq_work *wrk)
+{
+   struct i915_request *rq = container_of(wrk, typeof(*rq), submit_work);
+
+   might_lock(>engine->sched_engine->lock);
+   i915_sw_fence_complete(>submit);
+}
+
 static void __guc_signal_context_fence(struct intel_context *ce)
 {
struct i915_request *rq;
@@ -2026,8 +2034,12 @@ static void __guc_signal_context_fence(struct 
intel_context *ce)
if (!list_empty(>guc_state.fences))
trace_intel_context_fence_release(ce);
 
+   /*
+* Use an IRQ to ensure locking order of sched_engine->lock ->
+* ce->guc_state.lock is preserved.
+*/
list_for_each_entry(rq, >guc_state.fences, guc_fence_link)
-   i915_sw_fence_complete(>submit);
+   irq_work_queue(>submit_work);
 
INIT_LIST_HEAD(>guc_state.fences);
 }
@@ -2137,6 +2149,7 @@ static int guc_request_alloc(struct i915_request *rq)
spin_lock_irqsave(>guc_state.lock, flags);
if (context_wait_for_deregister_to_register(ce) ||
context_pending_disable(ce)) {
+   init_irq_work(>submit_work, submit_work_cb);
i915_sw_fence_await(>submit);
 
list_add_tail(>guc_fence_link, >guc_state.fences);
diff --git a/drivers/gpu/drm/i915/i915_request.h 
b/drivers/gpu/drm/i915/i915_request.h
index 1bc1349ba3c2..d818cfbfc41d 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -218,6 +218,11 @@ struct i915_request {
};
struct llist_head execute_cb;
struct i915_sw_fence semaphore;
+   /**
+* @submit_work: complete submit fence from an IRQ if needed for
+* locking hierarchy reasons.
+*/
+   struct irq_work submit_work;
 
/*
 * A list of everyone we wait upon, and everyone who waits upon us.
-- 
2.32.0

[Intel-gfx] [PATCH 13/22] drm/i915/guc: Reset LRC descriptor if register returns -ENODEV

Reset LRC descriptor if a context register returns -ENODEV as this means
we are mid-reset.

Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC interface")
Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 8d45585773f3..3a01743e09ea 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1399,10 +1399,12 @@ static int guc_lrc_desc_pin(struct intel_context *ce, 
bool loop)
} else {
with_intel_runtime_pm(runtime_pm, wakeref)
ret = register_context(ce, loop);
-   if (unlikely(ret == -EBUSY))
+   if (unlikely(ret == -EBUSY)) {
+   reset_lrc_desc(guc, desc_idx);
+   } else if (unlikely(ret == -ENODEV)) {
reset_lrc_desc(guc, desc_idx);
-   else if (unlikely(ret == -ENODEV))
ret = 0;/* Will get registered later */
+   }
}
 
return ret;
-- 
2.32.0

[Intel-gfx] [PATCH 21/22] drm/i915/guc: Move GuC priority fields in context under guc_active

Move GuC management fields in context under guc_active struct as this is
where the lock that protects theses fields lives. Also only set guc_prio
field once during context init.

Fixes: ee242ca704d3 ("drm/i915/guc: Implement GuC priority management")
Signed-off-by: Matthew Brost 
Cc: 
---
 drivers/gpu/drm/i915/gt/intel_context_types.h | 12 ++--
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 68 +++
 drivers/gpu/drm/i915/i915_trace.h |  2 +-
 3 files changed, 45 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 524a35a78bf4..f6989e6807f7 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -112,6 +112,7 @@ struct intel_context {
 #define CONTEXT_FORCE_SINGLE_SUBMISSION7
 #define CONTEXT_NOPREEMPT  8
 #define CONTEXT_LRCA_DIRTY 9
+#define CONTEXT_GUC_INIT   10
 
struct {
u64 timeout_us;
@@ -178,6 +179,11 @@ struct intel_context {
spinlock_t lock;
/** requests: active requests on this context */
struct list_head requests;
+   /*
+* GuC priority management
+*/
+   u8 guc_prio;
+   u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_active;
 
/* GuC LRC descriptor ID */
@@ -191,12 +197,6 @@ struct intel_context {
 */
struct list_head guc_id_link;
 
-   /*
-* GuC priority management
-*/
-   u8 guc_prio;
-   u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
-
 #ifdef CONFIG_DRM_I915_SELFTEST
/**
 * @drop_schedule_enable: Force drop of schedule enable G2H for selftest
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index c6ae6b4417c2..eb06a4c7534e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1354,8 +1354,6 @@ static void guc_context_policy_init(struct 
intel_engine_cs *engine,
desc->preemption_timeout = engine->props.preempt_timeout_ms * 1000;
 }
 
-static inline u8 map_i915_prio_to_guc_prio(int prio);
-
 static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
 {
struct intel_engine_cs *engine = ce->engine;
@@ -1363,8 +1361,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce, 
bool loop)
struct intel_guc *guc = >gt->uc.guc;
u32 desc_idx = ce->guc_id;
struct guc_lrc_desc *desc;
-   const struct i915_gem_context *ctx;
-   int prio = I915_CONTEXT_DEFAULT_PRIORITY;
bool context_registered;
intel_wakeref_t wakeref;
int ret = 0;
@@ -1381,12 +1377,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce, 
bool loop)
 
context_registered = lrc_desc_registered(guc, desc_idx);
 
-   rcu_read_lock();
-   ctx = rcu_dereference(ce->gem_context);
-   if (ctx)
-   prio = ctx->sched.priority;
-   rcu_read_unlock();
-
reset_lrc_desc(guc, desc_idx);
set_lrc_desc_registered(guc, desc_idx, ce);
 
@@ -1395,8 +1385,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce, 
bool loop)
desc->engine_submit_mask = adjust_engine_mask(engine->class,
  engine->mask);
desc->hw_context_desc = ce->lrc.lrca;
-   ce->guc_prio = map_i915_prio_to_guc_prio(prio);
-   desc->priority = ce->guc_prio;
+   desc->priority = ce->guc_active.guc_prio;
desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
guc_context_policy_init(engine, desc);
 
@@ -1798,10 +1787,10 @@ static inline void guc_lrc_desc_unpin(struct 
intel_context *ce)
 
 static void __guc_context_destroy(struct intel_context *ce)
 {
-   GEM_BUG_ON(ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] ||
-  ce->guc_prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
-  ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
-  ce->guc_prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
+   GEM_BUG_ON(ce->guc_active.guc_prio_count[GUC_CLIENT_PRIORITY_KMD_HIGH] 
||
+  ce->guc_active.guc_prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
+  
ce->guc_active.guc_prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
+  ce->guc_active.guc_prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
GEM_BUG_ON(ce->guc_state.number_committed_requests);
 
lrc_fini(ce);
@@ -1911,14 +1900,17 @@ static void guc_context_set_prio(struct intel_guc *guc,
 
GEM_BUG_ON(prio < GUC_CLIENT_PRIORITY_KMD_HIGH ||
   prio > GUC_CLIENT_PRIORITY_NORMAL);
+   lockdep_assert_held(>guc_active.lock);
 
-   if (ce->guc_prio == prio || submission_disabled(guc) ||
-   !context_registered(ce))
+   if (ce->guc_active.guc_prio ==

[Intel-gfx] [PATCH 20/22] drm/i915/guc: Drop pin count check trick between sched_disable and re-pin

Drop pin count check trick between a sched_disable and re-pin, now rely
on the lock and counter of the number of committed requests to determine
if scheduling should be disabled on the context.

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/intel_context_types.h |  2 +
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 49 ---
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index d5d643b04d54..524a35a78bf4 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -169,6 +169,8 @@ struct intel_context {
struct list_head fences;
/* GuC context blocked fence */
struct i915_sw_fence blocked_fence;
+   /* GuC committed requests */
+   int number_committed_requests;
} guc_state;
 
struct {
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 2ecb2f002bed..c6ae6b4417c2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -247,6 +247,25 @@ static inline void decr_context_blocked(struct 
intel_context *ce)
ce->guc_state.sched_state -= SCHED_STATE_BLOCKED;
 }
 
+static inline bool context_has_committed_requests(struct intel_context *ce)
+{
+   return !!ce->guc_state.number_committed_requests;
+}
+
+static inline void incr_context_committed_requests(struct intel_context *ce)
+{
+   lockdep_assert_held(>guc_state.lock);
+   ++ce->guc_state.number_committed_requests;
+   GEM_BUG_ON(ce->guc_state.number_committed_requests < 0);
+}
+
+static inline void decr_context_committed_requests(struct intel_context *ce)
+{
+   lockdep_assert_held(>guc_state.lock);
+   --ce->guc_state.number_committed_requests;
+   GEM_BUG_ON(ce->guc_state.number_committed_requests < 0);
+}
+
 static inline bool context_guc_id_invalid(struct intel_context *ce)
 {
return ce->guc_id == GUC_INVALID_LRC_ID;
@@ -1736,14 +1755,11 @@ static void guc_context_sched_disable(struct 
intel_context *ce)
spin_lock_irqsave(>guc_state.lock, flags);
 
/*
-* We have to check if the context has been disabled by another thread.
-* We also have to check if the context has been pinned again as another
-* pin operation is allowed to pass this function. Checking the pin
-* count, within ce->guc_state.lock, synchronizes this function with
-* guc_request_alloc ensuring a request doesn't slip through the
-* 'context_pending_disable' fence. Checking within the spin lock (can't
-* sleep) ensures another process doesn't pin this context and generate
-* a request before we set the 'context_pending_disable' flag here.
+* We have to check if the context has been disabled by another thread,
+* check if submssion has been disabled to seal a race with reset and
+* finally check if any more requests have been committed to the
+* context ensursing that a request doesn't slip through the
+* 'context_pending_disable' fence.
 */
enabled = context_enabled(ce);
if (unlikely(!enabled || submission_disabled(guc))) {
@@ -1752,7 +1768,8 @@ static void guc_context_sched_disable(struct 
intel_context *ce)
spin_unlock_irqrestore(>guc_state.lock, flags);
goto unpin;
}
-   if (unlikely(atomic_add_unless(>pin_count, -2, 2))) {
+   if (unlikely(context_has_committed_requests(ce))) {
+   intel_context_sched_disable_unpin(ce);
spin_unlock_irqrestore(>guc_state.lock, flags);
return;
}
@@ -1785,6 +1802,7 @@ static void __guc_context_destroy(struct intel_context 
*ce)
   ce->guc_prio_count[GUC_CLIENT_PRIORITY_HIGH] ||
   ce->guc_prio_count[GUC_CLIENT_PRIORITY_KMD_NORMAL] ||
   ce->guc_prio_count[GUC_CLIENT_PRIORITY_NORMAL]);
+   GEM_BUG_ON(ce->guc_state.number_committed_requests);
 
lrc_fini(ce);
intel_context_fini(ce);
@@ -2015,6 +2033,10 @@ static void remove_from_context(struct i915_request *rq)
 
spin_unlock_irq(>guc_active.lock);
 
+   spin_lock_irq(>guc_state.lock);
+   decr_context_committed_requests(ce);
+   spin_unlock_irq(>guc_state.lock);
+
atomic_dec(>guc_id_ref);
i915_request_notify_execute_cb_imm(rq);
 }
@@ -2162,15 +2184,7 @@ static int guc_request_alloc(struct i915_request *rq)
 * schedule enable or context registration if either G2H is pending
 * respectfully. Once a G2H returns, the fence is released that is
 * blocking these requests (see guc_signal_context_fence).
-*
-* We can safely check the below fields outside of the lock as it isn't
-*

[Intel-gfx] [PATCH 17/22] drm/i915/guc: Move guc_blocked fence to struct guc_state

Move guc_blocked fence to struct guc_state as the lock which protects
the fence lives there.

s/ce->guc_blocked/ce->guc_state.blocked_fence/g

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/intel_context.c|  5 +++--
 drivers/gpu/drm/i915/gt/intel_context_types.h  |  5 ++---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c  | 18 +-
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index 745e84c72c90..0e48939ec85f 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -405,8 +405,9 @@ intel_context_init(struct intel_context *ce, struct 
intel_engine_cs *engine)
 * Initialize fence to be complete as this is expected to be complete
 * unless there is a pending schedule disable outstanding.
 */
-   i915_sw_fence_init(>guc_blocked, sw_fence_dummy_notify);
-   i915_sw_fence_commit(>guc_blocked);
+   i915_sw_fence_init(>guc_state.blocked_fence,
+  sw_fence_dummy_notify);
+   i915_sw_fence_commit(>guc_state.blocked_fence);
 
i915_active_init(>active,
 __intel_context_active, __intel_context_retire, 0);
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 3a73f3117873..c06171ee8792 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -167,6 +167,8 @@ struct intel_context {
 * fence related to GuC submission
 */
struct list_head fences;
+   /* GuC context blocked fence */
+   struct i915_sw_fence blocked_fence;
} guc_state;
 
struct {
@@ -190,9 +192,6 @@ struct intel_context {
 */
struct list_head guc_id_link;
 
-   /* GuC context blocked fence */
-   struct i915_sw_fence guc_blocked;
-
/*
 * GuC priority management
 */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 9ae4633aa7cb..7aa16371908a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1482,24 +1482,24 @@ static void guc_blocked_fence_complete(struct 
intel_context *ce)
 {
lockdep_assert_held(>guc_state.lock);
 
-   if (!i915_sw_fence_done(>guc_blocked))
-   i915_sw_fence_complete(>guc_blocked);
+   if (!i915_sw_fence_done(>guc_state.blocked_fence))
+   i915_sw_fence_complete(>guc_state.blocked_fence);
 }
 
 static void guc_blocked_fence_reinit(struct intel_context *ce)
 {
lockdep_assert_held(>guc_state.lock);
-   GEM_BUG_ON(!i915_sw_fence_done(>guc_blocked));
+   GEM_BUG_ON(!i915_sw_fence_done(>guc_state.blocked_fence));
 
/*
 * This fence is always complete unless a pending schedule disable is
 * outstanding. We arm the fence here and complete it when we receive
 * the pending schedule disable complete message.
 */
-   i915_sw_fence_fini(>guc_blocked);
-   i915_sw_fence_reinit(>guc_blocked);
-   i915_sw_fence_await(>guc_blocked);
-   i915_sw_fence_commit(>guc_blocked);
+   i915_sw_fence_fini(>guc_state.blocked_fence);
+   i915_sw_fence_reinit(>guc_state.blocked_fence);
+   i915_sw_fence_await(>guc_state.blocked_fence);
+   i915_sw_fence_commit(>guc_state.blocked_fence);
 }
 
 static u16 prep_context_pending_disable(struct intel_context *ce)
@@ -1539,7 +1539,7 @@ static struct i915_sw_fence *guc_context_block(struct 
intel_context *ce)
if (enabled)
clr_context_enabled(ce);
spin_unlock_irqrestore(>guc_state.lock, flags);
-   return >guc_blocked;
+   return >guc_state.blocked_fence;
}
 
/*
@@ -1555,7 +1555,7 @@ static struct i915_sw_fence *guc_context_block(struct 
intel_context *ce)
with_intel_runtime_pm(runtime_pm, wakeref)
__guc_context_sched_disable(guc, ce, guc_id);
 
-   return >guc_blocked;
+   return >guc_state.blocked_fence;
 }
 
 static void guc_context_unblock(struct intel_context *ce)
-- 
2.32.0

[Intel-gfx] [PATCH 15/22] drm/i915/guc: Flush G2H work queue during reset

It isn't safe to scrub for missing G2H or continue with the reset until
all G2H processing is complete. Flush the G2H work queue during reset to
ensure it is done running.

Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC interface")
Signed-off-by: Matthew Brost 
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c  | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 3a01743e09ea..8c560ed14976 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -707,8 +707,6 @@ static void guc_flush_submissions(struct intel_guc *guc)
 
 void intel_guc_submission_reset_prepare(struct intel_guc *guc)
 {
-   int i;
-
if (unlikely(!guc_submission_initialized(guc))) {
/* Reset called during driver load? GuC not yet initialised! */
return;
@@ -724,20 +722,8 @@ void intel_guc_submission_reset_prepare(struct intel_guc 
*guc)
 
guc_flush_submissions(guc);
 
-   /*
-* Handle any outstanding G2Hs before reset. Call IRQ handler directly
-* each pass as interrupt have been disabled. We always scrub for
-* outstanding G2H as it is possible for outstanding_submission_g2h to
-* be incremented after the context state update.
-*/
-   for (i = 0; i < 4 && atomic_read(>outstanding_submission_g2h); 
++i) {
-   intel_guc_to_host_event_handler(guc);
-#define wait_for_reset(guc, wait_var) \
-   intel_guc_wait_for_pending_msg(guc, wait_var, false, (HZ / 20))
-   do {
-   wait_for_reset(guc, >outstanding_submission_g2h);
-   } while (!list_empty(>ct.requests.incoming));
-   }
+   flush_work(>ct.requests.worker);
+
scrub_guc_desc_for_outstanding_g2h(guc);
 }
 
-- 
2.32.0

[Intel-gfx] [PATCH 12/22] drm/i915/guc: Don't touch guc_state.sched_state without a lock

Before we did some clever tricks to not use the a lock when touching
guc_state.sched_state in certain cases. Don't do that, enforce the use
of the lock.

Part of this is removing a dead code path from guc_lrc_desc_pin where a
context could be deregistered when the aforementioned function was
called from the submission path. Remove this dead code and add a
GEM_BUG_ON if this path is ever attempted to be used.

Signed-off-by: Matthew Brost 
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 57 ++-
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 89126be26786..8d45585773f3 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -150,11 +150,22 @@ static inline void clr_context_registered(struct 
intel_context *ce)
 #define SCHED_STATE_BLOCKED_MASK   (0xfff << SCHED_STATE_BLOCKED_SHIFT)
 static inline void init_sched_state(struct intel_context *ce)
 {
-   /* Only should be called from guc_lrc_desc_pin() */
+   lockdep_assert_held(>guc_state.lock);
atomic_set(>guc_sched_state_no_lock, 0);
ce->guc_state.sched_state &= SCHED_STATE_BLOCKED_MASK;
 }
 
+static inline bool sched_state_is_init(struct intel_context *ce)
+{
+   /*
+* XXX: Kernel contexts can have SCHED_STATE_NO_LOCK_REGISTERED after
+* suspend.
+*/
+   return !(atomic_read(>guc_sched_state_no_lock) &
+~SCHED_STATE_NO_LOCK_REGISTERED) &&
+   !(ce->guc_state.sched_state &= ~SCHED_STATE_BLOCKED_MASK);
+}
+
 static inline bool
 context_wait_for_deregister_to_register(struct intel_context *ce)
 {
@@ -165,7 +176,7 @@ context_wait_for_deregister_to_register(struct 
intel_context *ce)
 static inline void
 set_context_wait_for_deregister_to_register(struct intel_context *ce)
 {
-   /* Only should be called from guc_lrc_desc_pin() without lock */
+   lockdep_assert_held(>guc_state.lock);
ce->guc_state.sched_state |=
SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER;
 }
@@ -599,9 +610,7 @@ static void scrub_guc_desc_for_outstanding_g2h(struct 
intel_guc *guc)
bool pending_disable, pending_enable, deregister, destroyed, banned;
 
xa_for_each(>context_lookup, index, ce) {
-   /* Flush context */
spin_lock_irqsave(>guc_state.lock, flags);
-   spin_unlock_irqrestore(>guc_state.lock, flags);
 
/*
 * Once we are at this point submission_disabled() is guaranteed
@@ -617,6 +626,8 @@ static void scrub_guc_desc_for_outstanding_g2h(struct 
intel_guc *guc)
banned = context_banned(ce);
init_sched_state(ce);
 
+   spin_unlock_irqrestore(>guc_state.lock, flags);
+
if (pending_enable || destroyed || deregister) {
atomic_dec(>outstanding_submission_g2h);
if (deregister)
@@ -1318,6 +1329,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce, 
bool loop)
int ret = 0;
 
GEM_BUG_ON(!engine->mask);
+   GEM_BUG_ON(!sched_state_is_init(ce));
 
/*
 * Ensure LRC + CT vmas are is same region as write barrier is done
@@ -1346,7 +1358,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce, 
bool loop)
desc->priority = ce->guc_prio;
desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
guc_context_policy_init(engine, desc);
-   init_sched_state(ce);
 
/*
 * The context_lookup xarray is used to determine if the hardware
@@ -1357,26 +1368,23 @@ static int guc_lrc_desc_pin(struct intel_context *ce, 
bool loop)
 * registering this context.
 */
if (context_registered) {
+   bool disabled;
+   unsigned long flags;
+
trace_intel_context_steal_guc_id(ce);
-   if (!loop) {
+   GEM_BUG_ON(!loop);
+
+   /* Seal race with Reset */
+   spin_lock_irqsave(>guc_state.lock, flags);
+   disabled = submission_disabled(guc);
+   if (likely(!disabled)) {
set_context_wait_for_deregister_to_register(ce);
intel_context_get(ce);
-   } else {
-   bool disabled;
-   unsigned long flags;
-
-   /* Seal race with Reset */
-   spin_lock_irqsave(>guc_state.lock, flags);
-   disabled = submission_disabled(guc);
-   if (likely(!disabled)) {
-   set_context_wait_for_deregister_to_register(ce);
-   intel_context_get(ce);
-   }
-   spin_unlock_irqrestore(>guc_state.lock, flags);
-   if (unlikely(disabled)) {
-

[Intel-gfx] [PATCH 22/22] drm/i915/guc: Add GuC kernel doc

Add GuC kernel doc for all structures added thus far for GuC submission
and update the main GuC submission section with the new interface
details.

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/intel_context_types.h |  42 +---
 drivers/gpu/drm/i915/gt/uc/intel_guc.h|  19 +++-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 101 ++
 drivers/gpu/drm/i915/i915_request.h   |  18 ++--
 4 files changed, 131 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index f6989e6807f7..75d609a1bc33 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -156,44 +156,56 @@ struct intel_context {
u8 wa_bb_page; /* if set, page num reserved for context workarounds */
 
struct {
-   /** lock: protects everything in guc_state */
+   /** @lock: protects everything in guc_state */
spinlock_t lock;
/**
-* sched_state: scheduling state of this context using GuC
+* @sched_state: scheduling state of this context using GuC
 * submission
 */
u32 sched_state;
/*
-* fences: maintains of list of requests that have a submit
-* fence related to GuC submission
+* @fences: maintains a list of requests are currently being
+* fenced until a GuC operation completes
 */
struct list_head fences;
-   /* GuC context blocked fence */
+   /**
+* @blocked_fence: fence used to signal when the blocking of a
+* contexts submissions is complete.
+*/
struct i915_sw_fence blocked_fence;
-   /* GuC committed requests */
+   /** @number_committed_requests: number of committed requests */
int number_committed_requests;
} guc_state;
 
struct {
-   /** lock: protects everything in guc_active */
+   /** @lock: protects everything in guc_active */
spinlock_t lock;
-   /** requests: active requests on this context */
+   /** @requests: list of active requests on this context */
struct list_head requests;
-   /*
-* GuC priority management
-*/
+   /** @guc_prio: the contexts current guc priority */
u8 guc_prio;
+   /**
+* @guc_prio_count: a counter of the number requests inflight in
+* each priority bucket
+*/
u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
} guc_active;
 
-   /* GuC LRC descriptor ID */
+   /**
+* @guc_id: unique handle which is used to communicate information with
+* the GuC about this context, protected by guc->contexts_lock
+*/
u16 guc_id;
 
-   /* GuC LRC descriptor reference count */
+   /**
+* @guc_id_ref: the number of references to the guc_id, when
+* transitioning in and out of zero protected by guc->contexts_lock
+*/
atomic_t guc_id_ref;
 
-   /*
-* GuC ID link - in list when unpinned but guc_id still valid in GuC
+   /**
+* @guc_id_link: in guc->guc_id_list when the guc_id has no refs but is
+* still valid, protected by guc->contexts_lock
 */
struct list_head guc_id_link;
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index 2e27fe59786b..c0b3fdb601f0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -41,6 +41,10 @@ struct intel_guc {
spinlock_t irq_lock;
unsigned int msg_enabled_mask;
 
+   /**
+* @outstanding_submission_g2h: number of outstanding G2H related to GuC
+* submission, used to determine if the GT is idle
+*/
atomic_t outstanding_submission_g2h;
 
struct {
@@ -49,12 +53,16 @@ struct intel_guc {
void (*disable)(struct intel_guc *guc);
} interrupts;
 
-   /*
-* contexts_lock protects the pool of free guc ids and a linked list of
-* guc ids available to be stolen
+   /**
+* @contexts_lock: protects guc_ids, guc_id_list, ce->guc_id, and
+* ce->guc_id_ref when transitioning in and out of zero
 */
spinlock_t contexts_lock;
+   /** @guc_ids: used to allocate new guc_ids */
struct ida guc_ids;
+   /**
+* @guc_id_list: list of intel_context with valid guc_ids but no refs
+*/
struct list_head guc_id_list;
 
bool submission_supported;
@@ -70,7 +78,10 @@ struct intel_guc {
struct i915_vma

[Intel-gfx] [PATCH 11/22] drm/i915/guc: Take context ref when cancelling request

A context can get destroyed after cancelling a request so take a
reference to context when cancelling a request.

Fixes: 62eaf0ae217d ("drm/i915/guc: Support request cancellation")
Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index bffd0199dc15..89126be26786 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1613,8 +1613,10 @@ static void guc_context_cancel_request(struct 
intel_context *ce,
   struct i915_request *rq)
 {
if (i915_sw_fence_signaled(>submit)) {
-   struct i915_sw_fence *fence = guc_context_block(ce);
+   struct i915_sw_fence *fence;
 
+   intel_context_get(ce);
+   fence = guc_context_block(ce);
i915_sw_fence_wait(fence);
if (!i915_request_completed(rq)) {
__i915_request_skip(rq);
@@ -1629,6 +1631,7 @@ static void guc_context_cancel_request(struct 
intel_context *ce,
flush_work(_to_guc(ce)->ct.requests.worker);
 
guc_context_unblock(ce);
+   intel_context_put(ce);
}
 }
 
-- 
2.32.0

[Intel-gfx] [PATCH 19/22] drm/i915/guc: Proper xarray usage for contexts_lookup

Lock the xarray and take ref to the context if needed.

v2:
 (Checkpatch)
  - Add new line after declaration

Signed-off-by: Matthew Brost 
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 84 ---
 1 file changed, 73 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index ba19b99173fc..2ecb2f002bed 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -599,8 +599,18 @@ static void scrub_guc_desc_for_outstanding_g2h(struct 
intel_guc *guc)
unsigned long index, flags;
bool pending_disable, pending_enable, deregister, destroyed, banned;
 
+   xa_lock_irqsave(>context_lookup, flags);
xa_for_each(>context_lookup, index, ce) {
-   spin_lock_irqsave(>guc_state.lock, flags);
+   /*
+* Corner case where the ref count on the object is zero but and
+* deregister G2H was lost. In this case we don't touch the ref
+* count and finish the destroy of the context.
+*/
+   bool do_put = kref_get_unless_zero(>ref);
+
+   xa_unlock(>context_lookup);
+
+   spin_lock(>guc_state.lock);
 
/*
 * Once we are at this point submission_disabled() is guaranteed
@@ -616,7 +626,9 @@ static void scrub_guc_desc_for_outstanding_g2h(struct 
intel_guc *guc)
banned = context_banned(ce);
init_sched_state(ce);
 
-   spin_unlock_irqrestore(>guc_state.lock, flags);
+   spin_unlock(>guc_state.lock);
+
+   GEM_BUG_ON(!do_put && !destroyed);
 
if (pending_enable || destroyed || deregister) {
atomic_dec(>outstanding_submission_g2h);
@@ -645,7 +657,12 @@ static void scrub_guc_desc_for_outstanding_g2h(struct 
intel_guc *guc)
 
intel_context_put(ce);
}
+
+   if (do_put)
+   intel_context_put(ce);
+   xa_lock(>context_lookup);
}
+   xa_unlock_irqrestore(>context_lookup, flags);
 }
 
 static inline bool
@@ -866,16 +883,26 @@ void intel_guc_submission_reset(struct intel_guc *guc, 
bool stalled)
 {
struct intel_context *ce;
unsigned long index;
+   unsigned long flags;
 
if (unlikely(!guc_submission_initialized(guc))) {
/* Reset called during driver load? GuC not yet initialised! */
return;
}
 
-   xa_for_each(>context_lookup, index, ce)
+   xa_lock_irqsave(>context_lookup, flags);
+   xa_for_each(>context_lookup, index, ce) {
+   intel_context_get(ce);
+   xa_unlock(>context_lookup);
+
if (intel_context_is_pinned(ce))
__guc_reset_context(ce, stalled);
 
+   intel_context_put(ce);
+   xa_lock(>context_lookup);
+   }
+   xa_unlock_irqrestore(>context_lookup, flags);
+
/* GuC is blown away, drop all references to contexts */
xa_destroy(>context_lookup);
 }
@@ -950,11 +977,21 @@ void intel_guc_submission_cancel_requests(struct 
intel_guc *guc)
 {
struct intel_context *ce;
unsigned long index;
+   unsigned long flags;
+
+   xa_lock_irqsave(>context_lookup, flags);
+   xa_for_each(>context_lookup, index, ce) {
+   intel_context_get(ce);
+   xa_unlock(>context_lookup);
 
-   xa_for_each(>context_lookup, index, ce)
if (intel_context_is_pinned(ce))
guc_cancel_context_requests(ce);
 
+   intel_context_put(ce);
+   xa_lock(>context_lookup);
+   }
+   xa_unlock_irqrestore(>context_lookup, flags);
+
guc_cancel_sched_engine_requests(guc->sched_engine);
 
/* GuC is blown away, drop all references to contexts */
@@ -2848,21 +2885,26 @@ void intel_guc_find_hung_context(struct intel_engine_cs 
*engine)
struct intel_context *ce;
struct i915_request *rq;
unsigned long index;
+   unsigned long flags;
 
/* Reset called during driver load? GuC not yet initialised! */
if (unlikely(!guc_submission_initialized(guc)))
return;
 
+   xa_lock_irqsave(>context_lookup, flags);
xa_for_each(>context_lookup, index, ce) {
+   intel_context_get(ce);
+   xa_unlock(>context_lookup);
+
if (!intel_context_is_pinned(ce))
-   continue;
+   goto next;
 
if (intel_engine_is_virtual(ce->engine)) {
if (!(ce->engine->mask & engine->mask))
-   continue;
+   goto next;
} else {
if (ce->engine != engine)
-

[Intel-gfx] [PATCH 08/22] drm/i915/guc: Don't enable scheduling on a banned context, guc_id invalid, not registered

When unblocking a context, do not enable scheduling if the context is
banned, guc_id invalid, or not registered.

Fixes: 62eaf0ae217d ("drm/i915/guc: Support request cancellation")
Signed-off-by: Matthew Brost 
Cc: 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index c3b7bf7319dd..353899634fa8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1579,6 +1579,9 @@ static void guc_context_unblock(struct intel_context *ce)
spin_lock_irqsave(>guc_state.lock, flags);
 
if (unlikely(submission_disabled(guc) ||
+intel_context_is_banned(ce) ||
+context_guc_id_invalid(ce) ||
+!lrc_desc_registered(guc, ce->guc_id) ||
 !intel_context_is_pinned(ce) ||
 context_pending_disable(ce) ||
 context_blocked(ce) > 1)) {
-- 
2.32.0

[Intel-gfx] [PATCH 09/22] drm/i915/selftests: Fix memory corruption in live_lrc_isolation

GuC submission has exposed an existing memory corruption in
live_lrc_isolation. We believe that some writes to the watchdog offsets
in the LRC (0x178 & 0x17c) can result in trashing of portions of the
address space. With GuC submission there are additional objects which
can move the context redzone into the space that is trashed. To
workaround this avoid poisoning the watchdog.

v2:
 (Daniel Vetter)
  - Add VLK ref in code to workaround

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/selftest_lrc.c | 29 +-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c 
b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index b0977a3b699b..cdc6ae48a1e1 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -1074,6 +1074,32 @@ record_registers(struct intel_context *ce,
goto err_after;
 }
 
+static u32 safe_offset(u32 offset, u32 reg)
+{
+   /* XXX skip testing of watchdog - VLK-22772 */
+   if (offset == 0x178 || offset == 0x17c)
+   reg = 0;
+
+   return reg;
+}
+
+static int get_offset_mask(struct intel_engine_cs *engine)
+{
+   if (GRAPHICS_VER(engine->i915) < 12)
+   return 0xfff;
+
+   switch (engine->class) {
+   default:
+   case RENDER_CLASS:
+   return 0x07ff;
+   case COPY_ENGINE_CLASS:
+   return 0x0fff;
+   case VIDEO_DECODE_CLASS:
+   case VIDEO_ENHANCEMENT_CLASS:
+   return 0x3fff;
+   }
+}
+
 static struct i915_vma *load_context(struct intel_context *ce, u32 poison)
 {
struct i915_vma *batch;
@@ -1117,7 +1143,8 @@ static struct i915_vma *load_context(struct intel_context 
*ce, u32 poison)
len = (len + 1) / 2;
*cs++ = MI_LOAD_REGISTER_IMM(len);
while (len--) {
-   *cs++ = hw[dw];
+   *cs++ = safe_offset(hw[dw] & 
get_offset_mask(ce->engine),
+   hw[dw]);
*cs++ = poison;
dw += 2;
}
-- 
2.32.0

[Intel-gfx] [PATCH 14/22] drm/i915: Allocate error capture in atomic context

Error captures can now be done in a work queue processing G2H messages.
These messages need to be completely done being processed in the reset
path, to avoid races in the missing G2H cleanup, which create a
dependency on memory allocations and dma fences (i915_requests).
Requests depend on resets, thus now we have a circular dependency. To
work around this, allocate the error capture in an atomic context.

Fixes: dc0dad365c5e ("Fix for error capture after full GPU reset with GuC")
Fixes: 573ba126aef3 ("Capture error state on context reset")
Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 37 +--
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0f08bcfbe964..453376aa6d9f 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -49,7 +49,6 @@
 #include "i915_memcpy.h"
 #include "i915_scatterlist.h"
 
-#define ALLOW_FAIL (GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
 #define ATOMIC_MAYFAIL (GFP_ATOMIC | __GFP_NOWARN)
 
 static void __sg_set_buf(struct scatterlist *sg,
@@ -79,7 +78,7 @@ static bool __i915_error_grow(struct drm_i915_error_state_buf 
*e, size_t len)
if (e->cur == e->end) {
struct scatterlist *sgl;
 
-   sgl = (typeof(sgl))__get_free_page(ALLOW_FAIL);
+   sgl = (typeof(sgl))__get_free_page(ATOMIC_MAYFAIL);
if (!sgl) {
e->err = -ENOMEM;
return false;
@@ -99,10 +98,10 @@ static bool __i915_error_grow(struct 
drm_i915_error_state_buf *e, size_t len)
}
 
e->size = ALIGN(len + 1, SZ_64K);
-   e->buf = kmalloc(e->size, ALLOW_FAIL);
+   e->buf = kmalloc(e->size, ATOMIC_MAYFAIL);
if (!e->buf) {
e->size = PAGE_ALIGN(len + 1);
-   e->buf = kmalloc(e->size, GFP_KERNEL);
+   e->buf = kmalloc(e->size, ATOMIC_MAYFAIL);
}
if (!e->buf) {
e->err = -ENOMEM;
@@ -243,12 +242,12 @@ static bool compress_init(struct i915_vma_compress *c)
 {
struct z_stream_s *zstream = >zstream;
 
-   if (pool_init(>pool, ALLOW_FAIL))
+   if (pool_init(>pool, ATOMIC_MAYFAIL))
return false;
 
zstream->workspace =
kmalloc(zlib_deflate_workspacesize(MAX_WBITS, MAX_MEM_LEVEL),
-   ALLOW_FAIL);
+   ATOMIC_MAYFAIL);
if (!zstream->workspace) {
pool_fini(>pool);
return false;
@@ -256,7 +255,7 @@ static bool compress_init(struct i915_vma_compress *c)
 
c->tmp = NULL;
if (i915_has_memcpy_from_wc())
-   c->tmp = pool_alloc(>pool, ALLOW_FAIL);
+   c->tmp = pool_alloc(>pool, ATOMIC_MAYFAIL);
 
return true;
 }
@@ -280,7 +279,7 @@ static void *compress_next_page(struct i915_vma_compress *c,
if (dst->page_count >= dst->num_pages)
return ERR_PTR(-ENOSPC);
 
-   page = pool_alloc(>pool, ALLOW_FAIL);
+   page = pool_alloc(>pool, ATOMIC_MAYFAIL);
if (!page)
return ERR_PTR(-ENOMEM);
 
@@ -376,7 +375,7 @@ struct i915_vma_compress {
 
 static bool compress_init(struct i915_vma_compress *c)
 {
-   return pool_init(>pool, ALLOW_FAIL) == 0;
+   return pool_init(>pool, ATOMIC_MAYFAIL) == 0;
 }
 
 static bool compress_start(struct i915_vma_compress *c)
@@ -391,7 +390,7 @@ static int compress_page(struct i915_vma_compress *c,
 {
void *ptr;
 
-   ptr = pool_alloc(>pool, ALLOW_FAIL);
+   ptr = pool_alloc(>pool, ATOMIC_MAYFAIL);
if (!ptr)
return -ENOMEM;
 
@@ -997,7 +996,7 @@ i915_vma_coredump_create(const struct intel_gt *gt,
 
num_pages = min_t(u64, vma->size, vma->obj->base.size) >> PAGE_SHIFT;
num_pages = DIV_ROUND_UP(10 * num_pages, 8); /* worstcase zlib growth */
-   dst = kmalloc(sizeof(*dst) + num_pages * sizeof(u32 *), ALLOW_FAIL);
+   dst = kmalloc(sizeof(*dst) + num_pages * sizeof(u32 *), ATOMIC_MAYFAIL);
if (!dst)
return NULL;
 
@@ -1433,7 +1432,7 @@ capture_engine(struct intel_engine_cs *engine,
struct i915_request *rq = NULL;
unsigned long flags;
 
-   ee = intel_engine_coredump_alloc(engine, GFP_KERNEL);
+   ee = intel_engine_coredump_alloc(engine, ATOMIC_MAYFAIL);
if (!ee)
return NULL;
 
@@ -1481,7 +1480,7 @@ gt_record_engines(struct intel_gt_coredump *gt,
struct intel_engine_coredump *ee;
 
/* Refill our page pool before entering atomic section */
-   pool_refill(>pool, ALLOW_FAIL);
+   pool_refill(>pool, ATOMIC_MAYFAIL);
 
ee = capture_engine(engine, compress);
if (!ee)
@@ -1507,7 +1506,7 @@ gt_record_uc(struct intel_gt_coredump *gt,
const struct intel_uc *uc = >_gt->uc;

[Intel-gfx] [PATCH 07/22] drm/i915/selftests: Add a cancel request selftest that triggers a reset

Add a cancel request selftest that results in an engine reset to cancel
the request as it is non-preemptable. Also insert a NOP request after
the cancelled request and confirm that it completely successfully.

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/selftests/i915_request.c | 100 ++
 1 file changed, 100 insertions(+)

diff --git a/drivers/gpu/drm/i915/selftests/i915_request.c 
b/drivers/gpu/drm/i915/selftests/i915_request.c
index d67710d10615..e2c5db77f087 100644
--- a/drivers/gpu/drm/i915/selftests/i915_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_request.c
@@ -772,6 +772,98 @@ static int __cancel_completed(struct intel_engine_cs 
*engine)
return err;
 }
 
+static int __cancel_reset(struct intel_engine_cs *engine)
+{
+   struct intel_context *ce;
+   struct igt_spinner spin;
+   struct i915_request *rq, *nop;
+   unsigned long preempt_timeout_ms;
+   int err = 0;
+
+   preempt_timeout_ms = engine->props.preempt_timeout_ms;
+   engine->props.preempt_timeout_ms = 100;
+
+   if (igt_spinner_init(, engine->gt))
+   goto out_restore;
+
+   ce = intel_context_create(engine);
+   if (IS_ERR(ce)) {
+   err = PTR_ERR(ce);
+   goto out_spin;
+   }
+
+   rq = igt_spinner_create_request(, ce, MI_NOOP);
+   if (IS_ERR(rq)) {
+   err = PTR_ERR(rq);
+   goto out_ce;
+   }
+
+   pr_debug("%s: Cancelling active request\n", engine->name);
+   i915_request_get(rq);
+   i915_request_add(rq);
+   if (!igt_wait_for_spinner(, rq)) {
+   struct drm_printer p = drm_info_printer(engine->i915->drm.dev);
+
+   pr_err("Failed to start spinner on %s\n", engine->name);
+   intel_engine_dump(engine, , "%s\n", engine->name);
+   err = -ETIME;
+   goto out_rq;
+   }
+
+   nop = intel_context_create_request(ce);
+   if (IS_ERR(nop))
+   goto out_nop;
+   i915_request_get(nop);
+   i915_request_add(nop);
+
+   i915_request_cancel(rq, -EINTR);
+
+   if (i915_request_wait(rq, 0, HZ) < 0) {
+   struct drm_printer p = drm_info_printer(engine->i915->drm.dev);
+
+   pr_err("%s: Failed to cancel hung request\n", engine->name);
+   intel_engine_dump(engine, , "%s\n", engine->name);
+   err = -ETIME;
+   goto out_nop;
+   }
+
+   if (rq->fence.error != -EINTR) {
+   pr_err("%s: fence not cancelled (%u)\n",
+  engine->name, rq->fence.error);
+   err = -EINVAL;
+   goto out_nop;
+   }
+
+   if (i915_request_wait(nop, 0, HZ) < 0) {
+   struct drm_printer p = drm_info_printer(engine->i915->drm.dev);
+
+   pr_err("%s: Failed to complete nop request\n", engine->name);
+   intel_engine_dump(engine, , "%s\n", engine->name);
+   err = -ETIME;
+   goto out_nop;
+   }
+
+   if (nop->fence.error != 0) {
+   pr_err("%s: Nop request errored (%u)\n",
+  engine->name, nop->fence.error);
+   err = -EINVAL;
+   }
+
+out_nop:
+   i915_request_put(nop);
+out_rq:
+   i915_request_put(rq);
+out_ce:
+   intel_context_put(ce);
+out_spin:
+   igt_spinner_fini();
+out_restore:
+   engine->props.preempt_timeout_ms = preempt_timeout_ms;
+   if (err)
+   pr_err("%s: %s error %d\n", __func__, engine->name, err);
+   return err;
+}
+
 static int live_cancel_request(void *arg)
 {
struct drm_i915_private *i915 = arg;
@@ -804,6 +896,14 @@ static int live_cancel_request(void *arg)
return err;
if (err2)
return err2;
+
+   /* Expects reset so call outside of igt_live_test_* */
+   err = __cancel_reset(engine);
+   if (err)
+   return err;
+
+   if (igt_flush_test(i915))
+   return -EIO;
}
 
return 0;
-- 
2.32.0

[Intel-gfx] [PATCH 05/22] drm/i915/guc: Workaround reset G2H is received after schedule done G2H

If the context is reset as a result of the request cancelation the
context reset G2H is received after schedule disable done G2H which is
likely the wrong order. The schedule disable done G2H release the
waiting request cancelation code which resubmits the context. This races
with the context reset G2H which also wants to resubmit the context but
in this case it really should be a NOP as request cancelation code owns
the resubmit. Use some clever tricks of checking the context state to
seal this race until if / when the GuC firmware is fixed.

v2:
 (Checkpatch)
  - Fix typos

Fixes: 62eaf0ae217d ("drm/i915/guc: Support request cancellation")
Signed-off-by: Matthew Brost 
Cc: 
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 43 ---
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 3cd2da6f5c03..c3b7bf7319dd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -826,17 +826,35 @@ __unwind_incomplete_requests(struct intel_context *ce)
 static void __guc_reset_context(struct intel_context *ce, bool stalled)
 {
struct i915_request *rq;
+   unsigned long flags;
u32 head;
+   bool skip = false;
 
intel_context_get(ce);
 
/*
-* GuC will implicitly mark the context as non-schedulable
-* when it sends the reset notification. Make sure our state
-* reflects this change. The context will be marked enabled
-* on resubmission.
+* GuC will implicitly mark the context as non-schedulable when it sends
+* the reset notification. Make sure our state reflects this change. The
+* context will be marked enabled on resubmission.
+*
+* XXX: If the context is reset as a result of the request cancellation
+* this G2H is received after the schedule disable complete G2H which is
+* likely wrong as this creates a race between the request cancellation
+* code re-submitting the context and this G2H handler. This likely
+* should be fixed in the GuC but until if / when that gets fixed we
+* need to workaround this. Convert this function to a NOP if a pending
+* enable is in flight as this indicates that a request cancellation has
+* occurred.
 */
-   clr_context_enabled(ce);
+   spin_lock_irqsave(>guc_state.lock, flags);
+   if (likely(!context_pending_enable(ce))) {
+   clr_context_enabled(ce);
+   } else {
+   skip = true;
+   }
+   spin_unlock_irqrestore(>guc_state.lock, flags);
+   if (unlikely(skip))
+   goto out_put;
 
rq = intel_context_find_active_request(ce);
if (!rq) {
@@ -855,6 +873,7 @@ static void __guc_reset_context(struct intel_context *ce, 
bool stalled)
 out_replay:
guc_reset_state(ce, head, stalled);
__unwind_incomplete_requests(ce);
+out_put:
intel_context_put(ce);
 }
 
@@ -1599,6 +1618,13 @@ static void guc_context_cancel_request(struct 
intel_context *ce,
guc_reset_state(ce, intel_ring_wrap(ce->ring, rq->head),
true);
}
+
+   /*
+* XXX: Racey if context is reset, see comment in
+* __guc_reset_context().
+*/
+   flush_work(_to_guc(ce)->ct.requests.worker);
+
guc_context_unblock(ce);
}
 }
@@ -2719,7 +2745,12 @@ static void guc_handle_context_reset(struct intel_guc 
*guc,
 {
trace_intel_context_reset(ce);
 
-   if (likely(!intel_context_is_banned(ce))) {
+   /*
+* XXX: Racey if request cancellation has occurred, see comment in
+* __guc_reset_context().
+*/
+   if (likely(!intel_context_is_banned(ce) &&
+  !context_blocked(ce))) {
capture_error_state(guc, ce);
guc_context_replay(ce);
}
-- 
2.32.0

[Intel-gfx] [PATCH 06/22] drm/i915/execlists: Do not propagate errors to dependent fences

Progagating errors to dependent fences is wrong, don't do it. Selftest
in following patch exposes this bug.

Fixes: 8e9f84cf5cac ("drm/i915/gt: Propagate change in error status to children 
on unhold")
Signed-off-by: Matthew Brost 
Cc: 
---
 drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c 
b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index de5f9c86b9a4..cafb0608ffb4 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -2140,10 +2140,6 @@ static void __execlists_unhold(struct i915_request *rq)
if (p->flags & I915_DEPENDENCY_WEAK)
continue;
 
-   /* Propagate any change in error status */
-   if (rq->fence.error)
-   i915_request_set_error_once(w, rq->fence.error);
-
if (w->engine != rq->engine)
continue;
 
-- 
2.32.0

[Intel-gfx] [PATCH 10/22] drm/i915/selftests: Add initial GuC selftest for scrubbing lost G2H

While debugging an issue with full GT resets I went down a rabbit hole
thinking the scrubbing of lost G2H wasn't working correctly. This proved
to be incorrect as this was working just fine but this chase inspired me
to write a selftest to prove that this works. This simple selftest
injects errors dropping various G2H and then issues a full GT reset
proving that the scrubbing of these G2H doesn't blow up.

v2:
 (Daniel Vetter)
  - Use ifdef instead of macros for selftests
v3:
 (Checkpatch)
  - A space after 'switch' statement

Signed-off-by: Matthew Brost 
---
 drivers/gpu/drm/i915/gt/intel_context_types.h |  18 +++
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  25 
 drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 126 ++
 .../drm/i915/selftests/i915_live_selftests.h  |   1 +
 .../i915/selftests/intel_scheduler_helpers.c  |  12 ++
 .../i915/selftests/intel_scheduler_helpers.h  |   2 +
 6 files changed, 184 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/selftest_guc.c

diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h 
b/drivers/gpu/drm/i915/gt/intel_context_types.h
index e54351a170e2..3a73f3117873 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -198,6 +198,24 @@ struct intel_context {
 */
u8 guc_prio;
u32 guc_prio_count[GUC_CLIENT_PRIORITY_NUM];
+
+#ifdef CONFIG_DRM_I915_SELFTEST
+   /**
+* @drop_schedule_enable: Force drop of schedule enable G2H for selftest
+*/
+   bool drop_schedule_enable;
+
+   /**
+* @drop_schedule_disable: Force drop of schedule disable G2H for
+* selftest
+*/
+   bool drop_schedule_disable;
+
+   /**
+* @drop_deregister: Force drop of deregister G2H for selftest
+*/
+   bool drop_deregister;
+#endif
 };
 
 #endif /* __INTEL_CONTEXT_TYPES__ */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 353899634fa8..bffd0199dc15 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -2634,6 +2634,13 @@ int intel_guc_deregister_done_process_msg(struct 
intel_guc *guc,
 
trace_intel_context_deregister_done(ce);
 
+#ifdef CONFIG_DRM_I915_SELFTEST
+   if (unlikely(ce->drop_deregister)) {
+   ce->drop_deregister = false;
+   return 0;
+   }
+#endif
+
if (context_wait_for_deregister_to_register(ce)) {
struct intel_runtime_pm *runtime_pm =
>engine->gt->i915->runtime_pm;
@@ -2688,10 +2695,24 @@ int intel_guc_sched_done_process_msg(struct intel_guc 
*guc,
trace_intel_context_sched_done(ce);
 
if (context_pending_enable(ce)) {
+#ifdef CONFIG_DRM_I915_SELFTEST
+   if (unlikely(ce->drop_schedule_enable)) {
+   ce->drop_schedule_enable = false;
+   return 0;
+   }
+#endif
+
clr_context_pending_enable(ce);
} else if (context_pending_disable(ce)) {
bool banned;
 
+#ifdef CONFIG_DRM_I915_SELFTEST
+   if (unlikely(ce->drop_schedule_disable)) {
+   ce->drop_schedule_disable = false;
+   return 0;
+   }
+#endif
+
/*
 * Unpin must be done before __guc_signal_context_fence,
 * otherwise a race exists between the requests getting
@@ -3068,3 +3089,7 @@ bool intel_guc_virtual_engine_has_heartbeat(const struct 
intel_engine_cs *ve)
 
return false;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_guc.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c 
b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
new file mode 100644
index ..264e2f705c17
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright �� 2021 Intel Corporation
+ */
+
+#include "selftests/intel_scheduler_helpers.h"
+
+static struct i915_request *nop_user_request(struct intel_context *ce,
+struct i915_request *from)
+{
+   struct i915_request *rq;
+   int ret;
+
+   rq = intel_context_create_request(ce);
+   if (IS_ERR(rq))
+   return rq;
+
+   if (from) {
+   ret = i915_sw_fence_await_dma_fence(>submit,
+   >fence, 0,
+   I915_FENCE_GFP);
+   if (ret < 0) {
+   i915_request_put(rq);
+   return ERR_PTR(ret);
+   }
+   }
+
+   i915_request_get(rq);
+   i915_request_add(rq);
+
+   return rq;
+}
+
+static int intel_guc_scrub_ctbs(void *arg)
+{
+   struct intel_gt *gt = arg;
+   int ret = 0;
+   int i;
+

[Intel-gfx] [PATCH 02/22] drm/i915/guc: Fix outstanding G2H accounting

A small race that could result in incorrect accounting of the number
of outstanding G2H. Basically prior to this patch we did not increment
the number of outstanding G2H if we encoutered a GT reset while sending
a H2G. This was incorrect as the context state had already been updated
to anticipate a G2H response thus the counter should be incremented.

Fixes: f4eb1f3fe946 ("drm/i915/guc: Ensure G2H response has space in buffer")
Signed-off-by: Matthew Brost 
Cc: 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 69faa39da178..b5d3972ae164 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -360,11 +360,13 @@ static int guc_submission_send_busy_loop(struct intel_guc 
*guc,
 {
int err;
 
-   err = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
-
-   if (!err && g2h_len_dw)
+   if (g2h_len_dw)
atomic_inc(>outstanding_submission_g2h);
 
+   err = intel_guc_send_busy_loop(guc, action, len, g2h_len_dw, loop);
+   if (err == -EBUSY && g2h_len_dw)
+   atomic_dec(>outstanding_submission_g2h);
+
return err;
 }
 
-- 
2.32.0

[Intel-gfx] [PATCH 04/22] drm/i915/guc: Don't drop ce->guc_active.lock when unwinding context

Don't drop ce->guc_active.lock when unwinding a context after reset.
At one point we had to drop this because of a lock inversion but that is
no longer the case. It is much safer to hold the lock so let's do that.

Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC interface")
Signed-off-by: Matthew Brost 
Cc: 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index bc51caba50d0..3cd2da6f5c03 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -806,8 +806,6 @@ __unwind_incomplete_requests(struct intel_context *ce)
continue;
 
list_del_init(>sched.link);
-   spin_unlock(>guc_active.lock);
-
__i915_request_unsubmit(rq);
 
/* Push the request back into the queue for later resubmission. 
*/
@@ -820,8 +818,6 @@ __unwind_incomplete_requests(struct intel_context *ce)
 
list_add(>sched.link, pl);
set_bit(I915_FENCE_FLAG_PQUEUE, >fence.flags);
-
-   spin_lock(>guc_active.lock);
}
spin_unlock(>guc_active.lock);
spin_unlock_irqrestore(_engine->lock, flags);
-- 
2.32.0

[Intel-gfx] [PATCH 00/22] Clean up GuC CI failures, simplify locking, and kernel DOC

Daniel Vetter pointed out that locking in the GuC submission code was
overly complicated, let's clean this up a bit before introducing more
features in the GuC submission backend.

Also fix some CI failures, port fixes from our internal tree, and add a
few more selftests for coverage.

Lastly, add some kernel DOC explaining how the GuC submission backend
works.

v2: Fix logic error in 'Workaround reset G2H is received after schedule
done G2H', don't propagate errors to dependent fences in execlists
submissiom, resolve checkpatch issues, resend to correct lists

Signed-off-by: Matthew Brost 

Matthew Brost (22):
  drm/i915/guc: Fix blocked context accounting
  drm/i915/guc: Fix outstanding G2H accounting
  drm/i915/guc: Unwind context requests in reverse order
  drm/i915/guc: Don't drop ce->guc_active.lock when unwinding context
  drm/i915/guc: Workaround reset G2H is received after schedule done G2H
  drm/i915/execlists: Do not propagate errors to dependent fences
  drm/i915/selftests: Add a cancel request selftest that triggers a
reset
  drm/i915/guc: Don't enable scheduling on a banned context, guc_id
invalid, not registered
  drm/i915/selftests: Fix memory corruption in live_lrc_isolation
  drm/i915/selftests: Add initial GuC selftest for scrubbing lost G2H
  drm/i915/guc: Take context ref when cancelling request
  drm/i915/guc: Don't touch guc_state.sched_state without a lock
  drm/i915/guc: Reset LRC descriptor if register returns -ENODEV
  drm/i915: Allocate error capture in atomic context
  drm/i915/guc: Flush G2H work queue during reset
  drm/i915/guc: Release submit fence from an IRQ
  drm/i915/guc: Move guc_blocked fence to struct guc_state
  drm/i915/guc: Rework and simplify locking
  drm/i915/guc: Proper xarray usage for contexts_lookup
  drm/i915/guc: Drop pin count check trick between sched_disable and
re-pin
  drm/i915/guc: Move GuC priority fields in context under guc_active
  drm/i915/guc: Add GuC kernel doc

 drivers/gpu/drm/i915/gt/intel_context.c   |   5 +-
 drivers/gpu/drm/i915/gt/intel_context_types.h |  68 +-
 .../drm/i915/gt/intel_execlists_submission.c  |   4 -
 drivers/gpu/drm/i915/gt/selftest_lrc.c|  29 +-
 drivers/gpu/drm/i915/gt/uc/intel_guc.h|  19 +-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 690 +++---
 drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 126 
 drivers/gpu/drm/i915/i915_gpu_error.c |  37 +-
 drivers/gpu/drm/i915/i915_request.h   |  23 +-
 drivers/gpu/drm/i915/i915_trace.h |   8 +-
 .../drm/i915/selftests/i915_live_selftests.h  |   1 +
 drivers/gpu/drm/i915/selftests/i915_request.c | 100 +++
 .../i915/selftests/intel_scheduler_helpers.c  |  12 +
 .../i915/selftests/intel_scheduler_helpers.h  |   2 +
 14 files changed, 813 insertions(+), 311 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/uc/selftest_guc.c

-- 
2.32.0

[Intel-gfx] [PATCH 03/22] drm/i915/guc: Unwind context requests in reverse order

When unwinding requests on a reset context, if other requests in the
context are in the priority list the requests could be resubmitted out
of seqno order. Traverse the list of active requests in reverse and
append to the head of the priority list to fix this.

Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC interface")
Signed-off-by: Matthew Brost 
Cc: 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index b5d3972ae164..bc51caba50d0 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -799,9 +799,9 @@ __unwind_incomplete_requests(struct intel_context *ce)
 
spin_lock_irqsave(_engine->lock, flags);
spin_lock(>guc_active.lock);
-   list_for_each_entry_safe(rq, rn,
->guc_active.requests,
-sched.link) {
+   list_for_each_entry_safe_reverse(rq, rn,
+>guc_active.requests,
+sched.link) {
if (i915_request_completed(rq))
continue;
 
@@ -818,7 +818,7 @@ __unwind_incomplete_requests(struct intel_context *ce)
}
GEM_BUG_ON(i915_sched_engine_is_empty(sched_engine));
 
-   list_add_tail(>sched.link, pl);
+   list_add(>sched.link, pl);
set_bit(I915_FENCE_FLAG_PQUEUE, >fence.flags);
 
spin_lock(>guc_active.lock);
-- 
2.32.0

[Intel-gfx] [PATCH 01/22] drm/i915/guc: Fix blocked context accounting

Prior to this patch the blocked context counter was cleared on
init_sched_state (used during registering a context & resets) which is
incorrect. This state needs to be persistent or the counter can read the
incorrect value resulting in scheduling never getting enabled again.

Fixes: 62eaf0ae217d ("drm/i915/guc: Support request cancellation")
Signed-off-by: Matthew Brost 
Reviewed-by: Daniel Vetter 
Cc: 
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 87d8dc8f51b9..69faa39da178 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -152,7 +152,7 @@ static inline void init_sched_state(struct intel_context 
*ce)
 {
/* Only should be called from guc_lrc_desc_pin() */
atomic_set(>guc_sched_state_no_lock, 0);
-   ce->guc_state.sched_state = 0;
+   ce->guc_state.sched_state &= SCHED_STATE_BLOCKED_MASK;
 }
 
 static inline bool
-- 
2.32.0

Re: [Intel-gfx] [PATCH] drm/i915: Ditch the i915_gem_ww_ctx loop member




On 8/16/21 3:34 PM, Maarten Lankhorst wrote:

Op 16-08-2021 om 15:30 schreef Thomas Hellström:

On 8/16/21 3:25 PM, Matthew Auld wrote:

On Mon, 16 Aug 2021 at 09:49, Thomas Hellström
 wrote:

It's only used by the for_i915_gem_ww() macro and we can use
the (typically) on-stack _err variable in its place.

While initially setting the _err variable to -EDEADLK to enter the
loop, we clear it before actually entering using fetch_and_zero() to
avoid empty loops or code not setting the _err variable running forever.

Suggested-by: Maarten Lankhorst 
Signed-off-by: Thomas Hellström 
---
   drivers/gpu/drm/i915/i915_gem_ww.h | 23 ---
   1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h 
b/drivers/gpu/drm/i915/i915_gem_ww.h
index f6b1a796667b..98348b1e6182 100644
--- a/drivers/gpu/drm/i915/i915_gem_ww.h
+++ b/drivers/gpu/drm/i915/i915_gem_ww.h
@@ -7,12 +7,13 @@

   #include 

+#include "i915_utils.h"
+
   struct i915_gem_ww_ctx {
  struct ww_acquire_ctx ctx;
  struct list_head obj_list;
  struct drm_i915_gem_object *contended;
-   unsigned short intr;
-   unsigned short loop;
+   bool intr;
   };

   void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr);
@@ -23,28 +24,20 @@ void i915_gem_ww_unlock_single(struct drm_i915_gem_object 
*obj);
   /* Internal functions used by the inlines! Don't use. */
   static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)
   {
-   ww->loop = 0;
  if (err == -EDEADLK) {
  err = i915_gem_ww_ctx_backoff(ww);
  if (!err)
-   ww->loop = 1;
+   err = -EDEADLK;
  }

-   if (!ww->loop)
+   if (err != -EDEADLK)
  i915_gem_ww_ctx_fini(ww);

  return err;
   }

-static inline void
-__i915_gem_ww_init(struct i915_gem_ww_ctx *ww, bool intr)
-{
-   i915_gem_ww_ctx_init(ww, intr);
-   ww->loop = 1;
-}
-
-#define for_i915_gem_ww(_ww, _err, _intr)  \
-   for (__i915_gem_ww_init(_ww, _intr); (_ww)->loop;   \
+#define for_i915_gem_ww(_ww, _err, _intr)    \
+   for (i915_gem_ww_ctx_init(_ww, _intr), (_err) = -EDEADLK; \
+    fetch_and_zero(&_err) == -EDEADLK;   \

Doesn't this now hide "normal" errors, like say get_pages() returning
-ENOSPC or so?

Yes, good catch. We should either just clear the -EDEADLK case, or not clear 
the error at all..

/Thomas

I believe not setting _err is a bug anyway. Why would you do such a loop without at 
least one err = ww_mutex_lock(); ?

Infinite loop would catch that at first test.


OK, I'll skip the clearing then.

/Thomas




~Maarten

[Intel-gfx] [PATCH V2] drm/i915/adl_s: Update ADL-S PCI IDs

2021-08-16 Thread Tejas Upadhyay

Sync PCI IDs with Bspec.

Bspec:53655

Changes since V1:
- All POR and Non POR Ids needs to be upstreamed - James Asmus

Signed-off-by: Tejas Upadhyay 
---
 include/drm/i915_pciids.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
index eee18fa53b54..55606f53ab7c 100644
--- a/include/drm/i915_pciids.h
+++ b/include/drm/i915_pciids.h
@@ -642,10 +642,14 @@
INTEL_VGA_DEVICE(0x4683, info), \
INTEL_VGA_DEVICE(0x4688, info), \
INTEL_VGA_DEVICE(0x4689, info), \
+   INTEL_VGA_DEVICE(0x468A, info), \
+   INTEL_VGA_DEVICE(0x468B, info), \
INTEL_VGA_DEVICE(0x4690, info), \
INTEL_VGA_DEVICE(0x4691, info), \
INTEL_VGA_DEVICE(0x4692, info), \
-   INTEL_VGA_DEVICE(0x4693, info)
+   INTEL_VGA_DEVICE(0x4693, info), \
+   INTEL_VGA_DEVICE(0x4698, info), \
+   INTEL_VGA_DEVICE(0x4699, info)
 
 /* ADL-P */
 #define INTEL_ADLP_IDS(info) \
-- 
2.31.1

Re: [Intel-gfx] [PATCH] drm/i915: Ditch the i915_gem_ww_ctx loop member

2021-08-16 Thread Maarten Lankhorst

Op 16-08-2021 om 15:30 schreef Thomas Hellström:
>
> On 8/16/21 3:25 PM, Matthew Auld wrote:
>> On Mon, 16 Aug 2021 at 09:49, Thomas Hellström
>>  wrote:
>>> It's only used by the for_i915_gem_ww() macro and we can use
>>> the (typically) on-stack _err variable in its place.
>>>
>>> While initially setting the _err variable to -EDEADLK to enter the
>>> loop, we clear it before actually entering using fetch_and_zero() to
>>> avoid empty loops or code not setting the _err variable running forever.
>>>
>>> Suggested-by: Maarten Lankhorst 
>>> Signed-off-by: Thomas Hellström 
>>> ---
>>>   drivers/gpu/drm/i915/i915_gem_ww.h | 23 ---
>>>   1 file changed, 8 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h 
>>> b/drivers/gpu/drm/i915/i915_gem_ww.h
>>> index f6b1a796667b..98348b1e6182 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_ww.h
>>> +++ b/drivers/gpu/drm/i915/i915_gem_ww.h
>>> @@ -7,12 +7,13 @@
>>>
>>>   #include 
>>>
>>> +#include "i915_utils.h"
>>> +
>>>   struct i915_gem_ww_ctx {
>>>  struct ww_acquire_ctx ctx;
>>>  struct list_head obj_list;
>>>  struct drm_i915_gem_object *contended;
>>> -   unsigned short intr;
>>> -   unsigned short loop;
>>> +   bool intr;
>>>   };
>>>
>>>   void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr);
>>> @@ -23,28 +24,20 @@ void i915_gem_ww_unlock_single(struct 
>>> drm_i915_gem_object *obj);
>>>   /* Internal functions used by the inlines! Don't use. */
>>>   static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)
>>>   {
>>> -   ww->loop = 0;
>>>  if (err == -EDEADLK) {
>>>  err = i915_gem_ww_ctx_backoff(ww);
>>>  if (!err)
>>> -   ww->loop = 1;
>>> +   err = -EDEADLK;
>>>  }
>>>
>>> -   if (!ww->loop)
>>> +   if (err != -EDEADLK)
>>>  i915_gem_ww_ctx_fini(ww);
>>>
>>>  return err;
>>>   }
>>>
>>> -static inline void
>>> -__i915_gem_ww_init(struct i915_gem_ww_ctx *ww, bool intr)
>>> -{
>>> -   i915_gem_ww_ctx_init(ww, intr);
>>> -   ww->loop = 1;
>>> -}
>>> -
>>> -#define for_i915_gem_ww(_ww, _err, _intr)  \
>>> -   for (__i915_gem_ww_init(_ww, _intr); (_ww)->loop;   \
>>> +#define for_i915_gem_ww(_ww, _err, _intr)    \
>>> +   for (i915_gem_ww_ctx_init(_ww, _intr), (_err) = -EDEADLK; \
>>> +    fetch_and_zero(&_err) == -EDEADLK;   \
>> Doesn't this now hide "normal" errors, like say get_pages() returning
>> -ENOSPC or so?
>
> Yes, good catch. We should either just clear the -EDEADLK case, or not clear 
> the error at all..
>
> /Thomas

I believe not setting _err is a bug anyway. Why would you do such a loop 
without at least one err = ww_mutex_lock(); ?

Infinite loop would catch that at first test.

~Maarten

Re: [Intel-gfx] [PATCH] drm/i915: Ditch the i915_gem_ww_ctx loop member




On 8/16/21 3:25 PM, Matthew Auld wrote:

On Mon, 16 Aug 2021 at 09:49, Thomas Hellström
 wrote:

It's only used by the for_i915_gem_ww() macro and we can use
the (typically) on-stack _err variable in its place.

While initially setting the _err variable to -EDEADLK to enter the
loop, we clear it before actually entering using fetch_and_zero() to
avoid empty loops or code not setting the _err variable running forever.

Suggested-by: Maarten Lankhorst 
Signed-off-by: Thomas Hellström 
---
  drivers/gpu/drm/i915/i915_gem_ww.h | 23 ---
  1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h 
b/drivers/gpu/drm/i915/i915_gem_ww.h
index f6b1a796667b..98348b1e6182 100644
--- a/drivers/gpu/drm/i915/i915_gem_ww.h
+++ b/drivers/gpu/drm/i915/i915_gem_ww.h
@@ -7,12 +7,13 @@

  #include 

+#include "i915_utils.h"
+
  struct i915_gem_ww_ctx {
 struct ww_acquire_ctx ctx;
 struct list_head obj_list;
 struct drm_i915_gem_object *contended;
-   unsigned short intr;
-   unsigned short loop;
+   bool intr;
  };

  void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr);
@@ -23,28 +24,20 @@ void i915_gem_ww_unlock_single(struct drm_i915_gem_object 
*obj);
  /* Internal functions used by the inlines! Don't use. */
  static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)
  {
-   ww->loop = 0;
 if (err == -EDEADLK) {
 err = i915_gem_ww_ctx_backoff(ww);
 if (!err)
-   ww->loop = 1;
+   err = -EDEADLK;
 }

-   if (!ww->loop)
+   if (err != -EDEADLK)
 i915_gem_ww_ctx_fini(ww);

 return err;
  }

-static inline void
-__i915_gem_ww_init(struct i915_gem_ww_ctx *ww, bool intr)
-{
-   i915_gem_ww_ctx_init(ww, intr);
-   ww->loop = 1;
-}
-
-#define for_i915_gem_ww(_ww, _err, _intr)  \
-   for (__i915_gem_ww_init(_ww, _intr); (_ww)->loop;   \
+#define for_i915_gem_ww(_ww, _err, _intr)\
+   for (i915_gem_ww_ctx_init(_ww, _intr), (_err) = -EDEADLK; \
+fetch_and_zero(&_err) == -EDEADLK;   \

Doesn't this now hide "normal" errors, like say get_pages() returning
-ENOSPC or so?


Yes, good catch. We should either just clear the -EDEADLK case, or not 
clear the error at all..


/Thomas

Re: [Intel-gfx] [PATCH] drm/i915: Ditch the i915_gem_ww_ctx loop member

2021-08-16 Thread Matthew Auld

On Mon, 16 Aug 2021 at 09:49, Thomas Hellström
 wrote:
>
> It's only used by the for_i915_gem_ww() macro and we can use
> the (typically) on-stack _err variable in its place.
>
> While initially setting the _err variable to -EDEADLK to enter the
> loop, we clear it before actually entering using fetch_and_zero() to
> avoid empty loops or code not setting the _err variable running forever.
>
> Suggested-by: Maarten Lankhorst 
> Signed-off-by: Thomas Hellström 
> ---
>  drivers/gpu/drm/i915/i915_gem_ww.h | 23 ---
>  1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h 
> b/drivers/gpu/drm/i915/i915_gem_ww.h
> index f6b1a796667b..98348b1e6182 100644
> --- a/drivers/gpu/drm/i915/i915_gem_ww.h
> +++ b/drivers/gpu/drm/i915/i915_gem_ww.h
> @@ -7,12 +7,13 @@
>
>  #include 
>
> +#include "i915_utils.h"
> +
>  struct i915_gem_ww_ctx {
> struct ww_acquire_ctx ctx;
> struct list_head obj_list;
> struct drm_i915_gem_object *contended;
> -   unsigned short intr;
> -   unsigned short loop;
> +   bool intr;
>  };
>
>  void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr);
> @@ -23,28 +24,20 @@ void i915_gem_ww_unlock_single(struct drm_i915_gem_object 
> *obj);
>  /* Internal functions used by the inlines! Don't use. */
>  static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)
>  {
> -   ww->loop = 0;
> if (err == -EDEADLK) {
> err = i915_gem_ww_ctx_backoff(ww);
> if (!err)
> -   ww->loop = 1;
> +   err = -EDEADLK;
> }
>
> -   if (!ww->loop)
> +   if (err != -EDEADLK)
> i915_gem_ww_ctx_fini(ww);
>
> return err;
>  }
>
> -static inline void
> -__i915_gem_ww_init(struct i915_gem_ww_ctx *ww, bool intr)
> -{
> -   i915_gem_ww_ctx_init(ww, intr);
> -   ww->loop = 1;
> -}
> -
> -#define for_i915_gem_ww(_ww, _err, _intr)  \
> -   for (__i915_gem_ww_init(_ww, _intr); (_ww)->loop;   \
> +#define for_i915_gem_ww(_ww, _err, _intr)\
> +   for (i915_gem_ww_ctx_init(_ww, _intr), (_err) = -EDEADLK; \
> +fetch_and_zero(&_err) == -EDEADLK;   \

Doesn't this now hide "normal" errors, like say get_pages() returning
-ENOSPC or so?

>  _err = __i915_gem_ww_fini(_ww, _err))
> -
>  #endif
> --
> 2.31.1
>

Re: [Intel-gfx] [PATCH v2] drm: avoid races with modesetting rights

On 16/8/21 5:04 pm, Daniel Vetter wrote:

On Mon, Aug 16, 2021 at 10:53 AM Desmond Cheong Zhi Xi
wrote:

On 16/8/21 2:47 am, kernel test robot wrote:

Hi Desmond,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210813]
[also build test ERROR on v5.14-rc5]
[cannot apply to linus/master v5.14-rc5 v5.14-rc4 v5.14-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
base:4b358aabb93a2c654cd1dcab1a25a589f6e2b153
config: i386-randconfig-a004-20210815 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
#
https://github.com/0day-ci/linux/commit/cf6d8354b7d7953cd866fad004cbb189adfa074f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
git checkout cf6d8354b7d7953cd866fad004cbb189adfa074f
# save the attached .config to linux build tree
make W=1 ARCH=i386

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot

All errors (new ones prefixed by >>, old ones prefixed by <<):

ERROR: modpost: "task_work_add" [drivers/gpu/drm/drm.ko] undefined!

I'm a bit uncertain about this. Looking into the .config used, this
error seems to happen because task_work_add isn't an exported symbol,
but DRM is being compiled as a loadable kernel module (CONFIG_DRM=m).

One way to deal with this is to export the symbol, but there was a
proposed patch to do this a few months back that wasn't picked up [1],
so I'm not sure what to make of this.

I'll export the symbol as part of a v3 series, and check in with the
task-work maintainers.

Link:
https://lore.kernel.org/lkml/20210127150029.13766-3-josh...@samsung.com/ [1]

Yeah that sounds best. I have two more thoughts on the patch:
- drm_master_flush isn't used by any modules outside of drm.ko, so we
can unexport it and drop the kerneldoc (the comment is still good).
These kind of internal functions have their declaration in
drm-internal.h - there's already a few there from drm_auth.c

Sounds good, I'll do that and move the declaration from drm_auth.h to
drm_internal.h.

- We know have 3 locks for master state, that feels a bit like
overkill. The spinlock I think we need to keep due to lock inversions,
but the master_mutex and master_rwsem look like we should be able to
merge them? I.e. anywhere we currently grab the master_mutex we could
instead grab the rwsem in either write mode (when we change stuff) or
read mode (when we just check, like in master_internal_acquire).

Thoughts?
-Daniel

Using rwsem in the places where we currently hold the mutex seems pretty
doable.

There are some tricky bits once we add rwsem read locks to the ioctl
handler. Some ioctl functions like drm_authmagic need a write lock.

In this particular case, it might make sense to break master_mutex down
into finer-grained locks, since the function doesn't change master
permissions. It just needs to prevent concurrent writes to the
drm_master.magic_map idr.

For other ioctls, I'll take a closer look on a case-by-case basis.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

[Intel-gfx] [PATCH 5.13 033/151] drm/i915/display: Fix the 12 BPC bits for PIPE_MISC reg

2021-08-16 Thread Greg Kroah-Hartman

From: Ankit Nautiyal 

commit abd9d66a055722393d33685214c08386694871d7 upstream.

Till DISPLAY12 the PIPE_MISC bits 5-7 are used to set the
Dithering BPC, with valid values of 6, 8, 10 BPC.
For ADLP+ these bits are used to set the PORT OUTPUT BPC, with valid
values of: 6, 8, 10, 12 BPC, and need to be programmed whether
dithering is enabled or not.

This patch:
-corrects the bits 5-7 for PIPE MISC register for 12 BPC.
-renames the bits and mask to have generic names for these bits for
dithering bpc and port output bpc.

v3: Added a note for MIPI DSI which uses the PIPE_MISC for readout
for pipe_bpp. (Uma Shankar)

v2: Added 'display' to the subject and fixes tag. (Uma Shankar)

Fixes: 756f85cffef2 ("drm/i915/bdw: Broadwell has PIPEMISC")
Cc: Paulo Zanoni  (v1)
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: Joonas Lahtinen 
Cc: Rodrigo Vivi 
Cc: intel-gfx@lists.freedesktop.org
Cc:  # v3.13+

Signed-off-by: Ankit Nautiyal 
Reviewed-by: Uma Shankar 
Signed-off-by: Uma Shankar 
Link: 
https://patchwork.freedesktop.org/patch/msgid/20210811051857.109723-1-ankit.k.nauti...@intel.com
(cherry picked from commit 70418a68713c13da3f36c388087d0220b456a430)
Signed-off-by: Rodrigo Vivi 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/gpu/drm/i915/display/intel_display.c |   34 +++
 drivers/gpu/drm/i915/i915_reg.h  |   16 
 2 files changed, 35 insertions(+), 15 deletions(-)

--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5424,16 +5424,18 @@ static void bdw_set_pipemisc(const struc
 
switch (crtc_state->pipe_bpp) {
case 18:
-   val |= PIPEMISC_DITHER_6_BPC;
+   val |= PIPEMISC_6_BPC;
break;
case 24:
-   val |= PIPEMISC_DITHER_8_BPC;
+   val |= PIPEMISC_8_BPC;
break;
case 30:
-   val |= PIPEMISC_DITHER_10_BPC;
+   val |= PIPEMISC_10_BPC;
break;
case 36:
-   val |= PIPEMISC_DITHER_12_BPC;
+   /* Port output 12BPC defined for ADLP+ */
+   if (DISPLAY_VER(dev_priv) > 12)
+   val |= PIPEMISC_12_BPC_ADLP;
break;
default:
MISSING_CASE(crtc_state->pipe_bpp);
@@ -5469,15 +5471,27 @@ int bdw_get_pipemisc_bpp(struct intel_cr
 
tmp = intel_de_read(dev_priv, PIPEMISC(crtc->pipe));
 
-   switch (tmp & PIPEMISC_DITHER_BPC_MASK) {
-   case PIPEMISC_DITHER_6_BPC:
+   switch (tmp & PIPEMISC_BPC_MASK) {
+   case PIPEMISC_6_BPC:
return 18;
-   case PIPEMISC_DITHER_8_BPC:
+   case PIPEMISC_8_BPC:
return 24;
-   case PIPEMISC_DITHER_10_BPC:
+   case PIPEMISC_10_BPC:
return 30;
-   case PIPEMISC_DITHER_12_BPC:
-   return 36;
+   /*
+* PORT OUTPUT 12 BPC defined for ADLP+.
+*
+* TODO:
+* For previous platforms with DSI interface, bits 5:7
+* are used for storing pipe_bpp irrespective of dithering.
+* Since the value of 12 BPC is not defined for these bits
+* on older platforms, need to find a workaround for 12 BPC
+* MIPI DSI HW readout.
+*/
+   case PIPEMISC_12_BPC_ADLP:
+   if (DISPLAY_VER(dev_priv) > 12)
+   return 36;
+   fallthrough;
default:
MISSING_CASE(tmp);
return 0;
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6134,11 +6134,17 @@ enum {
 #define   PIPEMISC_HDR_MODE_PRECISION  (1 << 23) /* icl+ */
 #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
 #define   PIPEMISC_PIXEL_ROUNDING_TRUNCREG_BIT(8) /* tgl+ */
-#define   PIPEMISC_DITHER_BPC_MASK (7 << 5)
-#define   PIPEMISC_DITHER_8_BPC(0 << 5)
-#define   PIPEMISC_DITHER_10_BPC   (1 << 5)
-#define   PIPEMISC_DITHER_6_BPC(2 << 5)
-#define   PIPEMISC_DITHER_12_BPC   (3 << 5)
+/*
+ * For Display < 13, Bits 5-7 of PIPE MISC represent DITHER BPC with
+ * valid values of: 6, 8, 10 BPC.
+ * ADLP+, the bits 5-7 represent PORT OUTPUT BPC with valid values of:
+ * 6, 8, 10, 12 BPC.
+ */
+#define   PIPEMISC_BPC_MASK(7 << 5)
+#define   PIPEMISC_8_BPC   (0 << 5)
+#define   PIPEMISC_10_BPC  (1 << 5)
+#define   PIPEMISC_6_BPC   (2 << 5)
+#define   PIPEMISC_12_BPC_ADLP (4 << 5) /* adlp+ */
 #define   PIPEMISC_DITHER_ENABLE   (1 << 4)
 #define   PIPEMISC_DITHER_TYPE_MASK(3 << 2)
 #define   PIPEMISC_DITHER_TYPE_SP  (0 << 2)

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/selftest: Fix use of err in igt_reset_{fail, nop}_engine()

== Series Details ==

Series: drm/i915/selftest: Fix use of err in igt_reset_{fail, nop}_engine()
URL   : https://patchwork.freedesktop.org/series/93713/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10487_full -> Patchwork_20830_full


Summary
---

  **SUCCESS**

  No regressions found.

  

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_ctx_persistence@legacy-engines-queued:
- shard-snb:  NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#1099]) +2 
similar issues
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-snb6/igt@gem_ctx_persiste...@legacy-engines-queued.html

  * igt@gem_eio@unwedge-stress:
- shard-tglb: [PASS][2] -> [TIMEOUT][3] ([i915#2369] / [i915#3063] 
/ [i915#3648])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-tglb8/igt@gem_...@unwedge-stress.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-tglb5/igt@gem_...@unwedge-stress.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
- shard-kbl:  NOTRUN -> [FAIL][4] ([i915#2842])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-kbl1/igt@gem_exec_fair@basic-none-s...@rcs0.html

  * igt@gem_exec_fair@basic-pace@vcs1:
- shard-iclb: NOTRUN -> [FAIL][5] ([i915#2842])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-iclb2/igt@gem_exec_fair@basic-p...@vcs1.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
- shard-glk:  [PASS][6] -> [FAIL][7] ([i915#2842])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-glk7/igt@gem_exec_fair@basic-throt...@rcs0.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-glk3/igt@gem_exec_fair@basic-throt...@rcs0.html

  * igt@gem_huc_copy@huc-copy:
- shard-tglb: [PASS][8] -> [SKIP][9] ([i915#2190])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-tglb7/igt@gem_huc_c...@huc-copy.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-tglb6/igt@gem_huc_c...@huc-copy.html
- shard-apl:  NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#2190])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-apl7/igt@gem_huc_c...@huc-copy.html
- shard-kbl:  NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#2190])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-kbl7/igt@gem_huc_c...@huc-copy.html

  * igt@gem_mmap_gtt@big-copy-xy:
- shard-skl:  [PASS][12] -> [FAIL][13] ([i915#1888] / [i915#307])
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-skl10/igt@gem_mmap_...@big-copy-xy.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-skl9/igt@gem_mmap_...@big-copy-xy.html

  * igt@gem_pread@exhaustion:
- shard-apl:  NOTRUN -> [WARN][14] ([i915#2658])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-apl2/igt@gem_pr...@exhaustion.html
- shard-snb:  NOTRUN -> [WARN][15] ([i915#2658])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-snb6/igt@gem_pr...@exhaustion.html

  * igt@gem_render_copy@x-tiled-to-vebox-yf-tiled:
- shard-kbl:  NOTRUN -> [SKIP][16] ([fdo#109271]) +100 similar 
issues
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-kbl2/igt@gem_render_c...@x-tiled-to-vebox-yf-tiled.html

  * igt@gem_softpin@noreloc-s3:
- shard-apl:  [PASS][17] -> [DMESG-WARN][18] ([i915#180]) +2 
similar issues
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-apl2/igt@gem_soft...@noreloc-s3.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-apl2/igt@gem_soft...@noreloc-s3.html

  * igt@gen9_exec_parse@cmd-crossing-page:
- shard-iclb: NOTRUN -> [SKIP][19] ([i915#2856])
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-iclb7/igt@gen9_exec_pa...@cmd-crossing-page.html

  * igt@i915_pm_dc@dc6-psr:
- shard-iclb: [PASS][20] -> [FAIL][21] ([i915#454])
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-iclb7/igt@i915_pm...@dc6-psr.html
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-iclb6/igt@i915_pm...@dc6-psr.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-hflip:
- shard-apl:  NOTRUN -> [SKIP][22] ([fdo#109271] / [i915#3777]) +4 
similar issues
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/shard-apl8/igt@kms_big...@y-tiled-max-hw-stride-32bpp-rotate-0-hflip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-hflip:
- shard-skl:  NOTRUN -> [SKIP][23] ([fdo#109271] / [i915#3777]) +1 
similar issue
   [23]:

[Intel-gfx] ✓ Fi.CI.BAT: success for drm: avoid races with modesetting rights

== Series Details ==

Series: drm: avoid races with modesetting rights
URL   : https://patchwork.freedesktop.org/series/93714/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10488 -> Patchwork_20831


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/index.html

Possible new issues
---

  Here are the unknown changes that may have been introduced in Patchwork_20831:

### IGT changes ###

 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@gt_heartbeat:
- {fi-ehl-2}: [PASS][1] -> [DMESG-FAIL][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/fi-ehl-2/igt@i915_selftest@live@gt_heartbeat.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-ehl-2/igt@i915_selftest@live@gt_heartbeat.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@amdgpu/amd_basic@query-info:
- fi-tgl-1115g4:  NOTRUN -> [SKIP][3] ([fdo#109315])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-tgl-1115g4/igt@amdgpu/amd_ba...@query-info.html

  * igt@amdgpu/amd_basic@semaphore:
- fi-bsw-nick:NOTRUN -> [SKIP][4] ([fdo#109271]) +17 similar issues
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-bsw-nick/igt@amdgpu/amd_ba...@semaphore.html

  * igt@amdgpu/amd_cs_nop@nop-gfx0:
- fi-tgl-1115g4:  NOTRUN -> [SKIP][5] ([fdo#109315] / [i915#2575]) +16 
similar issues
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-tgl-1115g4/igt@amdgpu/amd_cs_...@nop-gfx0.html

  * igt@i915_pm_backlight@basic-brightness:
- fi-tgl-1115g4:  NOTRUN -> [SKIP][6] ([i915#1155])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-tgl-1115g4/igt@i915_pm_backli...@basic-brightness.html

  * igt@i915_selftest@live@gt_lrc:
- fi-rkl-guc: [PASS][7] -> [DMESG-WARN][8] ([i915#3958])
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html

  * igt@kms_chamelium@common-hpd-after-suspend:
- fi-tgl-1115g4:  NOTRUN -> [SKIP][9] ([fdo#111827])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-tgl-1115g4/igt@kms_chamel...@common-hpd-after-suspend.html

  * igt@kms_force_connector_basic@force-load-detect:
- fi-tgl-1115g4:  NOTRUN -> [SKIP][10] ([fdo#109285])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-tgl-1115g4/igt@kms_force_connector_ba...@force-load-detect.html

  * igt@kms_psr@primary_mmap_gtt:
- fi-tgl-1115g4:  NOTRUN -> [SKIP][11] ([i915#1072]) +3 similar issues
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-tgl-1115g4/igt@kms_psr@primary_mmap_gtt.html

  * igt@prime_vgem@basic-userptr:
- fi-tgl-1115g4:  NOTRUN -> [SKIP][12] ([i915#3301])
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-tgl-1115g4/igt@prime_v...@basic-userptr.html

  
 Possible fixes 

  * igt@gem_exec_suspend@basic-s3:
- fi-tgl-1115g4:  [FAIL][13] ([i915#1888]) -> [PASS][14]
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/fi-tgl-1115g4/igt@gem_exec_susp...@basic-s3.html
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-tgl-1115g4/igt@gem_exec_susp...@basic-s3.html

  * igt@i915_pm_rpm@module-reload:
- fi-kbl-guc: [FAIL][15] ([i915#2203] / [i915#579]) -> [PASS][16]
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/fi-kbl-guc/igt@i915_pm_...@module-reload.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-kbl-guc/igt@i915_pm_...@module-reload.html

  * igt@i915_selftest@live@execlists:
- fi-bsw-nick:[INCOMPLETE][17] ([i915#2940]) -> [PASS][18]
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/fi-bsw-nick/igt@i915_selftest@l...@execlists.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-bsw-nick/igt@i915_selftest@l...@execlists.html

  * igt@i915_selftest@live@hangcheck:
- {fi-hsw-gt1}:   [DMESG-WARN][19] ([i915#3303]) -> [PASS][20]
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/fi-hsw-gt1/igt@i915_selftest@l...@hangcheck.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20831/fi-hsw-gt1/igt@i915_selftest@l...@hangcheck.html

  
 Warnings 

  * igt@kms_chamelium@vga-edid-read:
- fi-tgl-1115g4:  [SKIP][21] ([fdo#111827] / [i915#1887]) -> [SKIP][22] 
([fdo#111827])
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10488/fi-tgl-1115g4/igt@kms_chamel...@vga-edid-read.html

Re: [Intel-gfx] [PATCH] drm/i915/dp: remove superfluous EXPORT_SYMBOL()

On Mon, 16 Aug 2021, Imre Deak  wrote:
> On Mon, Aug 16, 2021 at 10:17:37AM +0300, Jani Nikula wrote:
>> The symbol isn't needed outside of i915.ko.
>> 
>> Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link 
>> training")
>> Fixes: 264613b406eb ("drm/i915: Disable LTTPR support when the DPCD rev < 
>> 1.4")
>> Cc: Imre Deak 
>> Signed-off-by: Jani Nikula 
>
> Thanks for catching it, not sure why I thought the export is needed :/
> Reviewed-by: Imre Deak 

Thanks, pushed.

BR,
Jani.

>
>> ---
>>  drivers/gpu/drm/i915/display/intel_dp_link_training.c | 1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c 
>> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> index 08bceae40aa8..053a3c2f7267 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
>> @@ -206,7 +206,6 @@ int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp 
>> *intel_dp)
>>  
>>  return lttpr_count;
>>  }
>> -EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);
>>  
>>  static u8 dp_voltage_max(u8 preemph)
>>  {
>> -- 
>> 2.20.1
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH] drm/i915/display: Drop redundant debug print

On Mon, 16 Aug 2021, "Sharma, Swati2"  wrote:
> On 16-Aug-21 5:40 PM, Jani Nikula wrote:
>> On Mon, 16 Aug 2021, "Sharma, Swati2"  wrote:
>>> On 13-Aug-21 1:16 PM, Jani Nikula wrote:
 On Thu, 12 Aug 2021, Swati Sharma  wrote:
> drm_dp_dpcd_read/write already has debug error message.
> Drop redundant error messages which gives false
> status even if correct value is read in drm_dp_dpcd_read().

 I guess the only problem is it gets harder to associate the preceding
 low level error messages with intel_dp_check_link_service_irq(). *shrug*

 Reviewed-by: Jani Nikula 


>>> Thanks Jani for the review. Can you please merge?
>> 
>> There was another version with open review?
>
> Yes. https://patchwork.freedesktop.org/series/93025/#rev3
> Should I add debug prints how Imre suggested in other IRQ func
> to make it generic or should it be dropped from here too?
> Quoting imre
> "Yes, that's why I suggested to return for the '0 value read' case 
> without any message printed, but still keep the message for the case 
> when the drm_dp_dpcd_readb() fails."
> "Ok, it's good to keep them in sync at least, so I'm ok with removing 
> the debug messages from here too."
>
> Please let me know what is the better approach.

IMO just nuke them.

BR,
Jani



>> 
>> BR,
>> Jani.
>> 
>> 
>>>
>
> Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and 
> recovery")
> Cc: Swati Sharma 
> Cc: Ankit Nautiyal 
> Cc: Uma Shankar  (v2)
> Cc: Jani Nikula 
> Cc: "Ville Syrj_l_" 
> Cc: Imre Deak 
> Cc: Manasi Navare 
> Cc: Uma Shankar 
> Cc: "Jos_ Roberto de Souza" 
> Cc: Sean Paul 
> Cc:  # v5.12+
>
> Link: 
> https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.k.nauti...@intel.com
> Signed-off-by: Swati Sharma 
> ---
>drivers/gpu/drm/i915/display/intel_dp.c | 8 ++--
>1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index c386ef8eb200..5c84f51ad41d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -3871,16 +3871,12 @@ static void 
> intel_dp_check_link_service_irq(struct intel_dp *intel_dp)
>   return;
>
>   if (drm_dp_dpcd_readb(_dp->aux,
> -   DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
> !val) {
> - drm_dbg_kms(>drm, "Error in reading link service irq 
> vector\n");
> +   DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
> !val)
>   return;
> - }
>
>   if (drm_dp_dpcd_writeb(_dp->aux,
> -DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {
> - drm_dbg_kms(>drm, "Error in writing link service irq 
> vector\n");
> +DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1)
>   return;
> - }
>
>   if (val & HDMI_LINK_STATUS_CHANGED)
>   intel_dp_handle_hdmi_link_status_change(intel_dp);

>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH] drm/i915/display: Drop redundant debug print

2021-08-16 Thread Sharma, Swati2





On 16-Aug-21 5:40 PM, Jani Nikula wrote:

On Mon, 16 Aug 2021, "Sharma, Swati2"  wrote:

On 13-Aug-21 1:16 PM, Jani Nikula wrote:

On Thu, 12 Aug 2021, Swati Sharma  wrote:

drm_dp_dpcd_read/write already has debug error message.
Drop redundant error messages which gives false
status even if correct value is read in drm_dp_dpcd_read().


I guess the only problem is it gets harder to associate the preceding
low level error messages with intel_dp_check_link_service_irq(). *shrug*

Reviewed-by: Jani Nikula 



Thanks Jani for the review. Can you please merge?


There was another version with open review?


Yes. https://patchwork.freedesktop.org/series/93025/#rev3
Should I add debug prints how Imre suggested in other IRQ func
to make it generic or should it be dropped from here too?
Quoting imre
"Yes, that's why I suggested to return for the '0 value read' case 
without any message printed, but still keep the message for the case 
when the drm_dp_dpcd_readb() fails."
"Ok, it's good to keep them in sync at least, so I'm ok with removing 
the debug messages from here too."


Please let me know what is the better approach.


BR,
Jani.






Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and 
recovery")
Cc: Swati Sharma 
Cc: Ankit Nautiyal 
Cc: Uma Shankar  (v2)
Cc: Jani Nikula 
Cc: "Ville Syrj_l_" 
Cc: Imre Deak 
Cc: Manasi Navare 
Cc: Uma Shankar 
Cc: "Jos_ Roberto de Souza" 
Cc: Sean Paul 
Cc:  # v5.12+

Link: 
https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.k.nauti...@intel.com
Signed-off-by: Swati Sharma 
---
   drivers/gpu/drm/i915/display/intel_dp.c | 8 ++--
   1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index c386ef8eb200..5c84f51ad41d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct 
intel_dp *intel_dp)
return;
   
   	if (drm_dp_dpcd_readb(_dp->aux,

- DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
!val) {
-   drm_dbg_kms(>drm, "Error in reading link service irq 
vector\n");
+ DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
!val)
return;
-   }
   
   	if (drm_dp_dpcd_writeb(_dp->aux,

-  DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {
-   drm_dbg_kms(>drm, "Error in writing link service irq 
vector\n");
+  DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1)
return;
-   }
   
   	if (val & HDMI_LINK_STATUS_CHANGED)

intel_dp_handle_hdmi_link_status_change(intel_dp);






--
~Swati Sharma

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: avoid races with modesetting rights

== Series Details ==

Series: drm: avoid races with modesetting rights
URL   : https://patchwork.freedesktop.org/series/93714/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+drivers/gpu/drm/drm_auth.c:483:6: warning: symbol 'master_flush' was not 
declared. Should it be static?

Re: [Intel-gfx] [PATCH] drm/i915/display: Drop redundant debug print

On Mon, 16 Aug 2021, "Sharma, Swati2"  wrote:
> On 13-Aug-21 1:16 PM, Jani Nikula wrote:
>> On Thu, 12 Aug 2021, Swati Sharma  wrote:
>>> drm_dp_dpcd_read/write already has debug error message.
>>> Drop redundant error messages which gives false
>>> status even if correct value is read in drm_dp_dpcd_read().
>> 
>> I guess the only problem is it gets harder to associate the preceding
>> low level error messages with intel_dp_check_link_service_irq(). *shrug*
>> 
>> Reviewed-by: Jani Nikula 
>> 
>> 
> Thanks Jani for the review. Can you please merge?

There was another version with open review?

BR,
Jani.


>
>>>
>>> Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and 
>>> recovery")
>>> Cc: Swati Sharma 
>>> Cc: Ankit Nautiyal 
>>> Cc: Uma Shankar  (v2)
>>> Cc: Jani Nikula 
>>> Cc: "Ville Syrj_l_" 
>>> Cc: Imre Deak 
>>> Cc: Manasi Navare 
>>> Cc: Uma Shankar 
>>> Cc: "Jos_ Roberto de Souza" 
>>> Cc: Sean Paul 
>>> Cc:  # v5.12+
>>>
>>> Link: 
>>> https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.k.nauti...@intel.com
>>> Signed-off-by: Swati Sharma 
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_dp.c | 8 ++--
>>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>> index c386ef8eb200..5c84f51ad41d 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>> @@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct 
>>> intel_dp *intel_dp)
>>> return;
>>>   
>>> if (drm_dp_dpcd_readb(_dp->aux,
>>> - DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
>>> !val) {
>>> -   drm_dbg_kms(>drm, "Error in reading link service irq 
>>> vector\n");
>>> + DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
>>> !val)
>>> return;
>>> -   }
>>>   
>>> if (drm_dp_dpcd_writeb(_dp->aux,
>>> -  DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {
>>> -   drm_dbg_kms(>drm, "Error in writing link service irq 
>>> vector\n");
>>> +  DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1)
>>> return;
>>> -   }
>>>   
>>> if (val & HDMI_LINK_STATUS_CHANGED)
>>> intel_dp_handle_hdmi_link_status_change(intel_dp);
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Ditch the i915_gem_ww_ctx loop member

== Series Details ==

Series: drm/i915: Ditch the i915_gem_ww_ctx loop member
URL   : https://patchwork.freedesktop.org/series/93711/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10487_full -> Patchwork_20829_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_20829_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_20829_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_20829_full:

### IGT changes ###

 Possible regressions 

  * igt@gem_ctx_persistence@many-contexts:
- shard-iclb: [PASS][1] -> [DMESG-WARN][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-iclb4/igt@gem_ctx_persiste...@many-contexts.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-iclb7/igt@gem_ctx_persiste...@many-contexts.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_ctx_persistence@legacy-engines-queued:
- shard-snb:  NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1099]) +3 
similar issues
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-snb6/igt@gem_ctx_persiste...@legacy-engines-queued.html

  * igt@gem_exec_fair@basic-none@vcs0:
- shard-kbl:  NOTRUN -> [FAIL][4] ([i915#2842]) +2 similar issues
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-kbl4/igt@gem_exec_fair@basic-n...@vcs0.html

  * igt@gem_exec_fair@basic-none@vecs0:
- shard-apl:  NOTRUN -> [FAIL][5] ([i915#2842] / [i915#3468])
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-apl2/igt@gem_exec_fair@basic-n...@vecs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
- shard-glk:  [PASS][6] -> [FAIL][7] ([i915#2842])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-glk7/igt@gem_exec_fair@basic-throt...@rcs0.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-glk1/igt@gem_exec_fair@basic-throt...@rcs0.html

  * igt@gem_exec_suspend@basic-s3:
- shard-kbl:  [PASS][8] -> [DMESG-WARN][9] ([i915#180]) +1 similar 
issue
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-kbl2/igt@gem_exec_susp...@basic-s3.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-kbl7/igt@gem_exec_susp...@basic-s3.html

  * igt@gem_huc_copy@huc-copy:
- shard-apl:  NOTRUN -> [SKIP][10] ([fdo#109271] / [i915#2190])
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-apl7/igt@gem_huc_c...@huc-copy.html

  * igt@gem_mmap_gtt@cpuset-big-copy:
- shard-iclb: [PASS][11] -> [FAIL][12] ([i915#307])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-iclb7/igt@gem_mmap_...@cpuset-big-copy.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-iclb1/igt@gem_mmap_...@cpuset-big-copy.html

  * igt@gem_pread@exhaustion:
- shard-snb:  NOTRUN -> [WARN][13] ([i915#2658])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-snb6/igt@gem_pr...@exhaustion.html

  * igt@gem_userptr_blits@input-checking:
- shard-snb:  NOTRUN -> [DMESG-WARN][14] ([i915#3002])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-snb6/igt@gem_userptr_bl...@input-checking.html

  * igt@gem_workarounds@suspend-resume:
- shard-apl:  [PASS][15] -> [DMESG-WARN][16] ([i915#180])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-apl3/igt@gem_workarou...@suspend-resume.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-apl7/igt@gem_workarou...@suspend-resume.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-hflip:
- shard-apl:  NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#3777]) +3 
similar issues
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-apl8/igt@kms_big...@y-tiled-max-hw-stride-32bpp-rotate-0-hflip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-0-hflip:
- shard-skl:  NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#3777]) +1 
similar issue
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-skl9/igt@kms_big...@y-tiled-max-hw-stride-64bpp-rotate-0-hflip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0:
- shard-apl:  NOTRUN -> [SKIP][19] ([fdo#109271]) +285 similar 
issues
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/shard-apl7/igt@kms_big...@yf-tiled-max-hw-stride-64bpp-rotate-0.html

  *

Re: [Intel-gfx] [v3][PATCH] drm/i915/display: Drop redundant debug print

2021-08-16 Thread Imre Deak

On Mon, Aug 16, 2021 at 05:23:00PM +0530, Sharma, Swati2 wrote:
> On 12-Aug-21 9:31 PM, Imre Deak wrote:
> > On Thu, Aug 12, 2021 at 06:41:07PM +0530, Swati Sharma wrote:
> > > drm_dp_dpcd_read/write already has debug error message.
> > > Drop redundant error messages which gives false
> > > status even if correct value is read in drm_dp_dpcd_read().
> > > 
> > > v2: -Added fixes tag (Ankit)
> > > v3: -Fixed build error (CI)
> > > 
> > > Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and 
> > > recovery")
> > > Cc: Swati Sharma 
> > > Cc: Ankit Nautiyal 
> > > Cc: Uma Shankar  (v2)
> > > Cc: Jani Nikula 
> > > Cc: "Ville Syrj_l_" 
> > > Cc: Imre Deak 
> > > Cc: Manasi Navare 
> > > Cc: Uma Shankar 
> > > Cc: "Jos_ Roberto de Souza" 
> > > Cc: Sean Paul 
> > > Cc:  # v5.12+
> > > 
> > > Link: 
> > > https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.k.nauti...@intel.com
> > > 
> > > Signed-off-by: Swati Sharma 
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_dp.c | 9 ++---
> > >   1 file changed, 2 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index c386ef8eb200..2526c9c8c690 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -3864,23 +3864,18 @@ static void 
> > > intel_dp_check_device_service_irq(struct intel_dp *intel_dp)
> > >   static void intel_dp_check_link_service_irq(struct intel_dp *intel_dp)
> > >   {
> > > - struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > >   u8 val;
> > >   if (intel_dp->dpcd[DP_DPCD_REV] < 0x11)
> > >   return;
> > >   if (drm_dp_dpcd_readb(_dp->aux,
> > > -   DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
> > > !val) {
> > > - drm_dbg_kms(>drm, "Error in reading link service irq 
> > > vector\n");
> > 
> > The only problem seems to be that for !val the debug print is incorrect,
> > so maybe just have a separate check for that after this one for the read()
> > and return w/o the debug message?
> > 
> > Is it really a stable material, since the change wouldn't have any
> > effect for regular users?
> 
> W/o this change in case of valid short pulse this error message will
> come even if it doesn't have anything to do with link service irq. For
> ex: in case of hdcp we keep getting unnecessary error message because
> the value read is 0 which will always be the case unless its really a
> link failure between PCON and HDMI2.1 sink.

Yes, that's why I suggested to return for the '0 value read' case
without any message printed, but still keep the message for the case
when the drm_dp_dpcd_readb() fails.

Note that it's not an error message but a debug message.

> Also, code is written in accordance with other IRQ func() above like
> intel_dp_check_device_service_irq().

Ok, it's good to keep them in sync at least, so I'm ok with removing the
debug messages from here too.

> > > +   DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
> > > !val)
> > >   return;
> > > - }
> > >   if (drm_dp_dpcd_writeb(_dp->aux,
> > > -DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {
> > > - drm_dbg_kms(>drm, "Error in writing link service irq 
> > > vector\n");
> > > +DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1)
> > >   return;
> > > - }
> > >   if (val & HDMI_LINK_STATUS_CHANGED)
> > >   intel_dp_handle_hdmi_link_status_change(intel_dp);
> > > -- 
> > > 2.25.1
> > > 
> 
> -- 
> ~Swati Sharma

Re: [Intel-gfx] [PATCH] drm/i915/display: Drop redundant debug print

2021-08-16 Thread Sharma, Swati2





On 13-Aug-21 1:16 PM, Jani Nikula wrote:

On Thu, 12 Aug 2021, Swati Sharma  wrote:

drm_dp_dpcd_read/write already has debug error message.
Drop redundant error messages which gives false
status even if correct value is read in drm_dp_dpcd_read().


I guess the only problem is it gets harder to associate the preceding
low level error messages with intel_dp_check_link_service_irq(). *shrug*

Reviewed-by: Jani Nikula 



Thanks Jani for the review. Can you please merge?



Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and 
recovery")
Cc: Swati Sharma 
Cc: Ankit Nautiyal 
Cc: Uma Shankar  (v2)
Cc: Jani Nikula 
Cc: "Ville Syrj_l_" 
Cc: Imre Deak 
Cc: Manasi Navare 
Cc: Uma Shankar 
Cc: "Jos_ Roberto de Souza" 
Cc: Sean Paul 
Cc:  # v5.12+

Link: 
https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.k.nauti...@intel.com
Signed-off-by: Swati Sharma 
---
  drivers/gpu/drm/i915/display/intel_dp.c | 8 ++--
  1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index c386ef8eb200..5c84f51ad41d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3871,16 +3871,12 @@ static void intel_dp_check_link_service_irq(struct 
intel_dp *intel_dp)
return;
  
  	if (drm_dp_dpcd_readb(_dp->aux,

- DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
!val) {
-   drm_dbg_kms(>drm, "Error in reading link service irq 
vector\n");
+ DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
!val)
return;
-   }
  
  	if (drm_dp_dpcd_writeb(_dp->aux,

-  DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {
-   drm_dbg_kms(>drm, "Error in writing link service irq 
vector\n");
+  DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1)
return;
-   }
  
  	if (val & HDMI_LINK_STATUS_CHANGED)

intel_dp_handle_hdmi_link_status_change(intel_dp);




--
~Swati Sharma

Re: [Intel-gfx] [v3][PATCH] drm/i915/display: Drop redundant debug print

2021-08-16 Thread Sharma, Swati2


On 12-Aug-21 9:31 PM, Imre Deak wrote:

On Thu, Aug 12, 2021 at 06:41:07PM +0530, Swati Sharma wrote:

drm_dp_dpcd_read/write already has debug error message.
Drop redundant error messages which gives false
status even if correct value is read in drm_dp_dpcd_read().

v2: -Added fixes tag (Ankit)
v3: -Fixed build error (CI)

Fixes: 9488a030ac91 ("drm/i915: Add support for enabling link status and 
recovery")
Cc: Swati Sharma 
Cc: Ankit Nautiyal 
Cc: Uma Shankar  (v2)
Cc: Jani Nikula 
Cc: "Ville Syrj_l_" 
Cc: Imre Deak 
Cc: Manasi Navare 
Cc: Uma Shankar 
Cc: "Jos_ Roberto de Souza" 
Cc: Sean Paul 
Cc:  # v5.12+

Link: 
https://patchwork.freedesktop.org/patch/msgid/20201218103723.30844-12-ankit.k.nauti...@intel.com

Signed-off-by: Swati Sharma 
---
  drivers/gpu/drm/i915/display/intel_dp.c | 9 ++---
  1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index c386ef8eb200..2526c9c8c690 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -3864,23 +3864,18 @@ static void intel_dp_check_device_service_irq(struct 
intel_dp *intel_dp)
  
  static void intel_dp_check_link_service_irq(struct intel_dp *intel_dp)

  {
-   struct drm_i915_private *i915 = dp_to_i915(intel_dp);
u8 val;
  
  	if (intel_dp->dpcd[DP_DPCD_REV] < 0x11)

return;
  
  	if (drm_dp_dpcd_readb(_dp->aux,

- DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
!val) {
-   drm_dbg_kms(>drm, "Error in reading link service irq 
vector\n");


The only problem seems to be that for !val the debug print is incorrect,
so maybe just have a separate check for that after this one for the read()
and return w/o the debug message?

Is it really a stable material, since the change wouldn't have any
effect for regular users?



W/o this change in case of valid short pulse this error message will 
come even if
it doesn't have anything to do with link service irq. For ex: in case of 
hdcp we keep getting unnecessary error message because the value read is 
0 which will always be the case unless its really a link failure between 
PCON and HDMI2.1 sink.


Also, code is written in accordance with other IRQ func() above like 
intel_dp_check_device_service_irq().



+ DP_LINK_SERVICE_IRQ_VECTOR_ESI0, ) != 1 || 
!val)
return;
-   }
  
  	if (drm_dp_dpcd_writeb(_dp->aux,

-  DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1) {
-   drm_dbg_kms(>drm, "Error in writing link service irq 
vector\n");
+  DP_LINK_SERVICE_IRQ_VECTOR_ESI0, val) != 1)
return;
-   }
  
  	if (val & HDMI_LINK_STATUS_CHANGED)

intel_dp_handle_hdmi_link_status_change(intel_dp);
--
2.25.1



--
~Swati Sharma

[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/dp: remove superfluous EXPORT_SYMBOL()

== Series Details ==

Series: drm/i915/dp: remove superfluous EXPORT_SYMBOL()
URL   : https://patchwork.freedesktop.org/series/93708/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10487_full -> Patchwork_20828_full


Summary
---

  **FAILURE**

  Serious unknown changes coming with Patchwork_20828_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_20828_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_20828_full:

### IGT changes ###

 Possible regressions 

  * igt@kms_pipe_crc_basic@disable-crc-after-crtc-pipe-b:
- shard-tglb: [PASS][1] -> [INCOMPLETE][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-tglb7/igt@kms_pipe_crc_ba...@disable-crc-after-crtc-pipe-b.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-tglb8/igt@kms_pipe_crc_ba...@disable-crc-after-crtc-pipe-b.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_ctx_persistence@legacy-engines-queued:
- shard-snb:  NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1099]) +2 
similar issues
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-snb2/igt@gem_ctx_persiste...@legacy-engines-queued.html

  * igt@gem_eio@unwedge-stress:
- shard-tglb: [PASS][4] -> [TIMEOUT][5] ([i915#2369] / [i915#3063] 
/ [i915#3648])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-tglb8/igt@gem_...@unwedge-stress.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-tglb6/igt@gem_...@unwedge-stress.html

  * igt@gem_exec_fair@basic-deadline:
- shard-glk:  [PASS][6] -> [FAIL][7] ([i915#2846])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-glk6/igt@gem_exec_f...@basic-deadline.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-glk6/igt@gem_exec_f...@basic-deadline.html

  * igt@gem_exec_fair@basic-pace@vcs1:
- shard-iclb: NOTRUN -> [FAIL][8] ([i915#2842])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-iclb4/igt@gem_exec_fair@basic-p...@vcs1.html
- shard-kbl:  [PASS][9] -> [SKIP][10] ([fdo#109271])
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-kbl2/igt@gem_exec_fair@basic-p...@vcs1.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-kbl4/igt@gem_exec_fair@basic-p...@vcs1.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
- shard-glk:  [PASS][11] -> [FAIL][12] ([i915#2842])
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-glk7/igt@gem_exec_fair@basic-throt...@rcs0.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-glk1/igt@gem_exec_fair@basic-throt...@rcs0.html

  * igt@gem_huc_copy@huc-copy:
- shard-apl:  NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#2190])
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-apl2/igt@gem_huc_c...@huc-copy.html
- shard-kbl:  NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#2190])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-kbl4/igt@gem_huc_c...@huc-copy.html

  * igt@gem_mmap_gtt@big-copy-xy:
- shard-skl:  [PASS][15] -> [FAIL][16] ([i915#1888] / [i915#307])
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-skl10/igt@gem_mmap_...@big-copy-xy.html
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-skl2/igt@gem_mmap_...@big-copy-xy.html

  * igt@gem_mmap_gtt@cpuset-basic-small-copy-xy:
- shard-glk:  [PASS][17] -> [FAIL][18] ([i915#1888] / [i915#307] / 
[i915#3468])
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-glk7/igt@gem_mmap_...@cpuset-basic-small-copy-xy.html
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-glk1/igt@gem_mmap_...@cpuset-basic-small-copy-xy.html

  * igt@gem_mmap_gtt@cpuset-big-copy:
- shard-iclb: [PASS][19] -> [FAIL][20] ([i915#307])
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/shard-iclb7/igt@gem_mmap_...@cpuset-big-copy.html
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-iclb7/igt@gem_mmap_...@cpuset-big-copy.html

  * igt@gem_pread@exhaustion:
- shard-snb:  NOTRUN -> [WARN][21] ([i915#2658])
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/shard-snb2/igt@gem_pr...@exhaustion.html

  * igt@gen9_exec_parse@cmd-crossing-page:
- shard-iclb: NOTRUN -> [SKIP][22] ([i915#2856])
   [22]:

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/selftest: Fix use of err in igt_reset_{fail, nop}_engine()

== Series Details ==

Series: drm/i915/selftest: Fix use of err in igt_reset_{fail, nop}_engine()
URL   : https://patchwork.freedesktop.org/series/93713/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10487 -> Patchwork_20830


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/index.html

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@amdgpu/amd_prime@i915-to-amd:
- fi-kbl-soraka:  NOTRUN -> [SKIP][1] ([fdo#109271])
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/fi-kbl-soraka/igt@amdgpu/amd_pr...@i915-to-amd.html

  * igt@gem_exec_parallel@engines@userptr:
- fi-pnv-d510:[PASS][2] -> [INCOMPLETE][3] ([i915#299])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/fi-pnv-d510/igt@gem_exec_parallel@engi...@userptr.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/fi-pnv-d510/igt@gem_exec_parallel@engi...@userptr.html

  * igt@i915_pm_rpm@module-reload:
- fi-kbl-guc: [PASS][4] -> [SKIP][5] ([fdo#109271])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/fi-kbl-guc/igt@i915_pm_...@module-reload.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/fi-kbl-guc/igt@i915_pm_...@module-reload.html

  * igt@runner@aborted:
- fi-pnv-d510:NOTRUN -> [FAIL][6] ([i915#2403] / [i915#2505] / 
[i915#2722])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/fi-pnv-d510/igt@run...@aborted.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#2505]: https://gitlab.freedesktop.org/drm/intel/issues/2505
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#299]: https://gitlab.freedesktop.org/drm/intel/issues/299
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303


Participating hosts (37 -> 34)
--

  Missing(3): fi-bdw-samus fi-bsw-cyan bat-jsl-1 


Build changes
-

  * Linux: CI_DRM_10487 -> Patchwork_20830

  CI-20190529: 20190529
  CI_DRM_10487: 51573da73ed1828367d6ea150155b85e347ab747 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6176: 3d8642170f2947b6aaad211e9e2e474fadedf6f9 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20830: 6021b4120d933a9ff840fd058403b5531e3e5778 @ 
git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6021b4120d93 drm/i915/selftest: Fix use of err in igt_reset_{fail, nop}_engine()

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20830/index.html

Re: [Intel-gfx] [PATCH v2 1/6] drm/i915: Export intel_hdmi_compute_avi_infoframe()

2021-08-16 Thread Nautiyal, Ankit K




On 8/16/2021 12:37 PM, Jani Nikula wrote:

On Fri, 13 Aug 2021, Ankit Nautiyal  wrote:

From: Swati Sharma 

Instead of re-writing the avi_infoframe_compute func in intel_dp;
exporting hdmi_compute_avi_infoframe func so that it can be called
directly while encapsulating AVI infoframes in GMP dip.

This is required when HDMI 2.1 PCON (dp to hdmi) is used and we need
to send AVI infoframes to PCON in source control mode.

Signed-off-by: Swati Sharma 
Signed-off-by: Ankit Nautiyal 
---
  drivers/gpu/drm/i915/display/intel_hdmi.c | 8 
  drivers/gpu/drm/i915/display/intel_hdmi.h | 3 +++
  2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c 
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b04685bb6439..0fbcdddb7ad5 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -702,10 +702,9 @@ void intel_read_infoframe(struct intel_encoder *encoder,
frame->any.type, type);
  }
  
-static bool

-intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder,
-struct intel_crtc_state *crtc_state,
-struct drm_connector_state *conn_state)
+bool intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder,
+ struct intel_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
  {
struct hdmi_avi_infoframe *frame = _state->infoframes.avi.avi;
const struct drm_display_mode *adjusted_mode =
@@ -758,6 +757,7 @@ intel_hdmi_compute_avi_infoframe(struct intel_encoder 
*encoder,
  
  	return true;

  }
+EXPORT_SYMBOL(intel_hdmi_compute_avi_infoframe);

This makes the function visible from other kernel modules; I don't think
you need or want that. Non-static is probably enough.

BR,
Jani.



Thanks Jani for the comments. Yes you are right, non-static would be 
enough for making this visible in intel_dp.


I will fix this in the next version of the patch.

Regards,

Ankit

  
  static bool

  intel_hdmi_compute_spd_infoframe(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h 
b/drivers/gpu/drm/i915/display/intel_hdmi.h
index b43a180d007e..2bdfd0838753 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.h
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
@@ -54,5 +54,8 @@ int intel_hdmi_dsc_get_num_slices(const struct 
intel_crtc_state *crtc_state,
  int src_max_slices, int src_max_slice_width,
  int hdmi_max_slices, int hdmi_throughput);
  int intel_hdmi_dsc_get_slice_height(int vactive);
+bool intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder,
+ struct intel_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state);
  
  #endif /* __INTEL_HDMI_H__ */

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Ditch the i915_gem_ww_ctx loop member

== Series Details ==

Series: drm/i915: Ditch the i915_gem_ww_ctx loop member
URL   : https://patchwork.freedesktop.org/series/93711/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10487 -> Patchwork_20829


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/index.html

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@amdgpu/amd_prime@amd-to-i915:
- fi-kbl-soraka:  NOTRUN -> [SKIP][1] ([fdo#109271]) +1 similar issue
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/fi-kbl-soraka/igt@amdgpu/amd_pr...@amd-to-i915.html

  
 Warnings 

  * igt@i915_pm_rpm@basic-rte:
- fi-kbl-guc: [SKIP][2] ([fdo#109271]) -> [FAIL][3] ([i915#579])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/fi-kbl-guc/igt@i915_pm_...@basic-rte.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/fi-kbl-guc/igt@i915_pm_...@basic-rte.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579


Participating hosts (37 -> 34)
--

  Missing(3): fi-bdw-samus fi-bsw-cyan bat-jsl-1 


Build changes
-

  * Linux: CI_DRM_10487 -> Patchwork_20829

  CI-20190529: 20190529
  CI_DRM_10487: 51573da73ed1828367d6ea150155b85e347ab747 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6176: 3d8642170f2947b6aaad211e9e2e474fadedf6f9 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20829: e82160475e8e931a8012bc7ed0cdcc387f562c23 @ 
git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e82160475e8e drm/i915: Ditch the i915_gem_ww_ctx loop member

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20829/index.html

Re: [Intel-gfx] [PATCH 02/10] drm/i915/bios: Start to support two integrated panels

On Wed, 21 Jul 2021, José Roberto de Souza  wrote:
> VBT has support for up two integrated panels but i915 only supports one.
>
> So here stating to add the basic support for two integrated panels
> and moving the DRRS to ddi_vbt_port_info instead of keeping a global
> one.
> Other VBT blocks will be converted in following patches.
>
> While at is also nucking lvds_dither as it is not used.

If you were to get a mysterious bisect result on this patch, would you
know what's wrong?

I think there's too much going on in one patch, and this needs to be
split. Small, incremental changes with functional and non-functional
separated if possible. This is incredibly fragile stuff.

Also, I think we'll need to be able to figure out the panel type based
on EDID, i.e. we need to parse some stuff first, and the panel stuff
only after we've read EDID. Obviously you don't have to do that here,
but this need to be done so that this doesn't become impossible.

BR,
Jani.


>
> Cc: Ville Syrjälä 
> Cc: Jani Nikula 
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 185 +-
>  drivers/gpu/drm/i915/display/intel_bios.h |   2 +
>  drivers/gpu/drm/i915/display/intel_dp.c   |   5 +-
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h |   3 +
>  drivers/gpu/drm/i915/i915_drv.h   |   5 +-
>  5 files changed, 150 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index 5bc2c944d99b4..2b90efb41ecce 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -211,22 +211,20 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
>   return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
>  }
>  
> -/* Parse general panel options */
> -static void
> -parse_panel_options(struct drm_i915_private *i915,
> - const struct bdb_header *bdb)
> +/*
> + * Parse and set vbt.panel_type, it will be used by the VBT blocks that are
> + * not being called from parse_integrated_panel() yet.
> + */
> +static void parse_panel_type(struct drm_i915_private *i915,
> +  const struct bdb_header *bdb)
>  {
>   const struct bdb_lvds_options *lvds_options;
> - int panel_type;
> - int drrs_mode;
> - int ret;
> + int ret, panel_type;
>  
>   lvds_options = find_section(bdb, BDB_LVDS_OPTIONS);
>   if (!lvds_options)
>   return;
>  
> - i915->vbt.lvds_dither = lvds_options->pixel_dither;
> -
>   ret = intel_opregion_get_panel_type(i915);
>   if (ret >= 0) {
>   drm_WARN_ON(>drm, ret > 0xf);
> @@ -246,9 +244,25 @@ parse_panel_options(struct drm_i915_private *i915,
>   }
>  
>   i915->vbt.panel_type = panel_type;
> +}
> +
> +/* Parse general panel options */
> +static void
> +parse_panel_options(struct drm_i915_private *i915,
> + const struct bdb_header *bdb,
> + struct ddi_vbt_port_info *info,
> + int panel_index)
> +{
> + const struct bdb_lvds_options *lvds_options;
> + int drrs_mode;
> +
> + lvds_options = find_section(bdb, BDB_LVDS_OPTIONS);
> + if (!lvds_options)
> + return;
> +
> + drrs_mode = lvds_options->dps_panel_type_bits >> (panel_index * 2);
> + drrs_mode &= MODE_MASK;
>  
> - drrs_mode = (lvds_options->dps_panel_type_bits
> - >> (panel_type * 2)) & MODE_MASK;
>   /*
>* VBT has static DRRS = 0 and seamless DRRS = 2.
>* The below piece of code is required to adjust vbt.drrs_type
> @@ -256,16 +270,16 @@ parse_panel_options(struct drm_i915_private *i915,
>*/
>   switch (drrs_mode) {
>   case 0:
> - i915->vbt.drrs_type = STATIC_DRRS_SUPPORT;
> + info->drrs_type = STATIC_DRRS_SUPPORT;
>   drm_dbg_kms(>drm, "DRRS supported mode is static\n");
>   break;
>   case 2:
> - i915->vbt.drrs_type = SEAMLESS_DRRS_SUPPORT;
> + info->drrs_type = SEAMLESS_DRRS_SUPPORT;
>   drm_dbg_kms(>drm,
>   "DRRS supported mode is seamless\n");
>   break;
>   default:
> - i915->vbt.drrs_type = DRRS_NOT_SUPPORTED;
> + info->drrs_type = DRRS_NOT_SUPPORTED;
>   drm_dbg_kms(>drm,
>   "DRRS not supported (VBT input)\n");
>   break;
> @@ -710,28 +724,42 @@ parse_driver_features(struct drm_i915_private *i915,
>   i915->vbt.int_lvds_support = 0;
>   }
>  
> - if (bdb->version < 228) {
> - drm_dbg_kms(>drm, "DRRS State Enabled:%d\n",
> - driver->drrs_enabled);
> - /*
> -  * If DRRS is not supported, drrs_type has to be set to 0.
> -  * This is because, VBT is configured in such a way that
> -  * static DRRS

Re: [Intel-gfx] [PATCH 01/10] drm/i915/bios: Allow DSI ports to be parsed by parse_ddi_port()

On Wed, 21 Jul 2021, José Roberto de Souza  wrote:
> Allow MIPI DSI ports to be parsed like any other DDI port.
> This will be helpful to integrate into just one function the parse of
> information about integrated panels(eDP and DSI).
>
> Allow MIPI DSI ports to be parsed to be parsed like any other DDI
> port.
> This will be helpful to integrate into just one function the parse of
> information about integrated panels(eDP and DSI).

Here be dragons.

For starters, parse_ddi_ports() is only run on DDI + CHV, most
significantly not VLV.

So intel_bios_encoder_supports_dsi() is not universal.

It's not trivial to extend parse_ddi_ports() to cover VLV in general or
CHV+DSI, because apparently they have the DSI ports in the same
"namespace", if you will, with other ports. I.e. you could have dupes,
which parse_ddi_port() does not allow.

There are existing patches on the list where I've attempted this, and
Ville has refuted them refuted me time and time again. :(


BR,
Jani.

>
> Cc: Ville Syrjälä 
> Cc: Jani Nikula 
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 22 +++---
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index 5b6922e28ef28..5bc2c944d99b4 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1709,10 +1709,10 @@ static enum port dvo_port_to_port(struct 
> drm_i915_private *i915,
>* so look for all the possible values for each port.
>*/
>   static const int port_mapping[][3] = {
> - [PORT_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, -1 },
> - [PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, -1 },
> - [PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, -1 },
> - [PORT_D] = { DVO_PORT_HDMID, DVO_PORT_DPD, -1 },
> + [PORT_A] = { DVO_PORT_HDMIA, DVO_PORT_DPA, DVO_PORT_MIPIA },
> + [PORT_B] = { DVO_PORT_HDMIB, DVO_PORT_DPB, DVO_PORT_MIPIB },
> + [PORT_C] = { DVO_PORT_HDMIC, DVO_PORT_DPC, DVO_PORT_MIPIC },
> + [PORT_D] = { DVO_PORT_HDMID, DVO_PORT_DPD, DVO_PORT_MIPID },
>   [PORT_E] = { DVO_PORT_HDMIE, DVO_PORT_DPE, DVO_PORT_CRT },
>   [PORT_F] = { DVO_PORT_HDMIF, DVO_PORT_DPF, -1 },
>   [PORT_G] = { DVO_PORT_HDMIG, DVO_PORT_DPG, -1 },
> @@ -1868,6 +1868,12 @@ intel_bios_encoder_supports_edp(const struct 
> intel_bios_encoder_data *devdata)
>   devdata->child.device_type & DEVICE_TYPE_INTERNAL_CONNECTOR;
>  }
>  
> +static bool
> +intel_bios_encoder_supports_dsi(const struct intel_bios_encoder_data 
> *devdata)
> +{
> + return devdata->child.device_type & DEVICE_TYPE_MIPI_OUTPUT;
> +}
> +
>  static bool is_port_valid(struct drm_i915_private *i915, enum port port)
>  {
>   /*
> @@ -1886,7 +1892,8 @@ static void parse_ddi_port(struct drm_i915_private 
> *i915,
>  {
>   const struct child_device_config *child = >child;
>   struct ddi_vbt_port_info *info;
> - bool is_dvi, is_hdmi, is_dp, is_edp, is_crt, supports_typec_usb, 
> supports_tbt;
> + bool is_dvi, is_hdmi, is_dp, is_edp, is_crt, supports_typec_usb;
> + bool supports_tbt, is_dsi;
>   int dp_boost_level, hdmi_boost_level;
>   enum port port;
>  
> @@ -1917,16 +1924,17 @@ static void parse_ddi_port(struct drm_i915_private 
> *i915,
>   is_crt = intel_bios_encoder_supports_crt(devdata);
>   is_hdmi = intel_bios_encoder_supports_hdmi(devdata);
>   is_edp = intel_bios_encoder_supports_edp(devdata);
> + is_dsi = intel_bios_encoder_supports_dsi(devdata);
>  
>   supports_typec_usb = intel_bios_encoder_supports_typec_usb(devdata);
>   supports_tbt = intel_bios_encoder_supports_tbt(devdata);
>  
>   drm_dbg_kms(>drm,
> - "Port %c VBT info: CRT:%d DVI:%d HDMI:%d DP:%d eDP:%d 
> LSPCON:%d USB-Type-C:%d TBT:%d DSC:%d\n",
> + "Port %c VBT info: CRT:%d DVI:%d HDMI:%d DP:%d eDP:%d 
> LSPCON:%d USB-Type-C:%d TBT:%d DSC:%d DSI:%d\n",
>   port_name(port), is_crt, is_dvi, is_hdmi, is_dp, is_edp,
>   HAS_LSPCON(i915) && child->lspcon,
>   supports_typec_usb, supports_tbt,
> - devdata->dsc != NULL);
> + devdata->dsc != NULL, is_dsi);
>  
>   if (is_dvi) {
>   u8 ddc_pin;

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] Support Intel HD Graphics in Mipad 2

2021-08-16 Thread Viết Ngọc

Hi everybody

I have a Mi Pad 2 and it is runing Android OS L.

When i tried to boot another Linux OS from usb, my tablet suspend and in screen 
have notify : "fb0: switching to inteldrmfb from EFI VGA"

 When i add "nomodeset" to boot option, the system is continue nomarly.  And 
this is information of hardware when i use lspci -v

VGA compatible controller: Intel Corporation Atom/Celeron/Pentium Processor 
x5-E8000/J3xxx/N3xxx Integrated Graphics Controller (rev 20) (prog-if 00 [VGA 
controller])

Subsystem: Xiaomi Device 1502

Flags: bus master, fast devsel, latency 0, IRQ 255

Memory at 9000 (64-bit, non-prefetchable) [size=16M]

Memory at 8000 (64-bit, prefetchable) [size=256M]

I/O ports at 1000 [size=64] Expansion ROM at 000c [virtual] [disabled] 
[size=128K]

Capabilities: [d0] Power Management version 2

Capabilities: [90] MSI: Enable- Count=1/1 Maskable- 64bit-

Capabilities: [b0] Vendor Specific Information: Len=07 

Kernel modules: i915

I think this problem that the Linux kernel after 3.14 not include custom of 
Xiaomi for Intel GPU.

Can you bring it from kernel source code of Mi Pad 2 to next version of Linux 
kernel?

This is source code of Mi Pad 2 
https://github.com/MiCode/Xiaomi_Kernel_OpenSource/tree/latte-l-oss

Thank you very much and hope you fix this soon

Viết Ngọc
[https://opengraph.githubassets.com/1c24a87bc9810d9708a2ec45696f8740ec523c00caed6047f6d77e83182a9d82/MiCode/Xiaomi_Kernel_OpenSource]
GitHub - MiCode/Xiaomi_Kernel_OpenSource at 
latte-l-oss
Xiaomi Mobile Phone Kernel OpenSource. Contribute to 
MiCode/Xiaomi_Kernel_OpenSource development by creating an account on GitHub.
github.com



Re: [Intel-gfx] [bug report] drm/i915/gem: Correct the locking and pin pattern for dma-buf (v8)

2021-08-16 Thread Nathan Chancellor


On 8/13/2021 4:31 AM, Dan Carpenter wrote:

Nathan has probably already sent fixes for these.  Nathan, could you CC
kernel-janitors on static checker fixes?  That way we wouldn't send so
many duplicate patches.


Sure. I did not send any fixes prior to this email but I just sent
https://lore.kernel.org/r/20210813171158.2665823-1-nat...@kernel.org/ 
with kernel-janitors and you CC'd.



Uninitialized variable warnings are really frustrating.  We disabled the
GCC warning for them because the new GCC is crap.  So now they take up
a lot of my time.  A few of these are because Smatch doesn't always know
when we enter loops, but more than half are clear bugs.

drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c:189 i915_gem_dmabuf_attach() error: 
uninitialized symbol 'err'.
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:80 
igt_dmabuf_import_self() error: uninitialized symbol 'import_obj'.
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:104 
igt_dmabuf_import_same_driver_lmem() error: uninitialized symbol 'dmabuf'.
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:138 
igt_dmabuf_import_same_driver_lmem() error: uninitialized symbol 'err'.
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:158 
igt_dmabuf_import_same_driver() error: uninitialized symbol 'dmabuf'.
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:231 
igt_dmabuf_import_same_driver() error: uninitialized symbol 'import_obj'.
drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c:197 
igt_lmem_pages_migrate() error: uninitialized symbol 'err'.


...


drivers/gpu/drm/i915/gt/selftest_hangcheck.c:465 igt_reset_nop_engine() error: 
uninitialized symbol 'err'.
drivers/gpu/drm/i915/gt/selftest_hangcheck.c:580 igt_reset_fail_engine() error: 
uninitialized symbol 'err'.


Clang caught these two but none of the rest and it is an obvious fix so 
that is the only one I am going to fix.



drivers/gpu/drm/i915/gt/selftest_migrate.c:102 copy() error: uninitialized 
symbol 'rq'.
drivers/gpu/drm/i915/gt/selftest_migrate.c:113 copy() error: uninitialized 
symbol 'vaddr'.
drivers/gpu/drm/i915/gt/selftest_migrate.c:182 clear() error: uninitialized 
symbol 'rq'.
drivers/gpu/drm/i915/gt/selftest_migrate.c:193 clear() error: uninitialized 
symbol 'vaddr'.
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:819 
__unwind_incomplete_requests() error: uninitialized symbol 'pl'.


Cheers,
Nathan

Re: [Intel-gfx] [PATCH 1/2] drm: avoid races with modesetting rights


On 13/8/21 11:49 pm, Daniel Vetter wrote:

On Fri, Aug 13, 2021 at 04:54:49PM +0800, Desmond Cheong Zhi Xi wrote:

In drm_client_modeset.c and drm_fb_helper.c,
drm_master_internal_{acquire,release} are used to avoid races with DRM
userspace. These functions hold onto drm_device.master_mutex while
committing, and bail if there's already a master.

However, ioctls can still race between themselves. A
time-of-check-to-time-of-use error can occur if an ioctl that changes
the modeset has its rights revoked after it validates its permissions,
but before it completes.

There are three ioctls that can affect modesetting permissions:

- DROP_MASTER ioctl removes rights for a master and its leases

- REVOKE_LEASE ioctl revokes rights for a specific lease

- SET_MASTER ioctl sets the device master if the master role hasn't
been acquired yet

All these races can be avoided by introducing an SRCU that acts as a
barrier for ioctls that can change modesetting permissions. Processes
that perform modesetting should hold a read lock on the new
drm_device.master_barrier_srcu, and ioctls that change these
permissions should call synchronize_srcu before returning.

This ensures that any process that might have seen old permissions are
flushed out before DROP_MASTER/REVOKE_LEASE/SET_MASTER ioctls return
to userspace.

Reported-by: Daniel Vetter 
Signed-off-by: Desmond Cheong Zhi Xi 


This looks pretty solid, but I think there's one gap where we can still
race. Scenario.

Process A has a drm fd with master rights and two threads:
- thread 1 does a long-running display operation (like a modeset or
   whatever)
- thread 2 does a drop-master

Then we start a new process B, which acquires master in drm_open (there is
no other one left). This is like setmaster ioctl, but your
DRM_MASTER_FLUSH bit doesn't work there.



Ah, I see the race. I think a good place to plug this would be in 
drm_master_open using the drm_master_flush (or 
drm_master_unlock_and_flush) that you suggested below.



The other thing is that for modeset stuff (which this all is) srcu is
probably massive overkill, and a simple rwsem should be good enough too.
Maybe even better, since the rwsem guarantees that no new reader can start
once you try to acquire the write side.



Makes sense, I'll switch to a rwsem then.


Finally, and this is a bit a bikeshed: I don't like much how
DRM_MASTER_FLUSH leaks the need of these very few places into the very
core drm_ioctl function. One idea I had was to use task_work in a special
function, roughly

void master_flush()
{
down_write(master_rwsem);
up_write(master_rwms);
}
void drm_master_flush()
{
init_task_work(fpriv->master_flush_work, master_flush)
task_work_add(fpriv->master_flush_work);
/* if task_work_add fails we're exiting, at which point the lack
 * of master flush doesn't matter);
}

And maybe put a comment above the function explaining why and how this
works.

We could even do a drm_master_unlock_and_flush helper, since that's really
what everyone wants, and it would make it very clear which master state
changes need this flush. Instead of setting a flag bit in an ioctl table
very far away ...

Thoughts?
-Daniel



Sounds good. I wasn't aware of the task_work_add mechanism to queue work 
before the task returns to usermode, but this seems like a more explicit 
way to flush.


Thanks for the feedback, Daniel. I'll fix this up in a v2.


---
  drivers/gpu/drm/drm_auth.c   | 17 ++---
  drivers/gpu/drm/drm_client_modeset.c | 10 ++
  drivers/gpu/drm/drm_drv.c|  2 ++
  drivers/gpu/drm/drm_fb_helper.c  | 20 
  drivers/gpu/drm/drm_internal.h   |  5 +++--
  drivers/gpu/drm/drm_ioctl.c  | 25 +
  include/drm/drm_device.h | 11 +++
  include/drm/drm_ioctl.h  |  7 +++
  8 files changed, 76 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 60a6b21474b1..004506608e76 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -29,6 +29,7 @@
   */
  
  #include 

+#include 
  
  #include 

  #include 
@@ -448,21 +449,31 @@ void drm_master_put(struct drm_master **master)
  EXPORT_SYMBOL(drm_master_put);
  
  /* Used by drm_client and drm_fb_helper */

-bool drm_master_internal_acquire(struct drm_device *dev)
+bool drm_master_internal_acquire(struct drm_device *dev, int *idx)
  {
+   *idx = srcu_read_lock(>master_barrier_srcu);
+
mutex_lock(>master_mutex);
if (dev->master) {
mutex_unlock(>master_mutex);
+   srcu_read_unlock(>master_barrier_srcu, *idx);
return false;
}
+   mutex_unlock(>master_mutex);
  
  	return true;

  }
  EXPORT_SYMBOL(drm_master_internal_acquire);
  
  /* Used by drm_client and drm_fb_helper */

-void drm_master_internal_release(struct drm_device *dev)
+void

[Intel-gfx] SR-IOV development status

2021-08-16 Thread Dr. Gregor Simenc


Hi there

is there any progress in development a SR-IOV driver for Xe graphics in 
tigerlake architecture für linux ?


Any comments are welcome.

I appreciate a driver to share my iGPU in KVM for other VMs like Windows.


Best wishes

Gregor

Re: [Intel-gfx] [PATCH v2] drm: avoid races with modesetting rights


On 16/8/21 2:47 am, kernel test robot wrote:

Hi Desmond,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20210813]
[also build test ERROR on v5.14-rc5]
[cannot apply to linus/master v5.14-rc5 v5.14-rc4 v5.14-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
base:4b358aabb93a2c654cd1dcab1a25a589f6e2b153
config: i386-randconfig-a004-20210815 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
 # 
https://github.com/0day-ci/linux/commit/cf6d8354b7d7953cd866fad004cbb189adfa074f
 git remote add linux-review https://github.com/0day-ci/linux
 git fetch --no-tags linux-review 
Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
 git checkout cf6d8354b7d7953cd866fad004cbb189adfa074f
 # save the attached .config to linux build tree
 make W=1 ARCH=i386

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>, old ones prefixed by <<):


ERROR: modpost: "task_work_add" [drivers/gpu/drm/drm.ko] undefined!




I'm a bit uncertain about this. Looking into the .config used, this 
error seems to happen because task_work_add isn't an exported symbol, 
but DRM is being compiled as a loadable kernel module (CONFIG_DRM=m).


One way to deal with this is to export the symbol, but there was a 
proposed patch to do this a few months back that wasn't picked up [1], 
so I'm not sure what to make of this.


I'll export the symbol as part of a v3 series, and check in with the 
task-work maintainers.


Link: 
https://lore.kernel.org/lkml/20210127150029.13766-3-josh...@samsung.com/ [1]



---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

[Intel-gfx] [BUG - BISECTED] display not detected anymore

2021-08-16 Thread Heiko Carstens

Hello,

I have Fedora 33 running, and with the Fedore kernel update from 5.11
series to 5.12 my external monitor was not detected anymore. Same is
true with the Fedora supplied 5.13 kernel version.

So I tried with vanilla kernel 5.11 and latest git head from Linus'
tree. 5.11 works while latest git head does not. Bisecting the problem
points to commit 32c3d9b0f51e ("Merge tag 'drm-intel-next-2021-01-27'
of git://anongit.freedesktop.org/drm/drm-intel into drm-next").

Unfortunately it is a merge commit, so it looks like conflicting
changes have been made in the parent branches.

Hardware in use:

- ThinkPad X1 Yoga 4th / Carbon 7th
- Intel(R) Core(TM) i5-8265U CPU @ 1.60GHz

The Thinkpad is connected to a ThinkPad Thunderbolt 3 Dock with a
Thunderbolt cable and a monitor (Eizo EV3285) is connected via
Displayport to the docking station.

The monitor is detected and works without any problems (4k@60HZ)
before the above mentioned merge commit. With the commit and
afterwards it is not detected anymore and only the Thinkpad builtin
display can be used.

Any idea what went wrong? I can provide more information, or test
debug patches if wanted. Just let me know.

Thanks,
Heiko

Re: [Intel-gfx] [PATCH v2] drm: avoid races with modesetting rights


On 16/8/21 2:47 am, kernel test robot wrote:

Hi Desmond,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20210813]
[also build test WARNING on v5.14-rc5]
[cannot apply to linus/master v5.14-rc5 v5.14-rc4 v5.14-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
base:4b358aabb93a2c654cd1dcab1a25a589f6e2b153
config: arc-randconfig-r031-20210815 (attached as .config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
 wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
 chmod +x ~/bin/make.cross
 # 
https://github.com/0day-ci/linux/commit/cf6d8354b7d7953cd866fad004cbb189adfa074f
 git remote add linux-review https://github.com/0day-ci/linux
 git fetch --no-tags linux-review 
Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
 git checkout cf6d8354b7d7953cd866fad004cbb189adfa074f
 # save the attached .config to linux build tree
 COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross 
ARCH=arc

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):


drivers/gpu/drm/drm_auth.c:483:6: warning: no previous prototype for 
'master_flush' [-Wmissing-prototypes]

  483 | void master_flush(struct callback_head *cb)
  |  ^~~~



My bad, this should have been declared with static. I'll add it in, thanks.



vim +/master_flush +483 drivers/gpu/drm/drm_auth.c

479 
480 /* After flushing, all readers that might have seen old master/lease
481  * permissions are guaranteed to have completed.
482  */
  > 483  void master_flush(struct callback_head *cb)
484 {
485 struct drm_device *dev = container_of(cb, struct drm_device,
486   master_flush_work);
487 
488 down_write(>master_rwsem);
489 up_write(>master_rwsem);
490 }
491 

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org

[Intel-gfx] [PATCH v2] drm: avoid races with modesetting rights

In drm_client_modeset.c and drm_fb_helper.c,
drm_master_internal_{acquire,release} are used to avoid races with DRM
userspace. These functions hold onto drm_device.master_mutex while
committing, and bail if there's already a master.

However, there are other places where modesetting rights can race. A
time-of-check-to-time-of-use error can occur if an ioctl that changes
the modeset has its rights revoked after it validates its permissions,
but before it completes.

There are four places where modesetting permissions can change:

- DROP_MASTER ioctl removes rights for a master and its leases

- REVOKE_LEASE ioctl revokes rights for a specific lease

- SET_MASTER ioctl sets the device master if the master role hasn't
been acquired yet

- drm_open which can create a new master for a device if one does not
currently exist

These races can be avoided by flushing all users that might have seen
old modesetting permissions before returning to userspace.

We do this using rwsem: users that perform modesetting should hold a
read lock on the new drm_device.master_rwsem, and users that change
these permissions should flush these readers before returning to
userspace.

Reported-by: Daniel Vetter 
Signed-off-by: Desmond Cheong Zhi Xi 
---

Hi,

I opted to leave the drm_master_unlock_and_flush helper out of this
patch, but happy to add it in if it'd be useful. Imo, when comparing it
with a mutex_unlock followed by drm_master_flush, it didn't add clarity.
And since we don't always hold the drm_device.master_mutex before
flushing (such as in drm_mode_revoke_lease_ioctl), perhaps it's better
to stick with one method to flush readers with drm_master_flush.

v1 -> v2 (suggested by Daniel Vetter):
- Address an additional race when drm_open runs.
- Switch from SRCU to rwsem to synchronise readers and writers.
- Implement drm_master_flush with task_work so that flushes can be
queued to run before returning to userspace without creating a new
DRM_MASTER_FLUSH ioctl flag.

Best wishes,
Desmond

 drivers/gpu/drm/drm_auth.c  | 45 -
 drivers/gpu/drm/drm_drv.c   |  1 +
 drivers/gpu/drm/drm_ioctl.c |  9 +++-
 drivers/gpu/drm/drm_lease.c |  1 +
 include/drm/drm_auth.h  |  1 +
 include/drm/drm_device.h| 18 +++
 6 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 60a6b21474b1..175bc4d1e4b4 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -29,6 +29,7 @@
  */
 
 #include 
+#include 
 
 #include 
 #include 
@@ -282,6 +283,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
drm_set_master(dev, file_priv, false);
 out_unlock:
mutex_unlock(>master_mutex);
+   drm_master_flush(dev);
return ret;
 }
 
@@ -321,8 +323,10 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void 
*data,
}
 
drm_drop_master(dev, file_priv);
+
 out_unlock:
mutex_unlock(>master_mutex);
+   drm_master_flush(dev);
return ret;
 }
 
@@ -344,6 +348,8 @@ int drm_master_open(struct drm_file *file_priv)
}
mutex_unlock(>master_mutex);
 
+   drm_master_flush(dev);
+
return ret;
 }
 
@@ -450,11 +456,15 @@ EXPORT_SYMBOL(drm_master_put);
 /* Used by drm_client and drm_fb_helper */
 bool drm_master_internal_acquire(struct drm_device *dev)
 {
+   down_read(>master_rwsem);
+
mutex_lock(>master_mutex);
if (dev->master) {
mutex_unlock(>master_mutex);
+   up_read(>master_rwsem);
return false;
}
+   mutex_unlock(>master_mutex);
 
return true;
 }
@@ -463,6 +473,39 @@ EXPORT_SYMBOL(drm_master_internal_acquire);
 /* Used by drm_client and drm_fb_helper */
 void drm_master_internal_release(struct drm_device *dev)
 {
-   mutex_unlock(>master_mutex);
+   up_read(>master_rwsem);
 }
 EXPORT_SYMBOL(drm_master_internal_release);
+
+/* After flushing, all readers that might have seen old master/lease
+ * permissions are guaranteed to have completed.
+ */
+void master_flush(struct callback_head *cb)
+{
+   struct drm_device *dev = container_of(cb, struct drm_device,
+ master_flush_work);
+
+   down_write(>master_rwsem);
+   up_write(>master_rwsem);
+}
+
+/**
+ * drm_master_flush - queues work to flush readers of master/lease permissions
+ * before returning to userspace
+ * @dev: DRM device
+ *
+ * Queues up work to flush all readers of master/lease permissions. This work
+ * is run before this task returns to user mode. Calling this function when a
+ * task changes modesetting rights ensures that other processes that perform
+ * modesetting do not race with userspace.
+ */
+void drm_master_flush(struct drm_device *dev)
+{
+   init_task_work(>master_flush_work, master_flush);
+   task_work_add(current, >master_flush_work, TWA_RESUME);
+   /* If task_work_add fails, then the task is

[Intel-gfx] [PATCH] drm/i915/selftest: Fix use of err in igt_reset_{fail, nop}_engine()

2021-08-16 Thread Nathan Chancellor

Clang warns:

In file included from drivers/gpu/drm/i915/gt/intel_reset.c:1514:
drivers/gpu/drm/i915/gt/selftest_hangcheck.c:465:62: warning: variable
'err' is uninitialized when used here [-Wuninitialized]
pr_err("[%s] Create context failed: %d!\n", engine->name, err);
  ^~~
...
drivers/gpu/drm/i915/gt/selftest_hangcheck.c:580:62: warning: variable
'err' is uninitialized when used here [-Wuninitialized]
pr_err("[%s] Create context failed: %d!\n", engine->name, err);
  ^~~
...
2 warnings generated.

This appears to be a copy and paste issue. Use ce directly using the %pe
specifier to pretty print the error code so that err is not used
uninitialized in these functions.

Fixes: 3a7b72665ea5 ("drm/i915/selftest: Bump selftest timeouts for hangcheck")
Reported-by: Dan Carpenter 
Signed-off-by: Nathan Chancellor 
---
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c 
b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 08f011f893b2..2c1ed32ca5ac 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -462,7 +462,7 @@ static int igt_reset_nop_engine(void *arg)
 
ce = intel_context_create(engine);
if (IS_ERR(ce)) {
-   pr_err("[%s] Create context failed: %d!\n", 
engine->name, err);
+   pr_err("[%s] Create context failed: %pe!\n", 
engine->name, ce);
return PTR_ERR(ce);
}
 
@@ -577,7 +577,7 @@ static int igt_reset_fail_engine(void *arg)
 
ce = intel_context_create(engine);
if (IS_ERR(ce)) {
-   pr_err("[%s] Create context failed: %d!\n", 
engine->name, err);
+   pr_err("[%s] Create context failed: %pe!\n", 
engine->name, ce);
return PTR_ERR(ce);
}
 

base-commit: 927dfdd09d8c03ba100ed0c8c3915f8e1d1f5556
-- 
2.33.0.rc2

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Ditch the i915_gem_ww_ctx loop member

== Series Details ==

Series: drm/i915: Ditch the i915_gem_ww_ctx loop member
URL   : https://patchwork.freedesktop.org/series/93711/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
e82160475e8e drm/i915: Ditch the i915_gem_ww_ctx loop member
-:67: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_ww' - possible side-effects?
#67: FILE: drivers/gpu/drm/i915/i915_gem_ww.h:39:
+#define for_i915_gem_ww(_ww, _err, _intr)\
+   for (i915_gem_ww_ctx_init(_ww, _intr), (_err) = -EDEADLK; \
+fetch_and_zero(&_err) == -EDEADLK;   \
 _err = __i915_gem_ww_fini(_ww, _err))

-:67: CHECK:MACRO_ARG_REUSE: Macro argument reuse '_err' - possible 
side-effects?
#67: FILE: drivers/gpu/drm/i915/i915_gem_ww.h:39:
+#define for_i915_gem_ww(_ww, _err, _intr)\
+   for (i915_gem_ww_ctx_init(_ww, _intr), (_err) = -EDEADLK; \
+fetch_and_zero(&_err) == -EDEADLK;   \
 _err = __i915_gem_ww_fini(_ww, _err))

total: 0 errors, 0 warnings, 2 checks, 48 lines checked

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/dp: remove superfluous EXPORT_SYMBOL()

== Series Details ==

Series: drm/i915/dp: remove superfluous EXPORT_SYMBOL()
URL   : https://patchwork.freedesktop.org/series/93708/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10487 -> Patchwork_20828


Summary
---

  **SUCCESS**

  No regressions found.

  External URL: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/index.html

Known issues


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

### IGT changes ###

 Issues hit 

  * igt@amdgpu/amd_prime@amd-to-i915:
- fi-kbl-soraka:  NOTRUN -> [SKIP][1] ([fdo#109271]) +1 similar issue
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/fi-kbl-soraka/igt@amdgpu/amd_pr...@amd-to-i915.html

  * igt@gem_exec_suspend@basic-s3:
- fi-tgl-1115g4:  [PASS][2] -> [FAIL][3] ([i915#1888])
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/fi-tgl-1115g4/igt@gem_exec_susp...@basic-s3.html
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/fi-tgl-1115g4/igt@gem_exec_susp...@basic-s3.html

  * igt@i915_selftest@live@execlists:
- fi-bsw-kefka:   [PASS][4] -> [INCOMPLETE][5] ([i915#2940])
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/fi-bsw-kefka/igt@i915_selftest@l...@execlists.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/fi-bsw-kefka/igt@i915_selftest@l...@execlists.html

  * igt@i915_selftest@live@workarounds:
- fi-rkl-guc: [PASS][6] -> [INCOMPLETE][7] ([i915#3920])
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10487/fi-rkl-guc/igt@i915_selftest@l...@workarounds.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/fi-rkl-guc/igt@i915_selftest@l...@workarounds.html

  * igt@runner@aborted:
- fi-bsw-kefka:   NOTRUN -> [FAIL][8] ([fdo#109271] / [i915#1436])
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/fi-bsw-kefka/igt@run...@aborted.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#2940]: https://gitlab.freedesktop.org/drm/intel/issues/2940
  [i915#3920]: https://gitlab.freedesktop.org/drm/intel/issues/3920


Participating hosts (37 -> 34)
--

  Missing(3): fi-bdw-samus fi-bsw-cyan bat-jsl-1 


Build changes
-

  * Linux: CI_DRM_10487 -> Patchwork_20828

  CI-20190529: 20190529
  CI_DRM_10487: 51573da73ed1828367d6ea150155b85e347ab747 @ 
git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6176: 3d8642170f2947b6aaad211e9e2e474fadedf6f9 @ 
https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20828: de0d884d06c91f72beab5d8c88c293a4410eba05 @ 
git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

de0d884d06c9 drm/i915/dp: remove superfluous EXPORT_SYMBOL()

== Logs ==

For more details see: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20828/index.html

Re: [Intel-gfx] [PATCH v2] drm: avoid races with modesetting rights

On Mon, Aug 16, 2021 at 10:53 AM Desmond Cheong Zhi Xi
 wrote:
> On 16/8/21 2:47 am, kernel test robot wrote:
> > Hi Desmond,
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on next-20210813]
> > [also build test ERROR on v5.14-rc5]
> > [cannot apply to linus/master v5.14-rc5 v5.14-rc4 v5.14-rc3]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
> >
> > url:
> > https://github.com/0day-ci/linux/commits/Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
> > base:4b358aabb93a2c654cd1dcab1a25a589f6e2b153
> > config: i386-randconfig-a004-20210815 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> > reproduce (this is a W=1 build):
> >  # 
> > https://github.com/0day-ci/linux/commit/cf6d8354b7d7953cd866fad004cbb189adfa074f
> >  git remote add linux-review https://github.com/0day-ci/linux
> >  git fetch --no-tags linux-review 
> > Desmond-Cheong-Zhi-Xi/drm-avoid-races-with-modesetting-rights/20210815-234145
> >  git checkout cf6d8354b7d7953cd866fad004cbb189adfa074f
> >  # save the attached .config to linux build tree
> >  make W=1 ARCH=i386
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot 
> >
> > All errors (new ones prefixed by >>, old ones prefixed by <<):
> >
> >>> ERROR: modpost: "task_work_add" [drivers/gpu/drm/drm.ko] undefined!
> >
>
> I'm a bit uncertain about this. Looking into the .config used, this
> error seems to happen because task_work_add isn't an exported symbol,
> but DRM is being compiled as a loadable kernel module (CONFIG_DRM=m).
>
> One way to deal with this is to export the symbol, but there was a
> proposed patch to do this a few months back that wasn't picked up [1],
> so I'm not sure what to make of this.
>
> I'll export the symbol as part of a v3 series, and check in with the
> task-work maintainers.
>
> Link:
> https://lore.kernel.org/lkml/20210127150029.13766-3-josh...@samsung.com/ [1]

Yeah that sounds best. I have two more thoughts on the patch:
- drm_master_flush isn't used by any modules outside of drm.ko, so we
can unexport it and drop the kerneldoc (the comment is still good).
These kind of internal functions have their declaration in
drm-internal.h - there's already a few there from drm_auth.c

- We know have 3 locks for master state, that feels a bit like
overkill. The spinlock I think we need to keep due to lock inversions,
but the master_mutex and master_rwsem look like we should be able to
merge them? I.e. anywhere we currently grab the master_mutex we could
instead grab the rwsem in either write mode (when we change stuff) or
read mode (when we just check, like in master_internal_acquire).

Thoughts?
-Daniel

>
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH] drm/i915/dp: remove superfluous EXPORT_SYMBOL()

2021-08-16 Thread Imre Deak

On Mon, Aug 16, 2021 at 10:17:37AM +0300, Jani Nikula wrote:
> The symbol isn't needed outside of i915.ko.
> 
> Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link 
> training")
> Fixes: 264613b406eb ("drm/i915: Disable LTTPR support when the DPCD rev < 
> 1.4")
> Cc: Imre Deak 
> Signed-off-by: Jani Nikula 

Thanks for catching it, not sure why I thought the export is needed :/
Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/display/intel_dp_link_training.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c 
> b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index 08bceae40aa8..053a3c2f7267 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -206,7 +206,6 @@ int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp 
> *intel_dp)
>  
>   return lttpr_count;
>  }
> -EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);
>  
>  static u8 dp_voltage_max(u8 preemph)
>  {
> -- 
> 2.20.1
>

[Intel-gfx] [PATCH] drm/i915: Ditch the i915_gem_ww_ctx loop member

It's only used by the for_i915_gem_ww() macro and we can use
the (typically) on-stack _err variable in its place.

While initially setting the _err variable to -EDEADLK to enter the
loop, we clear it before actually entering using fetch_and_zero() to
avoid empty loops or code not setting the _err variable running forever.

Suggested-by: Maarten Lankhorst 
Signed-off-by: Thomas Hellström 
---
 drivers/gpu/drm/i915/i915_gem_ww.h | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_ww.h 
b/drivers/gpu/drm/i915/i915_gem_ww.h
index f6b1a796667b..98348b1e6182 100644
--- a/drivers/gpu/drm/i915/i915_gem_ww.h
+++ b/drivers/gpu/drm/i915/i915_gem_ww.h
@@ -7,12 +7,13 @@
 
 #include 
 
+#include "i915_utils.h"
+
 struct i915_gem_ww_ctx {
struct ww_acquire_ctx ctx;
struct list_head obj_list;
struct drm_i915_gem_object *contended;
-   unsigned short intr;
-   unsigned short loop;
+   bool intr;
 };
 
 void i915_gem_ww_ctx_init(struct i915_gem_ww_ctx *ctx, bool intr);
@@ -23,28 +24,20 @@ void i915_gem_ww_unlock_single(struct drm_i915_gem_object 
*obj);
 /* Internal functions used by the inlines! Don't use. */
 static inline int __i915_gem_ww_fini(struct i915_gem_ww_ctx *ww, int err)
 {
-   ww->loop = 0;
if (err == -EDEADLK) {
err = i915_gem_ww_ctx_backoff(ww);
if (!err)
-   ww->loop = 1;
+   err = -EDEADLK;
}
 
-   if (!ww->loop)
+   if (err != -EDEADLK)
i915_gem_ww_ctx_fini(ww);
 
return err;
 }
 
-static inline void
-__i915_gem_ww_init(struct i915_gem_ww_ctx *ww, bool intr)
-{
-   i915_gem_ww_ctx_init(ww, intr);
-   ww->loop = 1;
-}
-
-#define for_i915_gem_ww(_ww, _err, _intr)  \
-   for (__i915_gem_ww_init(_ww, _intr); (_ww)->loop;   \
+#define for_i915_gem_ww(_ww, _err, _intr)\
+   for (i915_gem_ww_ctx_init(_ww, _intr), (_err) = -EDEADLK; \
+fetch_and_zero(&_err) == -EDEADLK;   \
 _err = __i915_gem_ww_fini(_ww, _err))
-
 #endif
-- 
2.31.1

[Intel-gfx] [PATCH] drm/i915/dp: remove superfluous EXPORT_SYMBOL()

The symbol isn't needed outside of i915.ko.

Fixes: b30edfd8d0b4 ("drm/i915: Switch to LTTPR non-transparent mode link 
training")
Fixes: 264613b406eb ("drm/i915: Disable LTTPR support when the DPCD rev < 1.4")
Cc: Imre Deak 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/display/intel_dp_link_training.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c 
b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index 08bceae40aa8..053a3c2f7267 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -206,7 +206,6 @@ int intel_dp_init_lttpr_and_dprx_caps(struct intel_dp 
*intel_dp)
 
return lttpr_count;
 }
-EXPORT_SYMBOL(intel_dp_init_lttpr_and_dprx_caps);
 
 static u8 dp_voltage_max(u8 preemph)
 {
-- 
2.20.1

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/gt: Initialize unused MOCS entries to L3_WB

== Series Details ==

Series: drm/i915/gt: Initialize unused MOCS entries to L3_WB
URL   : https://patchwork.freedesktop.org/series/93706/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10485_full -> Patchwork_20827_full


Summary
---

  **SUCCESS**

  No regressions found.

  

Possible new issues
---

  Here are the unknown changes that may have been introduced in 
Patchwork_20827_full:

### IGT changes ###

 Suppressed 

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_ctx_isolation@preservation-s3@vecs0:
- {shard-rkl}:[PASS][1] -> [DMESG-WARN][2]
   [1]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-rkl-5/igt@gem_ctx_isolation@preservation...@vecs0.html
   [2]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-rkl-1/igt@gem_ctx_isolation@preservation...@vecs0.html

  * igt@runner@aborted:
- {shard-rkl}:([FAIL][3], [FAIL][4], [FAIL][5], [FAIL][6], 
[FAIL][7]) ([i915#1602] / [i915#3002] / [i915#3810] / [i915#3811]) -> 
([FAIL][8], [FAIL][9], [FAIL][10], [FAIL][11], [FAIL][12], [FAIL][13]) 
([i915#1602] / [i915#2029] / [i915#3002] / [i915#3810] / [i915#3811])
   [3]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-rkl-1/igt@run...@aborted.html
   [4]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-rkl-2/igt@run...@aborted.html
   [5]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-rkl-2/igt@run...@aborted.html
   [6]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-rkl-5/igt@run...@aborted.html
   [7]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-rkl-5/igt@run...@aborted.html
   [8]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-rkl-1/igt@run...@aborted.html
   [9]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-rkl-1/igt@run...@aborted.html
   [10]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-rkl-2/igt@run...@aborted.html
   [11]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-rkl-2/igt@run...@aborted.html
   [12]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-rkl-1/igt@run...@aborted.html
   [13]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-rkl-5/igt@run...@aborted.html

  
Known issues


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

### IGT changes ###

 Issues hit 

  * igt@gem_create@create-massive:
- shard-snb:  NOTRUN -> [DMESG-WARN][14] ([i915#3002])
   [14]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-snb7/igt@gem_cre...@create-massive.html

  * igt@gem_ctx_persistence@legacy-engines-mixed:
- shard-snb:  NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#1099]) +4 
similar issues
   [15]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-snb2/igt@gem_ctx_persiste...@legacy-engines-mixed.html

  * igt@gem_exec_fair@basic-none@vcs0:
- shard-kbl:  [PASS][16] -> [FAIL][17] ([i915#2842]) +1 similar 
issue
   [16]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-kbl6/igt@gem_exec_fair@basic-n...@vcs0.html
   [17]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-kbl2/igt@gem_exec_fair@basic-n...@vcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
- shard-glk:  [PASS][18] -> [FAIL][19] ([i915#2842])
   [18]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-glk9/igt@gem_exec_fair@basic-throt...@rcs0.html
   [19]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-glk6/igt@gem_exec_fair@basic-throt...@rcs0.html
- shard-iclb: [PASS][20] -> [FAIL][21] ([i915#2849])
   [20]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-iclb1/igt@gem_exec_fair@basic-throt...@rcs0.html
   [21]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-iclb5/igt@gem_exec_fair@basic-throt...@rcs0.html

  * igt@gem_exec_schedule@independent@vecs0:
- shard-iclb: [PASS][22] -> [FAIL][23] ([i915#3795])
   [22]: 
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10485/shard-iclb8/igt@gem_exec_schedule@independ...@vecs0.html
   [23]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-iclb8/igt@gem_exec_schedule@independ...@vecs0.html

  * igt@gem_render_copy@yf-tiled-mc-ccs-to-vebox-yf-tiled:
- shard-iclb: NOTRUN -> [SKIP][24] ([i915#768]) +1 similar issue
   [24]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-iclb7/igt@gem_render_c...@yf-tiled-mc-ccs-to-vebox-yf-tiled.html

  * igt@gem_userptr_blits@dmabuf-sync:
- shard-apl:  NOTRUN -> [SKIP][25] ([fdo#109271] / [i915#3323])
   [25]: 
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20827/shard-apl7/igt@gem_userptr_bl...@dmabuf-sync.html

  * igt@gem_userptr_blits@input-checking:
- shard-apl:

Re: [Intel-gfx] [PATCH v2 1/6] drm/i915: Export intel_hdmi_compute_avi_infoframe()