Re: [Intel-gfx] [PATCH 10/14] drm/i915: Add MIPI_IO WA
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
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(&ppgtt->base);
> + i915_ppgtt_put(ppgtt);
> +err_unlock:
traditionally called out_unlock; Or being sole label, just out:
> + mutex_unlock(&dev_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 = &i915->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(&i915->drm.struct_mutex);
> + drm_mm_for_each_hole(node, &ggtt->base.mm, this_start, this_end) {
> + u64 this_size;
> +
> + if (ggtt->base.mm.color_adjust)
> + ggtt->base. mm.color_adjust(node, 0,
> + &this_start, &this_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_*()
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
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.
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
>-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
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(&intel_dp->aux, DP_RECEIVER_ALPM_CAP, &alpm_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(&intel_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(&intel_dp->aux,
>> + DP_RECEIVER_ALPM_CONFIG,
>> + DP_ALPM_ENABLE);
>> if (dev_priv->psr.link_standby)
>> drm_dp_dpcd_writeb(&intel_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
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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] DP compliance failure due to dithering for 18bpp video pattern
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
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
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.
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
== 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
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
== 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
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
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
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(&view, 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 7/7] drm/i915: Eliminate superfluous i915_ggtt_view_normal
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,
- &i915_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
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),
&ggtt->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_partial_pages(const struct i915_ggtt_view *view,
{
[Intel-gfx] [PATCH v2 1/7] drm/i915: Name the anonymous structs inside i915_ggtt_view
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] [PATCH v2 6/7] drm/i915: Eliminate superfluous i915_ggtt_view_rotated
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] Anonymouse ggtt_view params
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()
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(&vma->ggtt_view.params,
- &view->params,
- sizeof(view->params));
+ return memcmp(&vma->ggtt_view.params, &view->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
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
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)
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_*()
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
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(&crtc_state->base);
> > memset(crtc_state, 0, sizeof(*crtc_state));
> > crtc_state->base.crtc = &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(&crtc->base,
> > > &crtc->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
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
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
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 perhaps just iterate through the
>format and
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()
== 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
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
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
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
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 drm_i915_private *dev_priv = to_i915(dev);
>
> - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE);
> + if (dev_priv->psr.psr2_suppor
Re: [Intel-gfx] [PATCH v4 00/10] Execlist based engine-reset (v4)
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&id=de399a0a6baae97910796d81d8b9324db3fdd77c https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=prescheduler&id=67bea4dbb664b100c108af05d9a0f1f3b4078ab2 https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=prescheduler&id=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
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.
>
Re: [Intel-gfx] [PATCH i-g-t rfc 01/29] lib/igt_debugfs: Prevent buffer overflow
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, &buf, read_len);
> > > + bytes_read = read(pipe_crc->crc_fd, &buf, 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
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
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
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(&dev->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(&dev-
> >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.
> */
> void drm_kms_helper_poll_disable(struct drm_device *dev)
> {
> @@ -496,24 +503,6 @@ void drm_kms_helper_poll_disable(struct
> drm_device *dev)
> EXPORT_SYMBOL(drm_k
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Add format modifiers for Intel
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 == I915_FORMAT_MOD_Y_TILED ||
+ m
[Intel-gfx] [PATCH 03/10] drm/i915/psr: fix blank screen issue for psr2
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) & EDP_PSR_ENABLE);
WARN_ON(dev_priv->psr.active);
lockdep_assert_held(&dev_priv->psr.lock);
@@ -468,10
[Intel-gfx] ✓ Fi.CI.BAT: success for drm: Don't race connector registration
== 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
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
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
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, &i915->ggtt.base, &view);
+ 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(&vma->ggtt_view, &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 %llu, f
Re: [Intel-gfx] [PATCH] drm/i915: Fix up kerneldoc parameters for i915_gem_gtt_*()
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
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, &i915->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 = &i915->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(&resv, 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, &resv);
> + 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)
== 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
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
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 = &dev_priv->guc;
> + struct intel_huc *huc = &dev_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
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 = &dev_priv->guc;
+ struct intel_huc *huc = &dev_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,
+ HUC_STATUS2,
+ HUC_FW_VERIFIED,
+ HUC_FW_VERIFIED,
+ 50);
+
+ if (ret) {
+
Re: [Intel-gfx] [PATCH] drm: Don't race connector registration
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_*()
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 &struct i915_address_space
- * @node - the &struct 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 &struct i915_address_space
+ * @node: the &struct 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 &struct i915_address_space
- * @node - the &struct drm_mm_node (typically i915_vma.node)
- * @size - how much space to allocate inside the GTT,
+ * @vm: the &struct i915_address_space
+ * @node: the &struct 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
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
b/drivers/gpu/drm/i915/i915_guc_reg.h
index 6a0adaf..35cf991 100644
--- a/drivers/gpu/drm/i915/i915_guc_reg.h
+++ b/drivers/gpu/drm/i915/i915_guc_re
[Intel-gfx] [PATCH] drm/i915/huc: Support HuC authentication
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 = &dev_priv->guc;
+ struct intel_huc *huc = &dev_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,
+ HUC_STATUS2,
+ HUC_FW_VERIFIED,
+ HUC_FW_VERIFIED,
+ 50);
+
+ if (ret) {
+ DRM
Re: [Intel-gfx] [PATCH i-g-t rfc 01/29] lib/igt_debugfs: Prevent buffer overflow
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, &buf, read_len);
+ bytes_read = read(pipe_crc->crc_fd, &buf, 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
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(&connector->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
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(&connector->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
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
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
== 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
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
>-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)
== 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()
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()
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
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
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
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()
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 drm_detect_hdmi_monitor(struct edid *edid);
> > bool drm_detect_monitor_audio(struct edid *edid);
> > boo
[Intel-gfx] [PULL] drm-intel-fixes
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
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
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
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, &objects, batch_pool_link) {
> list_del(&obj->batch_pool_link);
> - vma = vma_lookup(obj, &ppgtt->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(&i915->drm.struct_mutex);
> + drm_mm_for_each_hole(node, &ggtt->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
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.
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&reviewed? >>> > >>> > 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
== 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
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, &blue, tiling,
> - &test.reference_crc);
> + if (!test_only)
> + test_grab_crc(data, output, pipe, true, &blue, tiling,
> + &test.reference_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(&data->display,
> - DRM_MODE_PAGE_FLIP_EVENT,
> - &data->display);
> + ret = igt_display_try_commit_atomic(&data->display,
> + flags,
> + &data->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, &crc);
> + n = igt_pipe_crc_get_crcs(data->pipe_crc, MAX_CRCS,
> &crc);
>
> - igt_assert_eq(n, MAX_CRCS);
> + igt_assert_eq(n, MAX_CRCS);
>
> - igt_assert_crc_equal(&test.reference_crc, crc);
> + igt_assert_crc_equal(&test.reference_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,
>
Re: [Intel-gfx] [PATCH v2] drm/i915: Declare i915_gem_object_create_internal() as taking phys_addr_t size
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
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(&dev_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(&obj->batch_pool_link, &objects);
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, &objects, batch_pool_link) {
> + vma = vma_lookup(obj, &ppgtt->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, &objects, batch_pool_link) {
> + vma = vma_lookup(obj, &ppgtt->base);
> + if (IS_ERR(vma))
> + continue;
> +
> + flags -= obj->base.size;
> + if (!drm_mm_node_allocated(&vma->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[
Re: [Intel-gfx] [PATCH 09/37] drm/i915: Mock infrastructure for request emission
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(&ctx->ref);
> >+INIT_LIST_HEAD(&ctx->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)
== 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
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(&i915->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(&ctx->ref);
+ INIT_LIST_HEAD(&ctx->link);
+ ctx->name = name ? kstrdup(name, GFP_KERNEL) : NULL;
Care or not whether allocation worked?
+ ctx->i915 = i915;
+
+ ret = ida_simple_get(&i915->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
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
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
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
== 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
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
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
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
> -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
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(&i915->gpu_error);
> >+unsigned long n, prime;
> >+ktime_t times[2] = {};
> >+int err;
> >+
> >+mutex_lock(&i915->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
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(&i915->gpu_error);
+ unsigned long n, prime;
+ ktime_t times[2] = {};
+ int err;
+
+ mutex_lock(&i915->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(&i915->gpu_error)) {
+ pr_err("GPU was reset %d times!\n",
+ i915_reset_count(&i915->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(&i915->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
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
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
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) |
+ PWM1_GATING_DIS | PWM2_GATING_DIS);
