Re: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA

[Jani Nikula]

On Thu, 12 Jan 2017, Mika Kahola  wrote:
> This is definitely needed to pass igt test on bxt
>
> 'gem_exec_suspend --run-subtest basic-S3'
>
> Tested-by: Mika Kahola 
>
> On Mon, 2017-01-09 at 14:46 +0530, Vidya Srinivas wrote:
>> From: Uma Shankar 
>> 
>> Enable MIPI IO WA for BXT DSI as per bspec.
>> 
>> Signed-off-by: Uma Shankar 
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h  | 3 +++
>>  drivers/gpu/drm/i915/intel_dsi.c | 9 +
>>  2 files changed, 12 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 71b978a..b9d7e98 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -8301,6 +8301,9 @@ enum {
>>  #define _BXT_MIPIC_PORT_CTRL0x6B8C0
>>  #define BXT_MIPI_PORT_CTRL(tc)  _MMIO_MIPI(tc,
>> _BXT_MIPIA_PORT_CTRL, _BXT_MIPIC_PORT_CTRL)
>>  
>> +#define BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR   _MMIO(0
>> x138090)

Observe that this register is already defined as BXT_P_CR_GT_DISP_PWRON,
and already used in intel_dpio_phy.c. It seems to me changing the bits
in this register should be hooked at the dpio level.

Imre?

>> +#define  MIPIO_RST_CTRL (1 <<
>> 2)
>> +
>>  #define  DPI_ENABLE (1 << 31)
>> /* A + C */
>>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT  27
>>  #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK   (0xf << 27)
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
>> b/drivers/gpu/drm/i915/intel_dsi.c
>> index a4bda92..9252490 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -366,6 +366,11 @@ static void bxt_dsi_device_ready(struct
>> intel_encoder *encoder)
>>  
>>  DRM_DEBUG_KMS("\n");
>>  
>> +/* Add MIPI IO reset programming for modeset */
>> +val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR);
>> +I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR,
>> +val | MIPIO_RST_CTRL);
>> +
> Should we move this WA to intel_dsi_pre_enable() as the counterpart of
> this WA is defined intel_dsi_post_disable()?

As I said, this should probably be managed in intel_dpio_phy.c.

And if not, this is BXT specific, and this hunk runs it on everything
else too.

BR,
Jani.


>
>>  /* Enable MIPI PHY transparent latch */
>>  for_each_dsi_port(port, intel_dsi->ports) {
>>  val = I915_READ(BXT_MIPI_PORT_CTRL(port));
>> @@ -757,6 +762,10 @@ static void intel_dsi_post_disable(struct
>> intel_encoder *encoder,
>>  drm_panel_power_off(intel_dsi->panel);
>>  msleep(intel_dsi->panel_off_delay);
>>  
>> +val = I915_READ(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR);
>> +I915_WRITE(BXT_P_CR_GT_DISP_PWRON_0_2_0_GTTMMADR,
>> +val & ~MIPIO_RST_CTRL);
>> +
>>  intel_disable_dsi_pll(encoder);
>>  
>>  /* Panel Disable over CRC PMIC */

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 29/37] drm/i915: Fill different pages of the GTT

[Joonas Lahtinen]

On ke, 2017-01-11 at 21:09 +, Chris Wilson wrote:
> Exercise filling different pages of the GTT
> 
> Signed-off-by: Chris Wilson 



> +static int walk_hole(struct drm_i915_private *i915,
> > +    struct i915_address_space *vm,
> > +    u64 hole_start, u64 hole_end)
> +{



> + for (addr = hole_start; addr < hole_end; addr += PAGE_SIZE)
{
> + cond_resched();

Maybe this should be in the previous tests too that fill the GTT (one
would imagine it to take a lot longer than just walking with single
vma).

> +static int igt_ppgtt_walk(void *arg)
> +{



> +
> + i915_ppgtt_close(>base);
> + i915_ppgtt_put(ppgtt);
> +err_unlock:

traditionally called out_unlock; Or being sole label, just out:

> + mutex_unlock(_priv->drm.struct_mutex);
> +
> + fake_file_free(dev_priv, file);
> + return err;
> +}
> +



> +static int igt_ggtt_walk(void *arg)
> +{
> + struct drm_i915_private *i915 = arg;
> + struct i915_ggtt *ggtt = >ggtt;
> + u64 hole_start = U64_MAX, hole_end = 0, hole_size = 0;

I think I missed this in the ppgtt, but; single assignment per line as
per CodingStyle. This is not obfuscation contest.

> + u64 this_start, this_end;
> + struct drm_mm_node *node;
> + int err;
> +
> + GEM_BUG_ON(ggtt->base.total & ~PAGE_MASK);

I think single instance of this somewhere early should be enough?

> +
> + mutex_lock(>drm.struct_mutex);
> + drm_mm_for_each_hole(node, >base.mm, this_start, this_end) {
> + u64 this_size;
> +
> + if (ggtt->base.mm.color_adjust)
> + ggtt->base. mm.color_adjust(node, 0,
> + _start, _end);
> +
> + this_size = this_end - this_start;
> + if (this_size > hole_size) {
> + hole_size = this_size;
> + hole_start = this_start;
> + hole_end = this_end;
> + }
> + }
> + pr_info("Found GGTT hole [%llx, %llx], size %llx\n",
> + hole_start, hole_end, hole_size);
> + GEM_BUG_ON(hole_start >= hole_end);
> +

Why not just walk all the holes big enough to accommodate GTT_PAGE_SIZE
for better coverage? But for just one hole, with above;

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()

[Joonas Lahtinen]

On to, 2017-01-12 at 16:45 +, Chris Wilson wrote:
> Parameter: good.
> Parameter - bad.
> 
> One day I'll learn the syntax.
> 
> Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a 
> helper")
> Fixes: e007b19d7ba7 ("drm/i915: Use the MRU stack search after evicting")
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Daniel Vetter 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] DP compliance failure due to dithering for 18bpp video pattern

[Jani Nikula]

On Fri, 13 Jan 2017, Manasi Navare  wrote:
> On Thu, Jan 12, 2017 at 03:41:07PM -0800, Rodrigo Vivi wrote:
>> On Thu, Jan 12, 2017 at 3:30 PM, Manasi Navare
>>  wrote:
>> > On Thu, Jan 12, 2017 at 12:32:09PM +0200, Jani Nikula wrote:
>> >> On Wed, 11 Jan 2017, Ville Syrjälä  wrote:
>> >> > On Wed, Jan 11, 2017 at 05:09:16PM +0200, Jani Nikula wrote:
>> >> >> On Tue, 10 Jan 2017, Manasi Navare  wrote:
>> >> >> > Hi All,
>> >> >> >
>> >> >> > We are seeing CRC check failures in some of the 18bpp video pattern
>> >> >> > DP Compliance tests causing the tests to fail. On further 
>> >> >> > investigation, it is
>> >> >> > rootcaused to dithering that the i915 driver enables in case of 
>> >> >> > 18bpp pipe
>> >> >> > configuration that messes up the CRC and causes the test to fail.
>> >> >>
>> >> >> The CTS spec actually accounts for CRC failures caused by dithering and
>> >> >> color space conversions. See section 3.2.1. However, it would be
>> >> >> preferrable to be able to automate this.
>> >> >>
>> >> >> > Some of the approaches that can solve this problem are:
>> >> >> > 1.  Add a new method in intel_dp.c to request the compliance test 
>> >> >> > state.
>> >> >> > Call this new method in intel_display.c to not enable dithering 
>> >> >> > during a
>> >> >> > compliance test. Issue with this is it makes the general portion of 
>> >> >> > the driver
>> >> >> > compliance aware.
>> >> >> >
>> >> >> > 2.  Move the dithering enable to compute_config methods in all 
>> >> >> > encoder source
>> >> >> > files. Issue: Lot of duplicate code and DP is the only encoder that 
>> >> >> > uses 18bpc.
>> >> >> >
>> >> >> > 3.  Disable dithering at all times in the driver. However this can 
>> >> >> > cause image
>> >> >> > quality issue with 8bpc plane and 6 bit pipe.
>> >> >> >
>> >> >> > Any suggestions on which approach can be implemented in order to pass
>> >> >> > compliance?
>> >> >>
>> >> >> I can't find any mention in the specs that we couldn't enable/disable
>> >> >> dithering on the fly. It's PIPE_MISC for BDW+ and PIPE_CONF for the
>> >> >> rest. So I'm wondering about doing...
>> >> >>
>> >> >> 4. Disable dithering at intel_dp_sink_crc_start() and enable it again
>> >> >>(according to config->dither) at intel_dp_sink_crc_stop(). It's
>> >> >>similar to the hsw_disable_ips() and hsw_enable_ips() calls, but
>> >> >>would have to cover more platforms.
>> >> >>
>> >
>> > intel_dp_sink_crc_start() gets called only through the debugfs interface.
>> 
>> Do you really use this sink crc for the compliance tests?
>> Or are you talking about some other CRC level check like in the compliance 
>> box?
>>
>
> Exactly, DPR 120 does a CRC check on the pixel values during the 18bpp video 
> pattern
> tests. So we need to disable dithering before that modeset or during 
> intel_modeset_pipe_config()
> where it sets pipe_config->dither to true.
> So I am not sure I understand how disabling the dithering at 
> intel_dp_sink_crc_start() would help.

Oh. I thought the test involved reading the DPCD for the CRC. I didn't
realize the test sink not only calculates the CRC for the source to read
from DPCD, but also makes a point of checking it.

If so, we do need to disable dithering at modeset.

Again, this is not a test failure according to the CTS (there's a
mention of a visual inspection) but automation FTW.

BR,
Jani.


>
> Manasi 
>> > If we disable dithering here, DPR 120 which is calculating the CRC of the
>> > pixels on the rendered video pattern would already have dithering enabled 
>> > on it.
>> > So how will this solve our issue of CRC failures in case of 18bpc test?
>> >
>> > Manasi
>> >
>> >
>> >> >> Ville, thoughts on changing dithering on the fly?
>> >> >
>> >> > Should be fine I think.
>> >> >
>> >> > BTW see
>> >> > https://lists.freedesktop.org/archives/intel-gfx/2016-December/115186.html
>> >> > if you intend to add more crc workaround type of things. There I'm
>> >> > changing the IPS w/a to force a full modeset because it was the easiest
>> >> > way to do things, and the current thing is just broken.
>> >>
>> >> I think forcing a modeset for sink crc would be rather annoying.
>> >>
>> >> BR,
>> >> Jani.
>> >>
>> >>
>> >> --
>> >> Jani Nikula, Intel Open Source Technology Center
>> > ___
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> 
>> 
>> -- 
>> Rodrigo Vivi
>> Blog: http://blog.vivi.eng.br

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] dim: Triple-check and tripe warning when merging patches that touch files outside i915.

[Jani Nikula]

On Fri, 13 Jan 2017, Rodrigo Vivi  wrote:
> Most of commiters already knows that by heart and also dim already
> have this warning. So maybe this is just Bart writing to blackboard. Duh!
>
> Anyway for my own usage this tripe check will help for sure.
> Maybe it help someone else in the future.

Well, this has changed with drm-misc; very few of the commits there have
acks from Dave. And that was sort of the point in that tree, to scale
better and not depend on his acks on every little detail.

I suppose the checks in dim could be branch specific.

BR,
Jani.


>
> Cc: Daniel Vetter 
> Cc: Jani Nikula 
> Signed-off-by: Rodrigo Vivi 
> ---
>  dim   | 4 
>  drm-intel.rst | 4 +++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/dim b/dim
> index eec5e43..73f6212 100755
> --- a/dim
> +++ b/dim
> @@ -933,6 +933,10 @@ function checkpatch_commit
>   echo -e "The following files are outside of i915 
> maintenance scope:\n"
>   echo "$non_i915_files"
>   echo -e "\nConfirm you have appropriate Acked-by and 
> Reviewed-by for above files."
> + local acked_by_airlie=$($cmd | grep "Acked-by: Dave 
> Airlie ")
> + if [ "$acked_by_airlie" == "" ]; then
> + echo -e "\nRemind that for drm you should have an 
> Acked-by from Dave Airlie "
> + fi
>   fi
>   fi
>  }
> diff --git a/drm-intel.rst b/drm-intel.rst
> index 79db1cf..583a687 100644
> --- a/drm-intel.rst
> +++ b/drm-intel.rst
> @@ -300,7 +300,9 @@ the right decisions, and for others to set their the 
> expectations right.
>  
>  The short list:
>  
> -* Only push patches changing `drivers/gpu/drm/i915`.
> +* Only push patches changing `drivers/gpu/drm/i915`. If dim mentioned that 
> you
> +  are touching files outside i915 stop and make sure that you have the proper
> +  Acked-by. Specially for drm files an Acked-by from Dave Airlie is required.
>  
>  * Only push patches to `drm-intel-next-queued` branch.

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v5 0/3] support DP MST audio

[Yang, Libin]

>-Original Message-
>From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
>Sent: Thursday, January 12, 2017 5:10 PM
>To: Jani Nikula 
>Cc: Yang, Libin ; intel-gfx@lists.freedesktop.org;
>ville.syrj...@linux.intel.com; Vetter, Daniel ;
>Pandiyan, Dhinakaran ; ti...@suse.de
>Subject: Re: [Intel-gfx] [PATCH v5 0/3] support DP MST audio
>
>On Thu, Jan 12, 2017 at 10:06:10AM +0100, Daniel Vetter wrote:
>> On Thu, Jan 12, 2017 at 10:53:35AM +0200, Jani Nikula wrote:
>> > On Thu, 12 Jan 2017, "Yang, Libin"  wrote:
>> > >>-Original Message-
>> > >>From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>> > >>Sent: Thursday, January 12, 2017 4:19 PM
>> > >>To: Yang, Libin ;
>> > >>intel-gfx@lists.freedesktop.org; ville.syrj...@linux.intel.com;
>> > >>Vetter, Daniel ; Pandiyan, Dhinakaran
>> > >>; ti...@suse.de
>> > >>Cc: Yang, Libin 
>> > >>Subject: Re: [PATCH v5 0/3] support DP MST audio
>> > >>
>> > >>On Thu, 12 Jan 2017, libin.y...@intel.com wrote:
>> > >>> From: Libin Yang 
>> > >>>
>> > >>> This patchset starts to support DP MST audio.
>> > >>
>> > >>Given that this is only sent to intel-gfx, I *presume* this is now
>> > >>intended to be merged via drm-intel. Is that right?
>> > >>
>> > >>Please don't assume your poor maintainers remember everything!
>> > >
>> > > These patches are for drm-tip.
>> > >
>> > > Which mail list I should send? I found the mail addresses are:
>> > > intel-gfx@lists.freedesktop.org and 
>> > > drm-intel-fi...@lists.freedesktop.org.
>> > > from
>> > > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.h
>> > > tml
>> >
>> > What I'm trying and apparently failing to say is that regardless of
>> > the tree and the mailing list, the patches only touch stuff under
>> > sound/, so it would be great to have a hint that these are really
>> > meant to be merged via us.
>>
>> And imo still good to cc: takashi and sound lists so they know I've
>> picked them up. Now I need to dig out that other thread (which is
>> somewhere) so I can confirm to Takashi that it has landed.
>
>Oops, Takashi _is_ already on cc, but ccing alsa wouldn't have hurt.
>Sorry about that, too early in the morning still ...
>
>Anyway, patches applied to drm-intel for 4.11.

Thanks. Yes, it is a good suggestion to cc ALSA. And as Jani said
it is better to describe it is for gfx.

Regards,
Libin

>
>Thanks, Daniel
>--
>Daniel Vetter
>Software Engineer, Intel Corporation
>http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/10] drm/i915/psr: enable ALPM for psr2

[Rodrigo Vivi]

patches 3, 4 and 5 merged to dinq.
Thanks for patches and reviews.

On Thu, Jan 5, 2017 at 12:38 PM, Jim Bride  wrote:
> On Mon, Jan 02, 2017 at 05:00:58PM +0530, vathsala nagaraju wrote:
>> As per edp1.4 spec , alpm is required for psr2 operation as it's
>> used for all psr2  main link power down management and alpm enable
>> bit must be set for psr2 operation.
>>
>> Cc: Rodrigo Vivi 
>> Cc: Jim Bride 
>> Signed-off-by: vathsala nagaraju 
>> Signed-off-by: Patil Deepti 
>
> Reviewed-by: Jim Bride 
>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>>  drivers/gpu/drm/i915/intel_dp.c  | 10 ++
>>  drivers/gpu/drm/i915/intel_psr.c |  6 +-
>>  3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 36dc835..0742b81 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1166,6 +1166,7 @@ struct i915_psr {
>>   bool link_standby;
>>   bool y_cord_support;
>>   bool colorimetry_support;
>> + bool alpm;
>>  };
>>
>>  enum intel_pch {
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index da577c9..9b313a3 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -3060,6 +3060,14 @@ static bool intel_dp_get_colorimetry_status(struct 
>> intel_dp *intel_dp)
>>   return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
>>  }
>>
>> +bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
>> +{
>> + uint8_t alpm_caps = 0;
>> +
>> + drm_dp_dpcd_readb(_dp->aux, DP_RECEIVER_ALPM_CAP, _caps);
>> + return alpm_caps & DP_ALPM_CAP;
>> +}
>> +
>>  /* These are source-specific values. */
>>  uint8_t
>>  intel_dp_voltage_max(struct intel_dp *intel_dp)
>> @@ -3644,6 +3652,8 @@ void intel_dp_set_idle_link_train(struct intel_dp 
>> *intel_dp)
>>   intel_dp_get_y_cord_status(intel_dp);
>>   dev_priv->psr.colorimetry_support =
>>   intel_dp_get_colorimetry_status(intel_dp);
>> + dev_priv->psr.alpm =
>> + intel_dp_get_alpm_status(intel_dp);
>>   }
>>
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
>> b/drivers/gpu/drm/i915/intel_psr.c
>> index 93eb0f0..494e4b2 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -209,7 +209,11 @@ static void hsw_psr_enable_sink(struct intel_dp 
>> *intel_dp)
>>   drm_dp_dpcd_writeb(_dp->aux,
>>   DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
>>   DP_AUX_FRAME_SYNC_ENABLE);
>> -
>> + /* Enable ALPM at sink for psr2 */
>> + if (dev_priv->psr.psr2_support && dev_priv->psr.alpm)
>> + drm_dp_dpcd_writeb(_dp->aux,
>> + DP_RECEIVER_ALPM_CONFIG,
>> + DP_ALPM_ENABLE);
>>   if (dev_priv->psr.link_standby)
>>   drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG,
>>  DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>> --
>> 1.9.1
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 07/10] drm/i915/psr: set PSR_MASK bits for deep sleep

[vathsala nagaraju]

Program EDP_PSR_DEBUG_CTL (PSR_MASK) to enable system
to go to deep sleep while in psr2.PSR2_STATUS bit 31:28
should report value 8 , if system enters deep sleep state.

Also, EDP_FRAMES_BEFORE_SU_ENTRY is set 1 , if not set,
flickering is observed on psr2 panel.

v2: (Ilia Mirkin)
- Remove duplicate bit definition 25:27

v3: rebase

v4: rebase

v5: rebase

Cc: Rodrigo Vivi 
Cc: Jim Bride 
Signed-off-by: Vathsala Nagaraju 
Signed-off-by: Patil Deepti 
Reviewed-by: Jim Bride 
---
 drivers/gpu/drm/i915/i915_reg.h  | 10 +++---
 drivers/gpu/drm/i915/intel_psr.c | 30 --
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c9c1ccd..ca76887 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3597,9 +3597,12 @@ enum {
 #define   EDP_PSR_PERF_CNT_MASK0xff
 
 #define EDP_PSR_DEBUG_CTL  _MMIO(dev_priv->psr_mmio_base + 0x60)
-#define   EDP_PSR_DEBUG_MASK_LPSP  (1<<27)
-#define   EDP_PSR_DEBUG_MASK_MEMUP (1<<26)
-#define   EDP_PSR_DEBUG_MASK_HPD   (1<<25)
+#define   EDP_PSR_DEBUG_MASK_MAX_SLEEP (1<<28)
+#define   EDP_PSR_DEBUG_MASK_LPSP  (1<<27)
+#define   EDP_PSR_DEBUG_MASK_MEMUP (1<<26)
+#define   EDP_PSR_DEBUG_MASK_HPD   (1<<25)
+#define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE(1<<16)
+#define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1<<15)
 
 #define EDP_PSR2_CTL   _MMIO(0x6f900)
 #define   EDP_PSR2_ENABLE  (1<<31)
@@ -3614,6 +3617,7 @@ enum {
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK(0xf<<4)
 #define   EDP_PSR2_IDLE_MASK   0xf
+#define   EDP_FRAMES_BEFORE_SU_ENTRY   (1<<4)
 
 #define EDP_PSR2_STATUS_CTL_MMIO(0x6f940)
 #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 935402e..3611c42 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -338,7 +338,9 @@ static void intel_enable_source_psr2(struct intel_dp 
*intel_dp)
/* FIXME: selective update is probably totally broken because it doesn't
 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 * good enough. */
-   val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
+   val |= EDP_PSR2_ENABLE |
+   EDP_SU_TRACK_ENABLE |
+   EDP_FRAMES_BEFORE_SU_ENTRY;
 
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
val |= EDP_PSR2_TP2_TIME_2500;
@@ -512,20 +514,28 @@ void intel_psr_enable(struct intel_dp *intel_dp)
if (dev_priv->psr.y_cord_support)
chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
+   I915_WRITE(EDP_PSR_DEBUG_CTL,
+  EDP_PSR_DEBUG_MASK_MEMUP |
+  EDP_PSR_DEBUG_MASK_HPD |
+  EDP_PSR_DEBUG_MASK_LPSP |
+  EDP_PSR_DEBUG_MASK_MAX_SLEEP |
+  EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
} else {
/* set up vsc header for psr1 */
hsw_psr_setup_vsc(intel_dp);
+   /*
+* Per Spec: Avoid continuous PSR exit by masking MEMUP
+* and HPD. also mask LPSP to avoid dependency on other
+* drivers that might block runtime_pm besides
+* preventing  other hw tracking issues now we can rely
+* on frontbuffer tracking.
+*/
+   I915_WRITE(EDP_PSR_DEBUG_CTL,
+  EDP_PSR_DEBUG_MASK_MEMUP |
+  EDP_PSR_DEBUG_MASK_HPD |
+  EDP_PSR_DEBUG_MASK_LPSP);
}
 
-   /*
-* Per Spec: Avoid continuous PSR exit by masking MEMUP and HPD.
-* Also mask LPSP to avoid dependency on other drivers that
-* might block runtime_pm besides preventing other hw tracking
-* issues now we can rely on frontbuffer tracking.
-*/
-   I915_WRITE(EDP_PSR_DEBUG_CTL, EDP_PSR_DEBUG_MASK_MEMUP |
-  EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
-
/* Enable PSR on the panel */
hsw_psr_enable_sink(intel_dp);
 
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org

Re: [Intel-gfx] DP compliance failure due to dithering for 18bpp video pattern

[Manasi Navare]

On Thu, Jan 12, 2017 at 03:41:07PM -0800, Rodrigo Vivi wrote:
> On Thu, Jan 12, 2017 at 3:30 PM, Manasi Navare
>  wrote:
> > On Thu, Jan 12, 2017 at 12:32:09PM +0200, Jani Nikula wrote:
> >> On Wed, 11 Jan 2017, Ville Syrjälä  wrote:
> >> > On Wed, Jan 11, 2017 at 05:09:16PM +0200, Jani Nikula wrote:
> >> >> On Tue, 10 Jan 2017, Manasi Navare  wrote:
> >> >> > Hi All,
> >> >> >
> >> >> > We are seeing CRC check failures in some of the 18bpp video pattern
> >> >> > DP Compliance tests causing the tests to fail. On further 
> >> >> > investigation, it is
> >> >> > rootcaused to dithering that the i915 driver enables in case of 18bpp 
> >> >> > pipe
> >> >> > configuration that messes up the CRC and causes the test to fail.
> >> >>
> >> >> The CTS spec actually accounts for CRC failures caused by dithering and
> >> >> color space conversions. See section 3.2.1. However, it would be
> >> >> preferrable to be able to automate this.
> >> >>
> >> >> > Some of the approaches that can solve this problem are:
> >> >> > 1.  Add a new method in intel_dp.c to request the compliance test 
> >> >> > state.
> >> >> > Call this new method in intel_display.c to not enable dithering 
> >> >> > during a
> >> >> > compliance test. Issue with this is it makes the general portion of 
> >> >> > the driver
> >> >> > compliance aware.
> >> >> >
> >> >> > 2.  Move the dithering enable to compute_config methods in all 
> >> >> > encoder source
> >> >> > files. Issue: Lot of duplicate code and DP is the only encoder that 
> >> >> > uses 18bpc.
> >> >> >
> >> >> > 3.  Disable dithering at all times in the driver. However this can 
> >> >> > cause image
> >> >> > quality issue with 8bpc plane and 6 bit pipe.
> >> >> >
> >> >> > Any suggestions on which approach can be implemented in order to pass
> >> >> > compliance?
> >> >>
> >> >> I can't find any mention in the specs that we couldn't enable/disable
> >> >> dithering on the fly. It's PIPE_MISC for BDW+ and PIPE_CONF for the
> >> >> rest. So I'm wondering about doing...
> >> >>
> >> >> 4. Disable dithering at intel_dp_sink_crc_start() and enable it again
> >> >>(according to config->dither) at intel_dp_sink_crc_stop(). It's
> >> >>similar to the hsw_disable_ips() and hsw_enable_ips() calls, but
> >> >>would have to cover more platforms.
> >> >>
> >
> > intel_dp_sink_crc_start() gets called only through the debugfs interface.
> 
> Do you really use this sink crc for the compliance tests?
> Or are you talking about some other CRC level check like in the compliance 
> box?
>

Exactly, DPR 120 does a CRC check on the pixel values during the 18bpp video 
pattern
tests. So we need to disable dithering before that modeset or during 
intel_modeset_pipe_config()
where it sets pipe_config->dither to true.
So I am not sure I understand how disabling the dithering at 
intel_dp_sink_crc_start() would help.

Manasi 
> > If we disable dithering here, DPR 120 which is calculating the CRC of the
> > pixels on the rendered video pattern would already have dithering enabled 
> > on it.
> > So how will this solve our issue of CRC failures in case of 18bpc test?
> >
> > Manasi
> >
> >
> >> >> Ville, thoughts on changing dithering on the fly?
> >> >
> >> > Should be fine I think.
> >> >
> >> > BTW see
> >> > https://lists.freedesktop.org/archives/intel-gfx/2016-December/115186.html
> >> > if you intend to add more crc workaround type of things. There I'm
> >> > changing the IPS w/a to force a full modeset because it was the easiest
> >> > way to do things, and the current thing is just broken.
> >>
> >> I think forcing a modeset for sink crc would be rather annoying.
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] DP compliance failure due to dithering for 18bpp video pattern

[Rodrigo Vivi]

On Thu, Jan 12, 2017 at 3:30 PM, Manasi Navare
 wrote:
> On Thu, Jan 12, 2017 at 12:32:09PM +0200, Jani Nikula wrote:
>> On Wed, 11 Jan 2017, Ville Syrjälä  wrote:
>> > On Wed, Jan 11, 2017 at 05:09:16PM +0200, Jani Nikula wrote:
>> >> On Tue, 10 Jan 2017, Manasi Navare  wrote:
>> >> > Hi All,
>> >> >
>> >> > We are seeing CRC check failures in some of the 18bpp video pattern
>> >> > DP Compliance tests causing the tests to fail. On further 
>> >> > investigation, it is
>> >> > rootcaused to dithering that the i915 driver enables in case of 18bpp 
>> >> > pipe
>> >> > configuration that messes up the CRC and causes the test to fail.
>> >>
>> >> The CTS spec actually accounts for CRC failures caused by dithering and
>> >> color space conversions. See section 3.2.1. However, it would be
>> >> preferrable to be able to automate this.
>> >>
>> >> > Some of the approaches that can solve this problem are:
>> >> > 1.  Add a new method in intel_dp.c to request the compliance test state.
>> >> > Call this new method in intel_display.c to not enable dithering during a
>> >> > compliance test. Issue with this is it makes the general portion of the 
>> >> > driver
>> >> > compliance aware.
>> >> >
>> >> > 2.  Move the dithering enable to compute_config methods in all encoder 
>> >> > source
>> >> > files. Issue: Lot of duplicate code and DP is the only encoder that 
>> >> > uses 18bpc.
>> >> >
>> >> > 3.  Disable dithering at all times in the driver. However this can 
>> >> > cause image
>> >> > quality issue with 8bpc plane and 6 bit pipe.
>> >> >
>> >> > Any suggestions on which approach can be implemented in order to pass
>> >> > compliance?
>> >>
>> >> I can't find any mention in the specs that we couldn't enable/disable
>> >> dithering on the fly. It's PIPE_MISC for BDW+ and PIPE_CONF for the
>> >> rest. So I'm wondering about doing...
>> >>
>> >> 4. Disable dithering at intel_dp_sink_crc_start() and enable it again
>> >>(according to config->dither) at intel_dp_sink_crc_stop(). It's
>> >>similar to the hsw_disable_ips() and hsw_enable_ips() calls, but
>> >>would have to cover more platforms.
>> >>
>
> intel_dp_sink_crc_start() gets called only through the debugfs interface.

Do you really use this sink crc for the compliance tests?
Or are you talking about some other CRC level check like in the compliance box?

> If we disable dithering here, DPR 120 which is calculating the CRC of the
> pixels on the rendered video pattern would already have dithering enabled on 
> it.
> So how will this solve our issue of CRC failures in case of 18bpc test?
>
> Manasi
>
>
>> >> Ville, thoughts on changing dithering on the fly?
>> >
>> > Should be fine I think.
>> >
>> > BTW see
>> > https://lists.freedesktop.org/archives/intel-gfx/2016-December/115186.html
>> > if you intend to add more crc workaround type of things. There I'm
>> > changing the IPS w/a to force a full modeset because it was the easiest
>> > way to do things, and the current thing is just broken.
>>
>> I think forcing a modeset for sink crc would be rather annoying.
>>
>> BR,
>> Jani.
>>
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] DP compliance failure due to dithering for 18bpp video pattern

[Manasi Navare]

On Thu, Jan 12, 2017 at 12:32:09PM +0200, Jani Nikula wrote:
> On Wed, 11 Jan 2017, Ville Syrjälä  wrote:
> > On Wed, Jan 11, 2017 at 05:09:16PM +0200, Jani Nikula wrote:
> >> On Tue, 10 Jan 2017, Manasi Navare  wrote:
> >> > Hi All,
> >> >
> >> > We are seeing CRC check failures in some of the 18bpp video pattern
> >> > DP Compliance tests causing the tests to fail. On further investigation, 
> >> > it is
> >> > rootcaused to dithering that the i915 driver enables in case of 18bpp 
> >> > pipe
> >> > configuration that messes up the CRC and causes the test to fail.
> >> 
> >> The CTS spec actually accounts for CRC failures caused by dithering and
> >> color space conversions. See section 3.2.1. However, it would be
> >> preferrable to be able to automate this.
> >> 
> >> > Some of the approaches that can solve this problem are:
> >> > 1.  Add a new method in intel_dp.c to request the compliance test state.
> >> > Call this new method in intel_display.c to not enable dithering during a
> >> > compliance test. Issue with this is it makes the general portion of the 
> >> > driver
> >> > compliance aware.
> >> >
> >> > 2.  Move the dithering enable to compute_config methods in all encoder 
> >> > source
> >> > files. Issue: Lot of duplicate code and DP is the only encoder that uses 
> >> > 18bpc.
> >> >
> >> > 3.  Disable dithering at all times in the driver. However this can cause 
> >> > image
> >> > quality issue with 8bpc plane and 6 bit pipe.
> >> >
> >> > Any suggestions on which approach can be implemented in order to pass
> >> > compliance?
> >> 
> >> I can't find any mention in the specs that we couldn't enable/disable
> >> dithering on the fly. It's PIPE_MISC for BDW+ and PIPE_CONF for the
> >> rest. So I'm wondering about doing...
> >> 
> >> 4. Disable dithering at intel_dp_sink_crc_start() and enable it again
> >>(according to config->dither) at intel_dp_sink_crc_stop(). It's
> >>similar to the hsw_disable_ips() and hsw_enable_ips() calls, but
> >>would have to cover more platforms.
> >>

intel_dp_sink_crc_start() gets called only through the debugfs interface.
If we disable dithering here, DPR 120 which is calculating the CRC of the
pixels on the rendered video pattern would already have dithering enabled on it.
So how will this solve our issue of CRC failures in case of 18bpc test?

Manasi

 
> >> Ville, thoughts on changing dithering on the fly?
> >
> > Should be fine I think.
> >
> > BTW see 
> > https://lists.freedesktop.org/archives/intel-gfx/2016-December/115186.html
> > if you intend to add more crc workaround type of things. There I'm
> > changing the IPS w/a to force a full modeset because it was the easiest
> > way to do things, and the current thing is just broken.
> 
> I think forcing a modeset for sink crc would be rather annoying.
> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] dim: Triple-check and tripe warning when merging patches that touch files outside i915.

[Rodrigo Vivi]

Most of commiters already knows that by heart and also dim already
have this warning. So maybe this is just Bart writing to blackboard. Duh!

Anyway for my own usage this tripe check will help for sure.
Maybe it help someone else in the future.

Cc: Daniel Vetter 
Cc: Jani Nikula 
Signed-off-by: Rodrigo Vivi 
---
 dim   | 4 
 drm-intel.rst | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/dim b/dim
index eec5e43..73f6212 100755
--- a/dim
+++ b/dim
@@ -933,6 +933,10 @@ function checkpatch_commit
echo -e "The following files are outside of i915 
maintenance scope:\n"
echo "$non_i915_files"
echo -e "\nConfirm you have appropriate Acked-by and 
Reviewed-by for above files."
+   local acked_by_airlie=$($cmd | grep "Acked-by: Dave 
Airlie ")
+   if [ "$acked_by_airlie" == "" ]; then
+   echo -e "\nRemind that for drm you should have an 
Acked-by from Dave Airlie "
+   fi
fi
fi
 }
diff --git a/drm-intel.rst b/drm-intel.rst
index 79db1cf..583a687 100644
--- a/drm-intel.rst
+++ b/drm-intel.rst
@@ -300,7 +300,9 @@ the right decisions, and for others to set their the 
expectations right.
 
 The short list:
 
-* Only push patches changing `drivers/gpu/drm/i915`.
+* Only push patches changing `drivers/gpu/drm/i915`. If dim mentioned that you
+  are touching files outside i915 stop and make sure that you have the proper
+  Acked-by. Specially for drm files an Acked-by from Dave Airlie is required.
 
 * Only push patches to `drm-intel-next-queued` branch.
 
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: reinstate call to trace_i915_vma_bind

[Patchwork]

== Series Details ==

Series: drm/i915: reinstate call to trace_i915_vma_bind
URL   : https://patchwork.freedesktop.org/series/17929/
State : success

== Summary ==

Series 17929v1 drm/i915: reinstate call to trace_i915_vma_bind
https://patchwork.freedesktop.org/api/1.0/series/17929/revisions/1/mbox/


fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

b72db2710635164c7b0495192fbf44e3a17362f6 drm-tip: 2017y-01m-12d-20h-39m-13s UTC 
integration manifest
6c31af4 drm/i915: reinstate call to trace_i915_vma_bind

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3508/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc

[Chris Wilson]

On Thu, Jan 12, 2017 at 04:24:50PM -0600, l...@pengaru.com wrote:
> On Thu, Jan 12, 2017 at 09:17:06PM +, Chris Wilson wrote:
> > On Mon, Jan 09, 2017 at 11:19:32AM +, Chris Wilson wrote:
> > > On a non-llc system, the objects are created with .cache_level =
> > > CACHE_NONE and so the transition to uncached for scanout is a no-op.
> > > However, if the object was never written to, it will still be in the CPU
> > > domain (having been zeroed out by shmemfs). Those cachelines need to be
> > > flushed prior to display.
> > > 
> > > Reported-by: Vito Caputo
> > > Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when 
> > > pinning the scanout")
> > > Signed-off-by: Chris Wilson 
> > > Cc:  # v4.10-rc1+
> > 
> > Ping?
> 
> This patch fixes the problem for me, in case that's what the ping's for.

Partly that, and trying to catch CI + reviewers.
 
> Out of curiosity the bug I reported described here be getting fixed in 4.10?
> https://lists.freedesktop.org/archives/dri-devel/2017-January/128405.html

It was fixed in the tree back in December, bit of a muddle to get that
particular patch into 4.10.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/7] drm/i915: Name the anonymous structs inside i915_ggtt_view

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/7] drm/i915: Name the anonymous structs 
inside i915_ggtt_view
URL   : https://patchwork.freedesktop.org/series/17923/
State : success

== Summary ==

Series 17923v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/17923/revisions/1/mbox/


fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hqtotal:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

b72db2710635164c7b0495192fbf44e3a17362f6 drm-tip: 2017y-01m-12d-20h-39m-13s UTC 
integration manifest
6a1ebef drm/i915: Eliminate superfluous i915_ggtt_view_normal
d2a2989 drm/i915: Eliminate superfluous i915_ggtt_view_rotated
9abf337 drm/i915: Convert i915_ggtt_view to use an anonymous union
3951d61 drm/i915: Stop clearing i915_ggtt_view
f979633 drm/i915: Compact memcmp in i915_vma_compare()
c1ad4d2 drm/i915: Mark the ggtt_view structs as packed
85788a4 drm/i915: Name the anonymous structs inside i915_ggtt_view

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3507/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: reinstate call to trace_i915_vma_bind

[daniele . ceraolospurio]

From: Daniele Ceraolo Spurio 

The call went away in:

commit 3b16525cc4c1a43e9053cfdc414356eea24bdfad
Author: Chris Wilson 
Date:   Thu Aug 4 16:32:25 2016 +0100

drm/i915: Split insertion/binding of an object into the VM

It is useful to have this trace as it pairs nicely with the vma_unbind
one to track vma activity.
Added inside the i915_vma_bind function (was outside before) to keep a
similar placement as trace_i915_vma_unbind.

Cc: Chris Wilson 
Signed-off-by: Daniele Ceraolo Spurio 
---
 drivers/gpu/drm/i915/i915_vma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index b74eeb7..b593748 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -207,6 +207,7 @@ int i915_vma_bind(struct i915_vma *vma, enum 
i915_cache_level cache_level,
return ret;
}
 
+   trace_i915_vma_bind(vma, flags);
ret = vma->vm->bind_vma(vma, cache_level, bind_flags);
if (ret)
return ret;
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm: Add new DRM_IOCTL_MODE_GETPLANE2

[Daniel Stone]

Hi,

On 12 January 2017 at 18:11, Ville Syrjälä
 wrote:
> On Thu, Jan 12, 2017 at 05:50:15PM +, Daniel Stone wrote:
>> struct drm_plane {
>> struct {
>> uint32_t format;
>> uint64_t modifiers[];
>> } formats[];
>> }
>
> Flipping formats[] vs. modifiers[] here would seem like it should make
> this easier with the proposed kernel API. And if the kernel will also
> uarantee that multiple instances of the same modifier must be returned
> contiguously, then it should be even easier.
>
> Oh and flipping formats[] and modifiers[] should also save a quite a
> bit of space since each format takes twice as much space as each
> modifier. But I suppose that might come at a runtime cost if you have
> to look for a specific format in each modifier's format list instead
> of having to look at just the modifier list of a specific format. So
> I suppose not flipping might be better after all, which I guess would
> complicate populating the infromation somewhat.
>
> Anyways, that's all a bit unrelated to the matter at hand, so I'll stop
> now and just state that I don't mind having an explicit offset if
> people really want it.

It would make sense, but then gbm_surface_create_with_modifiers takes
a fixed pixel format and a list of acceptable modifiers (which to me
seems like the right way around as an API), so whenever I was creating
a surface, I'd have to walk through and create a new list, flipped
back the other way.

Cheers,
Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 4/7] drm/i915: Stop clearing i915_ggtt_view

[Chris Wilson]

As we now use a compact memcmp in i915_vma_compare(), we can forgo
clearing the entire view and only set the precise parameters used in
this view.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3bf517e2430a..f034d8d2dd4c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1759,7 +1759,6 @@ compute_partial_view(struct drm_i915_gem_object *obj,
if (i915_gem_object_is_tiled(obj))
chunk = roundup(chunk, tile_row_pages(obj));
 
-   memset(, 0, sizeof(view));
view.type = I915_GGTT_VIEW_PARTIAL;
view.params.partial.offset = rounddown(page_offset, chunk);
view.params.partial.size =
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 6/7] drm/i915: Eliminate superfluous i915_ggtt_view_rotated

[Chris Wilson]

It is only being used to clear a struct and set the type, after which it
is overwritten. Since we no longer check the unset bits of the union,
skipping the clear is permissible.

Signed-off-by: Chris Wilson 
Reviewed-by: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c  | 3 ---
 drivers/gpu/drm/i915/i915_gem_gtt.h  | 1 -
 drivers/gpu/drm/i915/intel_display.c | 5 ++---
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 06cfd6951a5e..46e04676e011 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -106,9 +106,6 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma);
 const struct i915_ggtt_view i915_ggtt_view_normal = {
.type = I915_GGTT_VIEW_NORMAL,
 };
-const struct i915_ggtt_view i915_ggtt_view_rotated = {
-   .type = I915_GGTT_VIEW_ROTATED,
-};
 
 static void gen6_ggtt_invalidate(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 560fd8ddaf2c..2a8b25742d72 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -183,7 +183,6 @@ struct i915_ggtt_view {
 };
 
 extern const struct i915_ggtt_view i915_ggtt_view_normal;
-extern const struct i915_ggtt_view i915_ggtt_view_rotated;
 
 enum i915_cache_level;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f4be20f0198a..f523256ef77c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2137,11 +2137,10 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
const struct drm_framebuffer *fb,
unsigned int rotation)
 {
+   view->type = I915_GGTT_VIEW_NORMAL;
if (drm_rotation_90_or_270(rotation)) {
-   *view = i915_ggtt_view_rotated;
+   view->type = I915_GGTT_VIEW_ROTATED;
view->rotated = to_intel_framebuffer(fb)->rot_info;
-   } else {
-   *view = i915_ggtt_view_normal;
}
 }
 
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 7/7] drm/i915: Eliminate superfluous i915_ggtt_view_normal

[Chris Wilson]

Since commit 058d88c4330f ("drm/i915: Track pinned VMA"), there is only
one user of i915_ggtt_view_normal rodate. Just treat NULL as no special
view in pin_to_display() like everywhere else.

Signed-off-by: Chris Wilson 
Reviewed-by: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/i915_gem.c  | 2 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c  | 4 
 drivers/gpu/drm/i915/i915_gem_gtt.h  | 2 --
 drivers/gpu/drm/i915/intel_overlay.c | 3 +--
 4 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d8622fd23f5d..d4c59b53532e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3436,7 +3436,7 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
 * try to preserve the existing ABI).
 */
vma = ERR_PTR(-ENOSPC);
-   if (view->type == I915_GGTT_VIEW_NORMAL)
+   if (!view || view->type == I915_GGTT_VIEW_NORMAL)
vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment,
   PIN_MAPPABLE | PIN_NONBLOCK);
if (IS_ERR(vma)) {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 46e04676e011..d7f2aa82d810 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -103,10 +103,6 @@
 static int
 i915_get_ggtt_vma_pages(struct i915_vma *vma);
 
-const struct i915_ggtt_view i915_ggtt_view_normal = {
-   .type = I915_GGTT_VIEW_NORMAL,
-};
-
 static void gen6_ggtt_invalidate(struct drm_i915_private *dev_priv)
 {
/* Note that as an uncached mmio write, this should flush the
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 2a8b25742d72..4b351a2d812b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -182,8 +182,6 @@ struct i915_ggtt_view {
};
 };
 
-extern const struct i915_ggtt_view i915_ggtt_view_normal;
-
 enum i915_cache_level;
 
 struct i915_vma;
diff --git a/drivers/gpu/drm/i915/intel_overlay.c 
b/drivers/gpu/drm/i915/intel_overlay.c
index 4473a611c664..0608fad7f593 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -811,8 +811,7 @@ static int intel_overlay_do_put_image(struct intel_overlay 
*overlay,
if (ret != 0)
return ret;
 
-   vma = i915_gem_object_pin_to_display_plane(new_bo, 0,
-  _ggtt_view_normal);
+   vma = i915_gem_object_pin_to_display_plane(new_bo, 0, NULL);
if (IS_ERR(vma))
return PTR_ERR(vma);
 
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 5/7] drm/i915: Convert i915_ggtt_view to use an anonymous union

[Chris Wilson]

Save a lot of characters by making the union anonymous, with the
side-effect of ignoring unset bits when comparing views.

v2: Roll up the memcmps back into one.
v3: And split again as Ville points out we can't trust the compiler.

Signed-off-by: Chris Wilson 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_debugfs.c  | 20 ++--
 drivers/gpu/drm/i915/i915_gem.c  |  8 
 drivers/gpu/drm/i915/i915_gem_gtt.c  |  9 -
 drivers/gpu/drm/i915/i915_gem_gtt.h  |  2 +-
 drivers/gpu/drm/i915/i915_vma.c  |  9 -
 drivers/gpu/drm/i915/i915_vma.h  |  4 +++-
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 7 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index e367f06f5883..da13c4c3aa6b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -167,20 +167,20 @@ describe_obj(struct seq_file *m, struct 
drm_i915_gem_object *obj)
 
case I915_GGTT_VIEW_PARTIAL:
seq_printf(m, ", partial [%08llx+%x]",
-  vma->ggtt_view.params.partial.offset 
<< PAGE_SHIFT,
-  vma->ggtt_view.params.partial.size 
<< PAGE_SHIFT);
+  vma->ggtt_view.partial.offset << 
PAGE_SHIFT,
+  vma->ggtt_view.partial.size << 
PAGE_SHIFT);
break;
 
case I915_GGTT_VIEW_ROTATED:
seq_printf(m, ", rotated [(%ux%u, stride=%u, 
offset=%u), (%ux%u, stride=%u, offset=%u)]",
-  
vma->ggtt_view.params.rotated.plane[0].width,
-  
vma->ggtt_view.params.rotated.plane[0].height,
-  
vma->ggtt_view.params.rotated.plane[0].stride,
-  
vma->ggtt_view.params.rotated.plane[0].offset,
-  
vma->ggtt_view.params.rotated.plane[1].width,
-  
vma->ggtt_view.params.rotated.plane[1].height,
-  
vma->ggtt_view.params.rotated.plane[1].stride,
-  
vma->ggtt_view.params.rotated.plane[1].offset);
+  
vma->ggtt_view.rotated.plane[0].width,
+  
vma->ggtt_view.rotated.plane[0].height,
+  
vma->ggtt_view.rotated.plane[0].stride,
+  
vma->ggtt_view.rotated.plane[0].offset,
+  
vma->ggtt_view.rotated.plane[1].width,
+  
vma->ggtt_view.rotated.plane[1].height,
+  
vma->ggtt_view.rotated.plane[1].stride,
+  
vma->ggtt_view.rotated.plane[1].offset);
break;
 
default:
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f034d8d2dd4c..d8622fd23f5d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1760,10 +1760,10 @@ compute_partial_view(struct drm_i915_gem_object *obj,
chunk = roundup(chunk, tile_row_pages(obj));
 
view.type = I915_GGTT_VIEW_PARTIAL;
-   view.params.partial.offset = rounddown(page_offset, chunk);
-   view.params.partial.size =
+   view.partial.offset = rounddown(page_offset, chunk);
+   view.partial.size =
min_t(unsigned int, chunk,
- (obj->base.size >> PAGE_SHIFT) - 
view.params.partial.offset);
+ (obj->base.size >> PAGE_SHIFT) - view.partial.offset);
 
/* If the partial covers the entire object, just create a normal VMA. */
if (chunk >= obj->base.size >> PAGE_SHIFT)
@@ -1879,7 +1879,7 @@ int i915_gem_fault(struct vm_area_struct *area, struct 
vm_fault *vmf)
 
/* Finally, remap it using the new GTT offset */
ret = remap_io_mapping(area,
-  area->vm_start + 
(vma->ggtt_view.params.partial.offset << PAGE_SHIFT),
+  area->vm_start + (vma->ggtt_view.partial.offset 
<< PAGE_SHIFT),
   (ggtt->mappable_base + vma->node.start) >> 
PAGE_SHIFT,
   min_t(u64, vma->size, area->vm_end - 
area->vm_start),
   >mappable);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6d2ff20ec973..06cfd6951a5e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3490,7 +3490,7 @@ 

[Intel-gfx] [PATCH v2 1/7] drm/i915: Name the anonymous structs inside i915_ggtt_view

[Chris Wilson]

Naming this pair will become useful shortly...

Signed-off-by: Chris Wilson 
Reviewed-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_gem_gtt.h | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 6c40088f8cf4..5dd3755a5a45 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -152,20 +152,22 @@ enum i915_ggtt_view_type {
 };
 
 struct intel_rotation_info {
-   struct {
+   struct intel_rotation_plane_info {
/* tiles */
unsigned int width, height, stride, offset;
} plane[2];
 };
 
+struct intel_partial_info {
+   u64 offset;
+   unsigned int size;
+};
+
 struct i915_ggtt_view {
enum i915_ggtt_view_type type;
 
union {
-   struct {
-   u64 offset;
-   unsigned int size;
-   } partial;
+   struct intel_partial_info partial;
struct intel_rotation_info rotated;
} params;
 };
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] Anonymouse ggtt_view params

[Chris Wilson]

No one else liked packing the partial parameters into a single u64, so we
are back to a full 32bit for the partial size, and using __packed and a
bunch of asserts to ensure we have no unused bits inside each struct of
the union.

Hopefully that's the last contentios point!
-Chris

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 3/7] drm/i915: Compact memcmp in i915_vma_compare()

[Chris Wilson]

In preparation for the next patch to convert to using an anonymous union
and leaving the excess bytes in the union uninitialised, we first need
to make sure we do not compare using those uninitialised bytes. We also
want to preserve the compactness of the code, avoiding a second call to
memcmp or introducing a switch, so we take advantage of using the type
as an encoded size (as well as a unique identifier for each type of view).

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem_gtt.h | 14 +++---
 drivers/gpu/drm/i915/i915_vma.h | 15 +--
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 57cbd532dae1..20c03c30ce4e 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -145,12 +145,6 @@ typedef uint64_t gen8_ppgtt_pml4e_t;
 
 struct sg_table;
 
-enum i915_ggtt_view_type {
-   I915_GGTT_VIEW_NORMAL = 0,
-   I915_GGTT_VIEW_ROTATED,
-   I915_GGTT_VIEW_PARTIAL,
-};
-
 struct intel_rotation_info {
struct intel_rotation_plane_info {
/* tiles */
@@ -173,10 +167,16 @@ static inline void 
assert_intel_partial_info_is_packed(void)
BUILD_BUG_ON(sizeof(struct intel_partial_info) != sizeof(u64) + 
sizeof(unsigned int));
 }
 
+enum i915_ggtt_view_type {
+   I915_GGTT_VIEW_NORMAL = 0,
+   I915_GGTT_VIEW_ROTATED = sizeof(struct intel_rotation_info),
+   I915_GGTT_VIEW_PARTIAL = sizeof(struct intel_partial_info),
+};
+
 struct i915_ggtt_view {
enum i915_ggtt_view_type type;
-
union {
+   /* Members need to contain no holes/padding */
struct intel_partial_info partial;
struct intel_rotation_info rotated;
} params;
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index a969bbb65871..19f049cef9e3 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -199,15 +199,18 @@ i915_vma_compare(struct i915_vma *vma,
if (cmp)
return cmp;
 
+   BUILD_BUG_ON(I915_GGTT_VIEW_NORMAL != 0);
+   cmp = vma->ggtt_view.type;
if (!view)
-   return vma->ggtt_view.type;
+   return cmp;
+
+   cmp -= view->type;
+   if (cmp)
+   return cmp;
 
-   if (vma->ggtt_view.type != view->type)
-   return vma->ggtt_view.type - view->type;
+   BUILD_BUG_ON(I915_GGTT_VIEW_PARTIAL == I915_GGTT_VIEW_ROTATED);
 
-   return memcmp(>ggtt_view.params,
- >params,
- sizeof(view->params));
+   return memcmp(>ggtt_view.params, >params, view->type);
 }
 
 int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 2/7] drm/i915: Mark the ggtt_view structs as packed

[Chris Wilson]

In the next few patches, we will depend upon there being no
uninitialised bits inside the ggtt_view. To ensure this we add the
__packed attribute and double check with a build on that gcc hasn't
expanded the struct to include some padding bytes.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem_gtt.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 5dd3755a5a45..57cbd532dae1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -156,12 +156,22 @@ struct intel_rotation_info {
/* tiles */
unsigned int width, height, stride, offset;
} plane[2];
-};
+} __packed;
+
+static inline void assert_intel_rotation_info_is_packed(void)
+{
+   BUILD_BUG_ON(sizeof(struct intel_rotation_info) != 8*sizeof(unsigned 
int));
+}
 
 struct intel_partial_info {
u64 offset;
unsigned int size;
-};
+} __packed;
+
+static inline void assert_intel_partial_info_is_packed(void)
+{
+   BUILD_BUG_ON(sizeof(struct intel_partial_info) != sizeof(u64) + 
sizeof(unsigned int));
+}
 
 struct i915_ggtt_view {
enum i915_ggtt_view_type type;
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Flush untouched framebuffers before display on !llc

[Chris Wilson]

On Mon, Jan 09, 2017 at 11:19:32AM +, Chris Wilson wrote:
> On a non-llc system, the objects are created with .cache_level =
> CACHE_NONE and so the transition to uncached for scanout is a no-op.
> However, if the object was never written to, it will still be in the CPU
> domain (having been zeroed out by shmemfs). Those cachelines need to be
> flushed prior to display.
> 
> Reported-by: Vito Caputo
> Fixes: a6a7cc4b7db6 ("drm/i915: Always flush the dirty CPU cache when pinning 
> the scanout")
> Signed-off-by: Chris Wilson 
> Cc:  # v4.10-rc1+

Ping?

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 76689b59fc90..bdb113ef8cfe 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3514,7 +3514,7 @@ i915_gem_object_pin_to_display_plane(struct 
> drm_i915_gem_object *obj,
>   vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
>  
>   /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
> - if (obj->cache_dirty) {
> + if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) {
>   i915_gem_clflush_object(obj, true);
>   intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
>   }
> -- 
> 2.11.0

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2)

[Chris Wilson]

On Thu, Jan 12, 2017 at 05:24:12PM -, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Declare i915_gem_object_create_internal() as taking 
> phys_addr_t size (rev2)
> URL   : https://patchwork.freedesktop.org/series/17905/
> State : warning
> 
> == Summary ==
> 
> Series 17905v2 drm/i915: Declare i915_gem_object_create_internal() as taking 
> phys_addr_t size
> https://patchwork.freedesktop.org/api/1.0/series/17905/revisions/2/mbox/
> 
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-c:
> pass   -> DMESG-WARN (fi-bxt-j4205)

Nothing to see here, pushed. Thanks for the suggestion and review,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()

[Chris Wilson]

On Thu, Jan 12, 2017 at 05:33:44PM +, Matthew Auld wrote:
> On 12 January 2017 at 16:45, Chris Wilson  wrote:
> > Parameter: good.
> > Parameter - bad.
> >
> > One day I'll learn the syntax.
> >
> > Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a 
> > helper")
> > Fixes: e007b19d7ba7 ("drm/i915: Use the MRU stack search after evicting")
> > Signed-off-by: Chris Wilson 
> > Cc: Joonas Lahtinen 
> > Cc: Daniel Vetter 
> Reviewed-by: Matthew Auld 

Thanks, and pushed.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/14] drm/i915: Store the pipe pixel rate in the crtc state

[Rodrigo Vivi]

On Tue, Dec 20, 2016 at 03:29:54PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 20, 2016 at 03:10:53PM +0200, Ander Conselvan De Oliveira wrote:
> > On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> > > From: Ville Syrjälä 
> > > 
> > > Rather than recomptuing the pipe pixel rate on demand everwhere, let's
> > > just stick the precomputed value into the crtc state.
> > > 
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 31 ++-
> > >  drivers/gpu/drm/i915/intel_drv.h |  2 ++
> > >  drivers/gpu/drm/i915/intel_fbc.c |  3 +--
> > >  drivers/gpu/drm/i915/intel_pm.c  | 14 +++---
> > >  4 files changed, 36 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 0b0d7e8be630..1d979041c52c 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > 
> > 
> > [...]
> > 
> > > @@ -16919,7 +16938,7 @@ static void intel_modeset_readout_hw_state(struct 
> > > drm_device *dev)
> >                 __drm_atomic_helper_crtc_destroy_state(_state->base);
> > memset(crtc_state, 0, sizeof(*crtc_state));
> > crtc_state->base.crtc = >base;
> >  
> > crtc_state->base.active = crtc_state->base.enable =
> > dev_priv->display.get_pipe_config(crtc, crtc_state);
> >  
> > crtc->base.enabled = crtc_state->base.enable;
> > crtc->active = crtc_state->base.active;
> >  
> > if (crtc_state->base.active) {
> > >   dev_priv->active_crtcs |= 1 << crtc->pipe;
> > >  
> > >   if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> > > - pixclk = ilk_pipe_pixel_rate(crtc_state);
> > > + pixclk = crtc_state->pixel_rate;
> > 
> > Aren't you reading 0 here, because of the memset above? As far as I can 
> > tell,
> > haswell_get_pipe_config() doesn't set crtc_state->pixel_rate.
> 
> Hmm, yeah. Which means this whole piece of min_pixclk[] code is in
> the wrong place. You can't know the pixel rate until you know the
> clock, and you don't know that until you've done the full readout
> (meaning the encoder .get_config() hooks have been called as well).

Apparently this is the only piece blocking this already reviewed series, right?

So, what are the plans? drop this patch and move with the other patches?


> 
> > 
> > >   else if (IS_VALLEYVIEW(dev_priv) || 
> > > IS_CHERRYVIEW(dev_priv))
> > >   pixclk = 
> > > crtc_state->base.adjusted_mode.crtc_clock;
> > >   else
> > > @@ -17031,6 +17050,8 @@ static void intel_modeset_readout_hw_state(struct 
> > > drm_device *dev)
> > >    */
> > >   crtc->base.state->mode.private_flags = 
> > > I915_MODE_FLAG_INHERITED;
> > >  
> > > + intel_crtc_compute_pixel_rate(crtc->config);
> > > +
> > >   drm_calc_timestamping_constants(>base, 
> > > >base.hwmode);
> > >   update_scanline_offset(crtc);
> > >   }
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index f61ea43c7532..3969e786d566 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -541,6 +541,8 @@ struct intel_crtc_state {
> > >    * and get clipped at the edges. */
> > >   int pipe_src_w, pipe_src_h;
> > >  
> > > + unsigned int pixel_rate;
> > > +
> > 
> > Maybe add some comment about this parameter. This is not in kernel doc, but
> > having that already would probably make it easier for whoever does it in the
> > end.
> > 
> > Ander
> 
> -- 
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/10] drm/i915/psr: set CHICKEN_TRANS for psr2

[Vivi, Rodrigo]

Reviewed-by: Rodrigo Vivi 

On Fri, 2017-01-13 at 00:31 +0530, vathsala nagaraju wrote:
> As per bpsec, CHICKEN_TRANS_EDP bit 12 ,15 must be programmed in
> psr2 enable sequence.
> bit 12 : Program Transcoder EDP VSC DIP header with a valid setting for
> PSR2 and Set CHICKEN_TRANS_EDP(0x420cc) bit 12 for programmable
> header packet.
> bit 15 : Set CHICKEN_TRANS_EDP(0x420cc) bit 15 if Y coordinate is supported
> 
> v2: (Rodrigo)
> - move CHICKEN_TRANS_EDP bit set logic right after setup_vsc
> 
> v3:(Rodrigo)
> - initialize chicken_trans to CHICKEN_TRANS_BIT12 instead of 0
> 
> v4:(chris wilson)
> - use BIT(12), remove CHICKEN_TRANS_BIT12
> - remove unnecessary comments
> - update commit message
> 
> v5:
> - rename bit 12 PSR2_VSC_ENABLE_PROG_HEADER
> - rename bit 15 PSR2_ADD_VERTICAL_LINE_COUNT
> 
> v6:(Rodrigo)
> - remove TRANS_EDP=3, use cpu_transcoder
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: vathsala nagaraju 
> Signed-off-by: Patil Deepti 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 6 ++
>  drivers/gpu/drm/i915/intel_psr.c | 7 +++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7830e6e..c9c1ccd 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6449,6 +6449,12 @@ enum {
>  #define  BDW_DPRS_MASK_VBLANK_SRD(1 << 0)
>  #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, 
> _CHICKEN_PIPESL_1_B)
>  
> +#define CHICKEN_TRANS_A 0x420c0
> +#define CHICKEN_TRANS_B 0x420c4
> +#define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, 
> CHICKEN_TRANS_B)
> +#define PSR2_VSC_ENABLE_PROG_HEADER(1<<12)
> +#define PSR2_ADD_VERTICAL_LINE_COUNT   (1<<15)
> +
>  #define DISP_ARB_CTL _MMIO(0x45000)
>  #define  DISP_FBC_MEMORY_WAKE(1<<31)
>  #define  DISP_TILE_SURFACE_SWIZZLING (1<<13)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 36c4045..935402e 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -480,6 +480,9 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   struct drm_device *dev = intel_dig_port->base.base.dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
> + struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> + enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> + u32 chicken;
>  
>   if (!HAS_PSR(dev_priv)) {
>   DRM_DEBUG_KMS("PSR not supported on this platform\n");
> @@ -505,6 +508,10 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   if (HAS_DDI(dev_priv)) {
>   if (dev_priv->psr.psr2_support) {
>   skl_psr_setup_su_vsc(intel_dp);
> + chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> + if (dev_priv->psr.y_cord_support)
> + chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
> + I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
>   } else {
>   /* set up vsc header for psr1 */
>   hsw_psr_setup_vsc(intel_dp);

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 06/10] drm/i915/psr: set CHICKEN_TRANS for psr2

[vathsala nagaraju]

As per bpsec, CHICKEN_TRANS_EDP bit 12 ,15 must be programmed in
psr2 enable sequence.
bit 12 : Program Transcoder EDP VSC DIP header with a valid setting for
PSR2 and Set CHICKEN_TRANS_EDP(0x420cc) bit 12 for programmable
header packet.
bit 15 : Set CHICKEN_TRANS_EDP(0x420cc) bit 15 if Y coordinate is supported

v2: (Rodrigo)
- move CHICKEN_TRANS_EDP bit set logic right after setup_vsc

v3:(Rodrigo)
- initialize chicken_trans to CHICKEN_TRANS_BIT12 instead of 0

v4:(chris wilson)
- use BIT(12), remove CHICKEN_TRANS_BIT12
- remove unnecessary comments
- update commit message

v5:
- rename bit 12 PSR2_VSC_ENABLE_PROG_HEADER
- rename bit 15 PSR2_ADD_VERTICAL_LINE_COUNT

v6:(Rodrigo)
- remove TRANS_EDP=3, use cpu_transcoder

Cc: Rodrigo Vivi 
Cc: Jim Bride 
Signed-off-by: vathsala nagaraju 
Signed-off-by: Patil Deepti 
---
 drivers/gpu/drm/i915/i915_reg.h  | 6 ++
 drivers/gpu/drm/i915/intel_psr.c | 7 +++
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7830e6e..c9c1ccd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6449,6 +6449,12 @@ enum {
 #define  BDW_DPRS_MASK_VBLANK_SRD  (1 << 0)
 #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, 
_CHICKEN_PIPESL_1_B)
 
+#define CHICKEN_TRANS_A 0x420c0
+#define CHICKEN_TRANS_B 0x420c4
+#define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, 
CHICKEN_TRANS_B)
+#define PSR2_VSC_ENABLE_PROG_HEADER(1<<12)
+#define PSR2_ADD_VERTICAL_LINE_COUNT   (1<<15)
+
 #define DISP_ARB_CTL   _MMIO(0x45000)
 #define  DISP_FBC_MEMORY_WAKE  (1<<31)
 #define  DISP_TILE_SURFACE_SWIZZLING   (1<<13)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 36c4045..935402e 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -480,6 +480,9 @@ void intel_psr_enable(struct intel_dp *intel_dp)
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
+   struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
+   enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
+   u32 chicken;
 
if (!HAS_PSR(dev_priv)) {
DRM_DEBUG_KMS("PSR not supported on this platform\n");
@@ -505,6 +508,10 @@ void intel_psr_enable(struct intel_dp *intel_dp)
if (HAS_DDI(dev_priv)) {
if (dev_priv->psr.psr2_support) {
skl_psr_setup_su_vsc(intel_dp);
+   chicken = PSR2_VSC_ENABLE_PROG_HEADER;
+   if (dev_priv->psr.y_cord_support)
+   chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
+   I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
} else {
/* set up vsc header for psr1 */
hsw_psr_setup_vsc(intel_dp);
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Add format modifiers for Intel

[Ben Widawsky]

On 17-01-12 20:32:07, Ville Syrjälä wrote:

On Thu, Jan 12, 2017 at 10:00:55AM -0800, Ben Widawsky wrote:

On 17-01-12 12:51:20, Ville Syrjälä wrote:
>On Wed, Jan 11, 2017 at 04:51:17PM -0800, Ben Widawsky wrote:
>> This was based on a patch originally by Kristian. It has been modified
>> pretty heavily to use the new callbacks from the previous patch.
>>
>> Cc: Kristian H. Kristensen 
>> Signed-off-by: Ben Widawsky 
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 109 
++-
>>  drivers/gpu/drm/i915/intel_sprite.c  |  33 ++-
>>  2 files changed, 137 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
>> index 8715b1083d1d..26f3a911b999 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -61,6 +61,11 @@ static const uint32_t i8xx_primary_formats[] = {
>>DRM_FORMAT_XRGB,
>>  };
>>
>> +static const uint64_t i8xx_format_modifiers[] = {
>> +  I915_FORMAT_MOD_X_TILED,
>
>Did we want to list the linear modifier in these as well?
>

Yeah. My initial response was no, but yes. We should. I was using
DRM_FORMAT_MOD_NONE in its place, it should be linear, and it should be defined
in the array.

>> +  DRM_FORMAT_MOD_INVALID
>> +};
>> +
>>  /* Primary plane formats for gen >= 4 */
>>  static const uint32_t i965_primary_formats[] = {
>>DRM_FORMAT_C8,
>> @@ -71,6 +76,11 @@ static const uint32_t i965_primary_formats[] = {
>>DRM_FORMAT_XBGR2101010,
>>  };
>>
>> +static const uint64_t i965_format_modifiers[] = {
>> +  I915_FORMAT_MOD_X_TILED,
>> +  DRM_FORMAT_MOD_INVALID
>> +};
>
>We could just share the i8xx array. The name of the array should perhaps
>be i9xx_format_modifiers[] in that case. That's sort of the naming
>convention we've been left with for things that apply to more or less
>all the platforms.
>

Got it thanks. This is a relic from Kristian's original patch which tied the
modifiers to the formats in place. It made more sense there to have a separate
i8xx

>> +
>>  static const uint32_t skl_primary_formats[] = {
>>DRM_FORMAT_C8,
>>DRM_FORMAT_RGB565,
>> @@ -86,6 +96,12 @@ static const uint32_t skl_primary_formats[] = {
>>DRM_FORMAT_VYUY,
>>  };
>>
>> +static const uint64_t skl_format_modifiers[] = {
>> +  I915_FORMAT_MOD_Y_TILED,
>
>Yf missing? and linear
>

Yes, thanks. I'm kind of scared to add Yf to be honest :P

>> +  I915_FORMAT_MOD_X_TILED,
>> +  DRM_FORMAT_MOD_INVALID
>> +};
>> +
>>  /* Cursor formats */
>>  static const uint32_t intel_cursor_formats[] = {
>>DRM_FORMAT_ARGB,
>> @@ -15173,6 +15189,87 @@ void intel_plane_destroy(struct drm_plane *plane)
>>kfree(to_intel_plane(plane));
>>  }
>>
>> +static bool i8xx_mod_supported(uint32_t format, uint64_t modifier)
>> +{
>> +  if (modifier == DRM_FORMAT_MOD_NONE)
>> +  return true;
>> +
>> +  switch (format) {
>> +  case DRM_FORMAT_C8:
>> +  case DRM_FORMAT_RGB565:
>> +  case DRM_FORMAT_XRGB1555:
>> +  case DRM_FORMAT_XRGB:
>> +  return modifier == I915_FORMAT_MOD_X_TILED;
>> +  default:
>> +  return false;
>> +  }
>> +}
>> +
>> +static bool i965_mod_supported(uint32_t format, uint64_t modifier)
>> +{
>> +  switch (format) {
>> +  case DRM_FORMAT_C8:
>> +  case DRM_FORMAT_RGB565:
>> +  case DRM_FORMAT_XRGB:
>> +  case DRM_FORMAT_XBGR:
>> +  case DRM_FORMAT_XRGB2101010:
>> +  case DRM_FORMAT_XBGR2101010:
>> +  return modifier == I915_FORMAT_MOD_X_TILED;
>> +  default:
>> +  return false;
>> +  }
>> +}
>
>Hmm. There should be no format vs. tiling restrictions on these
>platforms, so presumably a simple "return true" should cover it all.
>That does perhaps remove the usefulness of these functions for
>verifying that the format or modifier is supported at all

One of the reasons for changing to this current format-modifier lookup at all
was Kristian's approach was considered fragile. If for whatever reason formats
are added, or removed, we'll catch it here. Also, it maybe let's us do something
on cursor plane at some point (I don't actually know). So yeah, we can return
true, but I like that it's spelled out explicitly. Makes it easy to compare it
to the docs as well to make sure our code is correct.

The benefit is of course I can combine i965_mod_supported() with
i8xx_mod_supported()

I'm honestly fine with changing it as well, I just don't see a huge reason to
change it since I've already typed it up. I'll leave it to you.


Feel free to keep it. We can always change it later if it becomes too much
work to maintain the duplicated format lists (the function and the array).
Not that I really expect these lists to be all that volatile.



[ ] Yes, change it.
[ ] No, leave it.

>but I've been thinking that addfb should 

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()

[Patchwork]

== Series Details ==

Series: drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()
URL   : https://patchwork.freedesktop.org/series/17915/
State : success

== Summary ==

Series 17915v1 drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()
https://patchwork.freedesktop.org/api/1.0/series/17915/revisions/1/mbox/


fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hqtotal:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

8f7458237916d6c1e5e1e9ebfc5923a6d1658385 drm-tip: 2017y-01m-12d-15h-39m-40s UTC 
integration manifest
4fbcc91 drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3505/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/7] i2c: designware-baytrail: Take punit lock on bus acquire

[Wolfram Sang]

On Sun, Jan 08, 2017 at 02:44:23PM +0100, Hans de Goede wrote:
> Take the punit lock to stop others from accessing the punit while the
> pmic i2c bus is in use. This is necessary because accessing the punit
> from the kernel may result in the punit trying to access the pmic i2c
> bus, which results in a hang when it happens while we own the pmic i2c
> bus semaphore.
> 
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
> Signed-off-by: Hans de Goede 
> Tested-by: tagorereddy 

I don't think the I2C patches need to go via I2C tree, so:

Acked-by: Wolfram Sang 

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/7] i2c: designware-baytrail: Call pmic_bus_access_notifier_chain

[Wolfram Sang]

On Sun, Jan 08, 2017 at 02:44:24PM +0100, Hans de Goede wrote:
> Call the iosf_mbi pmic_bus_access_notifier_chain on bus acquire / release.
> 
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
> Signed-off-by: Hans de Goede 
> Tested-by: tagorereddy 

Acked-by: Wolfram Sang 

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [GLK MIPI DSI V3 3/7] drm/i915/glk: Add MIPIIO Enable/disable sequence

[Julia Lawall]

Line 427 is under the if on line 422 and the subsequent lines are not.  It
is not clear at all what is intended.

julia

-- Forwarded message --
Date: Fri, 13 Jan 2017 02:13:07 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [Intel-gfx] [GLK MIPI DSI V3 3/7] drm/i915/glk: Add MIPIIO
Enable/disable sequence

In-Reply-To: <1483361668-2095-4-git-send-email-madhav.chau...@intel.com>

url:
https://github.com/0day-ci/linux/commits/Madhav-Chauhan/GLK-MIPI-DSI-VIDEO-MODE-PATCHES/20170103-015302
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
:: branch date: 10 days ago
:: commit date: 10 days ago

>> drivers/gpu/drm/i915/intel_dsi.c:427:2-43: code aligned with following code 
>> on line 428

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 8cf76366b507b9d1e251dc6d489be37f663b3252
vim +427 drivers/gpu/drm/i915/intel_dsi.c

8cf76366 Deepak M 2017-01-02  421   /* Wait for ULPS Not active */
8cf76366 Deepak M 2017-01-02  422   if 
(intel_wait_for_register(dev_priv,
8cf76366 Deepak M 2017-01-02  423   
MIPI_CTRL(port), GLK_ULPS_NOT_ACTIVE,
8cf76366 Deepak M 2017-01-02  424   
GLK_ULPS_NOT_ACTIVE, 20))
8cf76366 Deepak M 2017-01-02  425
8cf76366 Deepak M 2017-01-02  426   /* Exit ULPS */
8cf76366 Deepak M 2017-01-02 @427   val = 
I915_READ(MIPI_DEVICE_READY(port));
8cf76366 Deepak M 2017-01-02 @428   val &= ~ULPS_STATE_MASK;
8cf76366 Deepak M 2017-01-02  429   val |= (ULPS_STATE_EXIT | 
DEVICE_READY);
8cf76366 Deepak M 2017-01-02  430   
I915_WRITE(MIPI_DEVICE_READY(port), val);
8cf76366 Deepak M 2017-01-02  431

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/10] drm/i915/psr: fix blank screen issue for psr2

[Vivi, Rodrigo]

Reviewed-by: Rodrigo Vivi 

On Thu, 2017-01-12 at 23:30 +0530, vathsala nagaraju wrote:
> Psr1 and psr2 are mutually exclusive,ie when psr2 is enabled,
> psr1 should be disabled.When psr2 is exited , bit 31 of reg
> PSR2_CTL must be set to 0 but currently bit 31 of SRD_CTL
> (psr1 control register)is set to 0.
> Also ,PSR2_IDLE state is looked up from SRD_STATUS(psr1 register)
> instead of PSR2_STATUS register, which has wrong data, resulting
> in blankscreen.
> hsw_enable_source is split into hsw_enable_source_psr1 and
> hsw_enable_source_psr2 for easier code review and maintenance,
> as suggested by rodrigo and jim.
> 
> v2: (Rodrigo)
> - Rename hsw_enable_source_psr* to intel_enable_source_psr*
> 
> v3: (Rodrigo)
> - In hsw_psr_disable ,
>   1) for psr active case, handle psr2 followed by psr1.
>   2) psr inactive case, handle psr2 followed by psr1
> 
> v4:(Rodrigo)
> - move psr2 restriction(32X20) to match_conditions function
>   returning false and fully blocking PSR to a new patch before
>   this one.
> 
> v5: in source_psr2, removed val = EDP_PSR_ENABLE
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: Vathsala Nagaraju 
> Signed-off-by: Patil Deepti 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |   3 +
>  drivers/gpu/drm/i915/intel_psr.c | 122 
> +--
>  2 files changed, 95 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 00970aa..7830e6e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3615,6 +3615,9 @@ enum {
>  #define   EDP_PSR2_FRAME_BEFORE_SU_MASK  (0xf<<4)
>  #define   EDP_PSR2_IDLE_MASK 0xf
>  
> +#define EDP_PSR2_STATUS_CTL_MMIO(0x6f940)
> +#define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
> +
>  /* VGA port control */
>  #define ADPA _MMIO(0x61100)
>  #define PCH_ADPA_MMIO(0xe1100)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 707cae8..8827647 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -261,7 +261,7 @@ static void vlv_psr_activate(struct intel_dp *intel_dp)
>  VLV_EDP_PSR_ACTIVE_ENTRY);
>  }
>  
> -static void hsw_psr_enable_source(struct intel_dp *intel_dp)
> +static void intel_enable_source_psr1(struct intel_dp *intel_dp)
>  {
>   struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>   struct drm_device *dev = dig_port->base.base.dev;
> @@ -312,14 +312,29 @@ static void hsw_psr_enable_source(struct intel_dp 
> *intel_dp)
>   val |= EDP_PSR_TP1_TP2_SEL;
>  
>   I915_WRITE(EDP_PSR_CTL, val);
> +}
>  
> - if (!dev_priv->psr.psr2_support)
> - return;
> +static void intel_enable_source_psr2(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + /*
> +  * Let's respect VBT in case VBT asks a higher idle_frame value.
> +  * Let's use 6 as the minimum to cover all known cases including
> +  * the off-by-one issue that HW has in some cases. Also there are
> +  * cases where sink should be able to train
> +  * with the 5 or 6 idle patterns.
> +  */
> + uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
> + uint32_t val;
> +
> + val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>  
>   /* FIXME: selective update is probably totally broken because it doesn't
>* mesh at all with our frontbuffer tracking. And the hw alone isn't
>* good enough. */
> - val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
> + val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
>  
>   if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>   val |= EDP_PSR2_TP2_TIME_2500;
> @@ -333,6 +348,19 @@ static void hsw_psr_enable_source(struct intel_dp 
> *intel_dp)
>   I915_WRITE(EDP_PSR2_CTL, val);
>  }
>  
> +static void hsw_psr_enable_source(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + /* psr1 and psr2 are mutually exclusive.*/
> + if (dev_priv->psr.psr2_support)
> + intel_enable_source_psr2(intel_dp);
> + else
> + intel_enable_source_psr1(intel_dp);
> +}
> +
>  static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
>  {
>   struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> @@ -417,7 +445,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
>   struct drm_device *dev = intel_dig_port->base.base.dev;
>   struct 

Re: [Intel-gfx] [PATCH v4 00/10] Execlist based engine-reset (v4)

[Michel Thierry]

On 11/01/17 23:30, Chris Wilson wrote:

I'm sorry to do this, but there is a regression fix for gen3 required
first that makes this more complicated.

https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=prescheduler=de399a0a6baae97910796d81d8b9324db3fdd77c
https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=prescheduler=67bea4dbb664b100c108af05d9a0f1f3b4078ab2
https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=prescheduler=d91a3e43a8d6f94076b60caa1de3a9918c5cd766
-Chris



No worries, I'll rebase on top of these.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Add format modifiers for Intel

[Ville Syrjälä]

On Thu, Jan 12, 2017 at 10:00:55AM -0800, Ben Widawsky wrote:
> On 17-01-12 12:51:20, Ville Syrjälä wrote:
> >On Wed, Jan 11, 2017 at 04:51:17PM -0800, Ben Widawsky wrote:
> >> This was based on a patch originally by Kristian. It has been modified
> >> pretty heavily to use the new callbacks from the previous patch.
> >>
> >> Cc: Kristian H. Kristensen 
> >> Signed-off-by: Ben Widawsky 
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 109 
> >> ++-
> >>  drivers/gpu/drm/i915/intel_sprite.c  |  33 ++-
> >>  2 files changed, 137 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 8715b1083d1d..26f3a911b999 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -61,6 +61,11 @@ static const uint32_t i8xx_primary_formats[] = {
> >>DRM_FORMAT_XRGB,
> >>  };
> >>
> >> +static const uint64_t i8xx_format_modifiers[] = {
> >> +  I915_FORMAT_MOD_X_TILED,
> >
> >Did we want to list the linear modifier in these as well?
> >
> 
> Yeah. My initial response was no, but yes. We should. I was using
> DRM_FORMAT_MOD_NONE in its place, it should be linear, and it should be 
> defined
> in the array.
> 
> >> +  DRM_FORMAT_MOD_INVALID
> >> +};
> >> +
> >>  /* Primary plane formats for gen >= 4 */
> >>  static const uint32_t i965_primary_formats[] = {
> >>DRM_FORMAT_C8,
> >> @@ -71,6 +76,11 @@ static const uint32_t i965_primary_formats[] = {
> >>DRM_FORMAT_XBGR2101010,
> >>  };
> >>
> >> +static const uint64_t i965_format_modifiers[] = {
> >> +  I915_FORMAT_MOD_X_TILED,
> >> +  DRM_FORMAT_MOD_INVALID
> >> +};
> >
> >We could just share the i8xx array. The name of the array should perhaps
> >be i9xx_format_modifiers[] in that case. That's sort of the naming
> >convention we've been left with for things that apply to more or less
> >all the platforms.
> >
> 
> Got it thanks. This is a relic from Kristian's original patch which tied the
> modifiers to the formats in place. It made more sense there to have a separate
> i8xx
> 
> >> +
> >>  static const uint32_t skl_primary_formats[] = {
> >>DRM_FORMAT_C8,
> >>DRM_FORMAT_RGB565,
> >> @@ -86,6 +96,12 @@ static const uint32_t skl_primary_formats[] = {
> >>DRM_FORMAT_VYUY,
> >>  };
> >>
> >> +static const uint64_t skl_format_modifiers[] = {
> >> +  I915_FORMAT_MOD_Y_TILED,
> >
> >Yf missing? and linear
> >
> 
> Yes, thanks. I'm kind of scared to add Yf to be honest :P
> 
> >> +  I915_FORMAT_MOD_X_TILED,
> >> +  DRM_FORMAT_MOD_INVALID
> >> +};
> >> +
> >>  /* Cursor formats */
> >>  static const uint32_t intel_cursor_formats[] = {
> >>DRM_FORMAT_ARGB,
> >> @@ -15173,6 +15189,87 @@ void intel_plane_destroy(struct drm_plane *plane)
> >>kfree(to_intel_plane(plane));
> >>  }
> >>
> >> +static bool i8xx_mod_supported(uint32_t format, uint64_t modifier)
> >> +{
> >> +  if (modifier == DRM_FORMAT_MOD_NONE)
> >> +  return true;
> >> +
> >> +  switch (format) {
> >> +  case DRM_FORMAT_C8:
> >> +  case DRM_FORMAT_RGB565:
> >> +  case DRM_FORMAT_XRGB1555:
> >> +  case DRM_FORMAT_XRGB:
> >> +  return modifier == I915_FORMAT_MOD_X_TILED;
> >> +  default:
> >> +  return false;
> >> +  }
> >> +}
> >> +
> >> +static bool i965_mod_supported(uint32_t format, uint64_t modifier)
> >> +{
> >> +  switch (format) {
> >> +  case DRM_FORMAT_C8:
> >> +  case DRM_FORMAT_RGB565:
> >> +  case DRM_FORMAT_XRGB:
> >> +  case DRM_FORMAT_XBGR:
> >> +  case DRM_FORMAT_XRGB2101010:
> >> +  case DRM_FORMAT_XBGR2101010:
> >> +  return modifier == I915_FORMAT_MOD_X_TILED;
> >> +  default:
> >> +  return false;
> >> +  }
> >> +}
> >
> >Hmm. There should be no format vs. tiling restrictions on these
> >platforms, so presumably a simple "return true" should cover it all.
> >That does perhaps remove the usefulness of these functions for
> >verifying that the format or modifier is supported at all
> 
> One of the reasons for changing to this current format-modifier lookup at all
> was Kristian's approach was considered fragile. If for whatever reason formats
> are added, or removed, we'll catch it here. Also, it maybe let's us do 
> something
> on cursor plane at some point (I don't actually know). So yeah, we can return
> true, but I like that it's spelled out explicitly. Makes it easy to compare it
> to the docs as well to make sure our code is correct.
> 
> The benefit is of course I can combine i965_mod_supported() with
> i8xx_mod_supported()
> 
> I'm honestly fine with changing it as well, I just don't see a huge reason to
> change it since I've already typed it up. I'll leave it to you.

Feel free to keep it. We can always change it later if it becomes too much
work to maintain the duplicated format lists (the function and the array).
Not that I really expect these lists to be all that volatile.

> 
> [ 

Re: [Intel-gfx] [PATCH i-g-t rfc 01/29] lib/igt_debugfs: Prevent buffer overflow

[Lankhorst, Maarten]

Robert Foss schreef op do 12-01-2017 om 11:30 [-0500]:
> 
> On 2017-01-12 04:14 AM, Lankhorst, Maarten wrote:
> > 
> > Robert Foss schreef op wo 11-01-2017 om 15:41 [-0500]:
> > > 
> > > buf array may overflow with when writing '\0' if
> > > MAX_LINE_LEN bytes are read during read().
> > How?
> > 
> > char buf[MAX_LINE_LEN + 1];
> 
> I actually missed the + 1, but parts of the commit are still
> relevant 
> though, as the errno at least in theory could be != EAGAIN.
> 
> So I'd like to keep the below check, to prevent compiler warnings.
> if (bytes_read < 0)
> 
> Sounds ok?
Yes. :)
> 
> Rob.
> > 
> > 
> > > 
> > > Signed-off-by: Robert Foss 
> > > ---
> > >  lib/igt_debugfs.c | 8 +---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> > > index d828687a..8b8a627a 100644
> > > --- a/lib/igt_debugfs.c
> > > +++ b/lib/igt_debugfs.c
> > > @@ -594,13 +594,15 @@ static int read_crc(igt_pipe_crc_t
> > > *pipe_crc,
> > > igt_crc_t *out)
> > >   read_len = MAX_LINE_LEN;
> > > 
> > >   igt_set_timeout(5, "CRC reading");
> > > - bytes_read = read(pipe_crc->crc_fd, , read_len);
> > > + bytes_read = read(pipe_crc->crc_fd, , read_len - 1);
> > >   igt_reset_timeout();
> > > 
> > > - if (bytes_read < 0 && errno == EAGAIN) {
> > > + if (bytes_read < 0 && errno == EAGAIN)
> > >   igt_assert(pipe_crc->flags & O_NONBLOCK);
> > > +
> > > + if (bytes_read < 0)
> > >   bytes_read = 0;
> > > - }
> > > +
> > >   buf[bytes_read] = '\0';
> > > 
> > >   if (bytes_read && !pipe_crc_init_from_string(pipe_crc,
> > > out,
> > > buf))
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v6] drm/i915: Use __sg_alloc_table_from_pages for userptr allocations

[kbuild test robot]

Hi Tvrtko,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20170111]
[cannot apply to v4.10-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Tvrtko-Ursulin/drm-i915-Use-__sg_alloc_table_from_pages-for-userptr-allocations/20170113-004619
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/i915_drv.c:48:0:
   drivers/gpu/drm/i915/i915_drv.h: In function 'i915_sg_segment_size':
>> drivers/gpu/drm/i915/i915_drv.h:2605:10: error: 'SCATTERLIST_MAX_SEGMENT' 
>> undeclared (first use in this function)
  return SCATTERLIST_MAX_SEGMENT;
 ^~~
   drivers/gpu/drm/i915/i915_drv.h:2605:10: note: each undeclared identifier is 
reported only once for each function it appears in
--
   In file included from drivers/gpu/drm/i915/i915_gem_userptr.c:27:0:
   drivers/gpu/drm/i915/i915_drv.h: In function 'i915_sg_segment_size':
>> drivers/gpu/drm/i915/i915_drv.h:2605:10: error: 'SCATTERLIST_MAX_SEGMENT' 
>> undeclared (first use in this function)
  return SCATTERLIST_MAX_SEGMENT;
 ^~~
   drivers/gpu/drm/i915/i915_drv.h:2605:10: note: each undeclared identifier is 
reported only once for each function it appears in
   drivers/gpu/drm/i915/i915_gem_userptr.c: In function 
'__i915_gem_userptr_alloc_pages':
>> drivers/gpu/drm/i915/i915_gem_userptr.c:406:8: error: implicit declaration 
>> of function '__sg_alloc_table_from_pages' 
>> [-Werror=implicit-function-declaration]
 ret = __sg_alloc_table_from_pages(st, pvec, num_pages,
   ^~~
   cc1: some warnings being treated as errors

vim +/SCATTERLIST_MAX_SEGMENT +2605 drivers/gpu/drm/i915/i915_drv.h

  2599  
  2600  static inline unsigned int i915_sg_segment_size(void)
  2601  {
  2602  unsigned int size = swiotlb_max_segment();
  2603  
  2604  if (size == 0)
> 2605  return SCATTERLIST_MAX_SEGMENT;
  2606  
  2607  size = rounddown(size, PAGE_SIZE);
  2608  /* swiotlb_max_segment_size can return 1 byte when it means one 
page. */

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm: Add new DRM_IOCTL_MODE_GETPLANE2

[Ville Syrjälä]

On Thu, Jan 12, 2017 at 05:50:15PM +, Daniel Stone wrote:
> Hi,
> 
> On 12 January 2017 at 17:45, Ville Syrjälä
>  wrote:
> > On Thu, Jan 12, 2017 at 05:04:46PM +, Daniel Stone wrote:
> >> Implicit is clever but horrible. AFAICT, the only way to do it
> >> properly would be to have a nested forwards loop walk when you first
> >> hit a modifier, searching for further occurrences of that modifier to
> >> collect the complete set of formats that modifier applies to.
> >
> > Not sure for what that is the "only way". In fact I can't right now
> > think of any operation that would require an extra loop necessarily.
> > For some things you might just want to look for a specific
> > format+modifier combo, for that it doesn't matter how many blocks there
> > are.
> 
> Right, that just needs a local variable to act as a counter.
> 
> > And if you want to transform the reply into some less convoluted
> > form, well then you'd just need some modifier+dynamic format list thing,
> > or if you want to keep to bitmasks you'd either need a bitmask that can
> > grow when running out of bits or just make it big enough to handle a
> > sufficiently large worst case number of bits.
> >
> > Dunno, maybe I just lack imagination. Then again, I'm not even sure if
> > we're talking about userspace of kernel code here, which might explain
> > my general confusion :)
> 
> I'm talking about userspace, where I want to have:
> struct drm_plane {
> struct {
> uint32_t format;
> uint64_t modifiers[];
> } formats[];
> }

Flipping formats[] vs. modifiers[] here would seem like it should make
this easier with the proposed kernel API. And if the kernel will also
uarantee that multiple instances of the same modifier must be returned
contiguously, then it should be even easier.

Oh and flipping formats[] and modifiers[] should also save a quite a
bit of space since each format takes twice as much space as each
modifier. But I suppose that might come at a runtime cost if you have
to look for a specific format in each modifier's format list instead
of having to look at just the modifier list of a specific format. So
I suppose not flipping might be better after all, which I guess would
complicate populating the infromation somewhat.

Anyways, that's all a bit unrelated to the matter at hand, so I'll stop
now and just state that I don't mind having an explicit offset if
people really want it.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/probe-helpers: Drop locking from poll_enable

[Lyude Paul]

Fixes locking issues I've witnessed on the W541.

Tested-by: Lyude 
Reviewed-by: Lyude 

On Wed, 2017-01-11 at 10:01 +0100, Daniel Vetter wrote:
> It was only needed to protect the connector_list walking, see
> 
> commit 8c4ccc4ab6f64e859d4ff8d7c02c2ed2e956e07f
> Author: Daniel Vetter 
> Date:   Thu Jul 9 23:44:26 2015 +0200
> 
> drm/probe-helper: Grab mode_config.mutex in poll_init/enable
> 
> Unfortunately the commit message of that patch fails to mention that
> the new locking check was for the connector_list.
> 
> But that requirement disappeared in
> 
> commit c36a3254f7857f1ad9badbe3578ccc92be541a8e
> Author: Daniel Vetter 
> Date:   Thu Dec 15 16:58:43 2016 +0100
> 
> drm: Convert all helpers to drm_connector_list_iter
> 
> and so we can drop this again.
> 
> This fixes a locking inversion on nouveau, where the rpm code needs
> to
> re-enable. But in other places the rpm_get() calls are nested within
> the big modeset locks.
> 
> While at it, also improve the kerneldoc for these two functions a
> notch.
> 
> v2: Update the kerneldoc even more to explain that these functions
> can't be called concurrently, or bad things happen (Chris).
> 
> Cc: Dave Airlie 
> Reviewed-by: Chris Wilson 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_probe_helper.c   | 51 ++--
> 
>  drivers/gpu/drm/i915/intel_hotplug.c |  4 +--
>  include/drm/drm_crtc_helper.h|  1 -
>  3 files changed, 22 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index 20f48d1e2785..93381454bdf7 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -115,25 +115,28 @@ static int
> drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
>  
>  #define DRM_OUTPUT_POLL_PERIOD (10*HZ)
>  /**
> - * drm_kms_helper_poll_enable_locked - re-enable output polling.
> + * drm_kms_helper_poll_enable - re-enable output polling.
>   * @dev: drm_device
>   *
> - * This function re-enables the output polling work without
> - * locking the mode_config mutex.
> + * This function re-enables the output polling work, after it has
> been
> + * temporarily disabled using drm_kms_helper_poll_disable(), for
> example over
> + * suspend/resume.
>   *
> - * This is like drm_kms_helper_poll_enable() however it is to be
> - * called from a context where the mode_config mutex is locked
> - * already.
> + * Drivers can call this helper from their device resume
> implementation. It is
> + * an error to call this when the output polling support has not yet
> been set
> + * up.
> + *
> + * Note that calls to enable and disable polling must be strictly
> ordered, which
> + * is automatically the case when they're only call from
> suspend/resume
> + * callbacks.
>   */
> -void drm_kms_helper_poll_enable_locked(struct drm_device *dev)
> +void drm_kms_helper_poll_enable(struct drm_device *dev)
>  {
>   bool poll = false;
>   struct drm_connector *connector;
>   struct drm_connector_list_iter conn_iter;
>   unsigned long delay = DRM_OUTPUT_POLL_PERIOD;
>  
> - WARN_ON(!mutex_is_locked(>mode_config.mutex));
> -
>   if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
>   return;
>  
> @@ -163,7 +166,7 @@ void drm_kms_helper_poll_enable_locked(struct
> drm_device *dev)
>   if (poll)
>   schedule_delayed_work(
> >mode_config.output_poll_work, delay);
>  }
> -EXPORT_SYMBOL(drm_kms_helper_poll_enable_locked);
> +EXPORT_SYMBOL(drm_kms_helper_poll_enable);
>  
>  static enum drm_connector_status
>  drm_connector_detect(struct drm_connector *connector, bool force)
> @@ -290,7 +293,7 @@ int
> drm_helper_probe_single_connector_modes(struct drm_connector
> *connector,
>  
>   /* Re-enable polling in case the global poll config changed.
> */
>   if (drm_kms_helper_poll != dev->mode_config.poll_running)
> - drm_kms_helper_poll_enable_locked(dev);
> + drm_kms_helper_poll_enable(dev);
>  
>   dev->mode_config.poll_running = drm_kms_helper_poll;
>  
> @@ -484,8 +487,12 @@ static void output_poll_execute(struct
> work_struct *work)
>   * This function disables the output polling work.
>   *
>   * Drivers can call this helper from their device suspend
> implementation. It is
> - * not an error to call this even when output polling isn't enabled
> or arlready
> - * disabled.
> + * not an error to call this even when output polling isn't enabled
> or already
> + * disabled. Polling is re-enabled by calling
> drm_kms_helper_poll_enable().
> + *
> + * Note that calls to enable and disable polling must be strictly
> ordered, which
> + * is automatically the case when they're only call from
> suspend/resume
> + * callbacks.
>   

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Add format modifiers for Intel

[Ben Widawsky]

On 17-01-12 12:51:20, Ville Syrjälä wrote:

On Wed, Jan 11, 2017 at 04:51:17PM -0800, Ben Widawsky wrote:

This was based on a patch originally by Kristian. It has been modified
pretty heavily to use the new callbacks from the previous patch.

Cc: Kristian H. Kristensen 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/intel_display.c | 109 ++-
 drivers/gpu/drm/i915/intel_sprite.c  |  33 ++-
 2 files changed, 137 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 8715b1083d1d..26f3a911b999 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -61,6 +61,11 @@ static const uint32_t i8xx_primary_formats[] = {
DRM_FORMAT_XRGB,
 };

+static const uint64_t i8xx_format_modifiers[] = {
+   I915_FORMAT_MOD_X_TILED,


Did we want to list the linear modifier in these as well?



Yeah. My initial response was no, but yes. We should. I was using
DRM_FORMAT_MOD_NONE in its place, it should be linear, and it should be defined
in the array.


+   DRM_FORMAT_MOD_INVALID
+};
+
 /* Primary plane formats for gen >= 4 */
 static const uint32_t i965_primary_formats[] = {
DRM_FORMAT_C8,
@@ -71,6 +76,11 @@ static const uint32_t i965_primary_formats[] = {
DRM_FORMAT_XBGR2101010,
 };

+static const uint64_t i965_format_modifiers[] = {
+   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_INVALID
+};


We could just share the i8xx array. The name of the array should perhaps
be i9xx_format_modifiers[] in that case. That's sort of the naming
convention we've been left with for things that apply to more or less
all the platforms.



Got it thanks. This is a relic from Kristian's original patch which tied the
modifiers to the formats in place. It made more sense there to have a separate
i8xx


+
 static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_C8,
DRM_FORMAT_RGB565,
@@ -86,6 +96,12 @@ static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_VYUY,
 };

+static const uint64_t skl_format_modifiers[] = {
+   I915_FORMAT_MOD_Y_TILED,


Yf missing? and linear



Yes, thanks. I'm kind of scared to add Yf to be honest :P


+   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_INVALID
+};
+
 /* Cursor formats */
 static const uint32_t intel_cursor_formats[] = {
DRM_FORMAT_ARGB,
@@ -15173,6 +15189,87 @@ void intel_plane_destroy(struct drm_plane *plane)
kfree(to_intel_plane(plane));
 }

+static bool i8xx_mod_supported(uint32_t format, uint64_t modifier)
+{
+   if (modifier == DRM_FORMAT_MOD_NONE)
+   return true;
+
+   switch (format) {
+   case DRM_FORMAT_C8:
+   case DRM_FORMAT_RGB565:
+   case DRM_FORMAT_XRGB1555:
+   case DRM_FORMAT_XRGB:
+   return modifier == I915_FORMAT_MOD_X_TILED;
+   default:
+   return false;
+   }
+}
+
+static bool i965_mod_supported(uint32_t format, uint64_t modifier)
+{
+   switch (format) {
+   case DRM_FORMAT_C8:
+   case DRM_FORMAT_RGB565:
+   case DRM_FORMAT_XRGB:
+   case DRM_FORMAT_XBGR:
+   case DRM_FORMAT_XRGB2101010:
+   case DRM_FORMAT_XBGR2101010:
+   return modifier == I915_FORMAT_MOD_X_TILED;
+   default:
+   return false;
+   }
+}


Hmm. There should be no format vs. tiling restrictions on these
platforms, so presumably a simple "return true" should cover it all.
That does perhaps remove the usefulness of these functions for
verifying that the format or modifier is supported at all


One of the reasons for changing to this current format-modifier lookup at all
was Kristian's approach was considered fragile. If for whatever reason formats
are added, or removed, we'll catch it here. Also, it maybe let's us do something
on cursor plane at some point (I don't actually know). So yeah, we can return
true, but I like that it's spelled out explicitly. Makes it easy to compare it
to the docs as well to make sure our code is correct.

The benefit is of course I can combine i965_mod_supported() with
i8xx_mod_supported()

I'm honestly fine with changing it as well, I just don't see a huge reason to
change it since I've already typed it up. I'll leave it to you.

[ ] Yes, change it.
[ ] No, leave it.


but I've been thinking that addfb should perhaps just iterate through the
format and modifier lists for every plane. Would avoid having to effectively
maintain the same lists in multiple places.



I don't quite follow this. Can you elaborate?


+
+static bool skl_mod_supported(uint32_t format, uint64_t modifier)
+{
+   switch (format) {
+   case DRM_FORMAT_C8:
+   case DRM_FORMAT_RGB565:
+   case DRM_FORMAT_XRGB:
+   case DRM_FORMAT_XBGR:
+   case DRM_FORMAT_ARGB:
+   case DRM_FORMAT_ABGR:
+   return modifier == 

[Intel-gfx] [PATCH 03/10] drm/i915/psr: fix blank screen issue for psr2

[vathsala nagaraju]

Psr1 and psr2 are mutually exclusive,ie when psr2 is enabled,
psr1 should be disabled.When psr2 is exited , bit 31 of reg
PSR2_CTL must be set to 0 but currently bit 31 of SRD_CTL
(psr1 control register)is set to 0.
Also ,PSR2_IDLE state is looked up from SRD_STATUS(psr1 register)
instead of PSR2_STATUS register, which has wrong data, resulting
in blankscreen.
hsw_enable_source is split into hsw_enable_source_psr1 and
hsw_enable_source_psr2 for easier code review and maintenance,
as suggested by rodrigo and jim.

v2: (Rodrigo)
- Rename hsw_enable_source_psr* to intel_enable_source_psr*

v3: (Rodrigo)
- In hsw_psr_disable ,
  1) for psr active case, handle psr2 followed by psr1.
  2) psr inactive case, handle psr2 followed by psr1

v4:(Rodrigo)
- move psr2 restriction(32X20) to match_conditions function
  returning false and fully blocking PSR to a new patch before
  this one.

v5: in source_psr2, removed val = EDP_PSR_ENABLE

Cc: Rodrigo Vivi 
Cc: Jim Bride 
Signed-off-by: Vathsala Nagaraju 
Signed-off-by: Patil Deepti 
---
 drivers/gpu/drm/i915/i915_reg.h  |   3 +
 drivers/gpu/drm/i915/intel_psr.c | 122 +--
 2 files changed, 95 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 00970aa..7830e6e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3615,6 +3615,9 @@ enum {
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK(0xf<<4)
 #define   EDP_PSR2_IDLE_MASK   0xf
 
+#define EDP_PSR2_STATUS_CTL_MMIO(0x6f940)
+#define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
+
 /* VGA port control */
 #define ADPA   _MMIO(0x61100)
 #define PCH_ADPA_MMIO(0xe1100)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 707cae8..8827647 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -261,7 +261,7 @@ static void vlv_psr_activate(struct intel_dp *intel_dp)
   VLV_EDP_PSR_ACTIVE_ENTRY);
 }
 
-static void hsw_psr_enable_source(struct intel_dp *intel_dp)
+static void intel_enable_source_psr1(struct intel_dp *intel_dp)
 {
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = dig_port->base.base.dev;
@@ -312,14 +312,29 @@ static void hsw_psr_enable_source(struct intel_dp 
*intel_dp)
val |= EDP_PSR_TP1_TP2_SEL;
 
I915_WRITE(EDP_PSR_CTL, val);
+}
 
-   if (!dev_priv->psr.psr2_support)
-   return;
+static void intel_enable_source_psr2(struct intel_dp *intel_dp)
+{
+   struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = dig_port->base.base.dev;
+   struct drm_i915_private *dev_priv = to_i915(dev);
+   /*
+* Let's respect VBT in case VBT asks a higher idle_frame value.
+* Let's use 6 as the minimum to cover all known cases including
+* the off-by-one issue that HW has in some cases. Also there are
+* cases where sink should be able to train
+* with the 5 or 6 idle patterns.
+*/
+   uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
+   uint32_t val;
+
+   val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
 
/* FIXME: selective update is probably totally broken because it doesn't
 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 * good enough. */
-   val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
+   val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
 
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
val |= EDP_PSR2_TP2_TIME_2500;
@@ -333,6 +348,19 @@ static void hsw_psr_enable_source(struct intel_dp 
*intel_dp)
I915_WRITE(EDP_PSR2_CTL, val);
 }
 
+static void hsw_psr_enable_source(struct intel_dp *intel_dp)
+{
+   struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = dig_port->base.base.dev;
+   struct drm_i915_private *dev_priv = to_i915(dev);
+
+   /* psr1 and psr2 are mutually exclusive.*/
+   if (dev_priv->psr.psr2_support)
+   intel_enable_source_psr2(intel_dp);
+   else
+   intel_enable_source_psr1(intel_dp);
+}
+
 static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 {
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
@@ -417,7 +445,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
 
-   WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
+   if (dev_priv->psr.psr2_support)
+   WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE);
+   else
+   WARN_ON(I915_READ(EDP_PSR_CTL) & 

[Intel-gfx] ✓ Fi.CI.BAT: success for drm: Don't race connector registration

[Patchwork]

== Series Details ==

Series: drm: Don't race connector registration
URL   : https://patchwork.freedesktop.org/series/17910/
State : success

== Summary ==

Series 17910v1 drm: Don't race connector registration
https://patchwork.freedesktop.org/api/1.0/series/17910/revisions/1/mbox/


fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hqtotal:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

8f7458237916d6c1e5e1e9ebfc5923a6d1658385 drm-tip: 2017y-01m-12d-15h-39m-40s UTC 
integration manifest
f9e487c drm: Don't race connector registration

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3503/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm: Add new DRM_IOCTL_MODE_GETPLANE2

[Daniel Stone]

Hi,

On 12 January 2017 at 17:45, Ville Syrjälä
 wrote:
> On Thu, Jan 12, 2017 at 05:04:46PM +, Daniel Stone wrote:
>> Implicit is clever but horrible. AFAICT, the only way to do it
>> properly would be to have a nested forwards loop walk when you first
>> hit a modifier, searching for further occurrences of that modifier to
>> collect the complete set of formats that modifier applies to.
>
> Not sure for what that is the "only way". In fact I can't right now
> think of any operation that would require an extra loop necessarily.
> For some things you might just want to look for a specific
> format+modifier combo, for that it doesn't matter how many blocks there
> are.

Right, that just needs a local variable to act as a counter.

> And if you want to transform the reply into some less convoluted
> form, well then you'd just need some modifier+dynamic format list thing,
> or if you want to keep to bitmasks you'd either need a bitmask that can
> grow when running out of bits or just make it big enough to handle a
> sufficiently large worst case number of bits.
>
> Dunno, maybe I just lack imagination. Then again, I'm not even sure if
> we're talking about userspace of kernel code here, which might explain
> my general confusion :)

I'm talking about userspace, where I want to have:
struct drm_plane {
struct {
uint32_t format;
uint64_t modifiers[];
} formats[];
}

Rather than keeping the original format around and doing the lookup every time.

Cheers,
Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm: Add new DRM_IOCTL_MODE_GETPLANE2

[Ville Syrjälä]

On Thu, Jan 12, 2017 at 05:04:46PM +, Daniel Stone wrote:
> Hi,
> 
> On 12 January 2017 at 14:56, Rob Clark  wrote:
> > On Thu, Jan 12, 2017 at 4:38 AM, Ville Syrjälä
> >  wrote:
> >> Isn't an implicit offset enough? As in first mask for a specific
> >> modifier is for format indexes 0-63, second mask for the same modifier
> >> is for 64-127, and so on.
> >
> > hmm, hadn't thought of that approach.  Definitely if we go w/ implicit
> > then we want to have userspace support from the get-go.  For explicit,
> > I guess userspace could complain and ignore if it saw a non-zero
> > offset similar to what we do w/ pad and unknown flags in the other
> > direction?
> 
> Implicit is clever but horrible. AFAICT, the only way to do it
> properly would be to have a nested forwards loop walk when you first
> hit a modifier, searching for further occurrences of that modifier to
> collect the complete set of formats that modifier applies to.

Not sure for what that is the "only way". In fact I can't right now
think of any operation that would require an extra loop necessarily.
For some things you might just want to look for a specific
format+modifier combo, for that it doesn't matter how many blocks there
are. And if you want to transform the reply into some less convoluted
form, well then you'd just need some modifier+dynamic format list thing,
or if you want to keep to bitmasks you'd either need a bitmask that can
grow when running out of bits or just make it big enough to handle a
sufficiently large worst case number of bits.

Dunno, maybe I just lack imagination. Then again, I'm not even sure if
we're talking about userspace of kernel code here, which might explain
my general confusion :)

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 33/37] drm/i915: Verify page layout for rotated VMA

[Tvrtko Ursulin]

On 11/01/2017 21:09, Chris Wilson wrote:

Exercise creating rotated VMA and checking the page order within.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/selftests/i915_vma.c | 130 ++
 1 file changed, 130 insertions(+)

diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c 
b/drivers/gpu/drm/i915/selftests/i915_vma.c
index d229adabc5f8..95c5db2b0881 100644
--- a/drivers/gpu/drm/i915/selftests/i915_vma.c
+++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
@@ -287,11 +287,141 @@ static int igt_vma_pin1(void *arg)
return err;
 }

+static unsigned long rotated_index(const struct intel_rotation_info *r,
+  unsigned int n,
+  unsigned int x,
+  unsigned int y)
+{
+   return r->plane[n].stride * (r->plane[n].height - y - 1) + x;
+}
+
+static struct scatterlist *
+assert_rotated(struct drm_i915_gem_object *obj,
+  const struct intel_rotation_info *r, unsigned int n,
+  struct scatterlist *sg)
+{
+   unsigned int x, y;
+
+   for (x = 0; x < r->plane[n].width; x++) {
+   for (y = 0; y < r->plane[n].height; y++) {
+   unsigned long src_idx;
+   dma_addr_t src;
+
+   src_idx = rotated_index(r, n, x, y);
+   src = i915_gem_object_get_dma_address(obj, src_idx);


Right, this is actually more like unrotate, from rotated coords to 
unrotated index. I'll assume the formula is correct if it passes. :)



+
+   if (sg_dma_len(sg) != PAGE_SIZE) {
+   pr_err("Invalid sg.length, found %d, expected %lu 
for rotated page (%d, %d) [src index %lu]\n",
+  sg_dma_len(sg), PAGE_SIZE,
+  x, y, src_idx);
+   return NULL;
+   }
+
+   if (sg_dma_address(sg) != src) {
+   pr_err("Invalid address for rotated page (%d, %d) 
[src index %lu]\n",
+  x, y, src_idx);
+   return NULL;
+   }
+
+   sg = sg_next(sg);
+   }
+   }
+
+   return sg;
+}
+
+static int igt_vma_rotate(void *arg)
+{
+   struct drm_i915_private *i915 = arg;
+   struct drm_i915_gem_object *obj;
+   const unsigned int width = 6;
+   const unsigned int height = 4;
+   const unsigned int npages = 24;
+   struct i915_vma *vma;
+   struct i915_ggtt_view view;
+   struct scatterlist *sg;
+   unsigned int n;
+   int err = -ENOMEM;
+
+   obj = i915_gem_object_create_internal(i915, npages*PAGE_SIZE);
+   if (IS_ERR(obj))
+   goto err;
+
+   view.type = I915_GGTT_VIEW_ROTATED;
+   view.rotated.plane[0].offset = 0;
+   view.rotated.plane[0].width = width;
+   view.rotated.plane[0].height = height;
+   view.rotated.plane[0].stride = width;
+
+   view.rotated.plane[1].offset = 0;
+   view.rotated.plane[1].width = height;
+   view.rotated.plane[1].height = width;
+   view.rotated.plane[1].stride = height;


Ahem, why are the width & height assignments different between the 
planes and how can a single assert possibly work with this? Perhaps it 
all works totally differently after Ville's rewrite, I don't know any 
more obviously.



+
+   vma = i915_gem_obj_lookup_or_create_vma(obj, >ggtt.base, );
+   if (IS_ERR(vma)) {
+   err = PTR_ERR(vma);
+   goto err_object;
+   }
+
+   if (!i915_vma_is_ggtt(vma)) {
+   pr_err("VMA is not in the GGTT!\n");
+   err = -EINVAL;
+   goto err_object;
+   }
+
+   err = i915_vma_pin(vma, 0, 0, PIN_GLOBAL);
+   if (err)
+   goto err_object;


Log message would be good.


+
+   if (memcmp(>ggtt_view, , sizeof(view))) {
+   pr_err("VMA mismatch upon creation!\n");
+   err = -EINVAL;
+   goto err_object;
+   }


i915_vma_compare to cover any future tweaking in that area?


+
+   if (vma->size != 2*npages*PAGE_SIZE) {
+   pr_err("VMA is wrong size, expected %lu, found %llu\n",
+  2*npages*PAGE_SIZE, vma->size);
+   err = -EINVAL;
+   goto err_object;
+   }


Wait a minute, how can rotated view be bigger than the object?!

[after some head scratching and bringing up the code]

Right, you are playing tricks with pretend two planes. It would be good 
to split the test into one plane only first, and then the two-plane 
configuration. Probably with a "real" backing store, I mean object with 
enough pages for both planes and a proper offset.



+
+   if (vma->node.size < vma->size) {
+   pr_err("VMA binding too small, expected 

Re: [Intel-gfx] [PATCH] drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()

[Matthew Auld]

On 12 January 2017 at 16:45, Chris Wilson  wrote:
> Parameter: good.
> Parameter - bad.
>
> One day I'll learn the syntax.
>
> Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a 
> helper")
> Fixes: e007b19d7ba7 ("drm/i915: Use the MRU stack search after evicting")
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Daniel Vetter 
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 18/37] drm/i915: Test exhaustion of the mmap space

[Matthew Auld]

On 11 January 2017 at 21:09, Chris Wilson  wrote:
> An unlikely error condition that we can simulate by stealing the most of
s/the most/most/

> the range before trying to insert new objects.
>
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/selftests/i915_gem_object.c | 137 
> +++
>  1 file changed, 137 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c 
> b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
> index df3625f551aa..46512a67877d 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c
> @@ -25,6 +25,7 @@
>  #include "i915_selftest.h"
>
>  #include "mock_gem_device.h"
> +#include "huge_gem_object.h"
>
>  static int igt_gem_object(void *arg)
>  {
> @@ -383,6 +384,141 @@ static int igt_partial_tiling(void *arg)
> return err;
>  }
>
> +static int make_obj_busy(struct drm_i915_gem_object *obj)
> +{
> +   struct drm_i915_private *i915 = to_i915(obj->base.dev);
> +   struct drm_i915_gem_request *rq;
> +   struct i915_vma *vma;
> +   int err;
> +
> +   vma = i915_gem_obj_lookup_or_create_vma(obj, >ggtt.base, NULL);
> +   if (IS_ERR(vma))
> +   return PTR_ERR(vma);
> +
> +   err = i915_vma_pin(vma, 0, 0, PIN_USER);
> +   if (err)
> +   return err;
> +
> +   rq = i915_gem_request_alloc(i915->engine[RCS], i915->kernel_context);
> +   if (IS_ERR(rq)) {
> +   i915_vma_unpin(vma);
> +   return PTR_ERR(rq);
> +   }
> +
> +   i915_vma_move_to_active(vma, rq, 0);
> +   i915_add_request(rq);
> +
> +   i915_gem_object_set_active_reference(obj);
> +   i915_vma_unpin(vma);
> +   return 0;
> +}
> +
> +static bool assert_mmap_offset(struct drm_i915_private *i915,
> +  unsigned long size,
> +  int expected)
> +{
> +   struct drm_i915_gem_object *obj;
> +   int err;
> +
> +   obj = i915_gem_object_create_internal(i915, size);
> +   if (IS_ERR(obj))
> +   return PTR_ERR(obj);
> +
> +   err = i915_gem_object_create_mmap_offset(obj);
> +   i915_gem_object_put(obj);
> +
> +   return err == expected;
> +}
> +
> +static int igt_mmap_offset_exhaustion(void *arg)
> +{
> +   struct drm_i915_private *i915 = arg;
> +   struct drm_mm *mm = >drm.vma_offset_manager->vm_addr_space_mm;
> +   struct drm_i915_gem_object *obj;
> +   struct drm_mm_node resv, *hole;
> +   u64 hole_start, hole_end;
> +   int loop, err;
> +
> +   /* Trim the VMA mmap space to only a page */
> +   memset(, 0, sizeof(resv));
> +   drm_mm_for_each_hole(hole, mm, hole_start, hole_end) {
> +   resv.start = hole_start;
> +   resv.size = hole_end - hole_start - 1; /* PAGE_SIZE units */
> +   err = drm_mm_reserve_node(mm, );
> +   if (err) {
> +   pr_err("Failed to trim VMA manager, err=%d\n", err);
> +   return err;
> +   }
> +   break;
> +   }
> +
> +   /* Just fits! */
> +   if (!assert_mmap_offset(i915, PAGE_SIZE, 0)) {
> +   pr_err("Unable to insert object into single page hole\n");
> +   err = -EINVAL;
> +   goto err;
> +   }
> +
> +   /* Too large */
> +   if (!assert_mmap_offset(i915, 2*PAGE_SIZE, -ENOSPC)) {
> +   pr_err("Unexpectedly succeded in inserting too large object 
> into single page hole\n");
s/succeded/succeeded

> +   err = -EINVAL;
> +   goto err;
> +   }
> +
> +   /* Fill the hole, should then fail */
Fill the hole, further attempts should then fail; less confusing imo.

Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Declare i915_gem_object_create_internal() as taking 
phys_addr_t size (rev2)
URL   : https://patchwork.freedesktop.org/series/17905/
State : warning

== Summary ==

Series 17905v2 drm/i915: Declare i915_gem_object_create_internal() as taking 
phys_addr_t size
https://patchwork.freedesktop.org/api/1.0/series/17905/revisions/2/mbox/

Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-c:
pass   -> DMESG-WARN (fi-bxt-j4205)

fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:223  dwarn:1   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hqtotal:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

8f7458237916d6c1e5e1e9ebfc5923a6d1658385 drm-tip: 2017y-01m-12d-15h-39m-40s UTC 
integration manifest
eb09e09 drm/i915: Declare i915_gem_object_create_internal() as taking 
phys_addr_t size

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3502/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm: Add new DRM_IOCTL_MODE_GETPLANE2

[Daniel Stone]

Hi,

On 12 January 2017 at 14:56, Rob Clark  wrote:
> On Thu, Jan 12, 2017 at 4:38 AM, Ville Syrjälä
>  wrote:
>> Isn't an implicit offset enough? As in first mask for a specific
>> modifier is for format indexes 0-63, second mask for the same modifier
>> is for 64-127, and so on.
>
> hmm, hadn't thought of that approach.  Definitely if we go w/ implicit
> then we want to have userspace support from the get-go.  For explicit,
> I guess userspace could complain and ignore if it saw a non-zero
> offset similar to what we do w/ pad and unknown flags in the other
> direction?

Implicit is clever but horrible. AFAICT, the only way to do it
properly would be to have a nested forwards loop walk when you first
hit a modifier, searching for further occurrences of that modifier to
collect the complete set of formats that modifier applies to.
Depending on what you did with the structures, you'd either have to
destroy the drm_format_modifiers in the GetPlane return so further
instances of your outer loop didn't hit them, or have a _second_
nested loop walk into wherever you copied the formats/modifiers,
searching for anything with that.

Too clever by half, and everyone will get it wrong. Just add an explicit offset.

Cheers,
Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/huc: Support HuC authentication

[Chris Wilson]

On Thu, Jan 12, 2017 at 08:35:01AM -0800, Anusha Srivatsa wrote:
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index c6be352..f53ca8d 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -46,7 +46,7 @@ static bool intel_guc_recv(struct intel_guc *guc, u32 
> *status)
>  int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
>  {
>   struct drm_i915_private *dev_priv = guc_to_i915(guc);
> - u32 status;
> + u32 status = 0;
>   int i;
>   int ret;

This chunk is unrelated.

> +void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
> +{
> + struct intel_guc *guc = _priv->guc;
> + struct intel_huc *huc = _priv->huc;
> + struct i915_vma *vma;
> + int ret;
> + u32 data[2];
> +
> + vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
> + PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> + if (IS_ERR(vma)) {
> + DRM_DEBUG_DRIVER("failed to pin huc fw object %d\n",
> + (int)PTR_ERR(vma));
> + return;
> + }
> +
> + /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
> + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);

This invalidate will be redundant shortly.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/huc: Support HuC authentication

[Anusha Srivatsa]

From: Peter Antoine 

The HuC authentication is done by host2guc call. The HuC RSA keys
are sent to GuC for authentication.

v2: rebased on top of drm-intel-nightly.
changed name format and upped version 1.7.
v3: rebased on top of drm-intel-nightly.
v4: changed wait_for_automic to wait_for
v5: rebased.
v7: rebased.
v8: rebased.
v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc()
and place the prototype in intel_guc.h,correct the comments.
v10: rebased.
v11: rebased.
v12: rebased on top of drm-tip
v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c
to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc().
Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_
AUTHENTICATE_HUC
v14: rebased.
v15: rebased. Add newline on DRM_ERRORs that already dont have one.
v16: rebased. Replace wait_for with intel_wait_for_register() since
the latter employs sleep optimisations for quick responses- as pointed
out by Chris Wilson.
v17: rebased. Cleanup the intel_guc_auth_huc() by removing checks
already performed in earlier functions. Make comments more descriptive.
v18: rebased. Changed the bias for pinning the HuC object.Add return
values with errors.

Cc: Chris Wilson 
Cc: Arkadiusz Hiler 
Cc: Michal Wajdeczko 
Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   |  1 +
 drivers/gpu/drm/i915/intel_guc_loader.c |  2 ++
 drivers/gpu/drm/i915/intel_uc.c | 55 -
 drivers/gpu/drm/i915/intel_uc.h |  1 +
 4 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index ed1ab40..25691f0 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -505,6 +505,7 @@ enum intel_guc_action {
INTEL_GUC_ACTION_ENTER_S_STATE = 0x501,
INTEL_GUC_ACTION_EXIT_S_STATE = 0x502,
INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003,
+   INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000,
INTEL_GUC_ACTION_LIMIT
 };
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 861c157..c618d11 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -530,6 +530,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
intel_uc_fw_status_repr(guc_fw->fetch_status),
intel_uc_fw_status_repr(guc_fw->load_status));
 
+   intel_guc_auth_huc(dev_priv);
+
if (i915.enable_guc_submission) {
if (i915.guc_log_level >= 0)
gen9_enable_guc_interrupts(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index c6be352..8f95441 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -46,7 +46,6 @@ static bool intel_guc_recv(struct intel_guc *guc, u32 *status)
 int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
-   u32 status;
int i;
int ret;
 
@@ -140,3 +139,57 @@ int intel_guc_log_control(struct intel_guc *guc, u32 
control_val)
 
return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
+
+/**
+ * intel_guc_auth_huc() - authenticate ucode
+ * @dev_priv: the drm_i915_device
+ *
+ * Triggers a HuC fw authentication request to the GuC via intel_guc_action_
+ * authenticate_huc interface.
+ * interface.
+ */
+void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
+{
+   struct intel_guc *guc = _priv->guc;
+   struct intel_huc *huc = _priv->huc;
+   struct i915_vma *vma;
+   int ret;
+   u32 data[2];
+
+   vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
+   PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+   if (IS_ERR(vma)) {
+   DRM_DEBUG_DRIVER("failed to pin huc fw object %d\n",
+   (int)PTR_ERR(vma));
+   return;
+   }
+
+   /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
+   I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
+
+   /* Specify auth action and where public signature is. */
+   data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC;
+   data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset;
+
+   ret = intel_guc_send(guc, data, ARRAY_SIZE(data));
+   if (ret) {
+   DRM_ERROR("HuC: GuC did not ack Auth request: %d\n", ret);
+   goto out;
+   }
+
+   /* Check authentication status, it should be done by now */
+   ret = intel_wait_for_register(dev_priv,
+

Re: [Intel-gfx] [PATCH] drm: Don't race connector registration

[Chris Wilson]

On Thu, Jan 12, 2017 at 05:15:56PM +0100, Daniel Vetter wrote:
> I was under the misconception that the sysfs dev stuff can be fully
> set up, and then registered all in one step with device_add. That's
> true for properties and property groups, but not for parents and child
> devices. Those must be fully registered before you can register a
> child.
> 
> Add a bit of tracking to make sure that asynchronous mst connector
> hotplugging gets this right. For consistency we rely upon the implicit
> barriers of the connector->mutex, which is taken anyway, to ensure
> that at least either the connector or device registration call will
> work out.
> 
> Mildly tested since I can't reliably reproduce this on my mst box
> here.

Hmm, right it should prevent the oops on load. I think we need to
control the async dp-mst better and stop it running until we have the
device registered, and so if we use in the interim, plonk a WARN_ON on
top for -next and try and fix it for realz. (Probably a
drm_dp_mst_mgr_register() hook?)

Acked-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()

[Chris Wilson]

Parameter: good.
Parameter - bad.

One day I'll learn the syntax.

Fixes: 625d988acc28 ("drm/i915: Extract reserving space in the GTT to a helper")
Fixes: e007b19d7ba7 ("drm/i915: Use the MRU stack search after evicting")
Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 44 ++---
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0ed99adfd0da..43243eb3edc0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3557,16 +3557,16 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma)
 
 /**
  * i915_gem_gtt_reserve - reserve a node in an address_space (GTT)
- * @vm - the  i915_address_space
- * @node - the  drm_mm_node (typically i915_vma.mode)
- * @size - how much space to allocate inside the GTT,
- * must be #I915_GTT_PAGE_SIZE aligned
- * @offset - where to insert inside the GTT,
- *   must be #I915_GTT_MIN_ALIGNMENT aligned, and the node
- *   (@offset + @size) must fit within the address space
- * @color - color to apply to node, if this node is not from a VMA,
- *  color must be #I915_COLOR_UNEVICTABLE
- * @flags - control search and eviction behaviour
+ * @vm: the  i915_address_space
+ * @node: the  drm_mm_node (typically i915_vma.mode)
+ * @size: how much space to allocate inside the GTT,
+ *must be #I915_GTT_PAGE_SIZE aligned
+ * @offset: where to insert inside the GTT,
+ *  must be #I915_GTT_MIN_ALIGNMENT aligned, and the node
+ *  (@offset + @size) must fit within the address space
+ * @color: color to apply to node, if this node is not from a VMA,
+ * color must be #I915_COLOR_UNEVICTABLE
+ * @flags: control search and eviction behaviour
  *
  * i915_gem_gtt_reserve() tries to insert the @node at the exact @offset inside
  * the address space (using @size and @color). If the @node does not fit, it
@@ -3634,19 +3634,19 @@ static u64 random_offset(u64 start, u64 end, u64 len, 
u64 align)
 
 /**
  * i915_gem_gtt_insert - insert a node into an address_space (GTT)
- * @vm - the  i915_address_space
- * @node - the  drm_mm_node (typically i915_vma.node)
- * @size - how much space to allocate inside the GTT,
+ * @vm: the  i915_address_space
+ * @node: the  drm_mm_node (typically i915_vma.node)
+ * @size: how much space to allocate inside the GTT,
+ *must be #I915_GTT_PAGE_SIZE aligned
+ * @alignment: required alignment of starting offset, may be 0 but
+ * if specified, this must be a power-of-two and at least
+ * #I915_GTT_MIN_ALIGNMENT
+ * @color: color to apply to node
+ * @start: start of any range restriction inside GTT (0 for all),
  * must be #I915_GTT_PAGE_SIZE aligned
- * @alignment - required alignment of starting offset, may be 0 but
- *  if specified, this must be a power-of-two and at least
- *  #I915_GTT_MIN_ALIGNMENT
- * @color - color to apply to node
- * @start - start of any range restriction inside GTT (0 for all),
- *  must be #I915_GTT_PAGE_SIZE aligned
- * @end - end of any range restriction inside GTT (U64_MAX for all),
- *must be #I915_GTT_PAGE_SIZE aligned if not U64_MAX
- * @flags - control search and eviction behaviour
+ * @end: end of any range restriction inside GTT (U64_MAX for all),
+ *   must be #I915_GTT_PAGE_SIZE aligned if not U64_MAX
+ * @flags: control search and eviction behaviour
  *
  * i915_gem_gtt_insert() first searches for an available hole into which
  * is can insert the node. The hole address is aligned to @alignment and
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/huc: Add HuC fw loading support

[Anusha Srivatsa]

The HuC loading process is similar to GuC. The intel_uc_fw_fetch()
is used for both cases.

HuC loading needs to be before GuC loading. The WOPCM setting must
be done early before loading any of them.

v2: rebased on-top of drm-intel-nightly.
removed if(HAS_GUC()) before the guc call. (D.Gordon)
update huc_version number of format.
v3: rebased to drm-intel-nightly, changed the file name format to
match the one in the huc package.
Changed dev->dev_private to to_i915()
v4: moved function back to where it was.
change wait_for_atomic to wait_for.
v5: rebased + comment changes.
v7: rebased.
v8: rebased.
v9: rebased. Changed the year in the copyright message to reflect
the right year.Correct the comments,remove the unwanted WARN message,
replace drm_gem_object_unreference() with i915_gem_object_put().Make the
prototypes in intel_huc.h non-extern.
v10: rebased. Update the file construction done by HuC. It is similar to
GuC.Adopted the approach used in-
https://patchwork.freedesktop.org/patch/104355/ 
v11: Fix warnings remove old declaration
v12: Change dev to dev_priv in macro definition.
Corrected comments.
v13: rebased.
v14: rebased on top of drm-tip
v15: rebased. Updated functions intel_huc_load(),intel_huc_init() and
intel_uc_fw_fetch() to accept dev_priv instead of dev. Moved contents
of intel_huc.h to intel_uc.h
v16: change SKL_FW_ to SKL_HUC_FW_. Add intel_ prefix to guc_wopcm_size().
Remove unwanted checks in intel_uc.h. Rename huc_fw in struct intel_huc to
simply fw to avoid redundency.
v17: rebased.
v18: rebased. Correct comments.
v19: rebased. Correct comments. Make intel_huc_fini() accept dev_priv instead 
of dev
like intel_huc_init() and intel_huc_load().Move definition to i915_guc_reg.h 
from
intel_uc.h. Clean DMA_CTRL bits after HuC DMA transfer in huc_ucode_xfer()
instead of guc_ucode_xfer(). Add suitable WARNs to give extra info.
v20: rebased. Add proper bias for HuC and make sure there are
asserts on failure by using guc_ggtt_offset_vma()

Cc: Arkadiusz Hiler 
Cc: Michal Wajdeczko 
Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
---
 drivers/gpu/drm/i915/Makefile   |   1 +
 drivers/gpu/drm/i915/i915_drv.c |   3 +
 drivers/gpu/drm/i915/i915_drv.h |   2 +
 drivers/gpu/drm/i915/i915_guc_reg.h |   6 +
 drivers/gpu/drm/i915/intel_guc_loader.c |   7 +-
 drivers/gpu/drm/i915/intel_huc_loader.c | 265 
 drivers/gpu/drm/i915/intel_uc.h |  14 ++
 7 files changed, 295 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_huc_loader.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 5196509..45ae124 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -57,6 +57,7 @@ i915-y += i915_cmd_parser.o \
 # general-purpose microcontroller (GuC) support
 i915-y += intel_uc.o \
  intel_guc_loader.o \
+ intel_huc_loader.o \
  i915_guc_submission.o
 
 # autogenerated null render state
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4e5ea58..d7a0b49 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -599,6 +599,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
if (ret)
goto cleanup_irq;
 
+   intel_huc_init(dev_priv);
intel_guc_init(dev_priv);
 
ret = i915_gem_init(dev_priv);
@@ -627,6 +628,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
i915_gem_fini(dev_priv);
 cleanup_irq:
intel_guc_fini(dev_priv);
+   intel_huc_fini(dev_priv);
drm_irq_uninstall(dev);
intel_teardown_gmbus(dev_priv);
 cleanup_csr:
@@ -1314,6 +1316,7 @@ void i915_driver_unload(struct drm_device *dev)
drain_workqueue(dev_priv->wq);
 
intel_guc_fini(dev_priv);
+   intel_huc_fini(dev_priv);
i915_gem_fini(dev_priv);
intel_fbc_cleanup_cfb(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b84c1d1..2a17df2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2073,6 +2073,7 @@ struct drm_i915_private {
 
struct intel_gvt *gvt;
 
+   struct intel_huc huc;
struct intel_guc guc;
 
struct intel_csr csr;
@@ -2847,6 +2848,7 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define HAS_GUC(dev_priv)  ((dev_priv)->info.has_guc)
 #define HAS_GUC_UCODE(dev_priv)(HAS_GUC(dev_priv))
 #define HAS_GUC_SCHED(dev_priv)(HAS_GUC(dev_priv))
+#define HAS_HUC_UCODE(dev_priv)(HAS_GUC(dev_priv))
 
 #define HAS_RESOURCE_STREAMER(dev_priv) 
((dev_priv)->info.has_resource_streamer)
 
diff --git a/drivers/gpu/drm/i915/i915_guc_reg.h 

[Intel-gfx] [PATCH] drm/i915/huc: Support HuC authentication

[Anusha Srivatsa]

From: Peter Antoine 

The HuC authentication is done by host2guc call. The HuC RSA keys
are sent to GuC for authentication.

v2: rebased on top of drm-intel-nightly.
changed name format and upped version 1.7.
v3: rebased on top of drm-intel-nightly.
v4: changed wait_for_automic to wait_for
v5: rebased.
v7: rebased.
v8: rebased.
v9: rebased. Rename intel_huc_auh() to intel_guc_auth_huc()
and place the prototype in intel_guc.h,correct the comments.
v10: rebased.
v11: rebased.
v12: rebased on top of drm-tip
v13: rebased. Moved intel_guc_auth_huc from i915_guc_submission.c
to intel_uc.c.Update dev to dev_priv in intel_guc_auth_huc().
Renamed HOST2GUC_ACTION_AUTHENTICATE_HUC TO INTEL_GUC_ACTION_
AUTHENTICATE_HUC
v14: rebased.
v15: rebased. Add newline on DRM_ERRORs that already dont have one.
v16: rebased. Replace wait_for with intel_wait_for_register() since
the latter employs sleep optimisations for quick responses- as pointed
out by Chris Wilson.
v17: rebased. Cleanup the intel_guc_auth_huc() by removing checks
already performed in earlier functions. Make comments more descriptive.
v18: rebased. Changed the bias for pinning the HuC object.

Cc: Chris Wilson 
Cc: Arkadiusz Hiler 
Cc: Michal Wajdeczko 
Tested-by: Xiang Haihao 
Signed-off-by: Anusha Srivatsa 
Signed-off-by: Alex Dai 
Signed-off-by: Peter Antoine 
---
 drivers/gpu/drm/i915/intel_guc_fwif.h   |  1 +
 drivers/gpu/drm/i915/intel_guc_loader.c |  2 ++
 drivers/gpu/drm/i915/intel_uc.c | 56 -
 drivers/gpu/drm/i915/intel_uc.h |  1 +
 4 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index ed1ab40..25691f0 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -505,6 +505,7 @@ enum intel_guc_action {
INTEL_GUC_ACTION_ENTER_S_STATE = 0x501,
INTEL_GUC_ACTION_EXIT_S_STATE = 0x502,
INTEL_GUC_ACTION_SLPC_REQUEST = 0x3003,
+   INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000,
INTEL_GUC_ACTION_UK_LOG_ENABLE_LOGGING = 0x0E000,
INTEL_GUC_ACTION_LIMIT
 };
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c 
b/drivers/gpu/drm/i915/intel_guc_loader.c
index 861c157..c618d11 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -530,6 +530,8 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
intel_uc_fw_status_repr(guc_fw->fetch_status),
intel_uc_fw_status_repr(guc_fw->load_status));
 
+   intel_guc_auth_huc(dev_priv);
+
if (i915.enable_guc_submission) {
if (i915.guc_log_level >= 0)
gen9_enable_guc_interrupts(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index c6be352..f53ca8d 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -46,7 +46,7 @@ static bool intel_guc_recv(struct intel_guc *guc, u32 *status)
 int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
struct drm_i915_private *dev_priv = guc_to_i915(guc);
-   u32 status;
+   u32 status = 0;
int i;
int ret;
 
@@ -140,3 +140,57 @@ int intel_guc_log_control(struct intel_guc *guc, u32 
control_val)
 
return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
+
+/**
+ * intel_guc_auth_huc() - authenticate ucode
+ * @dev_priv: the drm_i915_device
+ *
+ * Triggers a HuC fw authentication request to the GuC via intel_guc_action_
+ * authenticate_huc interface.
+ * interface.
+ */
+void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
+{
+   struct intel_guc *guc = _priv->guc;
+   struct intel_huc *huc = _priv->huc;
+   struct i915_vma *vma;
+   int ret;
+   u32 data[2];
+
+   vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
+   PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+   if (IS_ERR(vma)) {
+   DRM_DEBUG_DRIVER("failed to pin huc fw object %d\n",
+   (int)PTR_ERR(vma));
+   return;
+   }
+
+   /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
+   I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
+
+   /* Specify auth action and where public signature is. */
+   data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC;
+   data[1] = i915_ggtt_offset(vma) + huc->fw.rsa_offset;
+
+   ret = intel_guc_send(guc, data, ARRAY_SIZE(data));
+   if (ret) {
+   DRM_ERROR("HuC: GuC did not ack Auth request\n");
+   goto out;
+   }
+
+   /* Check authentication status, it should be done by now */
+   ret = intel_wait_for_register(dev_priv,
+   

Re: [Intel-gfx] [PATCH i-g-t rfc 01/29] lib/igt_debugfs: Prevent buffer overflow

[Robert Foss]

On 2017-01-12 04:14 AM, Lankhorst, Maarten wrote:

Robert Foss schreef op wo 11-01-2017 om 15:41 [-0500]:

buf array may overflow with when writing '\0' if
MAX_LINE_LEN bytes are read during read().

How?

char buf[MAX_LINE_LEN + 1];


I actually missed the + 1, but parts of the commit are still relevant 
though, as the errno at least in theory could be != EAGAIN.


So I'd like to keep the below check, to prevent compiler warnings.
if (bytes_read < 0)

Sounds ok?


Rob.



Signed-off-by: Robert Foss 
---
 lib/igt_debugfs.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index d828687a..8b8a627a 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -594,13 +594,15 @@ static int read_crc(igt_pipe_crc_t *pipe_crc,
igt_crc_t *out)
read_len = MAX_LINE_LEN;

igt_set_timeout(5, "CRC reading");
-   bytes_read = read(pipe_crc->crc_fd, , read_len);
+   bytes_read = read(pipe_crc->crc_fd, , read_len - 1);
igt_reset_timeout();

-   if (bytes_read < 0 && errno == EAGAIN) {
+   if (bytes_read < 0 && errno == EAGAIN)
igt_assert(pipe_crc->flags & O_NONBLOCK);
+
+   if (bytes_read < 0)
bytes_read = 0;
-   }
+
buf[bytes_read] = '\0';

if (bytes_read && !pipe_crc_init_from_string(pipe_crc, out,
buf))

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm: Don't race connector registration

[Daniel Vetter]

On Thu, Jan 12, 2017 at 05:15:56PM +0100, Daniel Vetter wrote:
> I was under the misconception that the sysfs dev stuff can be fully
> set up, and then registered all in one step with device_add. That's
> true for properties and property groups, but not for parents and child
> devices. Those must be fully registered before you can register a
> child.
> 
> Add a bit of tracking to make sure that asynchronous mst connector
> hotplugging gets this right. For consistency we rely upon the implicit
> barriers of the connector->mutex, which is taken anyway, to ensure
> that at least either the connector or device registration call will
> work out.
> 
> Mildly tested since I can't reliably reproduce this on my mst box
> here.
> 
> Reported-by: Dave Hansen 
> Cc: Dave Hansen 
> Cc: Chris Wilson 
> Signed-off-by: Daniel Vetter 

Cc: sta...@vger.kernel.org

> ---
>  drivers/gpu/drm/drm_connector.c | 3 +++
>  drivers/gpu/drm/drm_drv.c   | 4 
>  include/drm/drmP.h  | 1 +
>  3 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index c75ab242f907..5999cb83d05b 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -378,6 +378,9 @@ int drm_connector_register(struct drm_connector 
> *connector)
>  {
>   int ret = 0;
>  
> + if (!connector->dev->registered)
> + return 0;
> +
>   mutex_lock(>mutex);
>   if (connector->registered)
>   goto unlock;
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7e24103c47f1..cad6df626678 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -749,6 +749,8 @@ int drm_dev_register(struct drm_device *dev, unsigned 
> long flags)
>   if (ret)
>   goto err_minors;
>  
> + dev->registered = true;
> +
>   if (dev->driver->load) {
>   ret = dev->driver->load(dev, flags);
>   if (ret)
> @@ -796,6 +798,8 @@ void drm_dev_unregister(struct drm_device *dev)
>  
>   drm_lastclose(dev);
>  
> + dev->registered = false;
> +
>   if (drm_core_check_feature(dev, DRIVER_MODESET))
>   drm_modeset_unregister_all(dev);
>  
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index c537c278a4be..ec105c339347 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -518,6 +518,7 @@ struct drm_device {
>   struct drm_minor *control;  /**< Control node */
>   struct drm_minor *primary;  /**< Primary node */
>   struct drm_minor *render;   /**< Render node */
> + bool registered;
>  
>   /* currently active master for this device. Protected by master_mutex */
>   struct drm_master *master;
> -- 
> 2.5.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm: Don't race connector registration

[Daniel Vetter]

I was under the misconception that the sysfs dev stuff can be fully
set up, and then registered all in one step with device_add. That's
true for properties and property groups, but not for parents and child
devices. Those must be fully registered before you can register a
child.

Add a bit of tracking to make sure that asynchronous mst connector
hotplugging gets this right. For consistency we rely upon the implicit
barriers of the connector->mutex, which is taken anyway, to ensure
that at least either the connector or device registration call will
work out.

Mildly tested since I can't reliably reproduce this on my mst box
here.

Reported-by: Dave Hansen 
Cc: Dave Hansen 
Cc: Chris Wilson 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_connector.c | 3 +++
 drivers/gpu/drm/drm_drv.c   | 4 
 include/drm/drmP.h  | 1 +
 3 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index c75ab242f907..5999cb83d05b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -378,6 +378,9 @@ int drm_connector_register(struct drm_connector *connector)
 {
int ret = 0;
 
+   if (!connector->dev->registered)
+   return 0;
+
mutex_lock(>mutex);
if (connector->registered)
goto unlock;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 7e24103c47f1..cad6df626678 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -749,6 +749,8 @@ int drm_dev_register(struct drm_device *dev, unsigned long 
flags)
if (ret)
goto err_minors;
 
+   dev->registered = true;
+
if (dev->driver->load) {
ret = dev->driver->load(dev, flags);
if (ret)
@@ -796,6 +798,8 @@ void drm_dev_unregister(struct drm_device *dev)
 
drm_lastclose(dev);
 
+   dev->registered = false;
+
if (drm_core_check_feature(dev, DRIVER_MODESET))
drm_modeset_unregister_all(dev);
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c537c278a4be..ec105c339347 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -518,6 +518,7 @@ struct drm_device {
struct drm_minor *control;  /**< Control node */
struct drm_minor *primary;  /**< Primary node */
struct drm_minor *render;   /**< Render node */
+   bool registered;
 
/* currently active master for this device. Protected by master_mutex */
struct drm_master *master;
-- 
2.5.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC PATCH 2/3] drm/i915: IOMMU based SVM implementation v16

[Jesse Barnes]

On Jan 12, 2017 8:04 AM, "Chris Wilson"  wrote:

On Thu, Jan 12, 2017 at 05:48:49PM +0200, Mika Kuoppala wrote:
> Chris Wilson  writes:
>
> > On Mon, Jan 09, 2017 at 06:52:53PM +0200, Mika Kuoppala wrote:
> >> +static int i915_gem_context_enable_svm(struct i915_gem_context *ctx)
> >> +{
> >> +  int ret;
> >> +
> >> +  if (!HAS_SVM(ctx->i915))
> >> +  return -ENODEV;
> >
> > How does legacy execbuf work with an svm context? It will write the
> > ppgtt, but those are no longer read by the GPU. So it will generate
> > faults at random addresses. Am I right in thinking we need to EINVAL if
> > using execbuf + context_is_svm?
>
> Yes without further experiments, it is best to block the legacy path
> with -EINVAL. I will add this.
>
> I guess with some tweaking the legacy interface could be made to work,
> but it would need is_svm_context() checks in rather many places
> in the execbuffer path to avoid relocations/pins.

Hmm. right. Basically we have to ignore all objects if svm. Basically we
strip off everything (having to EINVAL if passed in relocs etc) and more
or less call exec_svm. The advantage is that it keeps the request tracking
of the objects correct, but it can only work with softpinning of the
objects at their cpu addresses. (I don't propose we map the object in
the cpu table at the ppgtt offset!)

Anyway the decision has to be made upfront whether we want to support
the frankenapi.


FWIW I  think the Beignet team wanted this functionality to make their
implementation easier. It's a bit more invasive but might be worth it for
userspace to make their transition easier.

Jesse


-Chris

--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC PATCH 2/3] drm/i915: IOMMU based SVM implementation v16

[Chris Wilson]

On Thu, Jan 12, 2017 at 05:48:49PM +0200, Mika Kuoppala wrote:
> Chris Wilson  writes:
> 
> > On Mon, Jan 09, 2017 at 06:52:53PM +0200, Mika Kuoppala wrote:
> >> +static int i915_gem_context_enable_svm(struct i915_gem_context *ctx)
> >> +{
> >> +  int ret;
> >> +
> >> +  if (!HAS_SVM(ctx->i915))
> >> +  return -ENODEV;
> >
> > How does legacy execbuf work with an svm context? It will write the
> > ppgtt, but those are no longer read by the GPU. So it will generate
> > faults at random addresses. Am I right in thinking we need to EINVAL if
> > using execbuf + context_is_svm?
> 
> Yes without further experiments, it is best to block the legacy path
> with -EINVAL. I will add this.
> 
> I guess with some tweaking the legacy interface could be made to work,
> but it would need is_svm_context() checks in rather many places
> in the execbuffer path to avoid relocations/pins.

Hmm. right. Basically we have to ignore all objects if svm. Basically we
strip off everything (having to EINVAL if passed in relocs etc) and more
or less call exec_svm. The advantage is that it keeps the request tracking
of the objects correct, but it can only work with softpinning of the
objects at their cpu addresses. (I don't propose we map the object in
the cpu table at the ppgtt offset!)

Anyway the decision has to be made upfront whether we want to support
the frankenapi.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Add name for WaDisablePWMClockGating workaround

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Add name for 
WaDisablePWMClockGating workaround
URL   : https://patchwork.freedesktop.org/series/17904/
State : success

== Summary ==

Series 17904v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/17904/revisions/1/mbox/


fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hqtotal:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

beb34e26b0349bb8190f6c51d534ba5bf4611cf0 drm-tip: 2017y-01m-12d-14h-11m-49s UTC 
integration manifest
6f38f83 drm/i915/glk: Turn on workarounds that apply to Geminilake too
3b5b7a08 drm/i915: Apply WaSetHdcUnitClockGatingDisableInUcgctl6 only until B0
0205597 drm/i915: Add name for WaDisablePWMClockGating workaround

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3501/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC PATCH 2/3] drm/i915: IOMMU based SVM implementation v16

[Mika Kuoppala]

Chris Wilson  writes:

> On Mon, Jan 09, 2017 at 06:52:53PM +0200, Mika Kuoppala wrote:
>> +static int i915_gem_context_enable_svm(struct i915_gem_context *ctx)
>> +{
>> +int ret;
>> +
>> +if (!HAS_SVM(ctx->i915))
>> +return -ENODEV;
>
> How does legacy execbuf work with an svm context? It will write the
> ppgtt, but those are no longer read by the GPU. So it will generate
> faults at random addresses. Am I right in thinking we need to EINVAL if
> using execbuf + context_is_svm?

Yes without further experiments, it is best to block the legacy path
with -EINVAL. I will add this.

I guess with some tweaking the legacy interface could be made to work,
but it would need is_svm_context() checks in rather many places
in the execbuffer path to avoid relocations/pins.

-Mika

> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/huc: Add HuC fw loading support

[Srivatsa, Anusha]

>-Original Message-
>From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
>Sent: Wednesday, January 11, 2017 6:24 AM
>To: Wajdeczko, Michal 
>Cc: Srivatsa, Anusha ; intel-
>g...@lists.freedesktop.org; Alex Dai ; Peter Antoine
>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915/huc: Add HuC fw loading support
>
>On Wed, Jan 11, 2017 at 03:13:29PM +0100, Michal Wajdeczko wrote:
>> > +  vma = i915_gem_object_ggtt_pin(huc_fw->obj, NULL, 0, 0, 0);
>> > +  if (IS_ERR(vma)) {
>> > +  DRM_DEBUG_DRIVER("pin failed %d\n", (int)PTR_ERR(vma));
>> > +  return PTR_ERR(vma);
>> > +  }
>
>Just asking a stupid question: Does the HuC have the same limitation as the GuC
>on not being able to map certain ranges of the GuC? From the earlier discussion
>on the failures, I got the impression the HuC had the same limitations.

Hi Chris. I confirmed that HuC does have the limitations.
>> > +
>> > +  /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
>> > +  I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
>> > +
>> > +  intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>> > +
>> > +  /* init WOPCM */
>> > +  I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
>> > +  I915_WRITE(DMA_GUC_WOPCM_OFFSET,
>GUC_WOPCM_OFFSET_VALUE |
>> > +  HUC_LOADING_AGENT_GUC);
>> > +
>> > +  /* Set the source address for the uCode */
>> > +  offset = i915_ggtt_offset(vma) + huc_fw->header_offset;
>
>If huc does have the same limits as the guc, please use guc_ggtt_offset() for 
>the
>extra verification on the address before use.

Yes. Thanks a lot for bringing this up.
>-Chris
>
>--
>Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Expand ggtt_view paramters for debugfs (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Expand ggtt_view paramters for debugfs (rev3)
URL   : https://patchwork.freedesktop.org/series/17896/
State : success

== Summary ==

Series 17896v3 drm/i915: Expand ggtt_view paramters for debugfs
https://patchwork.freedesktop.org/api/1.0/series/17896/revisions/3/mbox/


fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hqtotal:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

beb34e26b0349bb8190f6c51d534ba5bf4611cf0 drm-tip: 2017y-01m-12d-14h-11m-49s UTC 
integration manifest
c4fd9d7 drm/i915: Expand ggtt_view paramters for debugfs

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3500/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/6] drm/i915: Compact memcmp in i915_vma_compare()

[Chris Wilson]

On Thu, Jan 12, 2017 at 03:00:20PM +, Tvrtko Ursulin wrote:
> 
> On 12/01/2017 11:33, Chris Wilson wrote:
> >On Wed, Jan 11, 2017 at 09:51:03PM +, Chris Wilson wrote:
> >>In preparation for the next patch to convert to using an anonymous union
> >>and leaving the excess bytes in the union uninitialised, we first need
> >>to make sure we do not compare using those uninitialised bytes. We also
> >>want to preserve the compactness of the code, avoiding a second call to
> >>memcmp or introducing a switch, so we take advantage of using the type
> >>as an encoded size (as well as a unique identifier for each type of view).
> >>
> >>Signed-off-by: Chris Wilson 
> >>---
> >> drivers/gpu/drm/i915/i915_gem_gtt.h | 14 +++---
> >> drivers/gpu/drm/i915/i915_vma.h | 15 +--
> >> 2 files changed, 16 insertions(+), 13 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
> >>b/drivers/gpu/drm/i915/i915_gem_gtt.h
> >>index 3187a260e6e1..36d85599ffc9 100644
> >>--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> >>+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> >>@@ -145,12 +145,6 @@ typedef uint64_t gen8_ppgtt_pml4e_t;
> >>
> >> struct sg_table;
> >>
> >>-enum i915_ggtt_view_type {
> >>-   I915_GGTT_VIEW_NORMAL = 0,
> >>-   I915_GGTT_VIEW_ROTATED,
> >>-   I915_GGTT_VIEW_PARTIAL,
> >>-};
> >>-
> >> struct intel_rotation_info {
> >>struct intel_rotation_plane_info {
> >>/* tiles */
> >>@@ -184,10 +178,16 @@ static inline u64 intel_partial_get_page_offset(const 
> >>struct intel_partial_info
> >>return pi->offset_size >> INTEL_PARTIAL_SIZE_BITS;
> >> }
> >>
> >>+enum i915_ggtt_view_type {
> >>+   I915_GGTT_VIEW_NORMAL = 0,
> >>+   I915_GGTT_VIEW_ROTATED = sizeof(struct intel_rotation_info),
> >>+   I915_GGTT_VIEW_PARTIAL = sizeof(struct intel_partial_info),
> >
> >One alternative to packing intel_partial_info into a u64 is to use
> >
> >I915_GGTT_VIEW_PARTIAL = 12, /* ideal sizeof(struct intel_partial_info) */
> >
> >How horrible is that?
> 
> Really horrible, nightmare to debug. :)
> 
> You are happy with 12 packed bytes I gather so no need for it?

I'd prefer the u64, but currently baking:

 struct intel_partial_info {
u64 offset;
unsigned int size;
-};
+} __packed;
+
+static inline void assert_intel_partial_info_is_packed(void)
+{
+   BUILD_BUG_ON(sizeof(struct intel_partial_info) != sizeof(u64) + 
sizeof(unsigned int));
+}

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/6] drm/i915: Compact memcmp in i915_vma_compare()

[Tvrtko Ursulin]

On 12/01/2017 11:33, Chris Wilson wrote:

On Wed, Jan 11, 2017 at 09:51:03PM +, Chris Wilson wrote:

In preparation for the next patch to convert to using an anonymous union
and leaving the excess bytes in the union uninitialised, we first need
to make sure we do not compare using those uninitialised bytes. We also
want to preserve the compactness of the code, avoiding a second call to
memcmp or introducing a switch, so we take advantage of using the type
as an encoded size (as well as a unique identifier for each type of view).

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem_gtt.h | 14 +++---
 drivers/gpu/drm/i915/i915_vma.h | 15 +--
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h 
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 3187a260e6e1..36d85599ffc9 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -145,12 +145,6 @@ typedef uint64_t gen8_ppgtt_pml4e_t;

 struct sg_table;

-enum i915_ggtt_view_type {
-   I915_GGTT_VIEW_NORMAL = 0,
-   I915_GGTT_VIEW_ROTATED,
-   I915_GGTT_VIEW_PARTIAL,
-};
-
 struct intel_rotation_info {
struct intel_rotation_plane_info {
/* tiles */
@@ -184,10 +178,16 @@ static inline u64 intel_partial_get_page_offset(const 
struct intel_partial_info
return pi->offset_size >> INTEL_PARTIAL_SIZE_BITS;
 }

+enum i915_ggtt_view_type {
+   I915_GGTT_VIEW_NORMAL = 0,
+   I915_GGTT_VIEW_ROTATED = sizeof(struct intel_rotation_info),
+   I915_GGTT_VIEW_PARTIAL = sizeof(struct intel_partial_info),


One alternative to packing intel_partial_info into a u64 is to use

I915_GGTT_VIEW_PARTIAL = 12, /* ideal sizeof(struct intel_partial_info) */

How horrible is that?


Really horrible, nightmare to debug. :)

You are happy with 12 packed bytes I gather so no need for it?

Regards,

Tvrtko

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm: Add new DRM_IOCTL_MODE_GETPLANE2

[Rob Clark]

On Thu, Jan 12, 2017 at 4:38 AM, Ville Syrjälä
 wrote:
> On Wed, Jan 11, 2017 at 08:43:16PM -0500, Rob Clark wrote:
>> On Wed, Jan 11, 2017 at 7:51 PM, Ben Widawsky  wrote:
>> >
>> > +struct drm_format_modifier {
>> > +   /* Bitmask of formats in get_plane format list this info
>> > +* applies to. */
>> > +   uint64_t formats;
>>
>> re: the uabi, I'd suggest to at least make this 'u32 offset; u32
>> formats'.. we can keep the existing implementation in this patch and
>> always set 'offset' to zero, and let the first one to hit more than 32
>> formats deal with the implementation.  (Maybe a strategically placed
>> WARN_ON() if you go that route..)
>
> Isn't an implicit offset enough? As in first mask for a specific
> modifier is for format indexes 0-63, second mask for the same modifier
> is for 64-127, and so on.

hmm, hadn't thought of that approach.  Definitely if we go w/ implicit
then we want to have userspace support from the get-go.  For explicit,
I guess userspace could complain and ignore if it saw a non-zero
offset similar to what we do w/ pad and unknown flags in the other
direction?

Not sure if that would fly or not..  I guess it is not a *critical*
fail, it just means userspace won't realize that some modifiers are
supported on some formats.. otoh the implicit approach could confuse a
userspace that didn't realize the offset into thinking modifiers
*were* supported on formats where they are not.. that seems like a
bigger problem.

BR,
-R

> The bigger issue is the userspace side I think. If we don't add the
> userspace side code to handle this case from the get go, it's going to
> be hard to actually start doing it from the kernel side.
>
>>
>> Otherwise I guess it is just a couple years until getplane3 ;-)
>>
>> BR,
>> -R
>>
>> > +
>> > +   /* This modifier can be used with the format for this plane. */
>> > +   uint64_t modifier;
>> > +};
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Remove useless casts to intel_plane_state

[Maarten Lankhorst]

Op 12-01-17 om 10:58 schreef Ville Syrjälä:
> On Thu, Jan 12, 2017 at 10:43:45AM +0100, Maarten Lankhorst wrote:
>> The visible member used to be in intel_plane_state->visible,
>> but has been moved to drm_plane_state->visible. In the conversion
>> some casts were left in that are now useless.
>>
>> to_intel_plane_state(x)->base.visible is the same as x->visible,
>> so use the latter to clear up the code a little.
> I would generally prefer switching over to intel_foo_state all over.
>
> But patch looks sane anyway
> Reviewed-by: Ville Syrjälä 
Applied, thanks for reviewing. :)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/37] drm/i915: Add unit tests for the breadcrumb rbtree, wakeups

[Chris Wilson]

On Thu, Jan 12, 2017 at 11:11:20AM +, Tvrtko Ursulin wrote:
> 
> On 11/01/2017 21:09, Chris Wilson wrote:
> >Third retroactive test, make sure that the seqno waiters are woken.
> 
> There are some open questions from the previous round (early
> December), not least of which is that I think we really need good
> comments in tests. Especially ones like this one which is pretty
> advanced. We need an overall description and some commentary on the
> stages. It is not straightforward to reverse engineer this, so doing
> it every time this code will need some attention for one reason or
> the other will be a PITA.

I added that comment!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 2/5] drm/edid: Introduce drm_default_rgb_quant_range()

[Ville Syrjälä]

On Thu, Jan 12, 2017 at 11:29:18AM +0200, Jani Nikula wrote:
> On Wed, 11 Jan 2017, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> >
> > Make the code selecting the RGB quantization range a little less magicy
> > by wrapping it up in a small helper.
> >
> > v2: s/adjusted_mode/mode in vc4 to make it actually compile
> >
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/drm_edid.c| 18 ++
> >  drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
> >  drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
> >  drivers/gpu/drm/vc4/vc4_hdmi.c|  4 +++-
> >  include/drm/drm_edid.h|  2 ++
> >  5 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 4ff04aa84dd0..304c583b8000 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid 
> > *edid)
> >  }
> >  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
> >  
> > +/**
> > + * drm_default_rgb_quant_range - default RGB quantization range
> > + * @mode: display mode
> > + *
> > + * Determine the default RGB quantization range for the mode,
> > + * as specified in CEA-861.
> > + *
> > + * Return: The default RGB quantization range for the mode
> > + */
> > +enum hdmi_quantization_range
> > +drm_default_rgb_quant_range(const struct drm_display_mode *mode)
> > +{
> > +   return drm_match_cea_mode(mode) > 1 ?
> > +   HDMI_QUANTIZATION_RANGE_LIMITED :
> > +   HDMI_QUANTIZATION_RANGE_FULL;
> > +}
> > +EXPORT_SYMBOL(drm_default_rgb_quant_range);
> 
> Or just bool drm_default_to_limited_range() or similar?

That's what I had initially, but then I thought it might be better to
start moving towards something a bit more explicit everwhere. But I
stopped short of actually making the drivers store the enum in their
state. We might want to do that, I think.

> 
> *shrug*
> 
> Reviewed-by: Jani Nikula 
> 
> 
> 
> > +
> >  static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
> >const u8 *hdmi)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 343e1d9fa761..d4befbbe834a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1713,7 +1713,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
> >  */
> > pipe_config->limited_color_range =
> > -   bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1;
> > +   bpp != 18 &&
> > +   drm_default_rgb_quant_range(adjusted_mode) ==
> > +   HDMI_QUANTIZATION_RANGE_LIMITED;
> > } else {
> > pipe_config->limited_color_range =
> > intel_dp->limited_color_range;
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 0bcfead14571..19bd13f53729 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1330,7 +1330,8 @@ bool intel_hdmi_compute_config(struct intel_encoder 
> > *encoder,
> > /* See CEA-861-E - 5.1 Default Encoding Parameters */
> > pipe_config->limited_color_range =
> > pipe_config->has_hdmi_sink &&
> > -   drm_match_cea_mode(adjusted_mode) > 1;
> > +   drm_default_rgb_quant_range(adjusted_mode) ==
> > +   HDMI_QUANTIZATION_RANGE_LIMITED;
> > } else {
> > pipe_config->limited_color_range =
> > intel_hdmi->limited_color_range;
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index c4cb2e26de32..5d49bf948162 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -463,7 +463,9 @@ static void vc4_hdmi_encoder_mode_set(struct 
> > drm_encoder *encoder,
> > csc_ctl = VC4_SET_FIELD(VC4_HD_CSC_CTL_ORDER_BGR,
> > VC4_HD_CSC_CTL_ORDER);
> >  
> > -   if (vc4_encoder->hdmi_monitor && drm_match_cea_mode(mode) > 1) {
> > +   if (vc4_encoder->hdmi_monitor &&
> > +   drm_default_rgb_quant_range(mode) ==
> > +   HDMI_QUANTIZATION_RANGE_LIMITED) {
> > /* CEA VICs other than #1 requre limited range RGB
> >  * output unless overridden by an AVI infoframe.
> >  * Apply a colorspace conversion to squash 0-255 down
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index 838eaf2b42e9..25cdf5f7a0d8 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -441,6 +441,8 @@ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const 
> > u8 video_code);
> >  bool 

[Intel-gfx] [PULL] drm-intel-fixes

[Jani Nikula]

Hi Dave -

Mostly GVT-g fixes, with a couple of other fixes from Chris.

BR,
Jani.

The following changes since commit a121103c922847ba5010819a3f250f1f7fc84ab8:

  Linux 4.10-rc3 (2017-01-08 14:18:17 -0800)

are available in the git repository at:

  git://anongit.freedesktop.org/git/drm-intel tags/drm-intel-fixes-2017-01-12

for you to fetch changes up to e4621b73b6b472fe2b434b4f0f76b8f33ee26a73:

  drm/i915: Fix phys pwrite for struct_mutex-less operation (2017-01-12 
10:15:44 +0200)


Changbin Du (5):
  drm/i915/gvt: fix error handing of tlb_control emulation
  drm/i915/gvt: fix return value in mul_force_wake_write
  drm/i915/gvt: always use readq and writeq
  drm/i915/gvt: fix use after free for workload
  drm/i915/gvt: dec vgpu->running_workload_num after the workload is really 
done

Chris Wilson (2):
  drm/i915: Clear ret before unbinding in i915_gem_evict_something()
  drm/i915: Fix phys pwrite for struct_mutex-less operation

Jani Nikula (1):
  Merge tag 'gvt-fixes-2017-01-10' of https://github.com/01org/gvt-linux 
into drm-intel-fixes

Jike Song (5):
  drm/i915/gvt: init/destroy vgpu_idr properly
  drm/i915/gvt: destroy the allocated idr on vgpu creating failures
  drm/i915/gvt: cleanup opregion memory allocation code
  drm/i915/gvt/kvmgt: return meaningful error for vgpu creating failure
  drm/i915/gvt: cleanup GFP flags

Nicolas Iooss (1):
  drm/i915/gvt: verify functions types in new_mmio_info()

Pei Zhang (1):
  drm/i915/gvt: print correct value for untracked mmio

Zhenyu Wang (2):
  drm/i915/gvt: adjust high memory size for default vGPU type
  drm/i915/gvt: remove duplicated definition

 drivers/gpu/drm/i915/gvt/aperture_gm.c |  7 -
 drivers/gpu/drm/i915/gvt/gtt.c | 54 +++---
 drivers/gpu/drm/i915/gvt/gvt.c |  8 -
 drivers/gpu/drm/i915/gvt/handlers.c| 13 
 drivers/gpu/drm/i915/gvt/kvmgt.c   | 14 ++---
 drivers/gpu/drm/i915/gvt/mmio.c| 31 +--
 drivers/gpu/drm/i915/gvt/opregion.c|  8 ++---
 drivers/gpu/drm/i915/gvt/reg.h |  3 +-
 drivers/gpu/drm/i915/gvt/scheduler.c   | 14 -
 drivers/gpu/drm/i915/gvt/vgpu.c|  8 +++--
 drivers/gpu/drm/i915/i915_gem.c| 34 +++--
 drivers/gpu/drm/i915/i915_gem_evict.c  |  1 +
 12 files changed, 78 insertions(+), 117 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/10] drm/i915: Update i915_reset parameter for kerneldoc

[Mika Kuoppala]

Michel Thierry  writes:

> Since commit c033666a94b57 ("drm/i915: Store a i915 backpointer from
> engine, and use it") i915_reset receives dev_priv, but the kerneldoc
> was not updated.
>
> Signed-off-by: Michel Thierry 

Pushed patches 01/10 and 02/10. Thanks for patches and review.
-Mika

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index aefab9a1a68e..4e5ea5898e06 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1746,7 +1746,7 @@ static void enable_engines_irq(struct drm_i915_private 
> *dev_priv)
>  
>  /**
>   * i915_reset - reset chip after a hang
> - * @dev: drm device to reset
> + * @dev_priv: device private to reset
>   *
>   * Reset the chip.  Useful if a hang is detected. Marks the device as wedged
>   * on failure.
> -- 
> 2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/6] drm/i915: Pack the partial view size and offset into a single u64

[Chris Wilson]

On Wed, Jan 11, 2017 at 09:51:02PM +, Chris Wilson wrote:
> Since the partial offset must be page aligned, we can use those low 12
> bits to encode the size of the partial view (which then cannot be larger
> than 8MiB in pages). A requirement for avoiding unused bits inside the
> struct is imposed later by avoiding the clear of the struct (or of
> copying around static initialisers). This is easier to guarantee by
> manual packing into a single u64 - the presence of the u64 inside a
> struct causes gcc to pad the struct out to a u64 boundary, even if we
> use the __packed attribute.

I made a mistake in my testing. :|

I assked gcc whether struct {u64; u32;} was the same size as a u64.
If we ask gcc whether struct {u64; u32;} is 12 bytes, all we need is the
__packed attribute.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 28/37] drm/i915: Exercising filling the top/bottom portions of the global GTT

[Joonas Lahtinen]

On ke, 2017-01-11 at 21:09 +, Chris Wilson wrote:
> Same test as previously for the per-process GTT instead applied to the
> global GTT.
> 
> Signed-off-by: Chris Wilson 



> @@ -218,28 +209,94 @@ static int igt_ppgtt_fill(void *arg)
>  
>   list_for_each_entry_safe(obj, on, , batch_pool_link) {
>   list_del(>batch_pool_link);
> - vma = vma_lookup(obj, >base);
> - if (!IS_ERR(vma))
> + vma = vma_lookup(obj, vm);
> + if (!IS_ERR(vma) && !i915_vma_is_ggtt(vma))

Reasoning is worthy a commenting here as discussed in IRC.

>   i915_vma_close(vma);
>  
>   i915_gem_object_put(obj);
>   }
>   }
> -err:



> +static int igt_ggtt_fill(void *arg)
> +{



> + mutex_lock(>drm.struct_mutex);
> + drm_mm_for_each_hole(node, >base.mm, this_start, this_end) {
> + u64 this_size;
> +
> + if (ggtt->base.mm.color_adjust)
> + ggtt->base. mm.color_adjust(node, 0,

You somehow managed to add space --^ 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [CI,1/2] drm/i915: Invalidate the guc ggtt TLB upon insertion

[Chris Wilson]

On Thu, Jan 12, 2017 at 01:53:40PM -, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [CI,1/2] drm/i915: Invalidate the guc ggtt TLB 
> upon insertion
> URL   : https://patchwork.freedesktop.org/series/17898/
> State : failure
> 
> == Summary ==
> 
> Series 17898v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/17898/revisions/1/mbox/
> 
> Test drv_module_reload:
> Subgroup basic-reload-final:
> pass   -> INCOMPLETE (fi-skl-6770hq)

That's a critical but unrelated issue. Looks like a window of
opportunity between submitting the request to the hw and it being
completed before it is signaled. Hmm.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCHv2] drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround.

[Mika Kuoppala]

Francisco Jerez  writes:

> Daniel Vetter  writes:
>
>> On Wed, Jan 11, 2017 at 12:24:59PM +, Chris Wilson wrote:
>>> On Wed, Jan 11, 2017 at 02:07:37PM +0200, Mika Kuoppala wrote:
>>> > Daniel Vetter  writes:
>>> > 
>>> > > On Mon, Jan 09, 2017 at 01:07:56PM -0800, Francisco Jerez wrote:
>>> > >> The WaDisableLSQCROPERFforOCL workaround has the side effect of
>>> > >> disabling an L3SQ optimization that has huge performance implications
>>> > >> and is unlikely to be necessary for the correct functioning of usual
>>> > >> graphic workloads.  Userspace is free to re-enable the workaround on
>>> > >> demand, and is generally in a better position to determine whether the
>>> > >> workaround is necessary than the DRM is (e.g. only during the
>>> > >> execution of compute kernels that rely on both L3 fences and HDC R/W
>>> > >> requests).
>>> > >> 
>>> > >> The same workaround seems to apply to BDW (at least to production
>>> > >> stepping G1) and SKL as well (the internal workaround database claims
>>> > >> that it does for all steppings, while the BSpec workaround table only
>>> > >> mentions pre-production steppings), but the DRM doesn't do anything
>>> > >> beyond whitelisting the L3SQCREG4 register so userspace can enable it
>>> > >> when it sees fit.  Do the same on KBL platforms.
>>> > >> 
>>> > >> Improves performance of the GFXBench4 gl_manhattan31 benchmark by 60%,
>>> > >> and gl_4 (AKA car chase) by 14% on a KBL GT2 running Mesa master --
>>> > >> This is followed by a regression of 35% and 10% respectively for the
>>> > >> same benchmarks and platform caused by my recent patch series
>>> > >> switching userspace to use the dataport constant cache instead of the
>>> > >> sampler to implement uniform pull constant loads, which caused us to
>>> > >> hit more heavily the L3 cache (and on platforms other than KBL had the
>>> > >> opposite effect of improving performance of the same two benchmarks).
>>> > >> The overall effect on KBL of this change combined with the recent
>>> > >> userspace change is respectively 4.6% and 2.6%.  SynMark2 OglShMapPcf
>>> > >> was affected by the constant cache changes (though it improved as it
>>> > >> did on other platforms rather than regressing), but is not
>>> > >> significantly affected by this patch (with statistical significance of
>>> > >> 5% and sample size 20).
>>> > >> 
>>> > >> v2: Drop some more code to avoid unused variable warning.
>>> > >> 
>>> > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99256
>>> > >> Signed-off-by: Francisco Jerez 
>>> > >> Cc: Eero Tamminen 
>>> > >> Cc: Jani Nikula 
>>> > >> Cc: Mika Kuoppala 
>>> > >> Cc: beig...@lists.freedesktop.org
>>> > >
>>> > > Don't we need some userspace flag/opt-in scheme to avoid stuff going 
>>> > > boom
>>> > > for compute kernels? Are the patches for mesa compute/beignet
>>> > > ready?
>>> > 
>>> > This is explicit setting on kbl/E0 only. So one could argue
>>> > that unless they filter based on PCI-IDs, things would already
>>> > blow up across the skl/kbl population, if they forgot
>>> > to set it. The whitelisting is in place and looks sane
>>> > so this E0 exception is a wart that got in by me reading wa
>>> > database slavishly without thinking.
>>> 
>>> Add Fixes then?
>>
>> Yeah, cc: stable would be good to make sure it shows up in all supported
>> kernels, fast. Otherwise we'll get some good wtf bug reports.
>
> Agreed -- It would be nice for this to get to stable kernel branches.
>

Added Fixes and stable tags and pushed to drm-intel-next-queued.

Thanks for patch,
-Mika

>> -Daniel
>> -- 
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [CI,1/2] drm/i915: Invalidate the guc ggtt TLB upon insertion

[Patchwork]

== Series Details ==

Series: series starting with [CI,1/2] drm/i915: Invalidate the guc ggtt TLB 
upon insertion
URL   : https://patchwork.freedesktop.org/series/17898/
State : failure

== Summary ==

Series 17898v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/17898/revisions/1/mbox/

Test drv_module_reload:
Subgroup basic-reload-final:
pass   -> INCOMPLETE (fi-skl-6770hq)

fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hqtotal:8pass:7dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

43b23e65d1802ffaa1811a3ee958b0e9e0aba10e drm-tip: 2017y-01m-12d-10h-54m-07s UTC 
integration manifest
6e6f301 HAX enable guc submission for CI
4b1aae7 drm/i915: Invalidate the guc ggtt TLB upon insertion

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3499/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 1/5] tests/kms_plane_multiple: Add TEST_ONLY flag

[Maarten Lankhorst]

Op 30-12-16 om 13:00 schreef Mika Kahola:
> Add TEST_ONLY flag to test atomic modesetting commits without
> actual real-life commit.
>
> Signed-off-by: Mika Kahola 
> ---
>  tests/kms_plane_multiple.c | 79 
> --
>  1 file changed, 49 insertions(+), 30 deletions(-)
>
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index 5e12be4..1a77a38 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -61,10 +61,12 @@ struct {
>   int iterations;
>   bool user_seed;
>   int seed;
> + bool test_only;
>  } opt = {
>   .iterations = 64,
>   .user_seed = false,
>   .seed = 1,
> + .test_only = false,
>  };
>  
>  static inline uint32_t pipe_select(int pipe)
> @@ -228,7 +230,7 @@ prepare_planes(data_t *data, enum pipe pipe, color_t 
> *color,
>  static void
>  test_atomic_plane_position_with_output(data_t *data, enum pipe pipe,
>  igt_output_t *output, int max_planes,
> -uint64_t tiling)
> +uint64_t tiling, bool test_only)
>  {
>   char buf[256];
>   struct drm_event *e = (void *)buf;
> @@ -240,6 +242,12 @@ test_atomic_plane_position_with_output(data_t *data, 
> enum pipe pipe,
>   int iterations = opt.iterations < 1 ? 1 : opt.iterations;
>   bool loop_forever;
>   char info[256];
> + int flags = DRM_MODE_ATOMIC_ALLOW_MODESET;
> +
> + if (test_only)
> + flags |= DRM_MODE_ATOMIC_TEST_ONLY;
> + else
> + flags |= DRM_MODE_PAGE_FLIP_EVENT;
I would only pass ALLOW_MODESET for the TEST_ONLY case.
>   if (opt.iterations == LOOP_FOREVER) {
>   loop_forever = true;
> @@ -256,8 +264,9 @@ test_atomic_plane_position_with_output(data_t *data, enum 
> pipe pipe,
>  
>   test_init(data, pipe);
>  
> - test_grab_crc(data, output, pipe, true, , tiling,
> -   _crc);
> + if (!test_only)
> + test_grab_crc(data, output, pipe, true, , tiling,
> +   _crc);
You still want to do some kind of pipe setup, else the state of the pipe will 
depend on what was set before it.

I'm not sure what you want the state to be though.
>   i = 0;
>   while (i < iterations || loop_forever) {
> @@ -265,24 +274,27 @@ test_atomic_plane_position_with_output(data_t *data, 
> enum pipe pipe,
>  
>   vblank_start = get_vblank(data->display.drm_fd, pipe, 
> DRM_VBLANK_NEXTONMISS);
>  
> - igt_display_commit_atomic(>display,
> -   DRM_MODE_PAGE_FLIP_EVENT,
> -   >display);
> + ret = igt_display_try_commit_atomic(>display,
> + flags,
> + >display);
> + igt_assert(ret != -EINVAL);
Other failures are fine? I would leave igt_display_commit_atomic here, it 
catches errors for you. :)
> - igt_set_timeout(1, "Stuck on page flip");
> + if (!test_only) {
> + igt_set_timeout(1, "Stuck on page flip");
>  
> - ret = read(data->display.drm_fd, buf, sizeof(buf));
> - igt_assert(ret >= 0);
> + ret = read(data->display.drm_fd, buf, sizeof(buf));
> + igt_assert(ret >= 0);
>  
> - igt_assert_eq(get_vblank(data->display.drm_fd, pipe, 0), 
> vblank_start + 1);
> - igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE);
> - igt_reset_timeout();
> + igt_assert_eq(get_vblank(data->display.drm_fd, pipe, 
> 0), vblank_start + 1);
> + igt_assert_eq(e->type, DRM_EVENT_FLIP_COMPLETE);
> + igt_reset_timeout();
>  
> - n = igt_pipe_crc_get_crcs(data->pipe_crc, MAX_CRCS, );
> + n = igt_pipe_crc_get_crcs(data->pipe_crc, MAX_CRCS, 
> );
>  
> - igt_assert_eq(n, MAX_CRCS);
> + igt_assert_eq(n, MAX_CRCS);
>  
> - igt_assert_crc_equal(_crc, crc);
> + igt_assert_crc_equal(_crc, crc);
> + }
>  
>   i++;
>   }
> @@ -345,7 +357,7 @@ test_legacy_plane_position_with_output(data_t *data, enum 
> pipe pipe,
>  
>  static void
>  test_plane_position(data_t *data, enum pipe pipe, bool atomic, int 
> max_planes,
> - uint64_t tiling)
> + uint64_t tiling, bool test_only)
>  {
>   igt_output_t *output;
>   int connected_outs;
> @@ -372,7 +384,8 @@ test_plane_position(data_t *data, enum pipe pipe, bool 
> atomic, int max_planes,
>   test_atomic_plane_position_with_output(data, pipe,
>  output,
>  max_planes,
> -  

Re: [Intel-gfx] [PATCH v2] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size

[Joonas Lahtinen]

On to, 2017-01-12 at 13:04 +, Chris Wilson wrote:
> The internal object is a collection of struct pages and so is
> intrinsically linked to the available physical memory on the machine,
> and not an arbitrary type from the uabi. Use phys_addr_t so the link
> between size and memory consumption is clear, and then double check that
> we don't overflow the maximum object size.
> 
> v2: Also assert that size is not zero - a mistake I made a few times
> while writing selftests.
> 
> Signed-off-by: Chris Wilson 
> Cc: Joonas Lahtinen 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 27/37] drm/i915: Exercising filling the top/bottom portions of the ppgtt

[Joonas Lahtinen]

On ke, 2017-01-11 at 21:09 +, Chris Wilson wrote:
> Allocate objects with varying number of pages (which should hopefully
> consist of a mixture of contiguous page chunks and so coalesced sg
> lists) and check that the sg walkers in insert_pages cope.
> 
> Signed-off-by: Chris Wilson 



>  struct drm_i915_gem_object *
>  i915_gem_object_create_internal(struct drm_i915_private *dev_priv,
> - unsigned int size);
> + unsigned long size);

As discussed in IRC, phys_size_t would be more documenting why it
deviates from u64.
 
> +static struct i915_vma *vma_lookup(struct drm_i915_gem_object *obj,
> +    struct i915_address_space *vm)
> +{
> + return i915_gem_obj_lookup_or_create_vma(obj, vm, NULL);
> +}

How about finally renaming the original function itself? 'vma_lookup'
is tad confusing, maybe 'vma_instance'? Lookup has strong implication
that it'll just look for existance. The name could be even better.

> +
> +static int igt_ppgtt_fill(void *arg)
> +{
> + struct drm_i915_private *dev_priv = arg;
> + unsigned long npages, max_pages = 1 << 20, prime;

Naww, assignment to it's own line (or rather, embed it to the min_t
expression). If you plan on making it a parameter, do so :P

> + struct drm_i915_gem_object *obj, *on;
> + struct i915_hw_ppgtt *ppgtt;
> + struct i915_vma *vma;
> + LIST_HEAD(objects);
> + int err = 0;
> +
> + if (!USES_FULL_PPGTT(dev_priv))
> + return 0;

This calls for return -ENXIO (or something else we're not using under
DRM and i915) and handling in the callchain.

> +
> + mutex_lock(_priv->drm.struct_mutex);
> + ppgtt = i915_ppgtt_create(dev_priv, NULL, "mock");
> + if (IS_ERR(ppgtt)) {
> + err = PTR_ERR(ppgtt);
> + goto err_unlock;
> + }
> + GEM_BUG_ON(ppgtt->base.total & ~PAGE_MASK);

IS_ALIGNED or offset_in_page

> + for_each_prime_number_from(prime, 2, 13) {
> + for (npages = 1; npages <= max_pages; npages *= prime) {
> + u64 flags;
> +
> + GEM_BUG_ON(!npages);
> + obj = huge_gem_object(dev_priv,
> +   PAGE_SIZE,
> +   npages << PAGE_SHIFT);
> + if (IS_ERR(obj))
> + break;
> +
> + list_add(>batch_pool_link, );

Urgh... anonymous union?

> +
> + /* Fill the GTT top down - hope we don't overstep the 
> end */
> + flags = ppgtt->base.total | PIN_OFFSET_FIXED | PIN_USER;
> + list_for_each_entry(obj, , batch_pool_link) {
> + vma = vma_lookup(obj, >base);
> + if (IS_ERR(vma))
> + continue;
> +

GEM_BUG_ON(flags & I915_GTT_PAGE_SIZE < obj->base.size);

> + flags -= obj->base.size;
> + err = i915_vma_pin(vma, 0, 0, flags);
> + if (err) {
> + pr_err("Fill top-down failed with 
> err=%d on size=%lu pages (prime=%lu)\n", err, npages, prime);

Alternatively dump flags in here so it's obvious from log.

> + goto err;
> + }
> +
> + i915_vma_unpin(vma);
> + }
> +
> + flags = ppgtt->base.total | PIN_OFFSET_FIXED | PIN_USER;
> + list_for_each_entry(obj, , batch_pool_link) {
> + vma = vma_lookup(obj, >base);
> + if (IS_ERR(vma))
> + continue;
> +
> + flags -= obj->base.size;
> + if (!drm_mm_node_allocated(>node) ||
> + i915_vma_misplaced(vma, 0, 0, flags)) {
> + pr_err("Fill top-down moved 
> vma.node=%llx + %llx, expected offset %llx\n",
> +    vma->node.start, vma->node.size,
> +    flags & PAGE_MASK);
> + err = -EINVAL;
> + goto err;
> + }
> +
> + err = i915_vma_unbind(vma);
> + if (err) {
> + pr_err("Fill top-down unbind of 
> vma.node=%llx + %llx failed with err=%d\n",
> +    vma->node.start, vma->node.size,
> +    err);
> + goto err;
> + }
> + }
> +

Maybe convert above into traditional phase[] array to dedup code 

Re: [Intel-gfx] [PATCH 09/37] drm/i915: Mock infrastructure for request emission

[Chris Wilson]

On Thu, Jan 12, 2017 at 01:11:50PM +, Tvrtko Ursulin wrote:
> 
> On 11/01/2017 21:09, Chris Wilson wrote:
> >+struct i915_gem_context *
> >+mock_context(struct drm_i915_private *i915,
> >+ const char *name)
> >+{
> >+struct i915_gem_context *ctx;
> >+int ret;
> >+
> >+ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> >+if (!ctx)
> >+return NULL;
> >+
> >+kref_init(>ref);
> >+INIT_LIST_HEAD(>link);
> >+ctx->name = name ? kstrdup(name, GFP_KERNEL) : NULL;
> 
> Care or not whether allocation worked?

Not really, ctx->name is a hint. But not checking is just asking for
trouble later.

> >+static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
> >+{
> >+const unsigned long sz = roundup_pow_of_two(sizeof(struct intel_ring));
> 
> You certainly like your longs. Never mind. :)

We have a habit of growing our structs. Consider this future proofing
;)

> >+struct intel_ring *ring;
> >+
> >+ring = kzalloc(sizeof(*ring) + sz, GFP_TEMPORARY);
> 
> Why GFP_TEMPORARY, the mocked context & co are not?

Depends on the phase I was writing the patch. The first tests were fast,
and so GFP_TEMPORARY seemed appropriate, and I used it everywhere I
remembered. With some tests being deliberately slow and others
deliberately using all memory, GFP_TEMPORARY seems more like misuse.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround. (rev2)
URL   : https://patchwork.freedesktop.org/series/17659/
State : success

== Summary ==

Series 17659v2 drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround.
https://patchwork.freedesktop.org/api/1.0/series/17659/revisions/2/mbox/


fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hqtotal:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

43b23e65d1802ffaa1811a3ee958b0e9e0aba10e drm-tip: 2017y-01m-12d-10h-54m-07s UTC 
integration manifest
650dc62 drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3498/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/37] drm/i915: Mock infrastructure for request emission

[Tvrtko Ursulin]

On 11/01/2017 21:09, Chris Wilson wrote:

Create a fake engine that runs requests using a timer to simulate hw.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem_context.c|   4 +
 drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c |  11 +-
 drivers/gpu/drm/i915/selftests/mock_context.c  |  70 +
 drivers/gpu/drm/i915/selftests/mock_context.h  |  34 
 drivers/gpu/drm/i915/selftests/mock_engine.c   | 172 +++--
 drivers/gpu/drm/i915/selftests/mock_engine.h   |  18 ++-
 drivers/gpu/drm/i915/selftests/mock_gem_device.c   |  95 +++-
 drivers/gpu/drm/i915/selftests/mock_gem_device.h   |   1 +
 drivers/gpu/drm/i915/selftests/mock_request.c  |  44 ++
 drivers/gpu/drm/i915/selftests/mock_request.h  |  44 ++
 10 files changed, 475 insertions(+), 18 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/selftests/mock_context.c
 create mode 100644 drivers/gpu/drm/i915/selftests/mock_context.h
 create mode 100644 drivers/gpu/drm/i915/selftests/mock_request.c
 create mode 100644 drivers/gpu/drm/i915/selftests/mock_request.h

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index fbd3b8ecbe20..91551b01a62c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -1185,3 +1185,7 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device 
*dev,

return 0;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftests/mock_context.c"
+#endif
diff --git a/drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c 
b/drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c
index bee86470a91d..2742103247c0 100644
--- a/drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c
@@ -25,6 +25,7 @@
 #include "i915_random.h"
 #include "i915_selftest.h"

+#include "mock_gem_device.h"
 #include "mock_engine.h"

 static int check_rbtree(struct intel_engine_cs *engine,
@@ -440,15 +441,15 @@ int intel_breadcrumbs_mock_selftests(void)
SUBTEST(igt_insert_complete),
SUBTEST(igt_wakeup),
};
-   struct intel_engine_cs *engine;
+   struct drm_i915_private *i915;
int err;

-   engine = mock_engine("mock");
-   if (!engine)
+   i915 = mock_gem_device();
+   if (!i915)
return -ENOMEM;

-   err = i915_subtests(tests, engine);
-   kfree(engine);
+   err = i915_subtests(tests, i915->engine[RCS]);
+   drm_dev_unref(>drm);

return err;
 }
diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c 
b/drivers/gpu/drm/i915/selftests/mock_context.c
new file mode 100644
index ..5098dbbc81d5
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/mock_context.c
@@ -0,0 +1,70 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "mock_context.h"
+#include "mock_gtt.h"
+
+struct i915_gem_context *
+mock_context(struct drm_i915_private *i915,
+const char *name)
+{
+   struct i915_gem_context *ctx;
+   int ret;
+
+   ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+   if (!ctx)
+   return NULL;
+
+   kref_init(>ref);
+   INIT_LIST_HEAD(>link);
+   ctx->name = name ? kstrdup(name, GFP_KERNEL) : NULL;


Care or not whether allocation worked?


+   ctx->i915 = i915;
+
+   ret = ida_simple_get(>context_hw_ida,
+0, MAX_CONTEXT_HW_ID, GFP_KERNEL);
+   if (ret < 0) {


if (name && name != ctx->name)
kfree(ctx->name);


+   kfree(ctx);
+   return NULL;
+   }
+   ctx->hw_id = ret;
+
+   if (name) {
+   ctx->ppgtt = mock_ppgtt(i915, name);
+   if (!ctx->ppgtt) {


Ditto.


+   kfree(ctx);
+ 

Re: [Intel-gfx] [PATCH 03/10] drm/i915: Update i915.reset to handle engine resets

[Chris Wilson]

On Thu, Jan 12, 2017 at 02:22:21PM +0200, Joonas Lahtinen wrote:
> On ke, 2017-01-11 at 20:18 -0800, Michel Thierry wrote:
> > From: Arun Siluvery 
> > 
> > In preparation for engine reset work update this parameter to handle more
> > than one type of reset. Default at the moment is still full gpu reset.
> > 
> > Cc: Chris Wilson 
> > Cc: Mika Kuoppala 
> > Signed-off-by: Arun Siluvery 
> > Signed-off-by: Michel Thierry 
> 
> 
> 
> > @@ -113,8 +113,8 @@ MODULE_PARM_DESC(vbt_sdvo_panel_type,
> > >   "Override/Ignore selection of SDVO panel mode in the VBT "
> > >   "(-2=ignore, -1=auto [default], index in VBT BIOS table)");
> >  
> > -module_param_named_unsafe(reset, i915.reset, bool, 0600);
> > -MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
> > +module_param_named_unsafe(reset, i915.reset, int, 0600);
> > +MODULE_PARM_DESC(reset, "Attempt GPU resets (0=disabled, 1=full gpu reset 
> > [default], 2=engine reset)");
> 
> Would it make sense to have this as bitflags? So you could disable full
> GPU reset but still enable engine reset?

As it stands from the code currently, not really. The per-engine reset
(conceptually) does the same operations as the full reset, just under an
engine mask. If we have issues with a global reset, those should plague
per-engine reset as well.

I would like to keep the user options to a minimum. If our only usecase
for this parameter is testing, let's develop those as kselftests.
Though disabling reset will still be desired to w/a the occasional hw
death.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size

[Chris Wilson]

The internal object is a collection of struct pages and so is
intrinsically linked to the available physical memory on the machine,
and not an arbitrary type from the uabi. Use phys_addr_t so the link
between size and memory consumption is clear, and then double check that
we don't overflow the maximum object size.

v2: Also assert that size is not zero - a mistake I made a few times
while writing selftests.

Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/i915_drv.h  | 2 +-
 drivers/gpu/drm/i915/i915_gem_internal.c | 7 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b84c1d1fa12c..d8635d2af7ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3506,7 +3506,7 @@ i915_gem_object_create_stolen_for_preallocated(struct 
drm_i915_private *dev_priv
 /* i915_gem_internal.c */
 struct drm_i915_gem_object *
 i915_gem_object_create_internal(struct drm_i915_private *dev_priv,
-   unsigned int size);
+   phys_addr_t size);
 
 /* i915_gem_shrinker.c */
 unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem_internal.c 
b/drivers/gpu/drm/i915/i915_gem_internal.c
index 863e505f..9b39472396ef 100644
--- a/drivers/gpu/drm/i915/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/i915_gem_internal.c
@@ -151,10 +151,15 @@ static const struct drm_i915_gem_object_ops 
i915_gem_object_internal_ops = {
  */
 struct drm_i915_gem_object *
 i915_gem_object_create_internal(struct drm_i915_private *i915,
-   unsigned int size)
+   phys_addr_t size)
 {
struct drm_i915_gem_object *obj;
 
+   GEM_BUG_ON(!size);
+
+   if (overflows_type(size, obj->base.size))
+   return ERR_PTR(-E2BIG);
+
obj = i915_gem_object_alloc(i915);
if (!obj)
return ERR_PTR(-ENOMEM);
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size

[Chris Wilson]

The internal object is a collection of struct pages and so is
intrinsically linked to the available physical memory on the machine,
and not an arbitrary type from the uabi. Use phys_addr_t so the link
between size and memory consumption is clear, and then double check that
we don't overflow the maximum object size.

Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
---
 drivers/gpu/drm/i915/i915_drv.h  | 2 +-
 drivers/gpu/drm/i915/i915_gem_internal.c | 5 -
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b84c1d1fa12c..d8635d2af7ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3506,7 +3506,7 @@ i915_gem_object_create_stolen_for_preallocated(struct 
drm_i915_private *dev_priv
 /* i915_gem_internal.c */
 struct drm_i915_gem_object *
 i915_gem_object_create_internal(struct drm_i915_private *dev_priv,
-   unsigned int size);
+   phys_addr_t size);
 
 /* i915_gem_shrinker.c */
 unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_gem_internal.c 
b/drivers/gpu/drm/i915/i915_gem_internal.c
index 863e505f..9fd3d94e0e9f 100644
--- a/drivers/gpu/drm/i915/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/i915_gem_internal.c
@@ -151,10 +151,13 @@ static const struct drm_i915_gem_object_ops 
i915_gem_object_internal_ops = {
  */
 struct drm_i915_gem_object *
 i915_gem_object_create_internal(struct drm_i915_private *i915,
-   unsigned int size)
+   phys_addr_t size)
 {
struct drm_i915_gem_object *obj;
 
+   if (overflows_type(size, obj->base.size))
+   return ERR_PTR(-E2BIG);
+
obj = i915_gem_object_alloc(i915);
if (!obj)
return ERR_PTR(-ENOMEM);
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Remove useless casts to intel_plane_state

[Patchwork]

== Series Details ==

Series: drm/i915: Remove useless casts to intel_plane_state
URL   : https://patchwork.freedesktop.org/series/17894/
State : success

== Summary ==

Series 17894v1 drm/i915: Remove useless casts to intel_plane_state
https://patchwork.freedesktop.org/api/1.0/series/17894/revisions/1/mbox/


fi-bdw-5557u total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050 total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205 total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700 total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900 total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820 total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770  total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770  total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hqtotal:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hqtotal:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600  total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

43b23e65d1802ffaa1811a3ee958b0e9e0aba10e drm-tip: 2017y-01m-12d-10h-54m-07s UTC 
integration manifest
5d6bfb8 drm/i915: Remove useless casts to intel_plane_state

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3497/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 26/37] drm/i915: Assert that we have allocated the drm_mm_node upon pinning

[Joonas Lahtinen]

On ke, 2017-01-11 at 21:09 +, Chris Wilson wrote:
> We currently check after the slow path that the vma is bound correctly,
> but we don't currently check after the fast path. This is important in
> case we accidentally take the fast path and leave the vma misplaced.
> 
> Signed-off-by: Chris Wilson 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 25/37] drm/i915: Move i915_ppgtt_close() into i915_gem_gtt.c

[Joonas Lahtinen]

On ke, 2017-01-11 at 21:09 +, Chris Wilson wrote:
> Move it alongside its ppgtt counterparts, in order to make it available
> for the ppgtt selftests.
> 
> Signed-off-by: Chris Wilson 

Positive side-effects.

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/10] drm/i915: Update i915.reset to handle engine resets

[Joonas Lahtinen]

On ke, 2017-01-11 at 20:18 -0800, Michel Thierry wrote:
> From: Arun Siluvery 
> 
> In preparation for engine reset work update this parameter to handle more
> than one type of reset. Default at the moment is still full gpu reset.
> 
> Cc: Chris Wilson 
> Cc: Mika Kuoppala 
> Signed-off-by: Arun Siluvery 
> Signed-off-by: Michel Thierry 



> @@ -113,8 +113,8 @@ MODULE_PARM_DESC(vbt_sdvo_panel_type,
> >     "Override/Ignore selection of SDVO panel mode in the VBT "
> >     "(-2=ignore, -1=auto [default], index in VBT BIOS table)");
>  
> -module_param_named_unsafe(reset, i915.reset, bool, 0600);
> -MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)");
> +module_param_named_unsafe(reset, i915.reset, int, 0600);
> +MODULE_PARM_DESC(reset, "Attempt GPU resets (0=disabled, 1=full gpu reset 
> [default], 2=engine reset)");

Would it make sense to have this as bitflags? So you could disable full
GPU reset but still enable engine reset?

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] tests/kms_plane_multiple: Change number of iterations

[Kahola, Mika]

> -Original Message-
> From: Chris Wilson [mailto:ch...@chris-wilson.co.uk]
> Sent: Thursday, January 12, 2017 2:00 PM
> To: Maarten Lankhorst 
> Cc: Kahola, Mika ; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH i-g-t] tests/kms_plane_multiple: Change number
> of iterations
> 
> On Thu, Jan 12, 2017 at 12:53:30PM +0100, Maarten Lankhorst wrote:
> > Op 12-01-17 om 12:12 schreef Mika Kahola:
> > > In CI system, the default 64 iterations of this test may cause CRC
> > > overflow warnings in dmesg when debugfs is enabled in kernel config.
> > > To keep dmesg warning noise in minimum, let's run this test only once by
> default.
> > >
> > > Signed-off-by: Mika Kahola 
> > > ---
> > >  tests/kms_plane_multiple.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> > > index 5e12be4..b94ec36 100644
> > > --- a/tests/kms_plane_multiple.c
> > > +++ b/tests/kms_plane_multiple.c
> > > @@ -62,7 +62,7 @@ struct {
> > >   bool user_seed;
> > >   int seed;
> > >  } opt = {
> > > - .iterations = 64,
> > > + .iterations = 1,
> > >   .user_seed = false,
> > >   .seed = 1,
> > >  };
> >
> > Reviewed-by: Maarten Lankhorst 
> >
> > Though such small changes don't need review. :)
> 
> I am also intrigued as to what a "CRC overflow warning" is and why userspace
> needs to take remedial action to hide the warning.
From CI runs you may see these errors

[drm:display_pipe_crc_irq_handler [i915]] *ERROR* CRC buffer overflowing

For a real fix, should we increase the size of ringbuffer where crc's are 
stored or in the kms_plane_multiple test wait for a while before running the 
next iteration?

> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/37] drm/i915: Simple selftest to exercise live requests

[Chris Wilson]

On Thu, Jan 12, 2017 at 12:10:08PM +, Tvrtko Ursulin wrote:
> 
> On 11/01/2017 21:09, Chris Wilson wrote:
> >Just create several batches of requests and expect it to not fall over!
> >
> >Signed-off-by: Chris Wilson 
> >---
> > drivers/gpu/drm/i915/selftests/i915_gem_request.c  | 78 
> > ++
> > .../gpu/drm/i915/selftests/i915_live_selftests.h   |  1 +
> > 2 files changed, 79 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_request.c 
> >b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
> >index f348f5f81351..63e69d360764 100644
> >--- a/drivers/gpu/drm/i915/selftests/i915_gem_request.c
> >+++ b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
> >@@ -22,6 +22,8 @@
> >  *
> >  */
> >
> >+#include 
> >+
> > #include "i915_selftest.h"
> >
> > #include "mock_gem_device.h"
> >@@ -155,3 +157,79 @@ int i915_gem_request_mock_selftests(void)
> >
> > return err;
> > }
> >+
> >+static int live_nop_request(void *arg)
> >+{
> >+I915_SELFTEST_TIMEOUT(end_time);
> >+struct drm_i915_private *i915 = arg;
> >+struct drm_i915_gem_request *request;
> >+unsigned int reset_count = i915_reset_count(>gpu_error);
> >+unsigned long n, prime;
> >+ktime_t times[2] = {};
> >+int err;
> >+
> >+mutex_lock(>drm.struct_mutex);
> >+
> >+err = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED);
> >+if (err) {
> >+pr_err("Failed to idle GPU before %s\n", __func__);
> >+goto out_unlock;
> >+}
> >+
> >+i915->gpu_error.missed_irq_rings = 0;
> >+
> >+for_each_prime_number_from(prime, 1, 8192) {
> 
> Feels like an overuse of primes. Wouldn't an exponential sequence
> make more sense here? Like 1, 10, 100, 1000, if not even 100 would
> be enough.

Heh, everything is a nail right! I do like testing with primes though,
since powers-of-two are frequently optimised (I don't claim we should
hit any on this path). It just feels more likely to find issues by
avoiding testing with "simple" values, and the goal is find bugs in next
year's patches so serendipity rules.
 
> Also since it is a live test, might be cool to loop over the engines.

Good idea.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 14/37] drm/i915: Simple selftest to exercise live requests

[Tvrtko Ursulin]

On 11/01/2017 21:09, Chris Wilson wrote:

Just create several batches of requests and expect it to not fall over!

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/selftests/i915_gem_request.c  | 78 ++
 .../gpu/drm/i915/selftests/i915_live_selftests.h   |  1 +
 2 files changed, 79 insertions(+)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_request.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
index f348f5f81351..63e69d360764 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_request.c
@@ -22,6 +22,8 @@
  *
  */

+#include 
+
 #include "i915_selftest.h"

 #include "mock_gem_device.h"
@@ -155,3 +157,79 @@ int i915_gem_request_mock_selftests(void)

return err;
 }
+
+static int live_nop_request(void *arg)
+{
+   I915_SELFTEST_TIMEOUT(end_time);
+   struct drm_i915_private *i915 = arg;
+   struct drm_i915_gem_request *request;
+   unsigned int reset_count = i915_reset_count(>gpu_error);
+   unsigned long n, prime;
+   ktime_t times[2] = {};
+   int err;
+
+   mutex_lock(>drm.struct_mutex);
+
+   err = i915_gem_wait_for_idle(i915, I915_WAIT_LOCKED);
+   if (err) {
+   pr_err("Failed to idle GPU before %s\n", __func__);
+   goto out_unlock;
+   }
+
+   i915->gpu_error.missed_irq_rings = 0;
+
+   for_each_prime_number_from(prime, 1, 8192) {


Feels like an overuse of primes. Wouldn't an exponential sequence make 
more sense here? Like 1, 10, 100, 1000, if not even 100 would be enough.


Also since it is a live test, might be cool to loop over the engines.


+   times[1] = ktime_get_raw();
+
+   for (n = 0; n < prime; n++) {
+   request = i915_gem_request_alloc(i915->engine[RCS],
+i915->kernel_context);
+   if (IS_ERR(request)) {
+   err = PTR_ERR(request);
+   goto out_unlock;
+   }
+
+   i915_add_request(request);
+   }
+   i915_wait_request(request, I915_WAIT_LOCKED, 
MAX_SCHEDULE_TIMEOUT);
+   times[1] = ktime_sub(ktime_get_raw(), times[1]);
+   if (prime == 1)
+   times[0] = times[1];
+
+   if (time_after(jiffies, end_time)) {
+   pr_warn("%s timed out: last batch size %lu\n",
+   __func__, prime);
+   break;
+   }
+   }
+
+   if (reset_count != i915_reset_count(>gpu_error)) {
+   pr_err("GPU was reset %d times!\n",
+  i915_reset_count(>gpu_error) - reset_count);
+   err = -EIO;
+   goto out_unlock;
+   }
+
+   if (i915->gpu_error.missed_irq_rings) {
+   pr_err("Missed interrupts on rings %lx\n",
+  i915->gpu_error.missed_irq_rings);
+   err = -EIO;
+   goto out_unlock;
+   }
+
+   pr_info("Request latencies: 1 = %lluns, %lu = %lluns\n",
+  ktime_to_ns(times[0]),
+  prime, div64_u64(ktime_to_ns(times[1]), prime));
+
+out_unlock:
+   mutex_unlock(>drm.struct_mutex);
+   return err;
+}
+
+int i915_gem_request_live_selftests(struct drm_i915_private *i915)
+{
+   static const struct i915_subtest tests[] = {
+   SUBTEST(live_nop_request),
+   };
+   return i915_subtests(tests, i915);
+}
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h 
b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index f3e17cb10e05..09bf538826df 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -9,3 +9,4 @@
  * Tests are executed in order by igt/drv_selftest
  */
 selftest(sanitycheck, i915_live_sanitycheck) /* keep first (igt selfcheck) */
+selftest(requests, i915_gem_request_live_selftests)



Regards,

Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] tests/kms_plane_multiple: Change number of iterations

[Chris Wilson]

On Thu, Jan 12, 2017 at 12:53:30PM +0100, Maarten Lankhorst wrote:
> Op 12-01-17 om 12:12 schreef Mika Kahola:
> > In CI system, the default 64 iterations of this test may cause CRC overflow
> > warnings in dmesg when debugfs is enabled in kernel config. To keep dmesg
> > warning noise in minimum, let's run this test only once by default.
> >
> > Signed-off-by: Mika Kahola 
> > ---
> >  tests/kms_plane_multiple.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> > index 5e12be4..b94ec36 100644
> > --- a/tests/kms_plane_multiple.c
> > +++ b/tests/kms_plane_multiple.c
> > @@ -62,7 +62,7 @@ struct {
> > bool user_seed;
> > int seed;
> >  } opt = {
> > -   .iterations = 64,
> > +   .iterations = 1,
> > .user_seed = false,
> > .seed = 1,
> >  };
> 
> Reviewed-by: Maarten Lankhorst 
> 
> Though such small changes don't need review. :)

I am also intrigued as to what a "CRC overflow warning" is and why
userspace needs to take remedial action to hide the warning.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] tests/kms_plane_multiple: Change number of iterations

[Maarten Lankhorst]

Op 12-01-17 om 12:12 schreef Mika Kahola:
> In CI system, the default 64 iterations of this test may cause CRC overflow
> warnings in dmesg when debugfs is enabled in kernel config. To keep dmesg
> warning noise in minimum, let's run this test only once by default.
>
> Signed-off-by: Mika Kahola 
> ---
>  tests/kms_plane_multiple.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index 5e12be4..b94ec36 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -62,7 +62,7 @@ struct {
>   bool user_seed;
>   int seed;
>  } opt = {
> - .iterations = 64,
> + .iterations = 1,
>   .user_seed = false,
>   .seed = 1,
>  };

Reviewed-by: Maarten Lankhorst 

Though such small changes don't need review. :)

~Maarten

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/3] drm/i915/glk: Turn on workarounds that apply to Geminilake too

[Ander Conselvan de Oliveira]

Apply workarounds to Geminilake, and annoatate those that are applied
uncondionally when they apply to GLK based on the workaround database.

Signed-off-by: Ander Conselvan de Oliveira 

---
 drivers/gpu/drm/i915/i915_gem_gtt.c |  4 ++--
 drivers/gpu/drm/i915/intel_lrc.c|  6 +++---
 drivers/gpu/drm/i915/intel_mocs.c   |  2 +-
 drivers/gpu/drm/i915/intel_pm.c | 23 
 drivers/gpu/drm/i915/intel_ringbuffer.c | 37 +
 5 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0ed99adf..7024144 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2167,14 +2167,14 @@ static void gtt_write_workarounds(struct 
drm_i915_private *dev_priv)
 * called on driver load and after a GPU reset, so you can place
 * workarounds here even if they get overwritten by GPU reset.
 */
-   /* WaIncreaseDefaultTLBEntries:chv,bdw,skl,bxt */
+   /* WaIncreaseDefaultTLBEntries:chv,bdw,skl,bxt,glk */
if (IS_BROADWELL(dev_priv))
I915_WRITE(GEN8_L3_LRA_1_GPGPU, 
GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_BDW);
else if (IS_CHERRYVIEW(dev_priv))
I915_WRITE(GEN8_L3_LRA_1_GPGPU, 
GEN8_L3_LRA_1_GPGPU_DEFAULT_VALUE_CHV);
else if (IS_SKYLAKE(dev_priv))
I915_WRITE(GEN8_L3_LRA_1_GPGPU, 
GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_SKL);
-   else if (IS_BROXTON(dev_priv))
+   else if (IS_GEN9_LP(dev_priv))
I915_WRITE(GEN8_L3_LRA_1_GPGPU, 
GEN9_L3_LRA_1_GPGPU_DEFAULT_VALUE_BXT);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index db714dc..7bb3d0a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1126,13 +1126,13 @@ static int gen9_init_indirectctx_bb(struct 
intel_engine_cs *engine,
if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1))
wa_ctx_emit(batch, index, MI_ARB_ON_OFF | MI_ARB_DISABLE);
 
-   /* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt */
+   /* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt,glk */
ret = gen8_emit_flush_coherentl3_wa(engine, batch, index);
if (ret < 0)
return ret;
index = ret;
 
-   /* WaDisableGatherAtSetShaderCommonSlice:skl,bxt,kbl */
+   /* WaDisableGatherAtSetShaderCommonSlice:skl,bxt,kbl,glk */
wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
wa_ctx_emit_reg(batch, index, COMMON_SLICE_CHICKEN2);
wa_ctx_emit(batch, index, _MASKED_BIT_DISABLE(
@@ -1156,7 +1156,7 @@ static int gen9_init_indirectctx_bb(struct 
intel_engine_cs *engine,
wa_ctx_emit(batch, index, 0);
}
 
-   /* WaMediaPoolStateCmdInWABB:bxt */
+   /* WaMediaPoolStateCmdInWABB:bxt,glk */
if (HAS_POOLED_EU(engine->i915)) {
/*
 * EU pool configuration is setup along with golden context
diff --git a/drivers/gpu/drm/i915/intel_mocs.c 
b/drivers/gpu/drm/i915/intel_mocs.c
index c787fc4..9c67534 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -191,7 +191,7 @@ static bool get_mocs_settings(struct drm_i915_private 
*dev_priv,
  "Platform that should have a MOCS table does not.\n");
}
 
-   /* WaDisableSkipCaching:skl,bxt,kbl */
+   /* WaDisableSkipCaching:skl,bxt,kbl,glk */
if (IS_GEN9(dev_priv)) {
int i;
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b257343..c8ebf1d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -65,12 +65,12 @@ static void gen9_init_clock_gating(struct drm_i915_private 
*dev_priv)
I915_WRITE(GEN8_CONFIG0,
   I915_READ(GEN8_CONFIG0) | GEN9_DEFAULT_FIXES);
 
-   /* WaEnableChickenDCPR:skl,bxt,kbl */
+   /* WaEnableChickenDCPR:skl,bxt,kbl,glk */
I915_WRITE(GEN8_CHICKEN_DCPR_1,
   I915_READ(GEN8_CHICKEN_DCPR_1) | MASK_WAKEMEM);
 
/* WaFbcTurnOffFbcWatermark:skl,bxt,kbl */
-   /* WaFbcWakeMemOn:skl,bxt,kbl */
+   /* WaFbcWakeMemOn:skl,bxt,kbl,glk */
I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
   DISP_FBC_WM_DIS |
   DISP_FBC_MEMORY_WAKE);
@@ -107,6 +107,19 @@ static void bxt_init_clock_gating(struct drm_i915_private 
*dev_priv)
   PWM1_GATING_DIS | PWM2_GATING_DIS);
 }
 
+static void glk_init_clock_gating(struct drm_i915_private *dev_priv)
+{
+   gen9_init_clock_gating(dev_priv);
+
+   /*
+* WaDisablePWMClockGating:glk
+* Backlight PWM may stop in the asserted state, causing backlight
+* to stay fully on.
+*/
+   I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
+