Re: [Intel-gfx] [RFC 02/10] drm/i915/gvt: get ready of memory for pvmmio
On 2018.09.27 12:37:47 -0400, Xiaolin Zhang wrote:
> To enable pvmmio feature, we need to prepare one 4K shared page
> which will be accessed by both guest and backend i915 driver.
>
> guest i915 allocate one page memory and then the guest physical address is
> passed to backend i915 driver through PVINFO register so that backend i915
> driver can access this shared page without hypeviser trap cost for shared
> data exchagne via hyperviser read_gpa functionality.
>
> Signed-off-by: Xiaolin Zhang
> ---
> drivers/gpu/drm/i915/i915_drv.c| 5 +
> drivers/gpu/drm/i915/i915_drv.h| 3 +++
> drivers/gpu/drm/i915/i915_pvinfo.h | 25 -
> drivers/gpu/drm/i915/i915_vgpu.c | 17 +
> 4 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ade9bca..815a4dd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -885,6 +885,7 @@ static int i915_driver_init_early(struct drm_i915_private
> *dev_priv)
> return -ENODEV;
>
> spin_lock_init(_priv->irq_lock);
> + spin_lock_init(_priv->shared_page_lock);
> spin_lock_init(_priv->gpu_error.lock);
> mutex_init(_priv->backlight_lock);
> spin_lock_init(_priv->uncore.lock);
> @@ -987,6 +988,8 @@ static void i915_mmio_cleanup(struct drm_i915_private
> *dev_priv)
>
> intel_teardown_mchbar(dev_priv);
> pci_iounmap(pdev, dev_priv->regs);
> + if (intel_vgpu_active(dev_priv) && dev_priv->shared_page)
> + free_pages((unsigned long)dev_priv->shared_page, 0);
> }
>
> /**
> @@ -1029,6 +1032,8 @@ static int i915_driver_init_mmio(struct
> drm_i915_private *dev_priv)
> return 0;
>
> err_uncore:
> + if (intel_vgpu_active(dev_priv) && dev_priv->shared_page)
> + free_pages((unsigned long)dev_priv->shared_page, 0);
> intel_uncore_fini(dev_priv);
> err_bridge:
> pci_dev_put(dev_priv->bridge_dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 174d618..76d7e9c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -56,6 +56,7 @@
>
> #include "i915_params.h"
> #include "i915_reg.h"
> +#include "i915_pvinfo.h"
> #include "i915_utils.h"
>
> #include "intel_bios.h"
> @@ -1623,6 +1624,8 @@ struct drm_i915_private {
> resource_size_t stolen_usable_size; /* Total size minus reserved
> ranges */
>
> void __iomem *regs;
> + struct gvt_shared_page *shared_page;
> + spinlock_t shared_page_lock;
>
> struct intel_uncore uncore;
>
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h
> b/drivers/gpu/drm/i915/i915_pvinfo.h
> index 697e998..ab839a7 100644
> --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> @@ -49,6 +49,25 @@ enum vgt_g2v_type {
> VGT_G2V_MAX,
> };
>
> +struct pv_ppgtt_update {
> + u64 pdp;
> + u64 start;
> + u64 length;
> + u32 cache_level;
> +};
> +
> +/*
> + * shared page(4KB) between gvt and VM, could be allocated by guest driver
> + * or a fixed location in PCI bar 0 region
> + */
> +struct gvt_shared_page {
> + u32 elsp_data[4];
> + u32 reg_addr;
> + u32 disable_irq;
> + struct pv_ppgtt_update pv_ppgtt;
> + u32 rsvd2[0x400 - 13];
> +};
Could we define offset for shared page fields instead of a struct?
Which is wasting space I think.
> +
> #define VGPU_PVMMIO(vgpu) vgpu_vreg_t(vgpu, vgtif_reg(enable_pvmmio))
>
> /*
> @@ -120,8 +139,12 @@ struct vgt_if {
> u32 execlist_context_descriptor_lo;
> u32 execlist_context_descriptor_hi;
> u32 enable_pvmmio;
> + struct {
> + u32 lo;
> + u32 hi;
> + } shared_page_gpa;
>
> - u32 rsv7[0x200 - 25];/* pad to one page */
> + u32 rsv7[0x200 - 27];/* pad to one page */
> } __packed;
>
> #define vgtif_reg(x) \
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> b/drivers/gpu/drm/i915/i915_vgpu.c
> index d22c5ca..10ae94b 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -62,6 +62,7 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv)
> {
> u64 magic;
> u16 version_major;
> + u64 shared_page_gpa;
>
> BUILD_BUG_ON(sizeof(struct vgt_if) != VGT_PVINFO_SIZE);
>
> @@ -89,6 +90,22 @@ void i915_check_vgpu(struct drm_i915_private *dev_priv)
> dev_priv->vgpu.active = true;
> DRM_INFO("Virtual GPU for Intel GVT-g detected with pvmmio 0x%x\n",
> i915_modparams.enable_pvmmio);
> +
> + if (intel_vgpu_active(dev_priv) && i915_modparams.enable_pvmmio) {
> + dev_priv->shared_page = (struct gvt_shared_page *)
> + __get_free_pages(GFP_KERNEL | __GFP_ZERO, 0);
> + if (!dev_priv->shared_page) {
> + DRM_ERROR("out of memory for shared page
Re: [Intel-gfx] [RFC 01/10] drm/i915/gvt: add module parameter enable_pvmmio
On 2018.09.28 14:09:45 +0800, Zhang, Xiaolin wrote: > On 09/27/2018 07:03 PM, Joonas Lahtinen wrote: > > Quoting Xiaolin Zhang (2018-09-27 19:37:46) > >> This int type module parameter is used to control the different > >> level pvmmio feature for MMIO emulation in GVT. > >> > >> This parameter is default zero, no pvmmio feature enabled. > >> > >> Its permission type is 0400 which means user could only change its > >> value through the cmdline, this is to prevent the dynamic modification > >> during runtime which would break the pvmmio internal logic. > >> > >> Signed-off-by: Xiaolin Zhang > > This shouldn't really be a module parameter. We should detect the > > capability from the vGPU device and use it always when possible. > > > > Regards, Joonas > > > for pv optimization, we should touch both guest driver and GVTg. this > parameter is used for > > guest pv capability because GVTg with pv capability will support both pv > and non pv capability guest. > That's the purpose of 'vgt_caps' in PVINFO to do capability check between host/guest. You need a new cap bit definition for PVMMIO and maybe another field for different PVMMIO level capability check. New parameter is not useful here. Thanks -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: Remove i915.enable_ppgtt override
On 2018.10.08 13:58:25 +, Wang, Zhi A wrote: > Thanks for pointing this. My bad. > > I take a look on the code and it looks like the GVT-g context is now quite > similar with the kernel context except the force single submission and ring > buffer size. (When we upstream the code, there was no kernel context at that > time. :/) Now there is already one API for single submission. If we can > introduce another one to configure the ring buffer size. Then maybe we can > remove the *create_gvt() in i915_gem_context.c and used kernel context > instead. > > Feel free to drop comments. :) > For ppgtt, you'd better change to use i915 ppgtt interface to handle shadow pages, then gvt context would be much like normal one. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Disable PSR when a PSR aux error happen
On Mon, 2018-10-08 at 17:49 -0700, Dhinakaran Pandiyan wrote:
> On Mon, 2018-10-08 at 17:30 -0700, Souza, Jose wrote:
> > On Mon, 2018-10-08 at 17:14 -0700, Dhinakaran Pandiyan wrote:
> > > On Fri, 2018-10-05 at 16:35 -0700, José Roberto de Souza wrote:
> > > > While PSR is active hardware will do aux transactions by it
> > > > self
> > > > to
> > > > wakeup sink to receive a new frame when necessary. If that
> > > > transaction is not acked by sink, hardware will trigger this
> > > > interruption.
> > > >
> > > > So let's disable PSR as it is a hint that there is problem with
> > > > this
> > > > sink.
> > > >
> > > > The removed FIXME was asking to manually train the link but we
> > > > don't
> > > > need to do that as by spec sink should do a short pulse when it
> > > > is
> > > > out of sync with source, we just need to make sure it is awaken
> > > > and
> > > > the SDP header with PSR disable will trigger this condition.
> > >
> > > That's a good point, the short pulse handler does handle
> > > retraining.
> > >
> > > > Cc: Dhinakaran Pandiyan
> > > > Signed-off-by: José Roberto de Souza
> > > > ---
> > > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > > drivers/gpu/drm/i915/intel_psr.c | 39
> > > >
> > > >
> > > > 2 files changed, 36 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > > b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 794a8a03c7e6..efbebe1c2ba3 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -638,6 +638,7 @@ struct i915_psr {
> > > > u8 sink_sync_latency;
> > > > ktime_t last_entry_attempt;
> > > > ktime_t last_exit;
> > > > + u32 irq_aux_error;
> > > > };
> > > >
> > > > enum intel_pch {
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index cd9a60d1efa1..74090fffea23 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -159,10 +159,16 @@ void intel_psr_irq_handler(struct
> > > > drm_i915_private *dev_priv, u32 psr_iir)
> > > >BIT(TRANSCODER_C);
> > > >
> > > > for_each_cpu_transcoder_masked(dev_priv,
> > > > cpu_transcoder,
> > > > transcoders) {
> > > > - /* FIXME: Exit PSR and link train manually when
> > > > this
> > > > happens. */
> > > > - if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> > > > - DRM_DEBUG_KMS("[transcoder %s] PSR aux
> > > > error\n",
> > > > - transcoder_name(cpu_trans
> > > > coder));
> > > > + if (psr_iir & EDP_PSR_ERROR(cpu_transcoder)) {
> > > > + DRM_WARN("[transcoder %s] PSR aux
> > > > error\n",
> > > > +transcoder_name(cpu_transcoder
> > > > ));
> > > > +
> > > > + spin_lock(_priv->irq_lock);
> > > > + dev_priv->psr.irq_aux_error |=
> > > > BIT(cpu_transcoder);
> > > > + spin_unlock(_priv->irq_lock);
> > > > +
> > > > + schedule_work(_priv->psr.work);
> > > > + }
> > > >
> > > > if (psr_iir &
> > > > EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> > > > dev_priv->psr.last_entry_attempt =
> > > > time_ns;
> > > > @@ -891,11 +897,36 @@ int intel_psr_set_debugfs_mode(struct
> > > > drm_i915_private *dev_priv,
> > > > return ret;
> > > > }
> > > >
> > > > +static void intel_psr_handle_irq(struct drm_i915_private
> > > > *dev_priv)
> > > > +{
> > > > + struct i915_psr *psr = _priv->psr;
> > > > + u32 irq_aux_error;
> > > > +
> > > > + spin_lock_irq(_priv->irq_lock);
> > > > + irq_aux_error = psr->irq_aux_error;
> > > > + psr->irq_aux_error = 0;
> > > > + spin_unlock_irq(_priv->irq_lock);
> > > > +
> > > > + /* right now PSR is only enabled in eDP */
> > > > + WARN_ON(irq_aux_error & ~BIT(TRANSCODER_EDP));
> > > > +
> > > > + mutex_lock(>lock);
> > > > +
> > > > + intel_psr_disable_locked(psr->dp);
> > > > + /* let's make sure that sink is awaken */
> > > > + drm_dp_dpcd_writeb(>dp->aux, DP_SET_POWER,
> > > > DP_SET_POWER_D0);
> > >
> > > We should be making sure the sink exits PSR after
> > > psr_invalidate()
> > > ->
> > > psr_exit() too? Which means, we have to figure out a cleaner way
> > > to
> > > handle all of this. I am not sure, at this point, what a cleaner
> > > solution will like. However, I'd like PSR disable from
> > > invalidate,
> > > PSR
> > > disable from a modeset and PSR disable due to an error share code
> > > and
> > > behavior. All of them should basically be
> > > 1) Disable PSR in PSR_CTL
> > > 2) Wait for idle in PSR_STATUS
> > > 3) Write to sink DP_SET_POWER
> >
> > We don't need to wait PSR to be disabled for invalidate(),
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Cache sink_count for eDP
On Mon, 2018-10-08 at 17:35 -0700, Souza, Jose wrote:
> On Mon, 2018-10-08 at 17:19 -0700, Dhinakaran Pandiyan wrote:
> > On Fri, 2018-10-05 at 16:35 -0700, José Roberto de Souza wrote:
> > > For eDP panels all the DPCD and EDID data is cached when
> > > initializing
> > > the eDP connector so in futher detection it do not call
> > > intel_dp_detect_dpcd() for eDP.
> > > The problem is on the first short pulse interruption it calls
> > > intel_dp_get_dpcd() for eDP and DP and it will read and set the
> > > sink
> > > count, causing a mismatch between old sink count and the new one
> > > triggering a full detection without needed.
> > >
> > > Cc: Dhinakaran Pandiyan
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 5 +
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 19f0c3f59cbe..4a1c31ec9065 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3926,6 +3926,7 @@ intel_edp_init_dpcd(struct intel_dp
> > > *intel_dp)
> > > {
> > > struct drm_i915_private *dev_priv =
> > > to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> > > + u8 val;
> > >
> > > /* this function is meant to be called only once */
> > > WARN_ON(intel_dp->dpcd[DP_DPCD_REV] != 0);
> > > @@ -3997,6 +3998,10 @@ intel_edp_init_dpcd(struct intel_dp
> > > *intel_dp)
> > >
> > > intel_dp_set_common_rates(intel_dp);
> > >
> > > + if (drm_dp_dpcd_readb(_dp->aux, DP_SINK_COUNT, ) <=
> > > 0)
> > > + return false;
> > > + intel_dp->sink_count = DP_GET_SINK_COUNT(val);
> >
> > Is this even relevant for eDPs? Seems unnecessary to read or
> > compare
> > sink count for eDP. I'd suggest skipping DP_SINK_COUNT checks for
> > eDP.
>
> I'm not sure as DP specs for DP_SINK_COUNT says:
>
> The Sink device shall add one more if it has a local Rendering
> Function.
>
> and eDP spec do not redefine or alter this, so I guess is more safe
> also read for eDP too.
>
We already special case eDP in several places, for example, don't
update link rates from the short pulse handler etc. And also don't
support hotplug, I don't see a point.
-DK
>
> >
> >
> > -DK
> >
> > > +
> > > return true;
> > > }
> > >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Disable PSR when a PSR aux error happen
On Mon, 2018-10-08 at 17:30 -0700, Souza, Jose wrote:
> On Mon, 2018-10-08 at 17:14 -0700, Dhinakaran Pandiyan wrote:
> > On Fri, 2018-10-05 at 16:35 -0700, José Roberto de Souza wrote:
> > > While PSR is active hardware will do aux transactions by it self
> > > to
> > > wakeup sink to receive a new frame when necessary. If that
> > > transaction is not acked by sink, hardware will trigger this
> > > interruption.
> > >
> > > So let's disable PSR as it is a hint that there is problem with
> > > this
> > > sink.
> > >
> > > The removed FIXME was asking to manually train the link but we
> > > don't
> > > need to do that as by spec sink should do a short pulse when it
> > > is
> > > out of sync with source, we just need to make sure it is awaken
> > > and
> > > the SDP header with PSR disable will trigger this condition.
> >
> > That's a good point, the short pulse handler does handle
> > retraining.
> >
> > > Cc: Dhinakaran Pandiyan
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > drivers/gpu/drm/i915/intel_psr.c | 39
> > >
> > >
> > > 2 files changed, 36 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 794a8a03c7e6..efbebe1c2ba3 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -638,6 +638,7 @@ struct i915_psr {
> > > u8 sink_sync_latency;
> > > ktime_t last_entry_attempt;
> > > ktime_t last_exit;
> > > + u32 irq_aux_error;
> > > };
> > >
> > > enum intel_pch {
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index cd9a60d1efa1..74090fffea23 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -159,10 +159,16 @@ void intel_psr_irq_handler(struct
> > > drm_i915_private *dev_priv, u32 psr_iir)
> > > BIT(TRANSCODER_C);
> > >
> > > for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > > transcoders) {
> > > - /* FIXME: Exit PSR and link train manually when this
> > > happens. */
> > > - if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> > > - DRM_DEBUG_KMS("[transcoder %s] PSR aux
> > > error\n",
> > > - transcoder_name(cpu_transcoder));
> > > + if (psr_iir & EDP_PSR_ERROR(cpu_transcoder)) {
> > > + DRM_WARN("[transcoder %s] PSR aux error\n",
> > > + transcoder_name(cpu_transcoder));
> > > +
> > > + spin_lock(_priv->irq_lock);
> > > + dev_priv->psr.irq_aux_error |=
> > > BIT(cpu_transcoder);
> > > + spin_unlock(_priv->irq_lock);
> > > +
> > > + schedule_work(_priv->psr.work);
> > > + }
> > >
> > > if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> > > dev_priv->psr.last_entry_attempt = time_ns;
> > > @@ -891,11 +897,36 @@ int intel_psr_set_debugfs_mode(struct
> > > drm_i915_private *dev_priv,
> > > return ret;
> > > }
> > >
> > > +static void intel_psr_handle_irq(struct drm_i915_private
> > > *dev_priv)
> > > +{
> > > + struct i915_psr *psr = _priv->psr;
> > > + u32 irq_aux_error;
> > > +
> > > + spin_lock_irq(_priv->irq_lock);
> > > + irq_aux_error = psr->irq_aux_error;
> > > + psr->irq_aux_error = 0;
> > > + spin_unlock_irq(_priv->irq_lock);
> > > +
> > > + /* right now PSR is only enabled in eDP */
> > > + WARN_ON(irq_aux_error & ~BIT(TRANSCODER_EDP));
> > > +
> > > + mutex_lock(>lock);
> > > +
> > > + intel_psr_disable_locked(psr->dp);
> > > + /* let's make sure that sink is awaken */
> > > + drm_dp_dpcd_writeb(>dp->aux, DP_SET_POWER,
> > > DP_SET_POWER_D0);
> >
> > We should be making sure the sink exits PSR after psr_invalidate()
> > ->
> > psr_exit() too? Which means, we have to figure out a cleaner way to
> > handle all of this. I am not sure, at this point, what a cleaner
> > solution will like. However, I'd like PSR disable from invalidate,
> > PSR
> > disable from a modeset and PSR disable due to an error share code
> > and
> > behavior. All of them should basically be
> > 1) Disable PSR in PSR_CTL
> > 2) Wait for idle in PSR_STATUS
> > 3) Write to sink DP_SET_POWER
>
> We don't need to wait PSR to be disabled for invalidate(), hardware
> will do the exit sequence including write to DP_SET_POWER, also in
> this
Yeah, but doesn't work consistently on the panel that I have, writing
to the sink DP_SET_POWER dpcd is needed. I guess, we could add a panel
specific quirk to work around it.
-DK
> case we want activate PSR as soon as all frontbuffer changes was
> commited and PSR is inactive.
>
> >
> >
> >
> > > +
> > > + mutex_unlock(_priv->psr.lock);
> > > +}
> > > +
> > > static void intel_psr_work(struct work_struct *work)
> > > {
> > > struct drm_i915_private *dev_priv =
> > >
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/dp: Add definitions for eDP Rev 1.4a and 1.4b
== Series Details == Series: drm/dp: Add definitions for eDP Rev 1.4a and 1.4b URL : https://patchwork.freedesktop.org/series/50720/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4951 -> Patchwork_10395 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/50720/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10395 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s3: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: PASS -> FAIL (fdo#103167) Possible fixes igt@drv_selftest@live_hangcheck: fi-cfl-8109u: INCOMPLETE (fdo#106070) -> PASS fi-kbl-7567u: DMESG-FAIL (fdo#107860) -> PASS igt@gem_exec_suspend@basic-s3: fi-cfl-8109u: DMESG-WARN (fdo#107345) -> PASS +3 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070 fdo#107345 https://bugs.freedesktop.org/show_bug.cgi?id=107345 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107860 https://bugs.freedesktop.org/show_bug.cgi?id=107860 == Participating hosts (47 -> 44) == Additional (2): fi-skl-guc fi-pnv-d510 Missing(5): fi-kbl-soraka fi-ctg-p8600 fi-byt-squawks fi-bsw-cyan fi-ilk-m540 == Build changes == * Linux: CI_DRM_4951 -> Patchwork_10395 CI_DRM_4951: 07b09b512d6ef0f5b0673ae611507c8a61ce6c7b @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4671: b121f7d42c260ae3a050c3f440d1c11f7cff7d1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10395: 5ee17799fe627790d7061f4d2b16c72d7f4c051d @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 5ee17799fe62 drm/dp: Add definitions for eDP Rev 1.4a and 1.4b == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10395/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Cache sink_count for eDP
On Mon, 2018-10-08 at 17:19 -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-10-05 at 16:35 -0700, José Roberto de Souza wrote:
> > For eDP panels all the DPCD and EDID data is cached when
> > initializing
> > the eDP connector so in futher detection it do not call
> > intel_dp_detect_dpcd() for eDP.
> > The problem is on the first short pulse interruption it calls
> > intel_dp_get_dpcd() for eDP and DP and it will read and set the
> > sink
> > count, causing a mismatch between old sink count and the new one
> > triggering a full detection without needed.
> >
> > Cc: Dhinakaran Pandiyan
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 19f0c3f59cbe..4a1c31ec9065 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3926,6 +3926,7 @@ intel_edp_init_dpcd(struct intel_dp
> > *intel_dp)
> > {
> > struct drm_i915_private *dev_priv =
> > to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> > + u8 val;
> >
> > /* this function is meant to be called only once */
> > WARN_ON(intel_dp->dpcd[DP_DPCD_REV] != 0);
> > @@ -3997,6 +3998,10 @@ intel_edp_init_dpcd(struct intel_dp
> > *intel_dp)
> >
> > intel_dp_set_common_rates(intel_dp);
> >
> > + if (drm_dp_dpcd_readb(_dp->aux, DP_SINK_COUNT, ) <=
> > 0)
> > + return false;
> > + intel_dp->sink_count = DP_GET_SINK_COUNT(val);
> Is this even relevant for eDPs? Seems unnecessary to read or compare
> sink count for eDP. I'd suggest skipping DP_SINK_COUNT checks for
> eDP.
I'm not sure as DP specs for DP_SINK_COUNT says:
The Sink device shall add one more if it has a local Rendering
Function.
and eDP spec do not redefine or alter this, so I guess is more safe
also read for eDP too.
>
>
> -DK
>
> > +
> > return true;
> > }
> >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Fix legacy DPMS changes with MST (rev7)
== Series Details == Series: Fix legacy DPMS changes with MST (rev7) URL : https://patchwork.freedesktop.org/series/49878/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4951 -> Patchwork_10394 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49878/revisions/7/mbox/ == Known issues == Here are the changes found in Patchwork_10394 that come from known issues: === IGT changes === Issues hit igt@drv_module_reload@basic-reload: fi-blb-e6850: NOTRUN -> INCOMPLETE (fdo#107718) Possible fixes igt@drv_selftest@live_hangcheck: fi-cfl-8109u: INCOMPLETE (fdo#106070) -> PASS fi-kbl-7567u: DMESG-FAIL (fdo#107860) -> PASS igt@gem_exec_suspend@basic-s3: fi-cfl-8109u: DMESG-WARN (fdo#107345) -> PASS +3 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070 fdo#107345 https://bugs.freedesktop.org/show_bug.cgi?id=107345 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107860 https://bugs.freedesktop.org/show_bug.cgi?id=107860 == Participating hosts (47 -> 45) == Additional (2): fi-skl-guc fi-pnv-d510 Missing(4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan == Build changes == * Linux: CI_DRM_4951 -> Patchwork_10394 CI_DRM_4951: 07b09b512d6ef0f5b0673ae611507c8a61ce6c7b @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4671: b121f7d42c260ae3a050c3f440d1c11f7cff7d1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10394: 8f3392feaa34d34ef04320168afd18eca2da1b42 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 8f3392feaa34 drm/i915: Fix intel_dp_mst_best_encoder() f5ef5c8de990 drm/i915: Skip vcpi allocation for MSTB ports that are gone b42e20df656a drm/i915: Don't unset intel_connector->mst_port 3cf7319a6799 drm/nouveau: Fix nv50_mstc->best_encoder() fb9e3593d7ac drm/atomic_helper: Disallow new modesets on unregistered connectors == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10394/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Disable PSR when a PSR aux error happen
On Mon, 2018-10-08 at 17:14 -0700, Dhinakaran Pandiyan wrote:
> On Fri, 2018-10-05 at 16:35 -0700, José Roberto de Souza wrote:
> > While PSR is active hardware will do aux transactions by it self to
> > wakeup sink to receive a new frame when necessary. If that
> > transaction is not acked by sink, hardware will trigger this
> > interruption.
> >
> > So let's disable PSR as it is a hint that there is problem with
> > this
> > sink.
> >
> > The removed FIXME was asking to manually train the link but we
> > don't
> > need to do that as by spec sink should do a short pulse when it is
> > out of sync with source, we just need to make sure it is awaken and
> > the SDP header with PSR disable will trigger this condition.
> That's a good point, the short pulse handler does handle retraining.
>
> > Cc: Dhinakaran Pandiyan
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_psr.c | 39
> >
> > 2 files changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 794a8a03c7e6..efbebe1c2ba3 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -638,6 +638,7 @@ struct i915_psr {
> > u8 sink_sync_latency;
> > ktime_t last_entry_attempt;
> > ktime_t last_exit;
> > + u32 irq_aux_error;
> > };
> >
> > enum intel_pch {
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index cd9a60d1efa1..74090fffea23 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -159,10 +159,16 @@ void intel_psr_irq_handler(struct
> > drm_i915_private *dev_priv, u32 psr_iir)
> >BIT(TRANSCODER_C);
> >
> > for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> > transcoders) {
> > - /* FIXME: Exit PSR and link train manually when this
> > happens. */
> > - if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> > - DRM_DEBUG_KMS("[transcoder %s] PSR aux
> > error\n",
> > - transcoder_name(cpu_transcoder));
> > + if (psr_iir & EDP_PSR_ERROR(cpu_transcoder)) {
> > + DRM_WARN("[transcoder %s] PSR aux error\n",
> > +transcoder_name(cpu_transcoder));
> > +
> > + spin_lock(_priv->irq_lock);
> > + dev_priv->psr.irq_aux_error |=
> > BIT(cpu_transcoder);
> > + spin_unlock(_priv->irq_lock);
> > +
> > + schedule_work(_priv->psr.work);
> > + }
> >
> > if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> > dev_priv->psr.last_entry_attempt = time_ns;
> > @@ -891,11 +897,36 @@ int intel_psr_set_debugfs_mode(struct
> > drm_i915_private *dev_priv,
> > return ret;
> > }
> >
> > +static void intel_psr_handle_irq(struct drm_i915_private
> > *dev_priv)
> > +{
> > + struct i915_psr *psr = _priv->psr;
> > + u32 irq_aux_error;
> > +
> > + spin_lock_irq(_priv->irq_lock);
> > + irq_aux_error = psr->irq_aux_error;
> > + psr->irq_aux_error = 0;
> > + spin_unlock_irq(_priv->irq_lock);
> > +
> > + /* right now PSR is only enabled in eDP */
> > + WARN_ON(irq_aux_error & ~BIT(TRANSCODER_EDP));
> > +
> > + mutex_lock(>lock);
> > +
> > + intel_psr_disable_locked(psr->dp);
> > + /* let's make sure that sink is awaken */
> > + drm_dp_dpcd_writeb(>dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
>
> We should be making sure the sink exits PSR after psr_invalidate() ->
> psr_exit() too? Which means, we have to figure out a cleaner way to
> handle all of this. I am not sure, at this point, what a cleaner
> solution will like. However, I'd like PSR disable from invalidate,
> PSR
> disable from a modeset and PSR disable due to an error share code and
> behavior. All of them should basically be
> 1) Disable PSR in PSR_CTL
> 2) Wait for idle in PSR_STATUS
> 3) Write to sink DP_SET_POWER
We don't need to wait PSR to be disabled for invalidate(), hardware
will do the exit sequence including write to DP_SET_POWER, also in this
case we want activate PSR as soon as all frontbuffer changes was
commited and PSR is inactive.
>
>
>
> > +
> > + mutex_unlock(_priv->psr.lock);
> > +}
> > +
> > static void intel_psr_work(struct work_struct *work)
> > {
> > struct drm_i915_private *dev_priv =
> > container_of(work, typeof(*dev_priv), psr.work);
> >
> > + if (READ_ONCE(dev_priv->psr.irq_aux_error))
> > + intel_psr_handle_irq(dev_priv);
> > +
> > mutex_lock(_priv->psr.lock);
> >
> > if (!dev_priv->psr.enabled)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/dp: Add definitions for eDP Rev 1.4a and 1.4b
VESA eDP 1.4 specification has separate fields defined in EDP_DPCD_REV for eDP 1.4a and 1.4b eDP revisions. This patch defines those. Found this when one of my eDP panels advertises eDP 1.4a (04h) in the EDP_DPCD_REV DPCD field. Cc: Jani Nikula Cc: Ville Syrjala Signed-off-by: Manasi Navare --- include/drm/drm_dp_helper.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 2a3843f248cf..9ad98e8d9ede 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -685,6 +685,8 @@ # define DP_EDP_12 0x01 # define DP_EDP_13 0x02 # define DP_EDP_14 0x03 +# define DP_EDP_14a 0x04/* eDP 1.4a */ +# define DP_EDP_14b 0x05/* eDP 1.4b */ #define DP_EDP_GENERAL_CAP_1 0x701 # define DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP (1 << 0) -- 2.18.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Cache sink_count for eDP
On Fri, 2018-10-05 at 16:35 -0700, José Roberto de Souza wrote:
> For eDP panels all the DPCD and EDID data is cached when initializing
> the eDP connector so in futher detection it do not call
> intel_dp_detect_dpcd() for eDP.
> The problem is on the first short pulse interruption it calls
> intel_dp_get_dpcd() for eDP and DP and it will read and set the sink
> count, causing a mismatch between old sink count and the new one
> triggering a full detection without needed.
>
> Cc: Dhinakaran Pandiyan
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/intel_dp.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 19f0c3f59cbe..4a1c31ec9065 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3926,6 +3926,7 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv =
> to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> + u8 val;
>
> /* this function is meant to be called only once */
> WARN_ON(intel_dp->dpcd[DP_DPCD_REV] != 0);
> @@ -3997,6 +3998,10 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
>
> intel_dp_set_common_rates(intel_dp);
>
> + if (drm_dp_dpcd_readb(_dp->aux, DP_SINK_COUNT, ) <=
> 0)
> + return false;
> + intel_dp->sink_count = DP_GET_SINK_COUNT(val);
Is this even relevant for eDPs? Seems unnecessary to read or compare
sink count for eDP. I'd suggest skipping DP_SINK_COUNT checks for eDP.
-DK
> +
> return true;
> }
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Disable PSR when a PSR aux error happen
On Fri, 2018-10-05 at 16:35 -0700, José Roberto de Souza wrote:
> While PSR is active hardware will do aux transactions by it self to
> wakeup sink to receive a new frame when necessary. If that
> transaction is not acked by sink, hardware will trigger this
> interruption.
>
> So let's disable PSR as it is a hint that there is problem with this
> sink.
>
> The removed FIXME was asking to manually train the link but we don't
> need to do that as by spec sink should do a short pulse when it is
> out of sync with source, we just need to make sure it is awaken and
> the SDP header with PSR disable will trigger this condition.
That's a good point, the short pulse handler does handle retraining.
>
> Cc: Dhinakaran Pandiyan
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_psr.c | 39
>
> 2 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 794a8a03c7e6..efbebe1c2ba3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -638,6 +638,7 @@ struct i915_psr {
> u8 sink_sync_latency;
> ktime_t last_entry_attempt;
> ktime_t last_exit;
> + u32 irq_aux_error;
> };
>
> enum intel_pch {
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index cd9a60d1efa1..74090fffea23 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -159,10 +159,16 @@ void intel_psr_irq_handler(struct
> drm_i915_private *dev_priv, u32 psr_iir)
> BIT(TRANSCODER_C);
>
> for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder,
> transcoders) {
> - /* FIXME: Exit PSR and link train manually when this
> happens. */
> - if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> - DRM_DEBUG_KMS("[transcoder %s] PSR aux
> error\n",
> - transcoder_name(cpu_transcoder));
> + if (psr_iir & EDP_PSR_ERROR(cpu_transcoder)) {
> + DRM_WARN("[transcoder %s] PSR aux error\n",
> + transcoder_name(cpu_transcoder));
> +
> + spin_lock(_priv->irq_lock);
> + dev_priv->psr.irq_aux_error |=
> BIT(cpu_transcoder);
> + spin_unlock(_priv->irq_lock);
> +
> + schedule_work(_priv->psr.work);
> + }
>
> if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> dev_priv->psr.last_entry_attempt = time_ns;
> @@ -891,11 +897,36 @@ int intel_psr_set_debugfs_mode(struct
> drm_i915_private *dev_priv,
> return ret;
> }
>
> +static void intel_psr_handle_irq(struct drm_i915_private *dev_priv)
> +{
> + struct i915_psr *psr = _priv->psr;
> + u32 irq_aux_error;
> +
> + spin_lock_irq(_priv->irq_lock);
> + irq_aux_error = psr->irq_aux_error;
> + psr->irq_aux_error = 0;
> + spin_unlock_irq(_priv->irq_lock);
> +
> + /* right now PSR is only enabled in eDP */
> + WARN_ON(irq_aux_error & ~BIT(TRANSCODER_EDP));
> +
> + mutex_lock(>lock);
> +
> + intel_psr_disable_locked(psr->dp);
> + /* let's make sure that sink is awaken */
> + drm_dp_dpcd_writeb(>dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
We should be making sure the sink exits PSR after psr_invalidate() ->
psr_exit() too? Which means, we have to figure out a cleaner way to
handle all of this. I am not sure, at this point, what a cleaner
solution will like. However, I'd like PSR disable from invalidate, PSR
disable from a modeset and PSR disable due to an error share code and
behavior. All of them should basically be
1) Disable PSR in PSR_CTL
2) Wait for idle in PSR_STATUS
3) Write to sink DP_SET_POWER
> +
> + mutex_unlock(_priv->psr.lock);
> +}
> +
> static void intel_psr_work(struct work_struct *work)
> {
> struct drm_i915_private *dev_priv =
> container_of(work, typeof(*dev_priv), psr.work);
>
> + if (READ_ONCE(dev_priv->psr.irq_aux_error))
> + intel_psr_handle_irq(dev_priv);
> +
> mutex_lock(_priv->psr.lock);
>
> if (!dev_priv->psr.enabled)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Fix legacy DPMS changes with MST (rev6)
== Series Details == Series: Fix legacy DPMS changes with MST (rev6) URL : https://patchwork.freedesktop.org/series/49878/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4950 -> Patchwork_10393 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49878/revisions/6/mbox/ == Known issues == Here are the changes found in Patchwork_10393 that come from known issues: === IGT changes === Issues hit igt@amdgpu/amd_cs_nop@fork-gfx0: fi-kbl-8809g: PASS -> DMESG-WARN (fdo#107762) igt@gem_exec_suspend@basic-s3: fi-kbl-soraka: NOTRUN -> INCOMPLETE (fdo#107774, fdo#107556, fdo#107859) Possible fixes igt@gem_exec_suspend@basic-s3: fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#107139, fdo#105128) -> PASS igt@kms_flip@basic-flip-vs-modeset: fi-hsw-4770r: DMESG-WARN (fdo#105602) -> PASS igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107762 https://bugs.freedesktop.org/show_bug.cgi?id=107762 fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774 fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859 == Participating hosts (50 -> 45) == Additional (1): fi-kbl-soraka Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-ctg-p8600 == Build changes == * Linux: CI_DRM_4950 -> Patchwork_10393 CI_DRM_4950: a9abc43bebfb6de62c2c3747a22fadfa17b61d8b @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4671: b121f7d42c260ae3a050c3f440d1c11f7cff7d1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10393: 55b7754f85b9634317bf5fed722b9545dd13d6d0 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 55b7754f85b9 drm/i915: Fix intel_dp_mst_best_encoder() 47e4fc9bbf7c drm/i915: Skip vcpi allocation for MSTB ports that are gone 75b8025bd2e8 drm/i915: Don't unset intel_connector->mst_port de38c26c2d84 drm/nouveau: Fix nv50_mstc->best_encoder() c7fbf3e3184e drm/atomic_helper: Disallow new modesets on unregistered connectors == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10393/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 5/5] drm/i915: Fix intel_dp_mst_best_encoder()
Currently, i915 appears to rely on blocking modesets on no-longer-present MSTB ports by simply returning NULL for ->best_encoder(), which in turn causes any new atomic commits that don't disable the CRTC to fail. This is wrong however, since we still want to allow userspace to disable CRTCs on no-longer-present MSTB ports by changing the DPMS state to off and this still requires that we retrieve an encoder. So, fix this by always returning a valid encoder regardless of the state of the MST port. Changes since v1: - Remove mst atomic helper, since this got replaced with a much simpler solution Signed-off-by: Lyude Paul Reviewed-by: Daniel Vetter Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/intel_dp_mst.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 0f14c0d1669c..7f155b4f1a7d 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -404,8 +404,6 @@ static struct drm_encoder *intel_mst_atomic_best_encoder(struct drm_connector *c struct intel_dp *intel_dp = intel_connector->mst_port; struct intel_crtc *crtc = to_intel_crtc(state->crtc); - if (!READ_ONCE(connector->registered)) - return NULL; return _dp->mst_encoders[crtc->pipe]->base.base; } -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 4/5] drm/i915: Skip vcpi allocation for MSTB ports that are gone
Since we need to be able to allow DPMS on->off prop changes after an MST
port has disappeared from the system, we need to be able to make sure we
can compute a config for the resulting atomic commit. Currently this is
impossible when the port has disappeared, since the VCPI slot searching
we try to do in intel_dp_mst_compute_config() will fail with -EINVAL.
Since the only commits we want to allow on no-longer-present MST ports
are ones that shut off display hardware, we already know that no VCPI
allocations are needed. So, hardcode the VCPI slot count to 0 when
intel_dp_mst_compute_config() is called on an MST port that's gone.
Changes since V4:
- Don't use mst_port_gone at all, just check whether or not the drm
connector is registered - Daniel Vetter
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/intel_dp_mst.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index aa21742d8634..0f14c0d1669c 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -38,11 +38,11 @@ static bool intel_dp_mst_compute_config(struct
intel_encoder *encoder,
struct intel_dp_mst_encoder *intel_mst = enc_to_mst(>base);
struct intel_digital_port *intel_dig_port = intel_mst->primary;
struct intel_dp *intel_dp = _dig_port->dp;
- struct intel_connector *connector =
- to_intel_connector(conn_state->connector);
+ struct drm_connector *connector = conn_state->connector;
+ void *port = to_intel_connector(connector)->port;
struct drm_atomic_state *state = pipe_config->base.state;
int bpp;
- int lane_count, slots;
+ int lane_count, slots = 0;
const struct drm_display_mode *adjusted_mode =
_config->base.adjusted_mode;
int mst_pbn;
bool constant_n = drm_dp_has_quirk(_dp->desc,
@@ -70,17 +70,23 @@ static bool intel_dp_mst_compute_config(struct
intel_encoder *encoder,
pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);
- if (drm_dp_mst_port_has_audio(_dp->mst_mgr, connector->port))
+ if (drm_dp_mst_port_has_audio(_dp->mst_mgr, port))
pipe_config->has_audio = true;
mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
pipe_config->pbn = mst_pbn;
- slots = drm_dp_atomic_find_vcpi_slots(state, _dp->mst_mgr,
- connector->port, mst_pbn);
- if (slots < 0) {
- DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);
- return false;
+ /* Zombie connectors can't have VCPI slots */
+ if (READ_ONCE(connector->registered)) {
+ slots = drm_dp_atomic_find_vcpi_slots(state,
+ _dp->mst_mgr,
+ port,
+ mst_pbn);
+ if (slots < 0) {
+ DRM_DEBUG_KMS("failed finding vcpi slots:%d\n",
+ slots);
+ return false;
+ }
}
intel_link_compute_m_n(bpp, lane_count,
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 1/5] drm/atomic_helper: Disallow new modesets on unregistered connectors
With the exception of modesets which would switch the DPMS state of a
connector from on to off, we want to make sure that we disallow all
modesets which would result in enabling a new monitor or a new mode
configuration on a monitor if the connector for the display in question
is no longer registered. This allows us to stop userspace from trying to
enable new displays on connectors for an MST topology that were just
removed from the system, without preventing userspace from disabling
DPMS on those connectors.
Changes since v5:
- Fix typo in comment, nothing else
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_atomic_helper.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 6f66777dca4b..e6a2cf72de5e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -319,6 +319,26 @@ update_connector_routing(struct drm_atomic_state *state,
return 0;
}
+ crtc_state = drm_atomic_get_new_crtc_state(state,
+ new_connector_state->crtc);
+ /*
+* For compatibility with legacy users, we want to make sure that
+* we allow DPMS On->Off modesets on unregistered connectors. Modesets
+* which would result in anything else must be considered invalid, to
+* avoid turning on new displays on dead connectors.
+*
+* Since the connector can be unregistered at any point during an
+* atomic check or commit, this is racy. But that's OK: all we care
+* about is ensuring that userspace can't do anything but shut off the
+* display on a connector that was destroyed after its been notified,
+* not before.
+*/
+ if (!READ_ONCE(connector->registered) && crtc_state->active) {
+ DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n",
+connector->base.id, connector->name);
+ return -EINVAL;
+ }
+
funcs = connector->helper_private;
if (funcs->atomic_best_encoder)
@@ -363,7 +383,6 @@ update_connector_routing(struct drm_atomic_state *state,
set_best_encoder(state, new_connector_state, new_encoder);
- crtc_state = drm_atomic_get_new_crtc_state(state,
new_connector_state->crtc);
crtc_state->connectors_changed = true;
DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on
[CRTC:%d:%s]\n",
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 3/5] drm/i915: Don't unset intel_connector->mst_port
Currently we set intel_connector->mst_port to NULL to signify that the
MST port has been removed from the system so that we can prevent further
action on the port such as connector probes, mode probing, etc.
However, we're going to need access to intel_connector->mst_port in
order to fixup ->best_encoder() so that it can always return the correct
encoder for an MST port to prevent legacy DPMS prop changes from
failing. This should be safe, so instead keep intel_connector->mst_port
always set and instead just check the status of
drm_connector->regustered to signify whether or not the connector has
disappeared from the system.
Changes since v2:
- Add a comment to mst_port_gone (Jani Nikula)
- Change mst_port_gone to a u8 instead of a bool, per the kernel bot.
Apparently bool is discouraged in structs these days
Changes since v4:
- Don't use mst_port_gone at all! Just check if the connector is
registered or not - Daniel Vetter
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/intel_dp_mst.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index 43db2e9ac575..aa21742d8634 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -307,9 +307,8 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector
*connector)
struct edid *edid;
int ret;
- if (!intel_dp) {
+ if (!READ_ONCE(connector->registered))
return intel_connector_update_modes(connector, NULL);
- }
edid = drm_dp_mst_get_edid(connector, _dp->mst_mgr,
intel_connector->port);
ret = intel_connector_update_modes(connector, edid);
@@ -324,9 +323,10 @@ intel_dp_mst_detect(struct drm_connector *connector, bool
force)
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = intel_connector->mst_port;
- if (!intel_dp)
+ if (!READ_ONCE(connector->registered))
return connector_status_disconnected;
- return drm_dp_mst_detect_port(connector, _dp->mst_mgr,
intel_connector->port);
+ return drm_dp_mst_detect_port(connector, _dp->mst_mgr,
+ intel_connector->port);
}
static void
@@ -366,7 +366,7 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
int bpp = 24; /* MST uses fixed bpp */
int max_rate, mode_rate, max_lanes, max_link_clock;
- if (!intel_dp)
+ if (!READ_ONCE(connector->registered))
return MODE_ERROR;
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
@@ -398,7 +398,7 @@ static struct drm_encoder
*intel_mst_atomic_best_encoder(struct drm_connector *c
struct intel_dp *intel_dp = intel_connector->mst_port;
struct intel_crtc *crtc = to_intel_crtc(state->crtc);
- if (!intel_dp)
+ if (!READ_ONCE(connector->registered))
return NULL;
return _dp->mst_encoders[crtc->pipe]->base.base;
}
@@ -499,7 +499,6 @@ static void intel_dp_register_mst_connector(struct
drm_connector *connector)
static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
struct drm_connector *connector)
{
- struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_i915_private *dev_priv = to_i915(connector->dev);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
connector->name);
@@ -508,10 +507,6 @@ static void intel_dp_destroy_mst_connector(struct
drm_dp_mst_topology_mgr *mgr,
if (dev_priv->fbdev)
drm_fb_helper_remove_one_connector(_priv->fbdev->helper,
connector);
- /* prevent race with the check in ->detect */
- drm_modeset_lock(>dev->mode_config.connection_mutex, NULL);
- intel_connector->mst_port = NULL;
- drm_modeset_unlock(>dev->mode_config.connection_mutex);
drm_connector_put(connector);
}
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 2/5] drm/nouveau: Fix nv50_mstc->best_encoder()
As mentioned in the previous commit, we currently prevent new modesets
on recently-removed MST connectors by returning no encoder from our
->best_encoder() callback once the MST port has disappeared. This is
wrong however, because it prevents legacy modesetting users from being
able to disable CRTCs on MST connectors after the connector's respective
topology has disappeared.
So, fix this by instead by just always returning a valid encoder.
Changes since v2:
- Remove usage of atomic MST helper for now, since that got replaced
with a much simpler solution
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 9f32b10c7c29..31b94bc9ec90 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -843,22 +843,16 @@ nv50_mstc_atomic_best_encoder(struct drm_connector
*connector,
{
struct nv50_head *head = nv50_head(connector_state->crtc);
struct nv50_mstc *mstc = nv50_mstc(connector);
- if (mstc->port) {
- struct nv50_mstm *mstm = mstc->mstm;
- return >msto[head->base.index]->encoder;
- }
- return NULL;
+
+ return >mstm->msto[head->base.index]->encoder;
}
static struct drm_encoder *
nv50_mstc_best_encoder(struct drm_connector *connector)
{
struct nv50_mstc *mstc = nv50_mstc(connector);
- if (mstc->port) {
- struct nv50_mstm *mstm = mstc->mstm;
- return >msto[0]->encoder;
- }
- return NULL;
+
+ return >mstm->msto[0]->encoder;
}
static enum drm_mode_status
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 0/5] Fix legacy DPMS changes with MST
Next version of https://patchwork.freedesktop.org/series/49878/ Still no functional changes, just removing a duplicate s-b to make CI happy. Lyude Paul (5): drm/atomic_helper: Disallow new modesets on unregistered connectors drm/nouveau: Fix nv50_mstc->best_encoder() drm/i915: Don't unset intel_connector->mst_port drm/i915: Skip vcpi allocation for MSTB ports that are gone drm/i915: Fix intel_dp_mst_best_encoder() drivers/gpu/drm/drm_atomic_helper.c | 21 - drivers/gpu/drm/i915/intel_dp_mst.c | 41 - drivers/gpu/drm/nouveau/dispnv50/disp.c | 14 +++-- 3 files changed, 44 insertions(+), 32 deletions(-) -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Fix legacy DPMS changes with MST (rev6)
== Series Details == Series: Fix legacy DPMS changes with MST (rev6) URL : https://patchwork.freedesktop.org/series/49878/ State : warning == Summary == $ dim checkpatch origin/drm-tip c7fbf3e3184e drm/atomic_helper: Disallow new modesets on unregistered connectors de38c26c2d84 drm/nouveau: Fix nv50_mstc->best_encoder() 75b8025bd2e8 drm/i915: Don't unset intel_connector->mst_port 47e4fc9bbf7c drm/i915: Skip vcpi allocation for MSTB ports that are gone -:25: WARNING:BAD_SIGN_OFF: Duplicate signature #25: Signed-off-by: Lyude Paul total: 0 errors, 1 warnings, 0 checks, 43 lines checked 55b7754f85b9 drm/i915: Fix intel_dp_mst_best_encoder() ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for Fix legacy DPMS changes with MST (rev5)
== Series Details == Series: Fix legacy DPMS changes with MST (rev5) URL : https://patchwork.freedesktop.org/series/49878/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4950 -> Patchwork_10392 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/49878/revisions/5/mbox/ == Known issues == Here are the changes found in Patchwork_10392 that come from known issues: === IGT changes === Issues hit igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191) Possible fixes igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#105128, fdo#107139) -> PASS igt@kms_flip@basic-flip-vs-modeset: fi-hsw-4770r: DMESG-WARN (fdo#105602) -> PASS igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 == Participating hosts (50 -> 44) == Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-ctg-p8600 == Build changes == * Linux: CI_DRM_4950 -> Patchwork_10392 CI_DRM_4950: a9abc43bebfb6de62c2c3747a22fadfa17b61d8b @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4671: b121f7d42c260ae3a050c3f440d1c11f7cff7d1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10392: fe8e2b97c2f3ae1cb276057b823353070689c53e @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == fe8e2b97c2f3 drm/i915: Fix intel_dp_mst_best_encoder() 38ea4d7ed971 drm/i915: Skip vcpi allocation for MSTB ports that are gone b4b4e8660291 drm/i915: Don't unset intel_connector->mst_port 492652e4f510 drm/nouveau: Fix nv50_mstc->best_encoder() e2f961e30125 drm/atomic_helper: Disallow new modesets on unregistered connectors == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10392/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Fix legacy DPMS changes with MST (rev5)
== Series Details == Series: Fix legacy DPMS changes with MST (rev5) URL : https://patchwork.freedesktop.org/series/49878/ State : warning == Summary == $ dim checkpatch origin/drm-tip e2f961e30125 drm/atomic_helper: Disallow new modesets on unregistered connectors 492652e4f510 drm/nouveau: Fix nv50_mstc->best_encoder() b4b4e8660291 drm/i915: Don't unset intel_connector->mst_port 38ea4d7ed971 drm/i915: Skip vcpi allocation for MSTB ports that are gone -:25: WARNING:BAD_SIGN_OFF: Duplicate signature #25: Signed-off-by: Lyude Paul total: 0 errors, 1 warnings, 0 checks, 43 lines checked fe8e2b97c2f3 drm/i915: Fix intel_dp_mst_best_encoder() ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 5/5] drm/i915: Fix intel_dp_mst_best_encoder()
Currently, i915 appears to rely on blocking modesets on no-longer-present MSTB ports by simply returning NULL for ->best_encoder(), which in turn causes any new atomic commits that don't disable the CRTC to fail. This is wrong however, since we still want to allow userspace to disable CRTCs on no-longer-present MSTB ports by changing the DPMS state to off and this still requires that we retrieve an encoder. So, fix this by always returning a valid encoder regardless of the state of the MST port. Changes since v1: - Remove mst atomic helper, since this got replaced with a much simpler solution Signed-off-by: Lyude Paul Reviewed-by: Daniel Vetter Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/intel_dp_mst.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 0f14c0d1669c..7f155b4f1a7d 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -404,8 +404,6 @@ static struct drm_encoder *intel_mst_atomic_best_encoder(struct drm_connector *c struct intel_dp *intel_dp = intel_connector->mst_port; struct intel_crtc *crtc = to_intel_crtc(state->crtc); - if (!READ_ONCE(connector->registered)) - return NULL; return _dp->mst_encoders[crtc->pipe]->base.base; } -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 1/5] drm/atomic_helper: Disallow new modesets on unregistered connectors
With the exception of modesets which would switch the DPMS state of a
connector from on to off, we want to make sure that we disallow all
modesets which would result in enabling a new monitor or a new mode
configuration on a monitor if the connector for the display in question
is no longer registered. This allows us to stop userspace from trying to
enable new displays on connectors for an MST topology that were just
removed from the system, without preventing userspace from disabling
DPMS on those connectors.
Changes since v5:
- Fix typo in comment, nothing else
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_atomic_helper.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 6f66777dca4b..e6a2cf72de5e 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -319,6 +319,26 @@ update_connector_routing(struct drm_atomic_state *state,
return 0;
}
+ crtc_state = drm_atomic_get_new_crtc_state(state,
+ new_connector_state->crtc);
+ /*
+* For compatibility with legacy users, we want to make sure that
+* we allow DPMS On->Off modesets on unregistered connectors. Modesets
+* which would result in anything else must be considered invalid, to
+* avoid turning on new displays on dead connectors.
+*
+* Since the connector can be unregistered at any point during an
+* atomic check or commit, this is racy. But that's OK: all we care
+* about is ensuring that userspace can't do anything but shut off the
+* display on a connector that was destroyed after its been notified,
+* not before.
+*/
+ if (!READ_ONCE(connector->registered) && crtc_state->active) {
+ DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n",
+connector->base.id, connector->name);
+ return -EINVAL;
+ }
+
funcs = connector->helper_private;
if (funcs->atomic_best_encoder)
@@ -363,7 +383,6 @@ update_connector_routing(struct drm_atomic_state *state,
set_best_encoder(state, new_connector_state, new_encoder);
- crtc_state = drm_atomic_get_new_crtc_state(state,
new_connector_state->crtc);
crtc_state->connectors_changed = true;
DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on
[CRTC:%d:%s]\n",
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 3/5] drm/i915: Don't unset intel_connector->mst_port
Currently we set intel_connector->mst_port to NULL to signify that the
MST port has been removed from the system so that we can prevent further
action on the port such as connector probes, mode probing, etc.
However, we're going to need access to intel_connector->mst_port in
order to fixup ->best_encoder() so that it can always return the correct
encoder for an MST port to prevent legacy DPMS prop changes from
failing. This should be safe, so instead keep intel_connector->mst_port
always set and instead just check the status of
drm_connector->regustered to signify whether or not the connector has
disappeared from the system.
Changes since v2:
- Add a comment to mst_port_gone (Jani Nikula)
- Change mst_port_gone to a u8 instead of a bool, per the kernel bot.
Apparently bool is discouraged in structs these days
Changes since v4:
- Don't use mst_port_gone at all! Just check if the connector is
registered or not - Daniel Vetter
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/intel_dp_mst.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index 43db2e9ac575..aa21742d8634 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -307,9 +307,8 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector
*connector)
struct edid *edid;
int ret;
- if (!intel_dp) {
+ if (!READ_ONCE(connector->registered))
return intel_connector_update_modes(connector, NULL);
- }
edid = drm_dp_mst_get_edid(connector, _dp->mst_mgr,
intel_connector->port);
ret = intel_connector_update_modes(connector, edid);
@@ -324,9 +323,10 @@ intel_dp_mst_detect(struct drm_connector *connector, bool
force)
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = intel_connector->mst_port;
- if (!intel_dp)
+ if (!READ_ONCE(connector->registered))
return connector_status_disconnected;
- return drm_dp_mst_detect_port(connector, _dp->mst_mgr,
intel_connector->port);
+ return drm_dp_mst_detect_port(connector, _dp->mst_mgr,
+ intel_connector->port);
}
static void
@@ -366,7 +366,7 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
int bpp = 24; /* MST uses fixed bpp */
int max_rate, mode_rate, max_lanes, max_link_clock;
- if (!intel_dp)
+ if (!READ_ONCE(connector->registered))
return MODE_ERROR;
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
@@ -398,7 +398,7 @@ static struct drm_encoder
*intel_mst_atomic_best_encoder(struct drm_connector *c
struct intel_dp *intel_dp = intel_connector->mst_port;
struct intel_crtc *crtc = to_intel_crtc(state->crtc);
- if (!intel_dp)
+ if (!READ_ONCE(connector->registered))
return NULL;
return _dp->mst_encoders[crtc->pipe]->base.base;
}
@@ -499,7 +499,6 @@ static void intel_dp_register_mst_connector(struct
drm_connector *connector)
static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
struct drm_connector *connector)
{
- struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_i915_private *dev_priv = to_i915(connector->dev);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
connector->name);
@@ -508,10 +507,6 @@ static void intel_dp_destroy_mst_connector(struct
drm_dp_mst_topology_mgr *mgr,
if (dev_priv->fbdev)
drm_fb_helper_remove_one_connector(_priv->fbdev->helper,
connector);
- /* prevent race with the check in ->detect */
- drm_modeset_lock(>dev->mode_config.connection_mutex, NULL);
- intel_connector->mst_port = NULL;
- drm_modeset_unlock(>dev->mode_config.connection_mutex);
drm_connector_put(connector);
}
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 4/5] drm/i915: Skip vcpi allocation for MSTB ports that are gone
Since we need to be able to allow DPMS on->off prop changes after an MST
port has disappeared from the system, we need to be able to make sure we
can compute a config for the resulting atomic commit. Currently this is
impossible when the port has disappeared, since the VCPI slot searching
we try to do in intel_dp_mst_compute_config() will fail with -EINVAL.
Since the only commits we want to allow on no-longer-present MST ports
are ones that shut off display hardware, we already know that no VCPI
allocations are needed. So, hardcode the VCPI slot count to 0 when
intel_dp_mst_compute_config() is called on an MST port that's gone.
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
Changes since V4:
- Don't use mst_port_gone at all, just check whether or not the drm
connector is registered - Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/i915/intel_dp_mst.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index aa21742d8634..0f14c0d1669c 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -38,11 +38,11 @@ static bool intel_dp_mst_compute_config(struct
intel_encoder *encoder,
struct intel_dp_mst_encoder *intel_mst = enc_to_mst(>base);
struct intel_digital_port *intel_dig_port = intel_mst->primary;
struct intel_dp *intel_dp = _dig_port->dp;
- struct intel_connector *connector =
- to_intel_connector(conn_state->connector);
+ struct drm_connector *connector = conn_state->connector;
+ void *port = to_intel_connector(connector)->port;
struct drm_atomic_state *state = pipe_config->base.state;
int bpp;
- int lane_count, slots;
+ int lane_count, slots = 0;
const struct drm_display_mode *adjusted_mode =
_config->base.adjusted_mode;
int mst_pbn;
bool constant_n = drm_dp_has_quirk(_dp->desc,
@@ -70,17 +70,23 @@ static bool intel_dp_mst_compute_config(struct
intel_encoder *encoder,
pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);
- if (drm_dp_mst_port_has_audio(_dp->mst_mgr, connector->port))
+ if (drm_dp_mst_port_has_audio(_dp->mst_mgr, port))
pipe_config->has_audio = true;
mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
pipe_config->pbn = mst_pbn;
- slots = drm_dp_atomic_find_vcpi_slots(state, _dp->mst_mgr,
- connector->port, mst_pbn);
- if (slots < 0) {
- DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);
- return false;
+ /* Zombie connectors can't have VCPI slots */
+ if (READ_ONCE(connector->registered)) {
+ slots = drm_dp_atomic_find_vcpi_slots(state,
+ _dp->mst_mgr,
+ port,
+ mst_pbn);
+ if (slots < 0) {
+ DRM_DEBUG_KMS("failed finding vcpi slots:%d\n",
+ slots);
+ return false;
+ }
}
intel_link_compute_m_n(bpp, lane_count,
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 2/5] drm/nouveau: Fix nv50_mstc->best_encoder()
As mentioned in the previous commit, we currently prevent new modesets
on recently-removed MST connectors by returning no encoder from our
->best_encoder() callback once the MST port has disappeared. This is
wrong however, because it prevents legacy modesetting users from being
able to disable CRTCs on MST connectors after the connector's respective
topology has disappeared.
So, fix this by instead by just always returning a valid encoder.
Changes since v2:
- Remove usage of atomic MST helper for now, since that got replaced
with a much simpler solution
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 9f32b10c7c29..31b94bc9ec90 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -843,22 +843,16 @@ nv50_mstc_atomic_best_encoder(struct drm_connector
*connector,
{
struct nv50_head *head = nv50_head(connector_state->crtc);
struct nv50_mstc *mstc = nv50_mstc(connector);
- if (mstc->port) {
- struct nv50_mstm *mstm = mstc->mstm;
- return >msto[head->base.index]->encoder;
- }
- return NULL;
+
+ return >mstm->msto[head->base.index]->encoder;
}
static struct drm_encoder *
nv50_mstc_best_encoder(struct drm_connector *connector)
{
struct nv50_mstc *mstc = nv50_mstc(connector);
- if (mstc->port) {
- struct nv50_mstm *mstm = mstc->mstm;
- return >msto[0]->encoder;
- }
- return NULL;
+
+ return >mstm->msto[0]->encoder;
}
static enum drm_mode_status
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 0/5] Fix legacy DPMS changes with MST
Next version of https://patchwork.freedesktop.org/series/49878/ No functional changes, just a typo fix Lyude Paul (5): drm/atomic_helper: Disallow new modesets on unregistered connectors drm/nouveau: Fix nv50_mstc->best_encoder() drm/i915: Don't unset intel_connector->mst_port drm/i915: Skip vcpi allocation for MSTB ports that are gone drm/i915: Fix intel_dp_mst_best_encoder() drivers/gpu/drm/drm_atomic_helper.c | 21 - drivers/gpu/drm/i915/intel_dp_mst.c | 41 - drivers/gpu/drm/nouveau/dispnv50/disp.c | 14 +++-- 3 files changed, 44 insertions(+), 32 deletions(-) -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/psr: Always wait for idle state when disabling PSR
On Fri, 2018-10-05 at 16:35 -0700, José Roberto de Souza wrote:
> It should always wait for idle state when disabling PSR because PSR
> could be inactive due a call to intel_psr_exit() and while PSR is
> still being disabled asynchronously userspace could change the
> modeset causing a call to psr_disable() that will not wait for PSR
> idle and then PSR will be enabled again while PSR is still not idle.
Agreed.
>
> Cc: Rodrigo Vivi
> Cc: Dhinakaran Pandiyan
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/intel_psr.c | 43 +++---
> --
> 1 file changed, 20 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 423cdf84059c..cd9a60d1efa1 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -661,40 +661,37 @@ static void
> intel_psr_disable_source(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> + i915_reg_t psr_status;
> + u32 psr_status_mask;
>
> if (dev_priv->psr.active) {
> - i915_reg_t psr_status;
> - u32 psr_status_mask;
> -
> - if (dev_priv->psr.psr2_enabled) {
> - psr_status = EDP_PSR2_STATUS;
> - psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> -
> + if (dev_priv->psr.psr2_enabled)
> I915_WRITE(EDP_PSR2_CTL,
> -I915_READ(EDP_PSR2_CTL) &
> -~(EDP_PSR2_ENABLE |
> EDP_SU_TRACK_ENABLE));
> -
> - } else {
> - psr_status = EDP_PSR_STATUS;
> - psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> -
> +I915_READ(EDP_PSR2_CTL) &
> ~EDP_PSR2_ENABLE);
Is there a way to reuse psr_exit() and move rest of the stuff to the
caller? We won't not need disable_source() if that can be done?
> + else
> I915_WRITE(EDP_PSR_CTL,
> I915_READ(EDP_PSR_CTL) &
> ~EDP_PSR_ENABLE);
> - }
> -
> - /* Wait till PSR is idle */
> - if (intel_wait_for_register(dev_priv,
> - psr_status,
> psr_status_mask, 0,
> - 2000))
> - DRM_ERROR("Timed out waiting for PSR Idle
> State\n");
> -
> - dev_priv->psr.active = false;
> } else {
> if (dev_priv->psr.psr2_enabled)
> WARN_ON(I915_READ(EDP_PSR2_CTL) &
> EDP_PSR2_ENABLE);
> else
> WARN_ON(I915_READ(EDP_PSR_CTL) &
> EDP_PSR_ENABLE);
> }
> +
> + if (dev_priv->psr.psr2_enabled) {
> + psr_status = EDP_PSR2_STATUS;
> + psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> + } else {
> + psr_status = EDP_PSR_STATUS;
> + psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> + }
> +
> + /* Wait till PSR is idle */
> + if (intel_wait_for_register(dev_priv, psr_status,
> psr_status_mask, 0,
> + 2000))
> + DRM_ERROR("Timed out waiting for PSR Idle State\n");
> +
> + dev_priv->psr.active = false;
> }
>
> static void intel_psr_disable_locked(struct intel_dp *intel_dp)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 5/5] drm/i915: Fix intel_dp_mst_best_encoder()
Currently, i915 appears to rely on blocking modesets on no-longer-present MSTB ports by simply returning NULL for ->best_encoder(), which in turn causes any new atomic commits that don't disable the CRTC to fail. This is wrong however, since we still want to allow userspace to disable CRTCs on no-longer-present MSTB ports by changing the DPMS state to off and this still requires that we retrieve an encoder. So, fix this by always returning a valid encoder regardless of the state of the MST port. Changes since v1: - Remove mst atomic helper, since this got replaced with a much simpler solution Signed-off-by: Lyude Paul Reviewed-by: Daniel Vetter Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/intel_dp_mst.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 0f14c0d1669c..7f155b4f1a7d 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -404,8 +404,6 @@ static struct drm_encoder *intel_mst_atomic_best_encoder(struct drm_connector *c struct intel_dp *intel_dp = intel_connector->mst_port; struct intel_crtc *crtc = to_intel_crtc(state->crtc); - if (!READ_ONCE(connector->registered)) - return NULL; return _dp->mst_encoders[crtc->pipe]->base.base; } -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 3/5] drm/i915: Don't unset intel_connector->mst_port
Currently we set intel_connector->mst_port to NULL to signify that the
MST port has been removed from the system so that we can prevent further
action on the port such as connector probes, mode probing, etc.
However, we're going to need access to intel_connector->mst_port in
order to fixup ->best_encoder() so that it can always return the correct
encoder for an MST port to prevent legacy DPMS prop changes from
failing. This should be safe, so instead keep intel_connector->mst_port
always set and instead just check the status of
drm_connector->regustered to signify whether or not the connector has
disappeared from the system.
Changes since v2:
- Add a comment to mst_port_gone (Jani Nikula)
- Change mst_port_gone to a u8 instead of a bool, per the kernel bot.
Apparently bool is discouraged in structs these days
Changes since v4:
- Don't use mst_port_gone at all! Just check if the connector is
registered or not - Daniel Vetter
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/intel_dp_mst.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index 43db2e9ac575..aa21742d8634 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -307,9 +307,8 @@ static int intel_dp_mst_get_ddc_modes(struct drm_connector
*connector)
struct edid *edid;
int ret;
- if (!intel_dp) {
+ if (!READ_ONCE(connector->registered))
return intel_connector_update_modes(connector, NULL);
- }
edid = drm_dp_mst_get_edid(connector, _dp->mst_mgr,
intel_connector->port);
ret = intel_connector_update_modes(connector, edid);
@@ -324,9 +323,10 @@ intel_dp_mst_detect(struct drm_connector *connector, bool
force)
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = intel_connector->mst_port;
- if (!intel_dp)
+ if (!READ_ONCE(connector->registered))
return connector_status_disconnected;
- return drm_dp_mst_detect_port(connector, _dp->mst_mgr,
intel_connector->port);
+ return drm_dp_mst_detect_port(connector, _dp->mst_mgr,
+ intel_connector->port);
}
static void
@@ -366,7 +366,7 @@ intel_dp_mst_mode_valid(struct drm_connector *connector,
int bpp = 24; /* MST uses fixed bpp */
int max_rate, mode_rate, max_lanes, max_link_clock;
- if (!intel_dp)
+ if (!READ_ONCE(connector->registered))
return MODE_ERROR;
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
@@ -398,7 +398,7 @@ static struct drm_encoder
*intel_mst_atomic_best_encoder(struct drm_connector *c
struct intel_dp *intel_dp = intel_connector->mst_port;
struct intel_crtc *crtc = to_intel_crtc(state->crtc);
- if (!intel_dp)
+ if (!READ_ONCE(connector->registered))
return NULL;
return _dp->mst_encoders[crtc->pipe]->base.base;
}
@@ -499,7 +499,6 @@ static void intel_dp_register_mst_connector(struct
drm_connector *connector)
static void intel_dp_destroy_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
struct drm_connector *connector)
{
- struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_i915_private *dev_priv = to_i915(connector->dev);
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
connector->name);
@@ -508,10 +507,6 @@ static void intel_dp_destroy_mst_connector(struct
drm_dp_mst_topology_mgr *mgr,
if (dev_priv->fbdev)
drm_fb_helper_remove_one_connector(_priv->fbdev->helper,
connector);
- /* prevent race with the check in ->detect */
- drm_modeset_lock(>dev->mode_config.connection_mutex, NULL);
- intel_connector->mst_port = NULL;
- drm_modeset_unlock(>dev->mode_config.connection_mutex);
drm_connector_put(connector);
}
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 4/5] drm/i915: Skip vcpi allocation for MSTB ports that are gone
Since we need to be able to allow DPMS on->off prop changes after an MST
port has disappeared from the system, we need to be able to make sure we
can compute a config for the resulting atomic commit. Currently this is
impossible when the port has disappeared, since the VCPI slot searching
we try to do in intel_dp_mst_compute_config() will fail with -EINVAL.
Since the only commits we want to allow on no-longer-present MST ports
are ones that shut off display hardware, we already know that no VCPI
allocations are needed. So, hardcode the VCPI slot count to 0 when
intel_dp_mst_compute_config() is called on an MST port that's gone.
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
Changes since V4:
- Don't use mst_port_gone at all, just check whether or not the drm
connector is registered - Daniel Vetter
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/i915/intel_dp_mst.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index aa21742d8634..0f14c0d1669c 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -38,11 +38,11 @@ static bool intel_dp_mst_compute_config(struct
intel_encoder *encoder,
struct intel_dp_mst_encoder *intel_mst = enc_to_mst(>base);
struct intel_digital_port *intel_dig_port = intel_mst->primary;
struct intel_dp *intel_dp = _dig_port->dp;
- struct intel_connector *connector =
- to_intel_connector(conn_state->connector);
+ struct drm_connector *connector = conn_state->connector;
+ void *port = to_intel_connector(connector)->port;
struct drm_atomic_state *state = pipe_config->base.state;
int bpp;
- int lane_count, slots;
+ int lane_count, slots = 0;
const struct drm_display_mode *adjusted_mode =
_config->base.adjusted_mode;
int mst_pbn;
bool constant_n = drm_dp_has_quirk(_dp->desc,
@@ -70,17 +70,23 @@ static bool intel_dp_mst_compute_config(struct
intel_encoder *encoder,
pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);
- if (drm_dp_mst_port_has_audio(_dp->mst_mgr, connector->port))
+ if (drm_dp_mst_port_has_audio(_dp->mst_mgr, port))
pipe_config->has_audio = true;
mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
pipe_config->pbn = mst_pbn;
- slots = drm_dp_atomic_find_vcpi_slots(state, _dp->mst_mgr,
- connector->port, mst_pbn);
- if (slots < 0) {
- DRM_DEBUG_KMS("failed finding vcpi slots:%d\n", slots);
- return false;
+ /* Zombie connectors can't have VCPI slots */
+ if (READ_ONCE(connector->registered)) {
+ slots = drm_dp_atomic_find_vcpi_slots(state,
+ _dp->mst_mgr,
+ port,
+ mst_pbn);
+ if (slots < 0) {
+ DRM_DEBUG_KMS("failed finding vcpi slots:%d\n",
+ slots);
+ return false;
+ }
}
intel_link_compute_m_n(bpp, lane_count,
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 1/5] drm/atomic_helper: Disallow new modesets on unregistered connectors
With the exception of modesets which would switch the DPMS state of a
connector from on to off, we want to make sure that we disallow all
modesets which would result in enabling a new monitor or a new mode
configuration on a monitor if the connector for the display in question
is no longer registered. This allows us to stop userspace from trying to
enable new displays on connectors for an MST topology that were just
removed from the system, without preventing userspace from disabling
DPMS on those connectors.
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_atomic_helper.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 6f66777dca4b..788749021ac9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -319,6 +319,26 @@ update_connector_routing(struct drm_atomic_state *state,
return 0;
}
+ crtc_state = drm_atomic_get_new_crtc_state(state,
+ new_connector_state->crtc);
+ /*
+* For compatibility with legacy users, we want to make sure that
+* we allow DPMS On->Off modesets on unregistered connectors. Modesets
+* which would result in anything else must be considered invalid, to
+* avoid turning on new displays on dead connectors.
+*
+* Since the connector can be unregistered at any point during an
+* atomic check or commit, this is racy. But that's OK: all we care
+* about is ensuring that userspace can't do anything but shut off the
+* display on a connector that was destroyed after it's been notified,
+* not before.
+*/
+ if (!READ_ONCE(connector->registered) && crtc_state->active) {
+ DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] is not registered\n",
+connector->base.id, connector->name);
+ return -EINVAL;
+ }
+
funcs = connector->helper_private;
if (funcs->atomic_best_encoder)
@@ -363,7 +383,6 @@ update_connector_routing(struct drm_atomic_state *state,
set_best_encoder(state, new_connector_state, new_encoder);
- crtc_state = drm_atomic_get_new_crtc_state(state,
new_connector_state->crtc);
crtc_state->connectors_changed = true;
DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] using [ENCODER:%d:%s] on
[CRTC:%d:%s]\n",
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 2/5] drm/nouveau: Fix nv50_mstc->best_encoder()
As mentioned in the previous commit, we currently prevent new modesets
on recently-removed MST connectors by returning no encoder from our
->best_encoder() callback once the MST port has disappeared. This is
wrong however, because it prevents legacy modesetting users from being
able to disable CRTCs on MST connectors after the connector's respective
topology has disappeared.
So, fix this by instead by just always returning a valid encoder.
Changes since v2:
- Remove usage of atomic MST helper for now, since that got replaced
with a much simpler solution
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 9f32b10c7c29..31b94bc9ec90 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -843,22 +843,16 @@ nv50_mstc_atomic_best_encoder(struct drm_connector
*connector,
{
struct nv50_head *head = nv50_head(connector_state->crtc);
struct nv50_mstc *mstc = nv50_mstc(connector);
- if (mstc->port) {
- struct nv50_mstm *mstm = mstc->mstm;
- return >msto[head->base.index]->encoder;
- }
- return NULL;
+
+ return >mstm->msto[head->base.index]->encoder;
}
static struct drm_encoder *
nv50_mstc_best_encoder(struct drm_connector *connector)
{
struct nv50_mstc *mstc = nv50_mstc(connector);
- if (mstc->port) {
- struct nv50_mstm *mstm = mstc->mstm;
- return >msto[0]->encoder;
- }
- return NULL;
+
+ return >mstm->msto[0]->encoder;
}
static enum drm_mode_status
--
2.17.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v5 0/5] Fix legacy DPMS changes with MST
Latest version of https://patchwork.freedesktop.org/series/49878/ Lyude Paul (5): drm/atomic_helper: Disallow new modesets on unregistered connectors drm/nouveau: Fix nv50_mstc->best_encoder() drm/i915: Don't unset intel_connector->mst_port drm/i915: Skip vcpi allocation for MSTB ports that are gone drm/i915: Fix intel_dp_mst_best_encoder() drivers/gpu/drm/drm_atomic_helper.c | 21 - drivers/gpu/drm/i915/intel_dp_mst.c | 41 - drivers/gpu/drm/nouveau/dispnv50/disp.c | 14 +++-- 3 files changed, 44 insertions(+), 32 deletions(-) -- 2.17.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Rename full ppgtt configuration to be more generic (rev6)
== Series Details ==
Series: drm/i915: Rename full ppgtt configuration to be more generic (rev6)
URL : https://patchwork.freedesktop.org/series/49021/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4950_full -> Patchwork_10391_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10391_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10391_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in
Patchwork_10391_full:
=== IGT changes ===
Warnings
igt@pm_rc6_residency@rc6-accuracy:
shard-snb: PASS -> SKIP
== Known issues ==
Here are the changes found in Patchwork_10391_full that come from known
issues:
=== IGT changes ===
Issues hit
igt@gem_cpu_reloc@full:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108073)
igt@gem_exec_big:
shard-hsw: PASS -> TIMEOUT (fdo#107937)
igt@kms_atomic@atomic_invalid_params:
shard-apl: PASS -> DMESG-WARN (fdo#103558, fdo#105602) +5
igt@kms_available_modes_crc@available_mode_test_crc:
shard-apl: PASS -> FAIL (fdo#106641)
igt@kms_cursor_crc@cursor-256x256-random:
shard-glk: PASS -> FAIL (fdo#103232)
igt@kms_cursor_crc@cursor-64x21-random:
shard-apl: PASS -> FAIL (fdo#103232) +4
igt@kms_cursor_legacy@2x-cursor-vs-flip-legacy:
shard-glk: NOTRUN -> INCOMPLETE (fdo#103359, k.org#198133)
igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
shard-glk: PASS -> INCOMPLETE (fdo#103359, k.org#198133)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
shard-apl: PASS -> FAIL (fdo#103167)
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-cpu:
shard-glk: PASS -> FAIL (fdo#103167) +2
{igt@kms_plane_alpha_blend@pipe-c-alpha-basic}:
shard-skl: NOTRUN -> FAIL (fdo#108145)
igt@kms_rotation_crc@primary-rotation-180:
shard-kbl: PASS -> DMESG-WARN (fdo#103558, fdo#105602) +10
igt@pm_rpm@debugfs-forcewake-user:
shard-skl: PASS -> INCOMPLETE (fdo#107807)
igt@pm_rpm@gem-execbuf:
shard-skl: NOTRUN -> INCOMPLETE (fdo#107803, fdo#107807)
Possible fixes
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
shard-hsw: DMESG-WARN (fdo#107956) -> PASS
igt@kms_color@pipe-a-legacy-gamma:
shard-apl: FAIL (fdo#104782, fdo#108145) -> PASS
igt@kms_color@pipe-b-legacy-gamma:
shard-apl: FAIL (fdo#104782) -> PASS
igt@kms_cursor_crc@cursor-256x256-suspend:
shard-kbl: INCOMPLETE (fdo#103665) -> PASS
igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
shard-skl: FAIL (fdo#103184) -> PASS
igt@kms_flip@2x-flip-vs-rmfb-interruptible:
shard-glk: INCOMPLETE (fdo#103359, k.org#198133) -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: FAIL (fdo#105363) -> PASS
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-skl: FAIL (fdo#100368) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-msflip-blt:
shard-skl: FAIL (fdo#103167) -> PASS +2
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
shard-apl: FAIL (fdo#103167) -> PASS
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
shard-glk: FAIL (fdo#103167) -> PASS +6
igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
shard-skl: INCOMPLETE (fdo#104108) -> PASS
{igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb}:
shard-glk: FAIL (fdo#108145) -> PASS
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-glk: FAIL (fdo#103166) -> PASS +2
shard-apl: FAIL (fdo#103166) -> PASS +1
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
Warnings
igt@kms_frontbuffer_tracking@fbc-1p-rte:
shard-apl: FAIL (fdo#105682, fdo#103167) -> DMESG-WARN
(fdo#103558, fdo#108131)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103359
Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/dp: Link train Fallback on eDP only if fallback link BW can fit panel's native mode
On Mon, Oct 01, 2018 at 07:03:52PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 12, 2018 at 03:57:16PM -0700, Manasi Navare wrote:
> > This patch fixes the original commit c0cfb10d9e1de49 ("drm/i915/edp:
> > Do not do link training fallback or prune modes on EDP") that causes
> > a blank screen in case of certain eDP panels (Eg: seen on Dell XPS13 9350)
> > where first link training fails and a retraining is required by falling
> > back to lower link rate/lane count.
> > In case of some panels they advertise higher link rate/lane count
> > than whats required for supporting the panel's native mode.
> > But we always link train at highest link rate/lane count for eDP
> > and if that fails we can still fallback to lower link rate/lane count
> > as long as the fallback link BW still fits the native mode to avoid
> > pruning the panel's native mode yet retraining at fallback values
> > to recover from a blank screen.
> >
> > v2:
> > * Send uevent if link failure on eDP unconditionally
> >
> > Cc: Clinton Taylor
> > Cc: Jani Nikula
> > Cc: Ville Syrjala
> > Cc: Daniel Vetter
> > Cc: Lucas De Marchi
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107489
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105338
> > Signed-off-by: Manasi Navare
> > Tested-by: Alexander Wilson
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 29 +++
> > drivers/gpu/drm/i915/intel_dp_link_training.c | 26 ++---
> > 2 files changed, 38 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 436c22de33b6..e4de5257cd87 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -557,6 +557,21 @@ static bool intel_dp_link_params_valid(struct intel_dp
> > *intel_dp, int link_rate,
> > return true;
> > }
> >
> > +static bool intel_dp_can_link_train_fallback_for_edp(struct intel_dp
> > *intel_dp,
> > +int link_rate,
> > +uint8_t lane_count)
> > +{
> > + struct drm_display_mode *fixed_mode =
> > intel_dp->attached_connector->panel.fixed_mode;
>
> const
>
Will add const to the fixed_mode.
> > + int mode_rate, max_rate;
> > +
> > + mode_rate = intel_dp_link_required(fixed_mode->clock, 18);
> > + max_rate = intel_dp_max_data_rate(link_rate, lane_count);
> > + if (mode_rate > max_rate)
> > + return false;
>
> I wonder if we should extract this into a helper and share with
> mode_valid(). Then again, it's just a few lines so maybe not worth it.
>
> > +
> > + return true;
> > +}
> > +
> > int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > int link_rate, uint8_t lane_count)
> > {
> > @@ -566,9 +581,23 @@ int intel_dp_get_link_train_fallback_values(struct
> > intel_dp *intel_dp,
> > intel_dp->num_common_rates,
> > link_rate);
> > if (index > 0) {
> > + if (intel_dp_is_edp(intel_dp) &&
> > + !intel_dp_can_link_train_fallback_for_edp(intel_dp,
> > +
> > intel_dp->common_rates[index - 1],
> > + lane_count)) {
> > + DRM_DEBUG_KMS("Retrying Link training for eDP with same
> > parameters\n");
> > + return 0;
> > + }
> > intel_dp->max_link_rate = intel_dp->common_rates[index - 1];
> > intel_dp->max_link_lane_count = lane_count;
> > } else if (lane_count > 1) {
> > + if (intel_dp_is_edp(intel_dp) &&
> > + !intel_dp_can_link_train_fallback_for_edp(intel_dp,
> > +
> > intel_dp_max_common_rate(intel_dp),
> > + lane_count >> 1))
> > {
> > + DRM_DEBUG_KMS("Retrying Link training for eDP with same
> > parameters\n");
> > + return 0;
> > + }
> > intel_dp->max_link_rate = intel_dp_max_common_rate(intel_dp);
> > intel_dp->max_link_lane_count = lane_count >> 1;
>
> This whole thing is getting a bit messy. I think it would worthwile to
> rewrite this as something like:
>
> intel_dp_update_link_train_fallback_values(intel_dp)
> {
> int link_rate = intel_dp->link_rate;
> int lane_count = intel_dp->lane_count;
>
> if (intel_dp_get_link_train_fallback_values(intel_dp, _rate,
> _count))
> return -1;
>
> if (is_edp() && !edp_can_link_train_thing(link_rate, lane_count)) {
> DRM_DEBUG_KMS("...");
> return 0;
> }
>
> intel_dp->max_link_rate = link_rate;
> intel_dp->max_link_lane_count = lane_count;
>
> return 0;
> }
>
> But
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Rename full ppgtt configuration to be more generic (rev6)
== Series Details == Series: drm/i915: Rename full ppgtt configuration to be more generic (rev6) URL : https://patchwork.freedesktop.org/series/49021/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4950 -> Patchwork_10391 = == Summary - WARNING == Minor unknown changes coming with Patchwork_10391 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10391, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/49021/revisions/6/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10391: === IGT changes === Warnings igt@drv_selftest@live_requests: fi-bxt-j4205: PASS -> SKIP +14 == Known issues == Here are the changes found in Patchwork_10391 that come from known issues: === IGT changes === Issues hit igt@amdgpu/amd_prime@amd-to-i915: fi-bxt-j4205: SKIP -> TIMEOUT (fdo#108075) +1 igt@drv_module_reload@basic-reload: fi-blb-e6850: NOTRUN -> INCOMPLETE (fdo#107718) igt@gem_exec_suspend@basic-s3: fi-cfl-8109u: PASS -> DMESG-WARN (fdo#107345) +2 fi-kbl-soraka: NOTRUN -> INCOMPLETE (fdo#107556, fdo#107859, fdo#107774) igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b: fi-byt-clapper: PASS -> FAIL (fdo#107362) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c: fi-cfl-8109u: PASS -> INCOMPLETE (fdo#108126, fdo#106070) igt@pm_rpm@module-reload: fi-bxt-j4205: PASS -> FAIL (fdo#107712) Possible fixes igt@gem_exec_suspend@basic-s3: fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#105128, fdo#107139) -> PASS igt@kms_flip@basic-flip-vs-modeset: fi-hsw-4770r: DMESG-WARN (fdo#105602) -> PASS igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107345 https://bugs.freedesktop.org/show_bug.cgi?id=107345 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556 fdo#107712 https://bugs.freedesktop.org/show_bug.cgi?id=107712 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774 fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859 fdo#108075 https://bugs.freedesktop.org/show_bug.cgi?id=108075 fdo#108126 https://bugs.freedesktop.org/show_bug.cgi?id=108126 == Participating hosts (50 -> 45) == Additional (1): fi-kbl-soraka Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-ctg-p8600 == Build changes == * Linux: CI_DRM_4950 -> Patchwork_10391 CI_DRM_4950: a9abc43bebfb6de62c2c3747a22fadfa17b61d8b @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4671: b121f7d42c260ae3a050c3f440d1c11f7cff7d1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10391: bcd9b888d547f6f6d8c03194e0c337bb09cc5863 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == bcd9b888d547 drm/i915: Make 48bit full ppgtt configuration generic (v7) == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10391/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Rename full ppgtt configuration to be more generic (rev6)
== Series Details == Series: drm/i915: Rename full ppgtt configuration to be more generic (rev6) URL : https://patchwork.freedesktop.org/series/49021/ State : warning == Summary == $ dim sparse origin/drm-tip Sparse version: v0.5.2 Commit: drm/i915: Make 48bit full ppgtt configuration generic (v7) -drivers/gpu/drm/i915/i915_gem_gtt.c:348:14: warning: expression using sizeof(void) -drivers/gpu/drm/i915/i915_gem_gtt.c:348:14: warning: expression using sizeof(void) +drivers/gpu/drm/i915/i915_gem_gtt.c:348:14: warning: expression using sizeof(void) +drivers/gpu/drm/i915/i915_gem_gtt.c:348:14: warning: expression using sizeof(void) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Rename full ppgtt configuration to be more generic (rev6)
== Series Details == Series: drm/i915: Rename full ppgtt configuration to be more generic (rev6) URL : https://patchwork.freedesktop.org/series/49021/ State : warning == Summary == $ dim checkpatch origin/drm-tip bcd9b888d547 drm/i915: Make 48bit full ppgtt configuration generic (v7) -:286: ERROR:SPACING: spaces required around that ':' (ctx:WxO) #286: FILE: drivers/gpu/drm/i915/i915_gem_gtt.c:2153: + (INTEL_GEN(i915) < 8) ? false :!intel_vgpu_active(i915); ^ -:286: ERROR:SPACING: space required before that '!' (ctx:OxV) #286: FILE: drivers/gpu/drm/i915/i915_gem_gtt.c:2153: + (INTEL_GEN(i915) < 8) ? false :!intel_vgpu_active(i915); ^ total: 2 errors, 0 warnings, 0 checks, 392 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v7)
48 bit ppgtt device configuration is really just extended address
range full ppgtt and may actually be something other than 48 bits.
Change HAS_FULL_48BIT_PPGTT() to HAS_4LVL_PPGTT() to better
describe that a 4 level walk table extended range PPGTT is being
used. Add a new device info field that specifies the number of
bits to prepare for cases where the range is not 32 or 48 bits.
Also rename other functions and comments from 48bit to 4-level.
Making use of the device info address range for gen6 highlights
simularities in the gen6 and gen8 code paths so move the common
code in to a common function.
v2: Keep HAS_FULL_PPGTT() unchanged (Chris)
v3: Simplify condition in gen8_ppgtt_create() (Chris)
Remove unnecessary line coninuations (Bob)
Rename functions/defines/comments from 48bit to 4lvl (Rodrigo/Bob)
v4: Rename FULL_4LVL_PPGTT to simply 4LVL_PPGTT (Rodrigo)
Be explised in setting vm.total to 1ULL << 32 (Rodrigo)
Gen 7 is 31 bits, not 32 (Chris)
v5: Mock device is 64b(63b) not 48b (Chris)
v6: Rebase to latest drm-tip (Bob)
v7: Combine common code for gen6/gen8 ppgtt create (Chris)
Improve comment on device info field (Chris)
Signed-off-by: Bob Paauwe
CC: Rodrigo Vivi
CC: Michel Thierry
CC: Chris Wilson
---
Chris, is this what you were looking for WRT handling GEN6/7?
drivers/gpu/drm/i915/gvt/vgpu.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 139 ++-
drivers/gpu/drm/i915/i915_gem_gtt.h | 4 +-
drivers/gpu/drm/i915/i915_pci.c | 6 +
drivers/gpu/drm/i915/i915_pvinfo.h | 2 +-
drivers/gpu/drm/i915/i915_vgpu.c | 4 +-
drivers/gpu/drm/i915/i915_vgpu.h | 2 +-
drivers/gpu/drm/i915/intel_device_info.h | 3 +
drivers/gpu/drm/i915/intel_lrc.c | 6 +-
drivers/gpu/drm/i915/selftests/huge_pages.c | 8 +-
drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 +
14 files changed, 89 insertions(+), 95 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index c628be05fbfe..6002ded0042b 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -44,7 +44,7 @@ void populate_pvinfo_page(struct intel_vgpu *vgpu)
vgpu_vreg_t(vgpu, vgtif_reg(display_ready)) = 0;
vgpu_vreg_t(vgpu, vgtif_reg(vgt_id)) = vgpu->id;
- vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_FULL_48BIT_PPGTT;
+ vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) = VGT_CAPS_4LVL_PPGTT;
vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) |= VGT_CAPS_HWSP_EMULATION;
vgpu_vreg_t(vgpu, vgtif_reg(vgt_caps)) |= VGT_CAPS_HUGE_GTT;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1b028f429e92..3b4852a89441 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1365,7 +1365,7 @@ static int i915_driver_init_hw(struct drm_i915_private
*dev_priv)
if (HAS_PPGTT(dev_priv)) {
if (intel_vgpu_active(dev_priv) &&
- !intel_vgpu_has_full_48bit_ppgtt(dev_priv)) {
+ !intel_vgpu_has_4lvl_ppgtt(dev_priv)) {
i915_report_error(dev_priv,
"incompatible vGPU found, support for
isolated ppGTT required\n");
return -ENXIO;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 30191523c309..54a44270d350 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2602,7 +2602,7 @@ intel_info(const struct drm_i915_private *dev_priv)
(INTEL_PPGTT(dev_priv) != INTEL_PPGTT_NONE)
#define HAS_FULL_PPGTT(dev_priv) \
(INTEL_PPGTT(dev_priv) >= INTEL_PPGTT_FULL)
-#define HAS_FULL_48BIT_PPGTT(dev_priv) \
+#define HAS_4LVL_PPGTT(dev_priv) \
(INTEL_PPGTT(dev_priv) >= INTEL_PPGTT_FULL_4LVL)
#define HAS_PAGE_SIZES(dev_priv, sizes) ({ \
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 15c92f75b1b8..5de54ae949c3 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -307,7 +307,7 @@ static u32 default_desc_template(const struct
drm_i915_private *i915,
desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
address_mode = INTEL_LEGACY_32B_CONTEXT;
- if (ppgtt && i915_vm_is_48bit(>vm))
+ if (ppgtt && i915_vm_is_4lvl(>vm))
address_mode = INTEL_LEGACY_64B_CONTEXT;
desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 29ca9007a704..2f603ce94ad4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: serialized performance queries
== Series Details ==
Series: drm/i915: serialized performance queries
URL : https://patchwork.freedesktop.org/series/50698/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4950_full -> Patchwork_10390_full =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_10390_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10390_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in
Patchwork_10390_full:
=== IGT changes ===
Possible regressions
igt@pm_rps@min-max-config-idle:
shard-hsw: PASS -> FAIL
Warnings
igt@kms_vblank@pipe-a-wait-forked-busy:
shard-snb: PASS -> SKIP +1
== Known issues ==
Here are the changes found in Patchwork_10390_full that come from known
issues:
=== IGT changes ===
Issues hit
igt@gem_cpu_reloc@full:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108073)
igt@gem_exec_big:
shard-hsw: PASS -> TIMEOUT (fdo#107937)
igt@kms_available_modes_crc@available_mode_test_crc:
shard-apl: PASS -> FAIL (fdo#106641)
igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
shard-skl: NOTRUN -> FAIL (fdo#105458)
igt@kms_color@pipe-c-legacy-gamma:
shard-skl: NOTRUN -> FAIL (fdo#104782)
igt@kms_cursor_crc@cursor-128x128-suspend:
shard-glk: PASS -> INCOMPLETE (k.org#198133, fdo#103359) +1
shard-snb: PASS -> DMESG-WARN (fdo#102365)
igt@kms_cursor_crc@cursor-64x21-random:
shard-apl: PASS -> FAIL (fdo#103232) +4
igt@kms_cursor_legacy@2x-cursor-vs-flip-legacy:
shard-glk: NOTRUN -> INCOMPLETE (k.org#198133, fdo#103359)
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-skl: PASS -> FAIL (fdo#105363)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
shard-apl: PASS -> FAIL (fdo#103167) +1
{igt@kms_plane_alpha_blend@pipe-a-coverage-7efc}:
shard-skl: NOTRUN -> FAIL (fdo#108145) +1
igt@kms_rotation_crc@exhaust-fences:
shard-skl: NOTRUN -> DMESG-WARN (fdo#105748)
igt@kms_setmode@basic:
shard-kbl: PASS -> FAIL (fdo#99912)
igt@kms_universal_plane@universal-plane-pipe-c-functional:
shard-apl: PASS -> FAIL (fdo#103166)
igt@perf_pmu@busy-accuracy-98-rcs0:
shard-snb: SKIP -> INCOMPLETE (fdo#105411)
igt@pm_rpm@legacy-planes:
shard-skl: PASS -> INCOMPLETE (fdo#105959, fdo#107807)
igt@pm_rpm@universal-planes:
shard-skl: PASS -> INCOMPLETE (fdo#107807)
Possible fixes
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
shard-skl: DMESG-WARN (fdo#107956) -> PASS
igt@kms_ccs@pipe-a-crc-primary-rotation-180:
shard-skl: FAIL (fdo#107725) -> PASS
igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
shard-glk: FAIL (fdo#108145) -> PASS
igt@kms_color@pipe-a-legacy-gamma:
shard-apl: FAIL (fdo#108145, fdo#104782) -> PASS
igt@kms_color@pipe-b-legacy-gamma:
shard-apl: FAIL (fdo#104782) -> PASS
igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
shard-skl: FAIL (fdo#103184) -> PASS
igt@kms_flip@2x-flip-vs-rmfb-interruptible:
shard-glk: INCOMPLETE (k.org#198133, fdo#103359) -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: FAIL (fdo#105363) -> PASS
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-skl: FAIL (fdo#100368) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-msflip-blt:
shard-skl: FAIL (fdo#103167) -> PASS +2
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
shard-apl: FAIL (fdo#103167) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-rte:
shard-apl: FAIL (fdo#105682, fdo#103167) -> PASS
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff:
shard-glk: FAIL (fdo#103167) -> PASS +7
igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
shard-skl: INCOMPLETE (fdo#104108) -> PASS
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-apl: FAIL (fdo#103166) -> PASS +1
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102365
Re: [Intel-gfx] [PATCH] pci: Add a few new IDs for Intel GPU "spurious interrupt" quirk
Hi Bjorn,
On Thu, Oct 4, 2018 at 4:12 AM Bjorn Helgaas wrote:
>
> On Thu, Sep 27, 2018 at 10:10:07AM +0800, Bin Meng wrote:
> > On Thu, Sep 27, 2018 at 12:57 AM Bjorn Helgaas wrote:
> > > On Wed, Sep 26, 2018 at 08:14:01AM -0700, Bin Meng wrote:
> > > > Add more PCI IDs to the Intel GPU "spurious interrupt" quirk table,
> > > > which are known to break.
> > >
> > > Do you have a reference for this? Any public bug reports, bugzilla,
> > > Intel spec reference or errata? "Which are known to break" is pretty
> > > vague.
> >
> > Sorry I used wrong words and should have been clearer. These devices
> > are validated to be broken. The test I used is very simple, just
> > unplug the VGA cable and plug it again, and "spurious interrupt" will
> > be seen on the interrupt line of the IGD device. I was not aware of
> > any public bugs filed to Intel, nor seen any errata from Intel.
>
> The original commit, f67fd55fa96f ("PCI: Add quirk for still enabled
> interrupts on Intel Sandy Bridge GPUs"), says some systems "crash"
> (not sure if that means an oops or an actual crash that requires a
> reboot) and on other systems, Linux disables the shared interrupt
> line. I assume disabling the interrupt line keeps devices using that
> line from working, but does not directly cause a crash.
>
Correct, disable the shared interrupt line keeps all devices using
that line from working, which is current kernel's behavior w/o this
quirk handling: it disables the (shared) interrupt line after 100.000+
generated interrupts. But the side effect is that other devices become
unusable after that (eg: USB devices which share the same interrupt
line with the Intel GPU). That's why the original commit, f67fd55fa96f
("PCI: Add quirk for still enabled interrupts on Intel Sandy Bridge
GPUs") disables the GPU's interrupt directly, which should really be
done by the VGA BIOS itself (a buggy VBIOS!).
> What specific symptom do you see here? I think it might be useful to
> collect details, e.g., dmesg logs, /proc/interrupts contents, output
> of "sudo lspci -vv", etc., for the systems you're quirking here. I'm
> hoping we can eventually figure out a solution that doesn't require a
> quirk for every new GPU, and maybe that info will help find it.
>
The symptom was described briefly in the original commit f67fd55fa96f
too, that disables the (shared) interrupt line after 100.000+
generated interrupts (can be observed via /proc/interrupts).
> > > > See commit f67fd55fa96f ("PCI: Add quirk for still enabled interrupts
> > > > on Intel Sandy Bridge GPUs"), and commit 7c82126a94e6 ("PCI: Add new
> > > > ID for Intel GPU "spurious interrupt" quirk") for some history.
> > > >
> > > > Based on current findings, it is highly possible that all Intel
> > > > 1st/2nd/3rd generation Core processors' IGD has such quirk.
> > >
> > > Can you include a reference to these "current findings"? I assume you
> > > have bug reports that include the device IDs you're adding? If not,
> > > how did you build this list of new IDs?
> >
> > By "current findings" I mean given the IDs we have here, plus previous
> > one added by Thomas, it's highly possible this VGA BIOS bug exists in
> > every 1st/2nd/3rd generation Core processors.
> >
> > > The function comment added by f67fd55fa96f ("PCI: Add quirk for still
> > > enabled interrupts on Intel Sandy Bridge GPUs") suggests that this is
> > > actually a BIOS issue, not a hardware erratum, i.e., I don't see
> > > anything there that suggests a hardware defect.
> > >
> > > But there must be a hole somewhere -- the kernel can't be expected to
> > > disable interrupts in device-specific ways when there's no driver
> > > loaded. Maybe it's simply a BIOS defect or maybe there's some
> > > interrupt or _PRT-related setup we're missing.
> >
> > It's a pure VGA BIOS bug, not the BIOS bug or _PRT etc. The VGA BIOS
> > forgot to turn off the interrupt on these devices.
>
> If this is a VGA BIOS defect, it's not very likely that it will
> magically be fixed for all new Intel GPUs, so in effect it sounds like
> we need to update this list of quirks in Linux every time a new Intel
> GPU comes out. That prospect is a little daunting.
>
I don't have a relatively newer Intel board at hand for testing right
now. I can try to locate one. But as I said, it's highly possible at
least all 1st/2nd/3rd generation Core processors are affected. Maybe
we can add all these known GPU devices of 1st/2nd/3rd generation Core
processors all together for now? For newer GPUs, let's wait until
someone reports the issue again?
> Do you happen to know if Windows has the same problem? I.e., if you
> boot an old version of Windows with a new GPU, and unplug the VGA
> cable, does Windows crash? If Windows can figure out how to handle
> that situation gracefully, Linux should be able to do it, too.
>
I suspect Windows cannot handle it too. Without the GPU awareness, the
interrupt line is simply on and no driver claims the devices and will
cause
Re: [Intel-gfx] [PATCH] pci: Add a few new IDs for Intel GPU "spurious interrupt" quirk
Hi David,
On Mon, Oct 8, 2018 at 6:06 PM David Laight wrote:
>
> From: Bin Meng
> > Sent: 08 October 2018 10:44
> ...
> > Correct, disable the shared interrupt line keeps all devices using
> > that line from working, which is current kernel's behavior w/o this
> > quirk handling: it disables the (shared) interrupt line after 100.000+
> > generated interrupts. But the side effect is that other devices become
> > unusable after that (eg: USB devices which share the same interrupt
> > line with the Intel GPU). That's why the original commit, f67fd55fa96f
> > ("PCI: Add quirk for still enabled interrupts on Intel Sandy Bridge
> > GPUs") disables the GPU's interrupt directly, which should really be
> > done by the VGA BIOS itself (a buggy VBIOS!).
>
> Shouldn't the kernel just disable all PCI(e) interrupts by writing
> 1 to the config space control register bit during grope?
> Can it ever by right for this to be set?
>
Do you mean PCI_COMMAND_INTX_DISABLE bit of the command register in
the configuration space? Setting this bit indeed could disable the
INTx interrupt, but it does not work for all PCI devices as this bit
was introduced in PCI spec v2.3.
> Apart from VGA the 'bus master' bit also needs to be clear.
>
> ISTR some very early PCI systems which failed to reset the PCI
> bus during reboot - at least the 'bus master' bit remained
> set for an ethernet card.
> On a private LAN the OS got reinstalled and rebooted without
> using all the ethernet receive buffers and then died because
> a receive frame got written into 'random' memory.
Regards,
Bin
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v2)
On Mon, 8 Oct 2018 22:35:34 +0800
Chris Chiu wrote:
> Thanks! I have no problem with this patch.
There are Fi.CI.BAT failures with the v2 (only with formatting fix
added) while the previous patch had passing results.
Now trying to identify why the failures happened with trybot
Thanks,
Guang
>
> On Thu, Oct 4, 2018 at 2:08 AM Guang Bai wrote:
>
> > On some platforms, slowly unplugging (wiggling) the HDMI cable makes
> > the kernel to believe the HDMI display still connected. This is
> > because the HDMI DDC lines are disconnected sometimes later after
> > the hot-plug interrupt triggered. Use the hot plug live states to
> > honor HDMI hot plug status in addtion to access the DDC channels.
> >
> > v2: Fix the formatting issue
> >
> > Cc: Jani Nikula
> > Cc: Chris Chiu
> > Signed-off-by: Guang Bai
> > ---
> > drivers/gpu/drm/i915/intel_hotplug.c | 32
> > +--- 1 file changed, 29 insertions(+),
> > 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 648a13c..98ab1ab 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -246,17 +246,43 @@ static void
> > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> > intel_runtime_pm_put(dev_priv);
> > }
> >
> > +#define MAX_SHORT_PULSE_MS 100
> > +#define PORT_CHECK_LOOP_COUNT 3
> > +
> > bool intel_encoder_hotplug(struct intel_encoder *encoder,
> >struct intel_connector *connector)
> > {
> > struct drm_device *dev = connector->base.dev;
> > - enum drm_connector_status old_status;
> > + enum drm_connector_status old_status, new_status;
> > + enum hpd_pin pin = encoder->hpd_pin;
> > + struct drm_i915_private *dev_priv =
> > to_i915(encoder->base.dev);
> > + u32 count = 0;
> >
> > WARN_ON(!mutex_is_locked(>mode_config.mutex));
> > old_status = connector->base.status;
> >
> > - connector->base.status =
> > - drm_helper_probe_detect(>base, NULL,
> > false);
> > + /*
> > +* Set HDMI connection status based on hot-plug live states
> > and
> > +* display probe results.
> > +*/
> > + if ((encoder->type == INTEL_OUTPUT_HDMI ||
> > +encoder->type == INTEL_OUTPUT_DDI) &&
> > + dev_priv->hotplug.stats[pin].state == HPD_ENABLED) {
> > + do {
> > + new_status = connector_status_disconnected;
> > + msleep(MAX_SHORT_PULSE_MS);
> > +
> > + if (intel_digital_port_connected(encoder))
> > + new_status =
> > drm_helper_probe_detect(>base,
> > +
> > NULL, false);
> > + if (new_status ==
> > connector_status_connected)
> > + break;
> > + } while (++count <= PORT_CHECK_LOOP_COUNT);
> > + connector->base.status = new_status;
> > + } else {
> > + connector->base.status =
> > + drm_helper_probe_detect(>base,
> > NULL, false);
> > + }
> >
> > if (old_status == connector->base.status)
> > return false;
> > --
> > 2.7.4
> >
> >
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: serialized performance queries
== Series Details == Series: drm/i915: serialized performance queries URL : https://patchwork.freedesktop.org/series/50698/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4950 -> Patchwork_10390 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/50698/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10390 that come from known issues: === IGT changes === Issues hit igt@drv_getparams_basic@basic-subslice-total: fi-snb-2520m: PASS -> DMESG-WARN (fdo#103713) +10 igt@gem_exec_suspend@basic-s3: fi-kbl-soraka: NOTRUN -> INCOMPLETE (fdo#107774, fdo#107859, fdo#107556) igt@kms_flip@basic-flip-vs-dpms: fi-skl-6700hq: PASS -> DMESG-WARN (fdo#105998) igt@kms_pipe_crc_basic@hang-read-crc-pipe-b: fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191) Possible fixes igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#105128, fdo#107139) -> PASS igt@kms_flip@basic-flip-vs-modeset: fi-hsw-4770r: DMESG-WARN (fdo#105602) -> PASS igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556 fdo#107774 https://bugs.freedesktop.org/show_bug.cgi?id=107774 fdo#107859 https://bugs.freedesktop.org/show_bug.cgi?id=107859 == Participating hosts (50 -> 45) == Additional (1): fi-kbl-soraka Missing(6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-ctg-p8600 == Build changes == * Linux: CI_DRM_4950 -> Patchwork_10390 CI_DRM_4950: a9abc43bebfb6de62c2c3747a22fadfa17b61d8b @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4671: b121f7d42c260ae3a050c3f440d1c11f7cff7d1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10390: 81dc092f58d50b34a5dd22efb926bc0c65142288 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 81dc092f58d5 drm/i915: add a new perf configuration execbuf parameter ccae77ecfc25 drm/i915/perf: allow for CS OA configs to be created lazily 40102d5976dd drm/i915/perf: allow holding preemption on filtered ctx == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10390/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 1/3] drm/i915/perf: allow holding preemption on filtered ctx
On 08/10/2018 16:35, Chris Wilson wrote: Quoting Lionel Landwerlin (2018-10-08 16:18:20) We would like to make use of perf in Vulkan. The Vulkan API is much lower level than OpenGL, with applications directly exposed to the concept of command buffers (pretty much equivalent to our batch buffers). In Vulkan, queries are always limited in scope to a command buffer. In OpenGL, the lack of command buffer concept meant that queries' duration could span multiple command buffers. With that restriction gone in Vulkan, we would like to simplify measuring performance just by measuring the deltas between the counter snapshots written by 2 MI_RECORD_PERF_COUNT commands, rather than the more complex scheme we currently have in the GL driver, using 2 MI_RECORD_PERF_COUNT commands and doing some post processing on the stream of OA reports, coming from the global OA buffer, to remove any unrelated deltas in between the 2 MI_RECORD_PERF_COUNT. Disabling preemption only apply to a single context with which want to query performance counters for and is considered a privileged operation, by default protected by CAP_SYS_ADMIN. It is possible to enable it for a normal user by disabling the paranoid stream setting. I'm am uncomfortable with disabling preemption like this. I suggest we at least have the preemption timeout in place first. -Chris Sure, I'm not super happy about that either but that's the hardware we have to work with :/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 3/3] drm/i915: add a new perf configuration execbuf parameter
Quoting Lionel Landwerlin (2018-10-08 16:18:22) > We want the ability to dispatch a set of command buffer to the > hardware, each with a different OA configuration. To achieve this, we > reuse a couple of fields from the execbuf2 struct (I CAN HAZ > execbuf3?) to notify what OA configuration should be used for a batch > buffer. This requires the process making the execbuf with this flag to > also own the perf fd at the time of execbuf. Sigh. It's a distinct step from emit_bb_start and should be using emit_bb_start itself to execute the batch. Use i915_vma_move_to_active to couple into retirement correctly, and use pin properly. If you feel emit_bb is doing too much, split it up into primitives rather than continue to overload it; emit_bb is used and will be used outside of execbuf. -Chris ___ 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/perf: allow for CS OA configs to be created lazily
On 08/10/2018 16:34, Chris Wilson wrote:
Quoting Lionel Landwerlin (2018-10-08 16:18:21)
Here we introduce a mechanism by which the execbuf part of the i915
driver will be able to request that a batch buffer containing the
programming for a particular OA config be created.
We'll execute these OA configuration buffers right before executing a
set of userspace commands so that a particular user batchbuffer be
executed with a given OA configuration.
This mechanism essentially allows the userspace driver to go through
several OA configuration without having to open/close the i915/perf
stream.
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/i915_drv.h | 22 ++-
drivers/gpu/drm/i915/i915_perf.c | 195 ++
drivers/gpu/drm/i915/intel_gpu_commands.h | 1 +
3 files changed, 187 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2264b30ce51a..a35715cd7608 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1378,6 +1378,10 @@ struct i915_oa_config {
struct attribute *attrs[2];
struct device_attribute sysfs_metric_id;
+ struct i915_vma *vma;
+
+ struct list_head vma_link;
+
atomic_t ref_count;
};
@@ -1979,11 +1983,21 @@ struct drm_i915_private {
struct mutex metrics_lock;
/*
-* List of dynamic configurations, you need to hold
-* dev_priv->perf.metrics_lock to access it.
+* List of dynamic configurations (struct i915_oa_config), you
+* need to hold dev_priv->perf.metrics_lock to access it.
*/
struct idr metrics_idr;
+ /*
+* List of dynamic configurations (struct i915_oa_config)
+* which have an allocated buffer in GGTT for reconfiguration,
+* you need to hold dev_priv->perf.metrics_lock to access it.
+* Elements are added to the list lazilly on execbuf (when a
+* particular configuration is requested). The list is freed
+* upon closing the perf stream.
+*/
+ struct list_head metrics_buffers;
+
/*
* Lock associated with anything below within this structure
* except exclusive_stream.
@@ -3315,6 +3329,10 @@ int i915_perf_remove_config_ioctl(struct drm_device
*dev, void *data,
void i915_oa_init_reg_state(struct intel_engine_cs *engine,
struct i915_gem_context *ctx,
uint32_t *reg_state);
+int i915_perf_get_oa_config(struct drm_i915_private *i915,
+ int metrics_set,
+ struct i915_oa_config **out_config,
+ struct i915_vma **out_vma);
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct i915_address_space *vm,
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index e2a96b6844fe..39c5b44862d4 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -364,9 +364,16 @@ struct perf_open_properties {
int oa_period_exponent;
};
-static void free_oa_config(struct drm_i915_private *dev_priv,
- struct i915_oa_config *oa_config)
+static void put_oa_config(struct i915_oa_config *oa_config)
{
+ if (!atomic_dec_and_test(_config->ref_count))
+ return;
+
+ if (oa_config->vma) {
+ list_del(_config->vma_link);
+ i915_vma_put(oa_config->vma);
+ }
+
if (!PTR_ERR(oa_config->flex_regs))
kfree(oa_config->flex_regs);
if (!PTR_ERR(oa_config->b_counter_regs))
@@ -376,38 +383,152 @@ static void free_oa_config(struct drm_i915_private
*dev_priv,
kfree(oa_config);
}
-static void put_oa_config(struct drm_i915_private *dev_priv,
- struct i915_oa_config *oa_config)
+static u32 *write_cs_mi_lri(u32 *cs, const struct i915_oa_reg *reg_data, u32
n_regs)
{
- if (!atomic_dec_and_test(_config->ref_count))
- return;
+ u32 i;
- free_oa_config(dev_priv, oa_config);
+ for (i = 0; i < n_regs; i++) {
+ if ((i % MI_LOAD_REGISTER_IMM_MAX_REGS) == 0) {
+ u32 n_lri = min(n_regs - i,
+ (u32) MI_LOAD_REGISTER_IMM_MAX_REGS);
+
+ *cs++ = MI_LOAD_REGISTER_IMM(n_lri);
+ }
+ *cs++ = i915_mmio_reg_offset(reg_data[i].addr);
+ *cs++ = reg_data[i].value;
+ }
+
+ return cs;
}
-static int get_oa_config(struct drm_i915_private *dev_priv,
-int metrics_set,
-struct i915_oa_config **out_config)
+static int
Re: [Intel-gfx] [RFC PATCH 1/3] drm/i915/perf: allow holding preemption on filtered ctx
Quoting Lionel Landwerlin (2018-10-08 16:18:20) > We would like to make use of perf in Vulkan. The Vulkan API is much > lower level than OpenGL, with applications directly exposed to the > concept of command buffers (pretty much equivalent to our batch > buffers). In Vulkan, queries are always limited in scope to a command > buffer. In OpenGL, the lack of command buffer concept meant that > queries' duration could span multiple command buffers. > > With that restriction gone in Vulkan, we would like to simplify > measuring performance just by measuring the deltas between the counter > snapshots written by 2 MI_RECORD_PERF_COUNT commands, rather than the > more complex scheme we currently have in the GL driver, using 2 > MI_RECORD_PERF_COUNT commands and doing some post processing on the > stream of OA reports, coming from the global OA buffer, to remove any > unrelated deltas in between the 2 MI_RECORD_PERF_COUNT. > > Disabling preemption only apply to a single context with which want to > query performance counters for and is considered a privileged > operation, by default protected by CAP_SYS_ADMIN. It is possible to > enable it for a normal user by disabling the paranoid stream setting. I'm am uncomfortable with disabling preemption like this. I suggest we at least have the preemption timeout in place first. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Do intel_panel_destroy_backlight() later
== Series Details ==
Series: series starting with [1/2] drm/i915: Do intel_panel_destroy_backlight()
later
URL : https://patchwork.freedesktop.org/series/50691/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4950_full -> Patchwork_10389_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_10389_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_10389_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
== Possible new issues ==
Here are the unknown changes that may have been introduced in
Patchwork_10389_full:
=== IGT changes ===
Warnings
igt@kms_cursor_legacy@cursora-vs-flipa-atomic-transitions-varying-size:
shard-snb: PASS -> SKIP +6
== Known issues ==
Here are the changes found in Patchwork_10389_full that come from known
issues:
=== IGT changes ===
Issues hit
igt@gem_cpu_reloc@full:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108073)
igt@kms_atomic@atomic_invalid_params:
shard-apl: PASS -> DMESG-WARN (fdo#105602, fdo#103558) +4
igt@kms_available_modes_crc@available_mode_test_crc:
shard-apl: PASS -> FAIL (fdo#106641)
igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
shard-skl: NOTRUN -> FAIL (fdo#105458)
igt@kms_color@pipe-c-legacy-gamma:
shard-skl: NOTRUN -> FAIL (fdo#104782)
igt@kms_cursor_crc@cursor-256x256-random:
shard-glk: PASS -> FAIL (fdo#103232)
shard-apl: PASS -> FAIL (fdo#103232) +1
igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
shard-glk: PASS -> DMESG-WARN (fdo#105763, fdo#106538)
igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
shard-glk: PASS -> INCOMPLETE (k.org#198133, fdo#103359)
igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-gtt:
shard-glk: PASS -> DMESG-FAIL (fdo#103167, fdo#106538)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
shard-apl: PASS -> FAIL (fdo#103167) +3
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-mmap-cpu:
shard-glk: PASS -> FAIL (fdo#103167) +2
{igt@kms_plane_alpha_blend@pipe-a-coverage-7efc}:
shard-glk: PASS -> DMESG-FAIL (fdo#106538)
{igt@kms_plane_alpha_blend@pipe-c-alpha-basic}:
shard-skl: NOTRUN -> FAIL (fdo#108145)
igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
shard-apl: PASS -> FAIL (fdo#103166)
igt@kms_rotation_crc@exhaust-fences:
shard-skl: NOTRUN -> DMESG-WARN (fdo#105748)
igt@pm_rpm@system-suspend:
shard-skl: NOTRUN -> INCOMPLETE (fdo#104108, fdo#107773,
fdo#107807)
Possible fixes
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
shard-kbl: DMESG-WARN (fdo#107956) -> PASS
igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
shard-hsw: DMESG-WARN (fdo#107956) -> PASS
igt@kms_ccs@pipe-a-crc-primary-rotation-180:
shard-skl: FAIL (fdo#107725) -> PASS
igt@kms_color@pipe-a-legacy-gamma:
shard-apl: FAIL (fdo#104782, fdo#108145) -> PASS
igt@kms_color@pipe-b-legacy-gamma:
shard-apl: FAIL (fdo#104782) -> PASS
igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
shard-skl: FAIL (fdo#103184) -> PASS
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: FAIL (fdo#105363) -> PASS
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-skl: FAIL (fdo#100368) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
shard-apl: FAIL (fdo#103167) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
shard-glk: FAIL (fdo#103167) -> PASS +3
igt@kms_plane@plane-position-covered-pipe-a-planes:
shard-apl: FAIL (fdo#103166) -> PASS
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-glk: FAIL (fdo#103166) -> PASS +2
igt@kms_setmode@basic:
shard-apl: FAIL (fdo#99912) -> PASS
Warnings
igt@kms_frontbuffer_tracking@fbc-1p-rte:
shard-apl: FAIL (fdo#103167, fdo#105682) -> DMESG-WARN
(fdo#108131, fdo#105602, fdo#103558)
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
Re: [Intel-gfx] [RFC PATCH 2/3] drm/i915/perf: allow for CS OA configs to be created lazily
Quoting Lionel Landwerlin (2018-10-08 16:18:21)
> Here we introduce a mechanism by which the execbuf part of the i915
> driver will be able to request that a batch buffer containing the
> programming for a particular OA config be created.
>
> We'll execute these OA configuration buffers right before executing a
> set of userspace commands so that a particular user batchbuffer be
> executed with a given OA configuration.
>
> This mechanism essentially allows the userspace driver to go through
> several OA configuration without having to open/close the i915/perf
> stream.
>
> Signed-off-by: Lionel Landwerlin
> ---
> drivers/gpu/drm/i915/i915_drv.h | 22 ++-
> drivers/gpu/drm/i915/i915_perf.c | 195 ++
> drivers/gpu/drm/i915/intel_gpu_commands.h | 1 +
> 3 files changed, 187 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2264b30ce51a..a35715cd7608 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1378,6 +1378,10 @@ struct i915_oa_config {
> struct attribute *attrs[2];
> struct device_attribute sysfs_metric_id;
>
> + struct i915_vma *vma;
> +
> + struct list_head vma_link;
> +
> atomic_t ref_count;
> };
>
> @@ -1979,11 +1983,21 @@ struct drm_i915_private {
> struct mutex metrics_lock;
>
> /*
> -* List of dynamic configurations, you need to hold
> -* dev_priv->perf.metrics_lock to access it.
> +* List of dynamic configurations (struct i915_oa_config), you
> +* need to hold dev_priv->perf.metrics_lock to access it.
> */
> struct idr metrics_idr;
>
> + /*
> +* List of dynamic configurations (struct i915_oa_config)
> +* which have an allocated buffer in GGTT for reconfiguration,
> +* you need to hold dev_priv->perf.metrics_lock to access it.
> +* Elements are added to the list lazilly on execbuf (when a
> +* particular configuration is requested). The list is freed
> +* upon closing the perf stream.
> +*/
> + struct list_head metrics_buffers;
> +
> /*
> * Lock associated with anything below within this structure
> * except exclusive_stream.
> @@ -3315,6 +3329,10 @@ int i915_perf_remove_config_ioctl(struct drm_device
> *dev, void *data,
> void i915_oa_init_reg_state(struct intel_engine_cs *engine,
> struct i915_gem_context *ctx,
> uint32_t *reg_state);
> +int i915_perf_get_oa_config(struct drm_i915_private *i915,
> + int metrics_set,
> + struct i915_oa_config **out_config,
> + struct i915_vma **out_vma);
>
> /* i915_gem_evict.c */
> int __must_check i915_gem_evict_something(struct i915_address_space *vm,
> diff --git a/drivers/gpu/drm/i915/i915_perf.c
> b/drivers/gpu/drm/i915/i915_perf.c
> index e2a96b6844fe..39c5b44862d4 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -364,9 +364,16 @@ struct perf_open_properties {
> int oa_period_exponent;
> };
>
> -static void free_oa_config(struct drm_i915_private *dev_priv,
> - struct i915_oa_config *oa_config)
> +static void put_oa_config(struct i915_oa_config *oa_config)
> {
> + if (!atomic_dec_and_test(_config->ref_count))
> + return;
> +
> + if (oa_config->vma) {
> + list_del(_config->vma_link);
> + i915_vma_put(oa_config->vma);
> + }
> +
> if (!PTR_ERR(oa_config->flex_regs))
> kfree(oa_config->flex_regs);
> if (!PTR_ERR(oa_config->b_counter_regs))
> @@ -376,38 +383,152 @@ static void free_oa_config(struct drm_i915_private
> *dev_priv,
> kfree(oa_config);
> }
>
> -static void put_oa_config(struct drm_i915_private *dev_priv,
> - struct i915_oa_config *oa_config)
> +static u32 *write_cs_mi_lri(u32 *cs, const struct i915_oa_reg *reg_data, u32
> n_regs)
> {
> - if (!atomic_dec_and_test(_config->ref_count))
> - return;
> + u32 i;
>
> - free_oa_config(dev_priv, oa_config);
> + for (i = 0; i < n_regs; i++) {
> + if ((i % MI_LOAD_REGISTER_IMM_MAX_REGS) == 0) {
> + u32 n_lri = min(n_regs - i,
> + (u32) MI_LOAD_REGISTER_IMM_MAX_REGS);
> +
> + *cs++ = MI_LOAD_REGISTER_IMM(n_lri);
> + }
> + *cs++ = i915_mmio_reg_offset(reg_data[i].addr);
> + *cs++ = reg_data[i].value;
> + }
> +
> + return cs;
> }
>
> -static int
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: serialized performance queries
== Series Details == Series: drm/i915: serialized performance queries URL : https://patchwork.freedesktop.org/series/50698/ State : warning == Summary == $ dim sparse origin/drm-tip Sparse version: v0.5.2 Commit: drm/i915/perf: allow holding preemption on filtered ctx Okay! Commit: drm/i915/perf: allow for CS OA configs to be created lazily +drivers/gpu/drm/i915/i915_perf.c:392:37: warning: expression using sizeof(void) -drivers/gpu/drm/i915/selftests/../i915_drv.h:3727:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3745:16: warning: expression using sizeof(void) Commit: drm/i915: add a new perf configuration execbuf parameter Okay! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: serialized performance queries
== Series Details == Series: drm/i915: serialized performance queries URL : https://patchwork.freedesktop.org/series/50698/ State : warning == Summary == $ dim checkpatch origin/drm-tip 40102d5976dd drm/i915/perf: allow holding preemption on filtered ctx ccae77ecfc25 drm/i915/perf: allow for CS OA configs to be created lazily -:109: CHECK:SPACING: No space is necessary after a cast #109: FILE: drivers/gpu/drm/i915/i915_perf.c:393: + (u32) MI_LOAD_REGISTER_IMM_MAX_REGS); total: 0 errors, 0 warnings, 1 checks, 336 lines checked 81dc092f58d5 drm/i915: add a new perf configuration execbuf parameter -:271: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #271: FILE: include/uapi/drm/i915_drm.h:1088: +#define I915_EXEC_PERF_CONFIG (1<<20) ^ -:273: CHECK:LINE_SPACING: Please don't use multiple blank lines #273: FILE: include/uapi/drm/i915_drm.h:1090: + + -:274: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #274: FILE: include/uapi/drm/i915_drm.h:1091: +#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_PERF_CONFIG<<1)) ^ total: 0 errors, 0 warnings, 3 checks, 218 lines checked ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC PATCH 3/3] drm/i915: add a new perf configuration execbuf parameter
We want the ability to dispatch a set of command buffer to the
hardware, each with a different OA configuration. To achieve this, we
reuse a couple of fields from the execbuf2 struct (I CAN HAZ
execbuf3?) to notify what OA configuration should be used for a batch
buffer. This requires the process making the execbuf with this flag to
also own the perf fd at the time of execbuf.
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/i915_drv.c| 4 ++
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 60 +++---
drivers/gpu/drm/i915/i915_request.c| 4 ++
drivers/gpu/drm/i915/i915_request.h| 2 +
drivers/gpu/drm/i915/intel_lrc.c | 13 -
drivers/gpu/drm/i915/intel_ringbuffer.c| 11 +++-
include/uapi/drm/i915_drm.h| 12 -
7 files changed, 97 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 193023427b40..564c2e749fd8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -444,6 +444,10 @@ static int i915_getparam_ioctl(struct drm_device *dev,
void *data,
case I915_PARAM_MMAP_GTT_COHERENT:
value = INTEL_INFO(dev_priv)->has_coherent_ggtt;
break;
+ case I915_PARAM_HAS_EXEC_PERF_CONFIG:
+ /* Obviously requires perf support. */
+ value = dev_priv->perf.initialized;
+ break;
default:
DRM_DEBUG("Unknown parameter %d\n", param->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 09187286d346..8b963641f142 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -286,6 +286,8 @@ struct i915_execbuffer {
*/
int lut_size;
struct hlist_head *buckets; /** ht for relocation handles */
+
+ struct i915_vma *oa_config; /** HW configuration for OA, NULL is not
needed. */
};
#define exec_entry(EB, VMA) (&(EB)->exec[(VMA)->exec_flags - (EB)->flags])
@@ -1121,6 +1123,32 @@ static void clflush_write32(u32 *addr, u32 value,
unsigned int flushes)
*addr = value;
}
+static int
+get_execbuf_oa_config(struct drm_i915_private *dev_priv,
+ int perf_fd, u32 oa_config_id,
+ struct i915_vma **out_oa_vma)
+{
+ struct file *perf_file;
+ int ret;
+
+ if (!dev_priv->perf.oa.exclusive_stream)
+ return -EINVAL;
+
+ perf_file = fget(perf_fd);
+ if (!perf_file)
+ return -EINVAL;
+
+ if (perf_file->private_data != dev_priv->perf.oa.exclusive_stream)
+ return -EINVAL;
+
+ fput(perf_file);
+
+ ret = i915_perf_get_oa_config(dev_priv, oa_config_id,
+ NULL, out_oa_vma);
+
+ return ret;
+}
+
static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
struct i915_vma *vma,
unsigned int len)
@@ -1173,6 +1201,9 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
goto err_unpin;
}
+ rq->oa_config = eb->oa_config;
+ eb->oa_config = NULL;
+
err = i915_request_await_object(rq, vma->obj, true);
if (err)
goto err_request;
@@ -1875,12 +1906,15 @@ static bool i915_gem_check_execbuffer(struct
drm_i915_gem_execbuffer2 *exec)
return false;
}
- if (exec->DR4 == 0x) {
- DRM_DEBUG("UXA submitting garbage DR4, fixing up\n");
- exec->DR4 = 0;
+ /* We reuse DR1 & DR4 fields for passing the perf config detail. */
+ if (!(exec->flags & I915_EXEC_PERF_CONFIG)) {
+ if (exec->DR4 == 0x) {
+ DRM_DEBUG("UXA submitting garbage DR4, fixing up\n");
+ exec->DR4 = 0;
+ }
+ if (exec->DR1 || exec->DR4)
+ return false;
}
- if (exec->DR1 || exec->DR4)
- return false;
if ((exec->batch_start_offset | exec->batch_len) & 0x7)
return false;
@@ -2224,6 +2258,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
eb.buffer_count = args->buffer_count;
eb.batch_start_offset = args->batch_start_offset;
eb.batch_len = args->batch_len;
+ eb.oa_config = NULL;
eb.batch_flags = 0;
if (args->flags & I915_EXEC_SECURE) {
@@ -2253,9 +2288,16 @@ i915_gem_do_execbuffer(struct drm_device *dev,
}
}
+ if (args->flags & I915_EXEC_PERF_CONFIG) {
+ err = get_execbuf_oa_config(eb.i915, args->DR1, args->DR4,
+ _config);
+ if (err)
+ goto err_out_fence;
+ }
+
err = eb_create();
if (err)
-
[Intel-gfx] [RFC PATCH 1/3] drm/i915/perf: allow holding preemption on filtered ctx
We would like to make use of perf in Vulkan. The Vulkan API is much
lower level than OpenGL, with applications directly exposed to the
concept of command buffers (pretty much equivalent to our batch
buffers). In Vulkan, queries are always limited in scope to a command
buffer. In OpenGL, the lack of command buffer concept meant that
queries' duration could span multiple command buffers.
With that restriction gone in Vulkan, we would like to simplify
measuring performance just by measuring the deltas between the counter
snapshots written by 2 MI_RECORD_PERF_COUNT commands, rather than the
more complex scheme we currently have in the GL driver, using 2
MI_RECORD_PERF_COUNT commands and doing some post processing on the
stream of OA reports, coming from the global OA buffer, to remove any
unrelated deltas in between the 2 MI_RECORD_PERF_COUNT.
Disabling preemption only apply to a single context with which want to
query performance counters for and is considered a privileged
operation, by default protected by CAP_SYS_ADMIN. It is possible to
enable it for a normal user by disabling the paranoid stream setting.
v2: Store preemption setting in intel_context (Chris)
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/i915_gem_context.c | 1 +
drivers/gpu/drm/i915/i915_gem_context.h | 3 +++
drivers/gpu/drm/i915/i915_perf.c| 32 +++--
drivers/gpu/drm/i915/intel_lrc.c| 2 +-
include/uapi/drm/i915_drm.h | 8 +++
5 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 8cbe58070561..c80655dbccc1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -343,6 +343,7 @@ __create_hw_context(struct drm_i915_private *dev_priv,
struct intel_context *ce = >__engine[n];
ce->gem_context = ctx;
+ ce->arb_enable = MI_ARB_ENABLE;
}
INIT_RADIX_TREE(>handles_vma, GFP_KERNEL);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h
b/drivers/gpu/drm/i915/i915_gem_context.h
index f6d870b1f73e..295d53763ee0 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -171,6 +171,9 @@ struct i915_gem_context {
int pin_count;
const struct intel_context_ops *ops;
+
+ /* arb_enable: Control preemption */
+ u32 arb_enable;
} __engine[I915_NUM_ENGINES];
/** ring_size: size for allocating the per-engine ring buffer */
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 664b96bb65a3..e2a96b6844fe 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -354,6 +354,7 @@ struct perf_open_properties {
u32 sample_flags;
u64 single_context:1;
+ u64 context_disable_preemption:1;
u64 ctx_handle;
/* OA sampling state */
@@ -1360,6 +1361,12 @@ static void i915_oa_stream_destroy(struct
i915_perf_stream *stream)
mutex_lock(_priv->drm.struct_mutex);
dev_priv->perf.oa.exclusive_stream = NULL;
dev_priv->perf.oa.ops.disable_metric_set(dev_priv);
+ if (stream->ctx) {
+ struct intel_context *ce =
+ to_intel_context(stream->ctx, dev_priv->engine[RCS]);
+
+ ce->arb_enable = MI_ARB_ENABLE;
+ }
mutex_unlock(_priv->drm.struct_mutex);
free_oa_buffer(dev_priv);
@@ -2099,6 +2106,13 @@ static int i915_oa_stream_init(struct i915_perf_stream
*stream,
goto err_enable;
}
+ if (props->context_disable_preemption) {
+ struct intel_context *ce =
+ to_intel_context(stream->ctx, dev_priv->engine[RCS]);
+
+ ce->arb_enable = MI_ARB_DISABLE;
+ }
+
stream->ops = _oa_stream_ops;
dev_priv->perf.oa.exclusive_stream = stream;
@@ -2555,6 +2569,15 @@ i915_perf_open_ioctl_locked(struct drm_i915_private
*dev_priv,
}
}
+ if (props->context_disable_preemption) {
+ if (!props->single_context) {
+ DRM_DEBUG("preemption disable with no context\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ privileged_op = true;
+ }
+
/*
* On Haswell the OA unit supports clock gating off for a specific
* context and in this mode there's no visibility of metrics for the
@@ -2569,8 +2592,10 @@ i915_perf_open_ioctl_locked(struct drm_i915_private
*dev_priv,
* MI_REPORT_PERF_COUNT commands and so consider it a privileged op to
* enable the OA unit by default.
*/
- if (IS_HASWELL(dev_priv) && specific_ctx)
+ if (IS_HASWELL(dev_priv) && specific_ctx &&
+ !props->context_disable_preemption) {
[Intel-gfx] [RFC PATCH 2/3] drm/i915/perf: allow for CS OA configs to be created lazily
Here we introduce a mechanism by which the execbuf part of the i915
driver will be able to request that a batch buffer containing the
programming for a particular OA config be created.
We'll execute these OA configuration buffers right before executing a
set of userspace commands so that a particular user batchbuffer be
executed with a given OA configuration.
This mechanism essentially allows the userspace driver to go through
several OA configuration without having to open/close the i915/perf
stream.
Signed-off-by: Lionel Landwerlin
---
drivers/gpu/drm/i915/i915_drv.h | 22 ++-
drivers/gpu/drm/i915/i915_perf.c | 195 ++
drivers/gpu/drm/i915/intel_gpu_commands.h | 1 +
3 files changed, 187 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2264b30ce51a..a35715cd7608 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1378,6 +1378,10 @@ struct i915_oa_config {
struct attribute *attrs[2];
struct device_attribute sysfs_metric_id;
+ struct i915_vma *vma;
+
+ struct list_head vma_link;
+
atomic_t ref_count;
};
@@ -1979,11 +1983,21 @@ struct drm_i915_private {
struct mutex metrics_lock;
/*
-* List of dynamic configurations, you need to hold
-* dev_priv->perf.metrics_lock to access it.
+* List of dynamic configurations (struct i915_oa_config), you
+* need to hold dev_priv->perf.metrics_lock to access it.
*/
struct idr metrics_idr;
+ /*
+* List of dynamic configurations (struct i915_oa_config)
+* which have an allocated buffer in GGTT for reconfiguration,
+* you need to hold dev_priv->perf.metrics_lock to access it.
+* Elements are added to the list lazilly on execbuf (when a
+* particular configuration is requested). The list is freed
+* upon closing the perf stream.
+*/
+ struct list_head metrics_buffers;
+
/*
* Lock associated with anything below within this structure
* except exclusive_stream.
@@ -3315,6 +3329,10 @@ int i915_perf_remove_config_ioctl(struct drm_device
*dev, void *data,
void i915_oa_init_reg_state(struct intel_engine_cs *engine,
struct i915_gem_context *ctx,
uint32_t *reg_state);
+int i915_perf_get_oa_config(struct drm_i915_private *i915,
+ int metrics_set,
+ struct i915_oa_config **out_config,
+ struct i915_vma **out_vma);
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct i915_address_space *vm,
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index e2a96b6844fe..39c5b44862d4 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -364,9 +364,16 @@ struct perf_open_properties {
int oa_period_exponent;
};
-static void free_oa_config(struct drm_i915_private *dev_priv,
- struct i915_oa_config *oa_config)
+static void put_oa_config(struct i915_oa_config *oa_config)
{
+ if (!atomic_dec_and_test(_config->ref_count))
+ return;
+
+ if (oa_config->vma) {
+ list_del(_config->vma_link);
+ i915_vma_put(oa_config->vma);
+ }
+
if (!PTR_ERR(oa_config->flex_regs))
kfree(oa_config->flex_regs);
if (!PTR_ERR(oa_config->b_counter_regs))
@@ -376,38 +383,152 @@ static void free_oa_config(struct drm_i915_private
*dev_priv,
kfree(oa_config);
}
-static void put_oa_config(struct drm_i915_private *dev_priv,
- struct i915_oa_config *oa_config)
+static u32 *write_cs_mi_lri(u32 *cs, const struct i915_oa_reg *reg_data, u32
n_regs)
{
- if (!atomic_dec_and_test(_config->ref_count))
- return;
+ u32 i;
- free_oa_config(dev_priv, oa_config);
+ for (i = 0; i < n_regs; i++) {
+ if ((i % MI_LOAD_REGISTER_IMM_MAX_REGS) == 0) {
+ u32 n_lri = min(n_regs - i,
+ (u32) MI_LOAD_REGISTER_IMM_MAX_REGS);
+
+ *cs++ = MI_LOAD_REGISTER_IMM(n_lri);
+ }
+ *cs++ = i915_mmio_reg_offset(reg_data[i].addr);
+ *cs++ = reg_data[i].value;
+ }
+
+ return cs;
}
-static int get_oa_config(struct drm_i915_private *dev_priv,
-int metrics_set,
-struct i915_oa_config **out_config)
+static int alloc_oa_config_buffer(struct drm_i915_private *i915,
+ struct i915_oa_config *oa_config)
{
+ struct
[Intel-gfx] [RFC PATCH 0/3] drm/i915: serialized performance queries
Hi all, This is a early stage series on which I'm looking for feedback. Some background : Performance queries (sampling performance counters through MI_REPORT_PERF_COUNT instruction) commands requires the hardware to be programmed with the desired configuration to allow particular performance data to be recorded. Up to this series, an application querying performance data would have to close/reopen the i915/perf stream each time it wanted to gather different type of performance data. This series introduce a new mechanism through the execbuf parameters to specify what configuration should be used for the set of commands given into the execbuf's batchbuffer. Motivation : Giving the configuration needed to gather performance data at execbuf time together with holding preemption on the batchs of the same application allows data to be gathered using just MI_REPORT_PERF_COUNT instructions and also to go through all many configuration without slowing down the application. The application can now serialize many queries of different types and doesn't have to wait for completion of a previous query to submit a new one with a different configuration. These patches are in no way final, in particular the execbuf uapi changes is probably unworkable in production (it was just a quick way to prove things are working). I heard discussions about execbuf3, this could probably be tied into that. Looking forward to your comments. Thanks, Lionel Landwerlin (3): drm/i915/perf: allow holding preemption on filtered ctx drm/i915/perf: allow for CS OA configs to be created lazily drm/i915: add a new perf configuration execbuf parameter drivers/gpu/drm/i915/i915_drv.c| 4 + drivers/gpu/drm/i915/i915_drv.h| 22 +- drivers/gpu/drm/i915/i915_gem_context.c| 1 + drivers/gpu/drm/i915/i915_gem_context.h| 3 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 60 +- drivers/gpu/drm/i915/i915_perf.c | 227 ++--- drivers/gpu/drm/i915/i915_request.c| 4 + drivers/gpu/drm/i915/i915_request.h| 2 + drivers/gpu/drm/i915/intel_gpu_commands.h | 1 + drivers/gpu/drm/i915/intel_lrc.c | 15 +- drivers/gpu/drm/i915/intel_ringbuffer.c| 11 +- include/uapi/drm/i915_drm.h| 20 +- 12 files changed, 327 insertions(+), 43 deletions(-) -- 2.19.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Do intel_panel_destroy_backlight() later
Quoting Ville Syrjala (2018-10-08 14:46:40) > From: Ville Syrjälä > > Currently we destroy the backlight during connector unregistration. > That means the final modeset performed by drm_atomic_helper_shutdown() > will leave the backlight on. We don't want that so let's just move > intel_panel_destroy_backlight() into intel_panel_fini() which gets > called during connector destuction. > > We still unregister the user visible backlight device during connector > unregistration. > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106386 > Cc: Jani Nikula > Cc: Chris Wilson > Cc: Daniel Vetter > Signed-off-by: Ville Syrjälä ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v2)
Thanks! I have no problem with this patch.
On Thu, Oct 4, 2018 at 2:08 AM Guang Bai wrote:
> On some platforms, slowly unplugging (wiggling) the HDMI cable makes
> the kernel to believe the HDMI display still connected. This is because
> the HDMI DDC lines are disconnected sometimes later after the hot-plug
> interrupt triggered. Use the hot plug live states to honor HDMI hot plug
> status in addtion to access the DDC channels.
>
> v2: Fix the formatting issue
>
> Cc: Jani Nikula
> Cc: Chris Chiu
> Signed-off-by: Guang Bai
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 32 +---
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 648a13c..98ab1ab 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -246,17 +246,43 @@ static void intel_hpd_irq_storm_reenable_work(struct
> work_struct *work)
> intel_runtime_pm_put(dev_priv);
> }
>
> +#define MAX_SHORT_PULSE_MS 100
> +#define PORT_CHECK_LOOP_COUNT 3
> +
> bool intel_encoder_hotplug(struct intel_encoder *encoder,
>struct intel_connector *connector)
> {
> struct drm_device *dev = connector->base.dev;
> - enum drm_connector_status old_status;
> + enum drm_connector_status old_status, new_status;
> + enum hpd_pin pin = encoder->hpd_pin;
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + u32 count = 0;
>
> WARN_ON(!mutex_is_locked(>mode_config.mutex));
> old_status = connector->base.status;
>
> - connector->base.status =
> - drm_helper_probe_detect(>base, NULL, false);
> + /*
> +* Set HDMI connection status based on hot-plug live states and
> +* display probe results.
> +*/
> + if ((encoder->type == INTEL_OUTPUT_HDMI ||
> +encoder->type == INTEL_OUTPUT_DDI) &&
> + dev_priv->hotplug.stats[pin].state == HPD_ENABLED) {
> + do {
> + new_status = connector_status_disconnected;
> + msleep(MAX_SHORT_PULSE_MS);
> +
> + if (intel_digital_port_connected(encoder))
> + new_status =
> drm_helper_probe_detect(>base,
> +NULL,
> false);
> + if (new_status == connector_status_connected)
> + break;
> + } while (++count <= PORT_CHECK_LOOP_COUNT);
> + connector->base.status = new_status;
> + } else {
> + connector->base.status =
> + drm_helper_probe_detect(>base, NULL,
> false);
> + }
>
> if (old_status == connector->base.status)
> return false;
> --
> 2.7.4
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Do intel_panel_destroy_backlight() later
Quoting Ville Syrjala (2018-10-08 14:46:40) > From: Ville Syrjälä > > Currently we destroy the backlight during connector unregistration. > That means the final modeset performed by drm_atomic_helper_shutdown() > will leave the backlight on. We don't want that so let's just move > intel_panel_destroy_backlight() into intel_panel_fini() which gets > called during connector destuction. > > We still unregister the user visible backlight device during connector > unregistration. > > Cc: Jani Nikula > Cc: Chris Wilson > Cc: Daniel Vetter > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Drop the eDP check from intel_dp_connector_destroy()
Quoting Ville Syrjala (2018-10-08 14:46:41) > From: Ville Syrjälä > > As long as the connector was zeroed during allocation calling > intel_panel_fini() is safe even if we haven't initialized > the panel struct explicitly. So let's drop the useless eDP > check from dp connector destruction. > > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Do intel_panel_destroy_backlight() later
== Series Details == Series: series starting with [1/2] drm/i915: Do intel_panel_destroy_backlight() later URL : https://patchwork.freedesktop.org/series/50691/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4950 -> Patchwork_10389 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/50691/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10389 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_hangcheck: fi-kbl-7560u: PASS -> INCOMPLETE (fdo#108044) Possible fixes igt@gem_exec_suspend@basic-s3: fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS igt@gem_exec_suspend@basic-s4-devices: fi-kbl-7500u: DMESG-WARN (fdo#105128, fdo#107139) -> PASS igt@kms_flip@basic-flip-vs-modeset: fi-hsw-4770r: DMESG-WARN (fdo#105602) -> PASS igt@pm_rpm@module-reload: fi-hsw-peppy: DMESG-WARN (fdo#107603, fdo#106386) -> PASS fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#106386 https://bugs.freedesktop.org/show_bug.cgi?id=106386 fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139 fdo#107603 https://bugs.freedesktop.org/show_bug.cgi?id=107603 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#108044 https://bugs.freedesktop.org/show_bug.cgi?id=108044 == Participating hosts (50 -> 42) == Missing(8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-snb-2520m fi-ctg-p8600 fi-skl-iommu == Build changes == * Linux: CI_DRM_4950 -> Patchwork_10389 CI_DRM_4950: a9abc43bebfb6de62c2c3747a22fadfa17b61d8b @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4671: b121f7d42c260ae3a050c3f440d1c11f7cff7d1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10389: 83ace5cf94e8a618cea7003b80492a13ae207af0 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 83ace5cf94e8 drm/i915: Drop the eDP check from intel_dp_connector_destroy() b9cdf0fd30aa drm/i915: Do intel_panel_destroy_backlight() later == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10389/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Stop calling intel_opregion unregister/register in suspend/resume
On Mon, Oct 08, 2018 at 09:44:08AM +0300, Jani Nikula wrote:
> On Fri, 05 Oct 2018, Chris Wilson wrote:
> > If we reduce the suspend function for intel_opregion to do the minimum
> > required, the resume function can also do the simple task of notifier
> > the ACPI bios that we are back. This avoid some nasty restrictions on
> > the likes of register_acpi_notifier() that are not allowed during the
> > early phase of resume.
>
> Something like this has been on the back of my mind for a long time, but
> never got around to it. Couple of nitpicks/observations inline.
>
> >
> > Signed-off-by: Chris Wilson
> > Cc: Imre Deak
> > Cc: Jani Nikula
Simplifies the task of moving some steps from the suspend/resume hooks
to the suspend_late/resume_early hooks, which is needed by the audio/
CDCLK workaround we are working towards, so:
Acked-by: Imre Deak
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 9 +-
> > drivers/gpu/drm/i915/intel_opregion.c | 155 +++---
> > drivers/gpu/drm/i915/intel_opregion.h | 15 +++
> > 3 files changed, 108 insertions(+), 71 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 193023427b40..9b0887746bac 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1919,9 +1919,7 @@ static int i915_drm_suspend(struct drm_device *dev)
> > i915_save_state(dev_priv);
> >
> > opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
> > - intel_opregion_notify_adapter(dev_priv, opregion_target_state);
> > -
> > - intel_opregion_unregister(dev_priv);
> > + intel_opregion_suspend(dev_priv, opregion_target_state);
> >
> > intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
> >
> > @@ -2040,7 +2038,6 @@ static int i915_drm_resume(struct drm_device *dev)
> >
> > i915_restore_state(dev_priv);
> > intel_pps_unlock_regs_wa(dev_priv);
> > - intel_opregion_setup(dev_priv);
> >
> > intel_init_pch_refclk(dev_priv);
> >
> > @@ -2082,12 +2079,10 @@ static int i915_drm_resume(struct drm_device *dev)
> > * */
> > intel_hpd_init(dev_priv);
> >
> > - intel_opregion_register(dev_priv);
> > + intel_opregion_resume(dev_priv);
> >
> > intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
> >
> > - intel_opregion_notify_adapter(dev_priv, PCI_D0);
> > -
> > intel_power_domains_enable(dev_priv);
> >
> > enable_rpm_wakeref_asserts(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> > b/drivers/gpu/drm/i915/intel_opregion.c
> > index e034b4166d32..379e8c64a248 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -773,70 +773,6 @@ static void intel_setup_cadls(struct drm_i915_private
> > *dev_priv)
> > opregion->acpi->cadl[i] = 0;
> > }
> >
> > -void intel_opregion_register(struct drm_i915_private *dev_priv)
> > -{
> > - struct intel_opregion *opregion = _priv->opregion;
> > -
> > - if (!opregion->header)
> > - return;
> > -
> > - if (opregion->acpi) {
> > - intel_didl_outputs(dev_priv);
> > - intel_setup_cadls(dev_priv);
> > -
> > - /* Notify BIOS we are ready to handle ACPI video ext notifs.
> > -* Right now, all the events are handled by the ACPI video
> > module.
> > -* We don't actually need to do anything with them. */
> > - opregion->acpi->csts = 0;
> > - opregion->acpi->drdy = 1;
> > -
> > - opregion->acpi_notifier.notifier_call =
> > intel_opregion_video_event;
> > - register_acpi_notifier(>acpi_notifier);
> > - }
> > -
> > - if (opregion->asle) {
> > - opregion->asle->tche = ASLE_TCHE_BLC_EN;
> > - opregion->asle->ardy = ASLE_ARDY_READY;
> > - }
> > -}
> > -
> > -void intel_opregion_unregister(struct drm_i915_private *dev_priv)
> > -{
> > - struct intel_opregion *opregion = _priv->opregion;
> > -
> > - if (!opregion->header)
> > - return;
> > -
> > - if (opregion->asle)
> > - opregion->asle->ardy = ASLE_ARDY_NOT_READY;
> > -
> > - cancel_work_sync(_priv->opregion.asle_work);
> > -
> > - if (opregion->acpi) {
> > - opregion->acpi->drdy = 0;
> > -
> > - unregister_acpi_notifier(>acpi_notifier);
> > - opregion->acpi_notifier.notifier_call = NULL;
> > - }
> > -
> > - /* just clear all opregion memory pointers now */
> > - memunmap(opregion->header);
> > - if (opregion->rvda) {
> > - memunmap(opregion->rvda);
> > - opregion->rvda = NULL;
> > - }
> > - if (opregion->vbt_firmware) {
> > - kfree(opregion->vbt_firmware);
> > - opregion->vbt_firmware = NULL;
> > - }
> > - opregion->header = NULL;
> > - opregion->acpi = NULL;
> > - opregion->swsci = NULL;
> > - opregion->asle = NULL;
> > - opregion->vbt = NULL;
> > -
Re: [Intel-gfx] [PATCH v2 6/6] drm/i915/icl:Add Wa_1606682166
Radhakrishna Sripada writes:
> From: Anuj Phogat
>
> Incorrect TDL's SSP address shift in SARB for 16:6 & 18:8 modes.
> Disable the Sampler state prefetch functionality in the SARB by
> programming 0xB000[30] to '1'. This is to be done at boot time
> and the feature must remain disabled permanently.
>
> Fixes flaky tex-mip-level-selection* piglit tests with Mesa i965
> driver.
>
> Cc: Radhakrishna Sripada
> Signed-off-by: Anuj Phogat
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_workarounds.c | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fa020425754f..0c544161ed47 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7415,6 +7415,7 @@ enum {
>
> #define GEN7_SARCHKMD_MMIO(0xB000)
> #define GEN7_DISABLE_DEMAND_PREFETCH (1 << 31)
> +#define GEN7_DISABLE_SAMPLER_PREFETCH (1 << 30)
>
> #define GEN7_L3SQCREG1 _MMIO(0xB010)
> #define VLV_B0_WA_L3SQCREG1_VALUE 0x00D3
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c
> b/drivers/gpu/drm/i915/intel_workarounds.c
> index cf4f4c1f86ab..7157115e5bc9 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -910,7 +910,8 @@ static void icl_gt_workarounds_apply(struct
> drm_i915_private *dev_priv)
> if (IS_ICL_REVID(dev_priv, ICL_REVID_A0, ICL_REVID_B0))
> I915_WRITE(GEN7_SARCHKMD,
> I915_READ(GEN7_SARCHKMD) |
> -GEN7_DISABLE_DEMAND_PREFETCH);
> +GEN7_DISABLE_DEMAND_PREFETCH |
> +GEN7_DISABLE_SAMPLER_PREFETCH);
> }
>
> void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> --
> 2.9.3
>
> ___
> 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 v4] drm/i915: Remove i915.enable_ppgtt override
Thanks for pointing this. My bad. I take a look on the code and it looks like the GVT-g context is now quite similar with the kernel context except the force single submission and ring buffer size. (When we upstream the code, there was no kernel context at that time. :/) Now there is already one API for single submission. If we can introduce another one to configure the ring buffer size. Then maybe we can remove the *create_gvt() in i915_gem_context.c and used kernel context instead. Feel free to drop comments. :) Thanks, Zhi. -Original Message- From: Zhenyu Wang [mailto:zhen...@linux.intel.com] Sent: Monday, October 8, 2018 12:54 PM To: Joonas Lahtinen Cc: He, Min ; Wang, Zhi A ; intel-gfx@lists.freedesktop.org; Chris Wilson ; Zhenyu Wang ; Auld, Matthew Subject: Re: [PATCH v4] drm/i915: Remove i915.enable_ppgtt override On 2018.09.26 11:02:15 +0300, Joonas Lahtinen wrote: > Quoting He, Min (2018-09-26 04:06:25) > > No. We changed to full 48bit ppgtt long time ago. :) > > So would that mean the change is OK to do? Looks that one unfortunately caused gvt regression, gvt context has no i915 managed i915_hw_ppgtt but only shadowed one, so need to check if ppgtt is valid before access. > > Acked-by from you, Zhi, would be good to have for documentation > purposes in that case :) > > Regards, Joonas > > > > > > -Original Message- > > > From: Wang, Zhi A > > > Sent: Wednesday, September 26, 2018 2:01 AM > > > To: Joonas Lahtinen ; Chris > > > Wilson ; > > > intel-gfx@lists.freedesktop.org; Zhenyu Wang > > > > > > Cc: Auld, Matthew ; He, Min > > > > > > Subject: Re: [PATCH v4] drm/i915: Remove i915.enable_ppgtt > > > override > > > > > > Hi Min: > > > > > > I remembered the IOTG guest kernel is using aliasing-ppgtt in the > > > last technical sharing (probably I didn't remember it correctly). > > > Can you confirm? > > > > > > Also, thanks Joonas for the reminding. :) > > > > > > thanks, > > > Zhi. > > > > > > On 09/25/18 09:22, Joonas Lahtinen wrote: > > > > + Zhi and Zhenyu > > > > > > > > For me the patch looks ok. > > > > > > > > It refuses to load GVT on systems where 48-bit is not supported, > > > > for not worsening the system security when virtualization is > > > > enabled (as anybody would probably expect virtualization to improve > > > > that). Please see code. > > > > > > > > Do we have such systems in the wild? Should we instruct the user > > > > to updating the hypervisor to specific kernel version when they > > > > hit the error? > > > > > > > > Regards, Joonas > > > > > > > > Quoting Chris Wilson (2018-09-25 14:48:20) > > > >> Now that we are confident in providing full-ppgtt where > > > >> supported, remove the ability to override the context isolation. > > > >> > > > >> v2: Remove faked aliasing-ppgtt for testing as it no longer is > > > >> accepted. > > > >> v3: s/USES/HAS/ to match usage and reject attempts to load the > > > >> module > > > on > > > >> old GVT-g setups that do not provide support for full-ppgtt. > > > >> > > > >> Signed-off-by: Chris Wilson > > > >> Cc: Joonas Lahtinen > > > >> Cc: Matthew Auld > > > >> --- > > > >> drivers/gpu/drm/i915/i915_drv.c | 22 +++-- > > > >> drivers/gpu/drm/i915/i915_drv.h | 14 +-- > > > >> drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > > > >> drivers/gpu/drm/i915/i915_gem_gtt.c | 88 > > > >> ++- > > > >> drivers/gpu/drm/i915/i915_gpu_error.c | 4 +- > > > >> drivers/gpu/drm/i915/i915_params.c| 4 - > > > >> drivers/gpu/drm/i915/i915_params.h| 1 - > > > >> drivers/gpu/drm/i915/i915_pci.c | 17 ++-- > > > >> drivers/gpu/drm/i915/intel_device_info.c | 6 ++ > > > >> drivers/gpu/drm/i915/intel_device_info.h | 11 ++- > > > >> drivers/gpu/drm/i915/intel_lrc.c | 13 ++- > > > >> drivers/gpu/drm/i915/selftests/huge_pages.c | 12 +-- > > > >> .../gpu/drm/i915/selftests/i915_gem_context.c | 59 + > > > >> .../gpu/drm/i915/selftests/i915_gem_evict.c | 2 +- > > > >> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 +- > > > >> 15 files changed, 62 insertions(+), 197 deletions(-) > > > >> > > > >> diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > >> index 44e2c0f5ec50..3efbbc71c3b4 100644 > > > >> --- a/drivers/gpu/drm/i915/i915_drv.c > > > >> +++ b/drivers/gpu/drm/i915/i915_drv.c > > > >> @@ -345,7 +345,7 @@ static int i915_getparam_ioctl(struct > > > >> drm_device > > > *dev, void *data, > > > >> value = HAS_WT(dev_priv); > > > >> break; > > > >> case I915_PARAM_HAS_ALIASING_PPGTT: > > > >> - value = USES_PPGTT(dev_priv); > > > >> + value = INTEL_PPGTT(dev_priv); > > > >> break; > > > >> case I915_PARAM_HAS_SEMAPHORES: > > > >> value =
Re: [Intel-gfx] [PATCH v2 5/6] drm/i915/icl: Add Wa_1406609255
Radhakrishna Sripada writes:
> Shader feature to prefetch binding tables does not support 16:6 18:8 BTP
> formats. Enabling fault handling could result in hangs with faults.
> Disabling demand prefetch would disable binding table prefetch.
>
> V2: Fix the stepping rivision to B0(Mika)
>
> References: HSDES#1406609255, HSDES#1406573985
> Cc: Mika Kuoppala
> Signed-off-by: Radhakrishna Sripada
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> drivers/gpu/drm/i915/intel_workarounds.c | 6 ++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c8a187d8db0f..fa020425754f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7413,6 +7413,9 @@ enum {
> #define GEN9_SLICE_COMMON_ECO_CHICKEN1 _MMIO(0x731c)
> #define GEN11_STATE_CACHE_REDIRECT_TO_CS (1 << 11)
>
> +#define GEN7_SARCHKMD_MMIO(0xB000)
> +#define GEN7_DISABLE_DEMAND_PREFETCH (1 << 31)
> +
> #define GEN7_L3SQCREG1 _MMIO(0xB010)
> #define VLV_B0_WA_L3SQCREG1_VALUE 0x00D3
>
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c
> b/drivers/gpu/drm/i915/intel_workarounds.c
> index 65cd36cd2957..cf4f4c1f86ab 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -905,6 +905,12 @@ static void icl_gt_workarounds_apply(struct
> drm_i915_private *dev_priv)
> I915_WRITE(GAMT_CHKN_BIT_REG,
> I915_READ(GAMT_CHKN_BIT_REG) |
> GAMT_CHKN_DISABLE_L3_COH_PIPE);
> +
> + /* Wa_1406609255:icl (pre-prod) */
> + if (IS_ICL_REVID(dev_priv, ICL_REVID_A0, ICL_REVID_B0))
> + I915_WRITE(GEN7_SARCHKMD,
> +I915_READ(GEN7_SARCHKMD) |
> +GEN7_DISABLE_DEMAND_PREFETCH);
> }
>
> void intel_gt_workarounds_apply(struct drm_i915_private *dev_priv)
> --
> 2.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Do intel_panel_destroy_backlight() later
From: Ville Syrjälä
Currently we destroy the backlight during connector unregistration.
That means the final modeset performed by drm_atomic_helper_shutdown()
will leave the backlight on. We don't want that so let's just move
intel_panel_destroy_backlight() into intel_panel_fini() which gets
called during connector destuction.
We still unregister the user visible backlight device during connector
unregistration.
Cc: Jani Nikula
Cc: Chris Wilson
Cc: Daniel Vetter
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 1 -
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_panel.c | 7 +++
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 741fc5b4f9d9..6a7fe89f3145 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15860,7 +15860,6 @@ void intel_connector_unregister(struct drm_connector
*connector)
struct intel_connector *intel_connector = to_intel_connector(connector);
intel_backlight_device_unregister(intel_connector);
- intel_panel_destroy_backlight(connector);
}
static void intel_hpd_poll_fini(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7a9f5ee4604f..8050d06c722a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1904,7 +1904,6 @@ int intel_panel_setup_backlight(struct drm_connector
*connector,
void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
const struct drm_connector_state *conn_state);
void intel_panel_disable_backlight(const struct drm_connector_state
*old_conn_state);
-void intel_panel_destroy_backlight(struct drm_connector *connector);
extern struct drm_display_mode *intel_find_panel_downclock(
struct drm_i915_private *dev_priv,
struct drm_display_mode *fixed_mode,
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 4a9f139e7b73..7df9bcd2bb20 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1814,11 +1814,8 @@ int intel_panel_setup_backlight(struct drm_connector
*connector, enum pipe pipe)
return 0;
}
-void intel_panel_destroy_backlight(struct drm_connector *connector)
+static void intel_panel_destroy_backlight(struct intel_panel *panel)
{
- struct intel_connector *intel_connector = to_intel_connector(connector);
- struct intel_panel *panel = _connector->panel;
-
/* dispose of the pwm */
if (panel->backlight.pwm)
pwm_put(panel->backlight.pwm);
@@ -1923,6 +1920,8 @@ void intel_panel_fini(struct intel_panel *panel)
struct intel_connector *intel_connector =
container_of(panel, struct intel_connector, panel);
+ intel_panel_destroy_backlight(panel);
+
if (panel->fixed_mode)
drm_mode_destroy(intel_connector->base.dev, panel->fixed_mode);
--
2.16.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Drop the eDP check from intel_dp_connector_destroy()
From: Ville Syrjälä As long as the connector was zeroed during allocation calling intel_panel_fini() is safe even if we haven't initialized the panel struct explicitly. So let's drop the useless eDP check from dp connector destruction. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_dp.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 19f0c3f59cbe..d12f987a6c43 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5261,12 +5261,7 @@ intel_dp_connector_destroy(struct drm_connector *connector) if (!IS_ERR_OR_NULL(intel_connector->edid)) kfree(intel_connector->edid); - /* -* Can't call intel_dp_is_edp() since the encoder may have been -* destroyed already. -*/ - if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) - intel_panel_fini(_connector->panel); + intel_panel_fini(_connector->panel); drm_connector_cleanup(connector); kfree(connector); -- 2.16.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] pci: Add a few new IDs for Intel GPU "spurious interrupt" quirk
From: Bin Meng
> Sent: 08 October 2018 13:34
> Hi David,
>
> On Mon, Oct 8, 2018 at 6:06 PM David Laight wrote:
> >
> > From: Bin Meng
> > > Sent: 08 October 2018 10:44
> > ...
> > > Correct, disable the shared interrupt line keeps all devices using
> > > that line from working, which is current kernel's behavior w/o this
> > > quirk handling: it disables the (shared) interrupt line after 100.000+
> > > generated interrupts. But the side effect is that other devices become
> > > unusable after that (eg: USB devices which share the same interrupt
> > > line with the Intel GPU). That's why the original commit, f67fd55fa96f
> > > ("PCI: Add quirk for still enabled interrupts on Intel Sandy Bridge
> > > GPUs") disables the GPU's interrupt directly, which should really be
> > > done by the VGA BIOS itself (a buggy VBIOS!).
> >
> > Shouldn't the kernel just disable all PCI(e) interrupts by writing
> > 1 to the config space control register bit during grope?
> > Can it ever by right for this to be set?
> >
>
> Do you mean PCI_COMMAND_INTX_DISABLE bit of the command register in
> the configuration space? Setting this bit indeed could disable the
> INTx interrupt, but it does not work for all PCI devices as this bit
> was introduced in PCI spec v2.3.
That's the one I was thinking of.
If it was introduced in v2.3 it explains why it is a 'disable' bit.
The v2.2 spec I just found doesn't seem to say anything about the
'reserved' bits. I guess the values are ignored (and probobly read
back as zeros).
In any case it should be implemented by the VGA devices in question.
I guess the kernel should also ensure that MSI and MSI-X interrupts
are also all disabled.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Fixup kernel doc for param name changes
== Series Details ==
Series: drm/i915: Fixup kernel doc for param name changes
URL : https://patchwork.freedesktop.org/series/50679/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4948_full -> Patchwork_10388_full =
== Summary - SUCCESS ==
No regressions found.
== Known issues ==
Here are the changes found in Patchwork_10388_full that come from known
issues:
=== IGT changes ===
Issues hit
igt@debugfs_test@read_all_entries_display_off:
shard-skl: PASS -> INCOMPLETE (fdo#104108)
igt@gem_userptr_blits@readonly-unsync:
shard-skl: NOTRUN -> INCOMPLETE (fdo#108074)
igt@kms_busy@extended-modeset-hang-newfb-render-b:
shard-skl: NOTRUN -> DMESG-WARN (fdo#107956)
igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
shard-glk: PASS -> FAIL (fdo#108145)
shard-skl: NOTRUN -> FAIL (fdo#105458)
igt@kms_color@pipe-b-legacy-gamma:
shard-apl: PASS -> FAIL (fdo#104782)
igt@kms_cursor_crc@cursor-64x21-random:
shard-apl: PASS -> FAIL (fdo#103232) +3
igt@kms_cursor_legacy@2x-cursor-vs-flip-legacy:
shard-glk: NOTRUN -> INCOMPLETE (fdo#103359, k.org#198133)
igt@kms_flip@basic-flip-vs-modeset:
shard-hsw: PASS -> DMESG-WARN (fdo#102614)
igt@kms_flip@flip-vs-fences-interruptible:
shard-snb: NOTRUN -> INCOMPLETE (fdo#105411)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-msflip-blt:
shard-skl: PASS -> FAIL (fdo#105682)
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
shard-apl: PASS -> FAIL (fdo#103167) +2
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
shard-glk: PASS -> FAIL (fdo#103167) +8
igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt:
shard-skl: PASS -> FAIL (fdo#103167)
igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
shard-glk: PASS -> FAIL (fdo#103166) +2
igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
shard-apl: PASS -> FAIL (fdo#103166) +1
igt@kms_rotation_crc@exhaust-fences:
shard-skl: NOTRUN -> DMESG-WARN (fdo#105748)
igt@kms_setmode@basic:
shard-snb: NOTRUN -> FAIL (fdo#99912)
igt@pm_rpm@legacy-planes:
shard-skl: PASS -> INCOMPLETE (fdo#105959, fdo#107807)
Possible fixes
igt@gem_exec_await@wide-contexts:
shard-kbl: FAIL (fdo#106680) -> PASS
igt@gem_softpin@noreloc-s3:
shard-skl: INCOMPLETE (fdo#107773, fdo#104108) -> PASS
igt@kms_chv_cursor_fail@pipe-a-64x64-left-edge:
shard-glk: DMESG-WARN (fdo#105763, fdo#106538) -> PASS +2
igt@kms_cursor_crc@cursor-128x128-onscreen:
shard-apl: FAIL (fdo#103232) -> PASS
igt@kms_cursor_crc@cursor-64x64-sliding:
shard-glk: FAIL (fdo#103232) -> PASS
igt@kms_flip@2x-flip-vs-rmfb-interruptible:
shard-glk: INCOMPLETE (fdo#103359, k.org#198133) -> PASS
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
shard-apl: FAIL (fdo#103167) -> PASS
igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
shard-glk: FAIL (fdo#103167) -> PASS
{igt@kms_plane_alpha_blend@pipe-a-coverage-7efc}:
shard-skl: FAIL (fdo#108145) -> PASS
igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
shard-glk: FAIL (fdo#103166) -> PASS
igt@pm_rpm@gem-execbuf-stress-pc8:
shard-skl: INCOMPLETE (fdo#107807) -> SKIP
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
fdo#105458 https://bugs.freedesktop.org/show_bug.cgi?id=105458
fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
fdo#105748 https://bugs.freedesktop.org/show_bug.cgi?id=105748
fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959
fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
fdo#107956
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Fixup kernel doc for param name changes
== Series Details == Series: drm/i915: Fixup kernel doc for param name changes URL : https://patchwork.freedesktop.org/series/50679/ State : success == Summary == = CI Bug Log - changes from CI_DRM_4948 -> Patchwork_10388 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/50679/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10388 that come from known issues: === IGT changes === Issues hit igt@gem_exec_suspend@basic-s3: fi-blb-e6850: PASS -> INCOMPLETE (fdo#107718) igt@kms_chamelium@dp-crc-fast: fi-kbl-7500u: PASS -> DMESG-FAIL (fdo#103841) igt@kms_flip@basic-flip-vs-modeset: fi-glk-j4005: PASS -> DMESG-WARN (fdo#106097, fdo#106000) igt@prime_vgem@basic-fence-flip: fi-cfl-8700k: PASS -> FAIL (fdo#104008) Possible fixes igt@drv_module_reload@basic-reload: fi-glk-j4005: DMESG-WARN (fdo#106248, fdo#106725) -> PASS igt@pm_rpm@module-reload: fi-glk-j4005: DMESG-FAIL (fdo#104767) -> PASS fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008 fdo#104767 https://bugs.freedesktop.org/show_bug.cgi?id=104767 fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000 fdo#106097 https://bugs.freedesktop.org/show_bug.cgi?id=106097 fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248 fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 == Participating hosts (47 -> 42) == Missing(5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-icl-u2 fi-bdw-samus == Build changes == * Linux: CI_DRM_4948 -> Patchwork_10388 CI_DRM_4948: 21b6e4d4acf2c5d218cdaa57fda4037ed3788af6 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4671: b121f7d42c260ae3a050c3f440d1c11f7cff7d1a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10388: adf251937d8e39b47ff0d8033214d84e0bb9d152 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == adf251937d8e drm/i915: Fixup kernel doc for param name changes == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10388/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fixup kernel doc for param name changes
Acked-by: Maarten Lankhorst
On 08.10.2018 14:13, Ville Syrjälä wrote:
On Mon, Oct 08, 2018 at 11:48:08AM +0100, Chris Wilson wrote:
s/crtc/crtc_state/ for the kernel doc as well as the params.
Fixes: 65c307fd08dd ("drm/i915: Make shared dpll functions take crtc_state,
v3.")
Signed-off-by: Chris Wilson
Cc: Maarten Lankhorst
Cc: Ville Syrjälä
Reviewed-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 10e820804eee..874646357ad1 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -126,7 +126,7 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
/**
* intel_prepare_shared_dpll - call a dpll's prepare hook
- * @crtc: CRTC which has a shared dpll
+ * @crtc_state: CRTC, and its state, which has a shared dpll
*
* This calls the PLL's prepare hook if it has one and if the PLL is not
* already enabled. The prepare hook is platform specific.
@@ -154,7 +154,7 @@ void intel_prepare_shared_dpll(const struct
intel_crtc_state *crtc_state)
/**
* intel_enable_shared_dpll - enable a CRTC's shared DPLL
- * @crtc: CRTC which has a shared DPLL
+ * @crtc_state: CRTC, and its state, which has a shared DPLL
*
* Enable the shared DPLL used by @crtc.
*/
@@ -199,7 +199,7 @@ void intel_enable_shared_dpll(const struct intel_crtc_state
*crtc_state)
/**
* intel_disable_shared_dpll - disable a CRTC's shared DPLL
- * @crtc: CRTC which has a shared DPLL
+ * @crtc_state: CRTC, and its state, which has a shared DPLL
*
* Disable the shared DPLL used by @crtc.
*/
--
2.19.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fixup kernel doc for param name changes
On Mon, Oct 08, 2018 at 11:48:08AM +0100, Chris Wilson wrote:
> s/crtc/crtc_state/ for the kernel doc as well as the params.
>
> Fixes: 65c307fd08dd ("drm/i915: Make shared dpll functions take crtc_state,
> v3.")
> Signed-off-by: Chris Wilson
> Cc: Maarten Lankhorst
> Cc: Ville Syrjälä
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 10e820804eee..874646357ad1 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -126,7 +126,7 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>
> /**
> * intel_prepare_shared_dpll - call a dpll's prepare hook
> - * @crtc: CRTC which has a shared dpll
> + * @crtc_state: CRTC, and its state, which has a shared dpll
> *
> * This calls the PLL's prepare hook if it has one and if the PLL is not
> * already enabled. The prepare hook is platform specific.
> @@ -154,7 +154,7 @@ void intel_prepare_shared_dpll(const struct
> intel_crtc_state *crtc_state)
>
> /**
> * intel_enable_shared_dpll - enable a CRTC's shared DPLL
> - * @crtc: CRTC which has a shared DPLL
> + * @crtc_state: CRTC, and its state, which has a shared DPLL
> *
> * Enable the shared DPLL used by @crtc.
> */
> @@ -199,7 +199,7 @@ void intel_enable_shared_dpll(const struct
> intel_crtc_state *crtc_state)
>
> /**
> * intel_disable_shared_dpll - disable a CRTC's shared DPLL
> - * @crtc: CRTC which has a shared DPLL
> + * @crtc_state: CRTC, and its state, which has a shared DPLL
> *
> * Disable the shared DPLL used by @crtc.
> */
> --
> 2.19.1
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fixup kernel doc for param name changes
s/crtc/crtc_state/ for the kernel doc as well as the params.
Fixes: 65c307fd08dd ("drm/i915: Make shared dpll functions take crtc_state,
v3.")
Signed-off-by: Chris Wilson
Cc: Maarten Lankhorst
Cc: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 10e820804eee..874646357ad1 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -126,7 +126,7 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
/**
* intel_prepare_shared_dpll - call a dpll's prepare hook
- * @crtc: CRTC which has a shared dpll
+ * @crtc_state: CRTC, and its state, which has a shared dpll
*
* This calls the PLL's prepare hook if it has one and if the PLL is not
* already enabled. The prepare hook is platform specific.
@@ -154,7 +154,7 @@ void intel_prepare_shared_dpll(const struct
intel_crtc_state *crtc_state)
/**
* intel_enable_shared_dpll - enable a CRTC's shared DPLL
- * @crtc: CRTC which has a shared DPLL
+ * @crtc_state: CRTC, and its state, which has a shared DPLL
*
* Enable the shared DPLL used by @crtc.
*/
@@ -199,7 +199,7 @@ void intel_enable_shared_dpll(const struct intel_crtc_state
*crtc_state)
/**
* intel_disable_shared_dpll - disable a CRTC's shared DPLL
- * @crtc: CRTC which has a shared DPLL
+ * @crtc_state: CRTC, and its state, which has a shared DPLL
*
* Disable the shared DPLL used by @crtc.
*/
--
2.19.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] /drm/i915/hdmi: SCDC Scrambling enable without CTS mode
On Fri, Oct 05, 2018 at 03:18:44PM -0700, clinton.a.tay...@intel.com wrote:
> From: Clint Taylor
>
> Setting the SCDC scrambling CTS mode causes HDMI Link Layer protocol tests
> HF1-12 and HF1-13 to fail. Added "Source Shall" entries from SCDC
> section before enabling scrambling.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107895
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107896
> Signed-off-by: Clint Taylor
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
> drivers/gpu/drm/i915/intel_hdmi.c | 8
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 9e82281..a1b877f 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1872,7 +1872,7 @@ void intel_ddi_enable_transcoder_func(const struct
> intel_crtc_state *crtc_state)
> temp |= TRANS_DDI_MODE_SELECT_DVI;
>
> if (crtc_state->hdmi_scrambling)
> - temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> + temp |= TRANS_DDI_HDMI_SCRAMBLING;
> if (crtc_state->hdmi_high_tmds_clock_ratio)
> temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
> } else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
> @@ -3394,8 +3394,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> if (intel_dig_port->infoframe_enabled(encoder, pipe_config))
> pipe_config->has_infoframe = true;
>
> - if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
> - TRANS_DDI_HDMI_SCRAMBLING_MASK)
> + if ((temp & TRANS_DDI_HDMI_SCRAMBLING) ==
> + TRANS_DDI_HDMI_SCRAMBLING)
It's a single bit so 'temp & TRANS_DDI_HDMI_SCRAMBLING' will do.
The spec isn't particularly clear about the CTS enable bit, but judging
from the name I guess you should only enable it when doing compliance
testing.
> pipe_config->hdmi_scrambling = true;
> if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> pipe_config->hdmi_high_tmds_clock_ratio = true;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 454f570..d181d67 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2148,6 +2148,14 @@ bool intel_hdmi_handle_sink_scrambling(struct
> intel_encoder *encoder,
> connector->base.id, connector->name,
> yesno(scrambling), high_tmds_clock_ratio ? 40 : 10);
>
> + /* SCDC source version 10.4.1.2 */
> + if (drm_scdc_writeb(adapter, SCDC_SOURCE_VERSION, 0x01) < 0)
> + DRM_DEBUG_KMS("Unable to set SCDC Source Version register\n");
These look unrelated to the scrambler fix, so should be a separate patch.
I don't think the spec section numbers are particularly helpful without
some indication as to which specification they refer to.
> +
> + /* Clear SCDC CONFIG_0 10.4.1.6 - RR_Enable Polling Only */
> + if (drm_scdc_writeb(adapter, SCDC_CONFIG_0, 0x00) < 0)
> + DRM_DEBUG_KMS("Unable to set SCDC CONFIG_0 register\n");
The spec is unfortunately vague about this stuff. It sort of implies
that polling is optional, but then it says that if either source or sink
doesn't set the RR bit then polling must be used, which to me seems
like polling is in fact mandatory.
The spec allows for a max polling interval of 250 ms. I don't particulary
cherish waking up every 250ms whenver a HDMI 2.0 sink is hooked up. I
guess maybe we could limit it to times when the link is actually active,
but it still feels very wasteful to poll for something that should
basically never happen.
This is rather like the eDP dpcd polling when hpd isn't support.
Except IIRC the eDP polling is actually opitonal and we haven't
bothered to implement it. But I'm not even sure whether there are
any machines w/o eDP hpd hooked up.
Anyway, back to the patch itself. It seems to me that we should
probably be configuring this stuff during detect rather than
during crtc enable.
> +
> /* Set TMDS bit clock ratio to 1/40 or 1/10, and enable/disable
> scrambling */
> return drm_scdc_set_high_tmds_clock_ratio(adapter,
> high_tmds_clock_ratio) &&
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] pci: Add a few new IDs for Intel GPU "spurious interrupt" quirk
From: Bin Meng
> Sent: 08 October 2018 10:44
...
> Correct, disable the shared interrupt line keeps all devices using
> that line from working, which is current kernel's behavior w/o this
> quirk handling: it disables the (shared) interrupt line after 100.000+
> generated interrupts. But the side effect is that other devices become
> unusable after that (eg: USB devices which share the same interrupt
> line with the Intel GPU). That's why the original commit, f67fd55fa96f
> ("PCI: Add quirk for still enabled interrupts on Intel Sandy Bridge
> GPUs") disables the GPU's interrupt directly, which should really be
> done by the VGA BIOS itself (a buggy VBIOS!).
Shouldn't the kernel just disable all PCI(e) interrupts by writing
1 to the config space control register bit during grope?
Can it ever by right for this to be set?
Apart from VGA the 'bus master' bit also needs to be clear.
ISTR some very early PCI systems which failed to reset the PCI
bus during reboot - at least the 'bus master' bit remained
set for an ethernet card.
On a private LAN the OS got reinstalled and rebooted without
using all the ethernet receive buffers and then died because
a receive frame got written into 'random' memory.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915: Remove i915.enable_ppgtt override
On 2018.09.26 11:02:15 +0300, Joonas Lahtinen wrote:
> Quoting He, Min (2018-09-26 04:06:25)
> > No. We changed to full 48bit ppgtt long time ago. :)
>
> So would that mean the change is OK to do?
Looks that one unfortunately caused gvt regression, gvt context has
no i915 managed i915_hw_ppgtt but only shadowed one, so need to check
if ppgtt is valid before access.
>
> Acked-by from you, Zhi, would be good to have for documentation purposes
> in that case :)
>
> Regards, Joonas
>
> >
> > > -Original Message-
> > > From: Wang, Zhi A
> > > Sent: Wednesday, September 26, 2018 2:01 AM
> > > To: Joonas Lahtinen ; Chris Wilson
> > > ; intel-gfx@lists.freedesktop.org; Zhenyu Wang
> > >
> > > Cc: Auld, Matthew ; He, Min
> > > Subject: Re: [PATCH v4] drm/i915: Remove i915.enable_ppgtt override
> > >
> > > Hi Min:
> > >
> > > I remembered the IOTG guest kernel is using aliasing-ppgtt in the last
> > > technical sharing (probably I didn't remember it correctly). Can you
> > > confirm?
> > >
> > > Also, thanks Joonas for the reminding. :)
> > >
> > > thanks,
> > > Zhi.
> > >
> > > On 09/25/18 09:22, Joonas Lahtinen wrote:
> > > > + Zhi and Zhenyu
> > > >
> > > > For me the patch looks ok.
> > > >
> > > > It refuses to load GVT on systems where 48-bit is not supported, for not
> > > > worsening the system security when virtualization is enabled (as anybody
> > > > would probably expect virtualization to improve that). Please see code.
> > > >
> > > > Do we have such systems in the wild? Should we instruct the user to
> > > > updating the hypervisor to specific kernel version when they hit the
> > > > error?
> > > >
> > > > Regards, Joonas
> > > >
> > > > Quoting Chris Wilson (2018-09-25 14:48:20)
> > > >> Now that we are confident in providing full-ppgtt where supported,
> > > >> remove the ability to override the context isolation.
> > > >>
> > > >> v2: Remove faked aliasing-ppgtt for testing as it no longer is
> > > >> accepted.
> > > >> v3: s/USES/HAS/ to match usage and reject attempts to load the module
> > > on
> > > >> old GVT-g setups that do not provide support for full-ppgtt.
> > > >>
> > > >> Signed-off-by: Chris Wilson
> > > >> Cc: Joonas Lahtinen
> > > >> Cc: Matthew Auld
> > > >> ---
> > > >> drivers/gpu/drm/i915/i915_drv.c | 22 +++--
> > > >> drivers/gpu/drm/i915/i915_drv.h | 14 +--
> > > >> drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
> > > >> drivers/gpu/drm/i915/i915_gem_gtt.c | 88
> > > >> ++-
> > > >> drivers/gpu/drm/i915/i915_gpu_error.c | 4 +-
> > > >> drivers/gpu/drm/i915/i915_params.c| 4 -
> > > >> drivers/gpu/drm/i915/i915_params.h| 1 -
> > > >> drivers/gpu/drm/i915/i915_pci.c | 17 ++--
> > > >> drivers/gpu/drm/i915/intel_device_info.c | 6 ++
> > > >> drivers/gpu/drm/i915/intel_device_info.h | 11 ++-
> > > >> drivers/gpu/drm/i915/intel_lrc.c | 13 ++-
> > > >> drivers/gpu/drm/i915/selftests/huge_pages.c | 12 +--
> > > >> .../gpu/drm/i915/selftests/i915_gem_context.c | 59 +
> > > >> .../gpu/drm/i915/selftests/i915_gem_evict.c | 2 +-
> > > >> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 +-
> > > >> 15 files changed, 62 insertions(+), 197 deletions(-)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > >> index 44e2c0f5ec50..3efbbc71c3b4 100644
> > > >> --- a/drivers/gpu/drm/i915/i915_drv.c
> > > >> +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > >> @@ -345,7 +345,7 @@ static int i915_getparam_ioctl(struct drm_device
> > > *dev, void *data,
> > > >> value = HAS_WT(dev_priv);
> > > >> break;
> > > >> case I915_PARAM_HAS_ALIASING_PPGTT:
> > > >> - value = USES_PPGTT(dev_priv);
> > > >> + value = INTEL_PPGTT(dev_priv);
> > > >> break;
> > > >> case I915_PARAM_HAS_SEMAPHORES:
> > > >> value = HAS_LEGACY_SEMAPHORES(dev_priv);
> > > >> @@ -1049,17 +1049,6 @@ static void i915_driver_cleanup_mmio(struct
> > > drm_i915_private *dev_priv)
> > > >>
> > > >> static void intel_sanitize_options(struct drm_i915_private *dev_priv)
> > > >> {
> > > >> - /*
> > > >> -* i915.enable_ppgtt is read-only, so do an early pass to
> > > >> validate the
> > > >> -* user's requested state against the hardware/driver
> > > >> capabilities.
> > > We
> > > >> -* do this now so that we can print out any log messages once
> > > >> rather
> > > >> -* than every time we check intel_enable_ppgtt().
> > > >> -*/
> > > >> - i915_modparams.enable_ppgtt =
> > > >> - intel_sanitize_enable_ppgtt(dev_priv,
> > > >> -
> > > >> i915_modparams.enable_ppgtt);
> > > >> - DRM_DEBUG_DRIVER("ppgtt mode: %i\n",
> > > i915_modparams.enable_ppgtt);
> >
Re: [Intel-gfx] [PATCH] drm/i915: Stop calling intel_opregion unregister/register in suspend/resume
On Fri, 05 Oct 2018, Chris Wilson wrote:
> If we reduce the suspend function for intel_opregion to do the minimum
> required, the resume function can also do the simple task of notifier
> the ACPI bios that we are back. This avoid some nasty restrictions on
> the likes of register_acpi_notifier() that are not allowed during the
> early phase of resume.
Something like this has been on the back of my mind for a long time, but
never got around to it. Couple of nitpicks/observations inline.
>
> Signed-off-by: Chris Wilson
> Cc: Imre Deak
> Cc: Jani Nikula
> ---
> drivers/gpu/drm/i915/i915_drv.c | 9 +-
> drivers/gpu/drm/i915/intel_opregion.c | 155 +++---
> drivers/gpu/drm/i915/intel_opregion.h | 15 +++
> 3 files changed, 108 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 193023427b40..9b0887746bac 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1919,9 +1919,7 @@ static int i915_drm_suspend(struct drm_device *dev)
> i915_save_state(dev_priv);
>
> opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
> - intel_opregion_notify_adapter(dev_priv, opregion_target_state);
> -
> - intel_opregion_unregister(dev_priv);
> + intel_opregion_suspend(dev_priv, opregion_target_state);
>
> intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
>
> @@ -2040,7 +2038,6 @@ static int i915_drm_resume(struct drm_device *dev)
>
> i915_restore_state(dev_priv);
> intel_pps_unlock_regs_wa(dev_priv);
> - intel_opregion_setup(dev_priv);
>
> intel_init_pch_refclk(dev_priv);
>
> @@ -2082,12 +2079,10 @@ static int i915_drm_resume(struct drm_device *dev)
>* */
> intel_hpd_init(dev_priv);
>
> - intel_opregion_register(dev_priv);
> + intel_opregion_resume(dev_priv);
>
> intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
>
> - intel_opregion_notify_adapter(dev_priv, PCI_D0);
> -
> intel_power_domains_enable(dev_priv);
>
> enable_rpm_wakeref_asserts(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> b/drivers/gpu/drm/i915/intel_opregion.c
> index e034b4166d32..379e8c64a248 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -773,70 +773,6 @@ static void intel_setup_cadls(struct drm_i915_private
> *dev_priv)
> opregion->acpi->cadl[i] = 0;
> }
>
> -void intel_opregion_register(struct drm_i915_private *dev_priv)
> -{
> - struct intel_opregion *opregion = _priv->opregion;
> -
> - if (!opregion->header)
> - return;
> -
> - if (opregion->acpi) {
> - intel_didl_outputs(dev_priv);
> - intel_setup_cadls(dev_priv);
> -
> - /* Notify BIOS we are ready to handle ACPI video ext notifs.
> - * Right now, all the events are handled by the ACPI video
> module.
> - * We don't actually need to do anything with them. */
> - opregion->acpi->csts = 0;
> - opregion->acpi->drdy = 1;
> -
> - opregion->acpi_notifier.notifier_call =
> intel_opregion_video_event;
> - register_acpi_notifier(>acpi_notifier);
> - }
> -
> - if (opregion->asle) {
> - opregion->asle->tche = ASLE_TCHE_BLC_EN;
> - opregion->asle->ardy = ASLE_ARDY_READY;
> - }
> -}
> -
> -void intel_opregion_unregister(struct drm_i915_private *dev_priv)
> -{
> - struct intel_opregion *opregion = _priv->opregion;
> -
> - if (!opregion->header)
> - return;
> -
> - if (opregion->asle)
> - opregion->asle->ardy = ASLE_ARDY_NOT_READY;
> -
> - cancel_work_sync(_priv->opregion.asle_work);
> -
> - if (opregion->acpi) {
> - opregion->acpi->drdy = 0;
> -
> - unregister_acpi_notifier(>acpi_notifier);
> - opregion->acpi_notifier.notifier_call = NULL;
> - }
> -
> - /* just clear all opregion memory pointers now */
> - memunmap(opregion->header);
> - if (opregion->rvda) {
> - memunmap(opregion->rvda);
> - opregion->rvda = NULL;
> - }
> - if (opregion->vbt_firmware) {
> - kfree(opregion->vbt_firmware);
> - opregion->vbt_firmware = NULL;
> - }
> - opregion->header = NULL;
> - opregion->acpi = NULL;
> - opregion->swsci = NULL;
> - opregion->asle = NULL;
> - opregion->vbt = NULL;
> - opregion->lid_state = NULL;
> -}
> -
> static void swsci_setup(struct drm_i915_private *dev_priv)
> {
> struct intel_opregion *opregion = _priv->opregion;
> @@ -1115,3 +1051,94 @@ intel_opregion_get_panel_type(struct drm_i915_private
> *dev_priv)
>
> return ret - 1;
> }
> +
> +void intel_opregion_register(struct drm_i915_private *i915)
> +{
> + struct intel_opregion *opregion = >opregion;
> +
> + if
