Re: [PATCH] drm/doc/rfc: SR-IOV support on the new Xe driver

[Vivi, Rodrigo]

On Tue, 2023-11-14 at 12:37 +, Tvrtko Ursulin wrote:
> 
> On 10/11/2023 18:22, Michal Wajdeczko wrote:
> > The Single Root I/O Virtualization (SR-IOV) extension to the PCI
> > Express (PCIe) specification suite is supported starting from 12th
> > generation of Intel Graphics processors.
> > 
> > This RFC aims to explain how do we want to add support for SR-IOV
> > to the new Xe driver and to propose related additions to the sysfs.
> > 
> > Signed-off-by: Michal Wajdeczko 
> > Cc: Oded Gabbay 
> > Cc: Rodrigo Vivi 
> > Cc: Joonas Lahtinen 
> > Cc: Tvrtko Ursulin 
> > Cc: Daniel Vetter 
> > ---
> >   Documentation/gpu/rfc/index.rst |   5 +
> >   Documentation/gpu/rfc/sysfs-driver-xe-sriov | 501
> > 
> >   Documentation/gpu/rfc/xe_sriov.rst  | 192 
> >   3 files changed, 698 insertions(+)
> >   create mode 100644 Documentation/gpu/rfc/sysfs-driver-xe-sriov
> >   create mode 100644 Documentation/gpu/rfc/xe_sriov.rst
> > 
> > diff --git a/Documentation/gpu/rfc/index.rst
> > b/Documentation/gpu/rfc/index.rst
> > index e4f7b005138d..fc5bc447f30d 100644
> > --- a/Documentation/gpu/rfc/index.rst
> > +++ b/Documentation/gpu/rfc/index.rst
> > @@ -35,3 +35,8 @@ host such documentation:
> >   .. toctree::
> >   
> >  xe.rst
> > +
> > +.. toctree::
> > +   :maxdepth: 1
> > +
> > +   xe_sriov.rst
> > diff --git a/Documentation/gpu/rfc/sysfs-driver-xe-sriov
> > b/Documentation/gpu/rfc/sysfs-driver-xe-sriov
> > new file mode 100644
> > index ..77748204dd83
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/sysfs-driver-xe-sriov
> > @@ -0,0 +1,501 @@
> > +.. Documentation/ABI/testing/sysfs-driver-xe-sriov
> > +..
> > +.. Intel Xe driver ABI (SR-IOV extensions)
> > +..
> > +    The Single Root I/O Virtualization (SR-IOV) extension to
> > +    the PCI Express (PCIe) specification suite is supported
> > +    starting from 12th generation of Intel Graphics processors.
> > +
> > +    This document describes Xe driver specific additions.
> > +
> > +    For description of generic SR-IOV sysfs attributes see
> > +    "Documentation/ABI/testing/sysfs-bus-pci" document.
> > +
> > +    /sys/bus/pci/drivers/xe/BDF/
> > +    ├── sriov_auto_provisioning
> > +    │   ├── admin_mode
> > +    │   ├── enabled
> > +    │   ├── reset_defaults
> > +    │   ├── resources
> > +    │   │   ├── default_contexts_quota
> > +    │   │   ├── default_doorbells_quota
> > +    │   │   ├── default_ggtt_quota
> > +    │   │   └── default_lmem_quota
> > +    │   ├── scheduling
> > +    │   │   ├── default_exec_quantum_ms
> > +    │   │   └── default_preempt_timeout_us
> > +    │   └── monitoring
> > +    │       ├── default_cat_error_count
> > +    │       ├── default_doorbell_time_us
> > +    │       ├── default_engine_reset_count
> > +    │       ├── default_h2g_time_us
> > +    │       ├── default_irq_time_us
> > +    │       └── default_page_fault_count
> 
>  From the department of bike-shedding, one alternative could be to
> have 
> a directory called defaults which avoids having to have the default_ 
> prefix on everything under it.

good idea. probably with a '.' prefix to make it hidden like we have
for other stuff already.

'.defaults'

> 
> > +
> > +    /sys/bus/pci/drivers/xe/BDF/
> > +    ├── sriov_extensions
> 
> Should this be xe_sriov_extensions or if not doesn't it need
> agreement 
> to reserve the keyword in Documentation/ABI/testing/sysfs-bus-pci? 
> Sriov_auto_provisioning too I guess.
> 
> > +    │   ├── monitoring_period_ms
> > +    │   ├── strict_scheduling_enabled
> > +    │   ├── pf
> > +    │   │   ├── device -> ../../../BDF
> > +    │   │   ├── priority
> > +    │   │   ├── tile0
> > +    │   │   │   ├── gt0
> > +    │   │   │   │   ├── exec_quantum_ms
> > +    │   │   │   │   ├── preempt_timeout_us
> > +    │   │   │   │   └── thresholds
> > +    │   │   │   │   ├── cat_error_count
> > +    │   │   │   │   ├── doorbell_time_us
> > +    │   │   │   │   ├── engine_reset_count
> > +    │   │   │   │   ├── h2g_time_us
> > +    │   │   │   │   ├── irq_time_us
> > +    │   │   │   │   └── page_fault_count
> > +    │   │   │   └── gtX
> > +    │   │   └── tileT
> > +    │   ├── vf1
> > +    │   │   ├── device -> ../../../BDF+1
> > +    │   │   ├── stop
> > +    │   │   ├── tile0
> > +    │   │   │   ├── ggtt_quota
> > +    │   │   │   ├── lmem_quota
> > +    │   │   │   ├── gt0
> > +    │   │   │   │   ├── contexts_quota
> > +    │   │   │   │   ├── doorbells_quota
> > +    │   │   │   │   ├── exec_quantum_ms
> > +    │   │   │   │   ├── preempt_timeout_us
> > +    │   │   │   │   └── thresholds
> > +    │   │   │   │   ├── cat_error_count
> > +    │   │   │   │   ├── doorbell_time_us
> > +    │   │   │   │   ├── engine_reset_count
> > +    │   │   │   │   ├── h2g_time_us
> > +    │   │   │   │   ├── irq_time_us
> > +    │   │   │   │   └── page_fault_count
> > +    │   │   │   └── gtX
> > +    │   │   └── tileT
> > +    │ 

Re: [PATCH] gpu: drm: i915: fix documentation style

[Vivi, Rodrigo]

On Mon, 2023-08-21 at 14:00 -0700, Ceraolo Spurio, Daniele wrote:
> 
> 
> On 8/21/2023 9:22 AM, Jani Nikula wrote:
> > On Mon, 21 Aug 2023, "Ricardo B. Marliere" 
> > wrote:
> > > This patch fixes the following sphinx warnings in the htmldocs
> > > make target:
> > > 
> > > Documentation/gpu/i915:546:
> > > ./drivers/gpu/drm/i915/gt/uc/intel_huc.c:29: ERROR: Unexpected
> > > indentation.
> > > Documentation/gpu/i915:546:
> > > ./drivers/gpu/drm/i915/gt/uc/intel_huc.c:30: WARNING: Block quote
> > > ends without a blank line; unexpected unindent.
> > > Documentation/gpu/i915:546:
> > > ./drivers/gpu/drm/i915/gt/uc/intel_huc.c:35: WARNING: Bullet list
> > > ends without a blank line; unexpected unindent.
> > > 
> > > Signed-off-by: Ricardo B. Marliere 
> > Already fixed by commit 175b036472f6 ("drm/i915: fix Sphinx
> > indentation
> > warning") in drm-next.
> 
> Should we send this commit through the -fixes path, so it gets
> included 
> in 6.5?

175b036472f6 cherry-picked to drm-intel-fixes. Should be in this
week's pull request towards 6.5

> 
> Daniele
> 
> > BR,
> > Jani.
> > 
> > > ---
> > >   drivers/gpu/drm/i915/gt/uc/intel_huc.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> > > b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> > > index ddd146265beb..fa70defcb5b2 100644
> > > --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> > > @@ -26,6 +26,7 @@
> > >    * The kernel driver is only responsible for loading the HuC
> > > firmware and
> > >    * triggering its security authentication. This is done
> > > differently depending
> > >    * on the platform:
> > > + *
> > >    * - older platforms (from Gen9 to most Gen12s): the load is
> > > performed via DMA
> > >    *   and the authentication via GuC
> > >    * - DG2: load and authentication are both performed via GSC.
> > > @@ -33,6 +34,7 @@
> > >    *   not-DG2 older platforms), while the authentication is done
> > > in 2-steps,
> > >    *   a first auth for clear-media workloads via GuC and a
> > > second one for all
> > >    *   workloads via GSC.
> > > + *
> > >    * On platforms where the GuC does the authentication, to
> > > correctly do so the
> > >    * HuC binary must be loaded before the GuC one.
> > >    * Loading the HuC is optional; however, not using the HuC
> > > might negatively
> 

Re: [PATCH v2] drm/i915/hwconfig: Remove comment block

[Vivi, Rodrigo]

On Tue, 2022-12-13 at 13:50 +0800, Jiapeng Chong wrote:
> No functional modification involved.
> 
> drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c:112: warning:
> expecting prototype for intel_guc_hwconfig_init(). Prototype was for
> guc_hwconfig_init() instead.

Thank you for the patch and for addressing the comment.
But now the commit message is not explaining what's really going on.

Could you please improve the message saying that the function has
changed to static and we don't need doc comment in the static
functions, etc?!

Also, while doing this, please find a better commit message.
This is way to generic. Something like
drm/i915: Remove unnecessary doc from static hwconfig_init
sounds better and easier to understand from a log --oneline.

Oh, and it also looks this patch deserves a "Fixes:" tag,
pointing to the patch that created the mess. Either the original
patch if it already introduced like this, or to the patch that
transformed this function in static. git blame to find the culprit out.

With the commit msg fixed as above I will add my rv-b while merging it.

> 
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=3414
> Reported-by: Abaci Robot 
> Signed-off-by: Jiapeng Chong 
> ---
> Changes in v2:
>   -Remove the comment block.
> 
>  drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> index 4781fccc2687..5559d39881ee 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
> @@ -102,12 +102,6 @@ static bool has_table(struct drm_i915_private
> *i915)
> return false;
>  }
>  
> -/**
> - * intel_guc_hwconfig_init - Initialize the HWConfig
> - *
> - * Retrieve the HWConfig table from the GuC and save it locally.
> - * It can then be queried on demand by other users later on.
> - */
>  static int guc_hwconfig_init(struct intel_gt *gt)
>  {
> struct intel_hwconfig *hwconfig = >info.hwconfig;

Re: [PATCH] drm/i915/gt: Reset twice

[Vivi, Rodrigo]

On Tue, 2022-12-13 at 00:08 +0100, Andi Shyti wrote:
> Hi Rodrigo,
> 
> On Mon, Dec 12, 2022 at 11:55:10AM -0500, Rodrigo Vivi wrote:
> > On Mon, Dec 12, 2022 at 05:13:38PM +0100, Andi Shyti wrote:
> > > From: Chris Wilson 
> > > 
> > > After applying an engine reset, on some platforms like
> > > Jasperlake, we
> > > occasionally detect that the engine state is not cleared until
> > > shortly
> > > after the resume. As we try to resume the engine with volatile
> > > internal
> > > state, the first request fails with a spurious CS event (it looks
> > > like
> > > it reports a lite-restore to the hung context, instead of the
> > > expected
> > > idle->active context switch).
> > > 
> > > Signed-off-by: Chris Wilson 
> > 
> > There's a typo in the signature email I'm afraid...
> 
> oh yes, I forgot the 'C' :)

you forgot?
who signed it off?

> 
> > Other than that, have we checked the possibility of using the
> > driver-initiated-flr bit
> > instead of this second loop? That should be the right way to
> > guarantee everything is
> > cleared on gen11+...
> 
> maybe I am misinterpreting it, but is FLR the same as resetting
> hardware domains individually?

No, it is bigger than that... almost the PCI FLR with some exceptions:

https://lists.freedesktop.org/archives/intel-gfx/2022-December/313956.html

> How am I supposed to use driver_initiated_flr() in this context?

Some drivers are not even using this gt reset anymore and going
directly with the driver-initiated FLR in case that guc reset failed.

I believe we can still keep the gt reset in our case as we currently
have, but instead of keep retrying it until it succeeds we probably
should go to the next level and do the driver initiated FLR when the GT
reset failed.

> 
> Thanks,
> Andi
> 
> > > Cc: sta...@vger.kernel.org
> > > Cc: Mika Kuoppala 
> > > Signed-off-by: Andi Shyti 
> > > ---
> > >  drivers/gpu/drm/i915/gt/intel_reset.c | 34
> > > ++-
> > >  1 file changed, 28 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c
> > > b/drivers/gpu/drm/i915/gt/intel_reset.c
> > > index ffde89c5835a4..88dfc0c5316ff 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> > > @@ -268,6 +268,7 @@ static int ilk_do_reset(struct intel_gt *gt,
> > > intel_engine_mask_t engine_mask,
> > >  static int gen6_hw_domain_reset(struct intel_gt *gt, u32
> > > hw_domain_mask)
> > >  {
> > > struct intel_uncore *uncore = gt->uncore;
> > > +   int loops = 2;
> > > int err;
> > >  
> > > /*
> > > @@ -275,18 +276,39 @@ static int gen6_hw_domain_reset(struct
> > > intel_gt *gt, u32 hw_domain_mask)
> > >  * for fifo space for the write or forcewake the chip for
> > >  * the read
> > >  */
> > > -   intel_uncore_write_fw(uncore, GEN6_GDRST,
> > > hw_domain_mask);
> > > +   do {
> > > +   intel_uncore_write_fw(uncore, GEN6_GDRST,
> > > hw_domain_mask);
> > >  
> > > -   /* Wait for the device to ack the reset requests */
> > > -   err = __intel_wait_for_register_fw(uncore,
> > > -  GEN6_GDRST,
> > > hw_domain_mask, 0,
> > > -  500, 0,
> > > -  NULL);
> > > +   /*
> > > +    * Wait for the device to ack the reset requests.
> > > +    *
> > > +    * On some platforms, e.g. Jasperlake, we see see
> > > that the
> > > +    * engine register state is not cleared until
> > > shortly after
> > > +    * GDRST reports completion, causing a failure as
> > > we try
> > > +    * to immediately resume while the internal state
> > > is still
> > > +    * in flux. If we immediately repeat the reset,
> > > the second
> > > +    * reset appears to serialise with the first, and
> > > since
> > > +    * it is a no-op, the registers should retain
> > > their reset
> > > +    * value. However, there is still a concern that
> > > upon
> > > +    * leaving the second reset, the internal engine
> > > state
> > > +    * is still in flux and not ready for resuming.
> > > +    */
> > > +   err = __intel_wait_for_register_fw(uncore,
> > > GEN6_GDRST,
> > > + 
> > > hw_domain_mask, 0,
> > > +  2000, 0,
> > > +  NULL);
> > > +   } while (err == 0 && --loops);
> > > if (err)
> > > GT_TRACE(gt,
> > >  "Wait for 0x%08x engines reset
> > > failed\n",
> > >  hw_domain_mask);
> > >  
> > > +   /*
> > > +    * As we have observed that the engine state is still
> > > volatile
> > > +    * after GDRST is 

Re: [Intel-gfx] [PATCH v7 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915

[Vivi, Rodrigo]

On Fri, 2022-12-02 at 19:21 +, Teres Alexis, Alan Previn wrote:
> 
> 
> On Fri, 2022-12-02 at 11:22 -0500, Vivi, Rodrigo wrote:
> > On Thu, Dec 01, 2022 at 05:14:07PM -0800, Alan Previn wrote:
> > > Starting with MTL, there will be two GT-tiles, a render and media
> > > tile. PXP as a service for supporting workloads with protected
> > > contexts and protected buffers can be subscribed by process
> > > workloads on any tile. However, depending on the platform,
> > > only one of the tiles is used for control events pertaining to
> > > PXP
> > > 
> Alan: [snip]
> 
> > > @@ -1168,6 +1176,8 @@ static int i915_drm_prepare(struct
> > > drm_device *dev)
> > >  {
> > > struct drm_i915_private *i915 = to_i915(dev);
> > >  
> > > +   intel_pxp_suspend_prepare(i915->pxp);
> > > +
> > > /*
> > >  * NB intel_display_suspend() may issue new requests
> > > after we've
> > >  * ostensibly marked the GPU as ready-to-sleep here. We
> > > need to
> > > @@ -1248,6 +1258,8 @@ static int i915_drm_suspend_late(struct
> > > drm_device *dev, bool hibernation)
> > >  
> > > disable_rpm_wakeref_asserts(rpm);
> > >  
> > > +   intel_pxp_suspend(dev_priv->pxp);
> > 
> > is this really needed here in the suspend_late?
> > why not in suspend()?
> > 
> > In general what is needed in suspend_late is resumed from the
> > resume_early,
> > what doesn't look the case here. So something looks off.
> > 
> 
> Actually this patch is NOT changing the code flow of when these pxp
> pm functions are getting called from an i915-level
> perspective - i am merely moving it from inside gt level to top level
> up:
> 
> Original --> i915_drm_suspend_late calls i915_gem_suspend_late calls
> intel_gt_suspend_late calls intel_pxp_suspend
> Patch --> i915_drm_suspend_late calls intel_pxp_suspend (before
> calling i915_gem_suspend_late)
> 
> Putting that aside, i think the original code was designed to have
> the suspend-prepare do nearly everything except
> disable the KCR registers which is postponed in order to handle a
> failed system-suspend-prepare (after pxp's suspend-
> prepare). A failed system-suspend-prepare (after pxp's suspend-
> prepare) would be recoverable with no-op from pxp's
> perspective as the UMD would be forced to recreate the pxp context
> that recreates arb session again and because the KCR
> registers hadnt been disabled, nothing more is required. I'm not 100%
> sure so I'll ping Daniele jump in to correct me. 
> 
> That said, the better way, for code readibility, would be completely
> skip having an intel_pxp_suspend and just disable
> the KCR in intel_pxp_suspend_prepare and instead add an i915 callback
> for resume_complete (which is the symmetrical
> opposite of suspend_prepare and surprisingly non-existend in i915
> codebase) in order to re-start kcr registers there for
> the case of a failed-system-suspend-prepare or completion of resume.
> I have a separate series that is attempting to fix
> some of this lack of symmetry
> here: https://patchwork.freedesktop.org/patch/513279/?series=111409
> ev=1 but i hadn't
> removed the intel_pxp_suspend because i am not sure if the i915-
> resume-complete callback would still be called if i915
> itself was the reason for the failed suspend-prepare AND the pxp-
> suspend-prepare had occured. So i need to craft out a
> way to test that.
> 
> Do you want to continue pursuing the final fixups for pxp's suspend-
> resume flows in this patch? Again, i am NOT changing
> this flow - just moving it from inside-gt to outside gem-gt where for
> suspend we move it outside-and-before and for
> resume we move it outside-and-after.

Oh! okay, let's move without changing this flow in this patch and work
in a follow up.

> 
> > > 
> > > 
> 
> Alan: [snip]
> 
> > > +
> > > i915_gem_suspend_late(dev_priv);
> > >  
> > > for_each_gt(gt, dev_priv, i)
> > > +int intel_pxp_init(struct drm_i915_private *i915)
> > > +{
> > > +   struct intel_pxp *newpxp;
> > > +
> > > +   newpxp = kzalloc(sizeof(*newpxp), GFP_KERNEL);
> > > +   if (!newpxp)
> > > +   return -ENOMEM;
> > > +
> > > +   i915->pxp = newpxp;
> > 
> > i915->pxp is already an intel_pxp *, why can't we simply
> > do 
> > i915->pxp = kzalloc(sizeof(...
> >   if (!i915->pxp)
> >   return -ENOMEM;
> > ?
> > 
> 

Re: [Intel-gfx] [PATCH v6 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915

[Vivi, Rodrigo]

On Wed, 2022-11-30 at 09:32 -0800, Matt Roper wrote:
> On Tue, Nov 29, 2022 at 06:02:45PM -0800, Alan Previn wrote:
> > Starting with MTL, there will be two GT-tiles, a render and media
> > tile. PXP as a service for supporting workloads with protected
> 
> Drive-by comment:  we've been a bit inconsistent about terminology in
> the past, but my understanding is that we're trying to standardize on
> "GT" for the unit that MTL has two of, and keeping the term "tile"
> for
> the PVC-style unit that is a combination of (GT+lmem).

I agree that this gets really confusing... but it will be hard to
standardize this I'm afraid. Specially when we do the PVC-style-tile a
intel_gt struct and we apparently are doing the same on MTL, no?!

So, unless the topology gets organized in the code with a standard
name, it is hard to demand everyone to use the same one.

Besides that whenever we were discussing the pvc's one we all had
agreed that the term "tile" was bad, hence we focused on keep the
intel_gt ready for that.

Whenever I hear tile I think of the display buffer organization...
and there are other "tiles" examples iirc.

> 
> 
> Matt
> 
> > contexts and protected buffers can be subscribed by process
> > workloads on any tile. However, depending on the platform,
> > only one of the tiles is used for control events pertaining to PXP
> > operation (such as creating the arbitration session and session
> > tear-down).
> > 
> > PXP as a global feature is accessible via batch buffer instructions
> > on any engine/tile and the coherency across tiles is handled
> > implicitly
> > by the HW. In fact, for the foreseeable future, we are expecting
> > this
> > single-control-tile for the PXP subsystem.
> > 
> > In MTL, it's the standalone media tile (not the root tile) because
> > it contains the VDBOX and KCR engine (among the assets PXP relies
> > on
> > for those events).
> > 
> > Looking at the current code design, each tile is represented by the
> > intel_gt structure while the intel_pxp structure currently hangs
> > off the
> > intel_gt structure.
> > 
> > Keeping the intel_pxp structure within the intel_gt structure makes
> > some
> > internal functionalities more straight forward but adds code
> > complexity to
> > code readibility and maintainibility to many external-to-pxp
> > subsystems
> > which may need to pick the correct intel_gt structure. An example
> > of this
> > would be the intel_pxp_is_active or intel_pxp_is_enabled
> > functionality
> > which should be viewed as a global level inquiry, not a per-gt
> > inquiry.
> > 
> > That said, this series promotes the intel_pxp structure into the
> > drm_i915_private structure making it a top-level subsystem and the
> > PXP
> > subsystem will select the control gt internally and keep a pointer
> > to
> > it for internal reference.
> > 
> > Changes from prior revs:
> >    v5: - Switch from series to single patch (Rodrigo).
> >    - change function name from pxp_get_kcr_owner_gt to
> >  pxp_get_ctrl_gt.
> >    - Fix CI BAT failure by removing redundant call to
> > intel_pxp_fini
> >  from driver-remove.
> >    - NOTE: remaining open still persists on using ptr-to-ptr
> >  and back-ptr.
> >    v4: - Instead of maintaining intel_pxp as an intel_gt structure
> > member
> >  and creating a number of convoluted helpers that takes in
> > i915 as
> >  input and redirects to the correct intel_gt or takes any
> > intel_gt
> >  and internally replaces with the correct intel_gt, promote
> > it to
> >  be a top-level i915 structure.
> >    v3: - Rename gt level helper functions to "intel_pxp_is_enabled/
> >  supported/ active_on_gt" (Daniele)
> >    - Upgrade _gt_supports_pxp to replace what was
> > intel_gtpxp_is
> >  supported as the new intel_pxp_is_supported_on_gt to check
> > for
> >  PXP feature support vs the tee support for huc
> > authentication.
> >  Fix pxp-debugfs-registration to use only the former to
> > decide
> >  support. (Daniele)
> >    - Couple minor optimizations.
> >    v2: - Avoid introduction of new device info or gt variables and
> > use
> >  existing checks / macros to differentiate the correct GT-
> > >PXP
> >  control ownership (Daniele Ceraolo Spurio)
> >    - Don't reuse the updated global-checkers for per-GT callers
> > (such
> >  as other files within PXP) to avoid unnecessary GT-
> > reparsing,
> >  expose a replacement helper like the prior ones.
> > (Daniele).
> >    v1: - Add one more patch to the series for the intel_pxp
> > suspend/resume
> >  for similar refactoring
> > 
> > Signed-off-by: Alan Previn 
> > ---
> >  .../drm/i915/display/skl_universal_plane.c    |  2 +-
> >  drivers/gpu/drm/i915/gem/i915_gem_context.c   |  6 +-
> >  drivers/gpu/drm/i915/gem/i915_gem_create.c    |  2 +-
> >  .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  2 +-
> >  drivers/gpu/drm/i915/gt/intel_gt.c 

Re: [PATCH v3] drm/i915/mtl: Enable Idle Messaging for GSC CS

[Vivi, Rodrigo]

On Sat, 2022-11-19 at 09:02 +0530, Nilawar, Badal wrote:
> 
> 
> On 19-11-2022 00:07, Vivi, Rodrigo wrote:
> > On Sat, 2022-11-19 at 00:03 +0530, Badal Nilawar wrote:
> > > From: Vinay Belgaumkar 
> > > 
> > > By defaut idle messaging is disabled for GSC CS so to unblock RC6
> > > entry on media tile idle messaging need to be enabled.
> > > 
> > > v2:
> > >   - Fix review comments (Vinay)
> > >   - Set GSC idle hysteresis as per spec (Badal)
> > > v3:
> > >   - Fix review comments (Rodrigo)
> > > 
> > > Bspec: 71496
> > > 
> > > Cc: Daniele Ceraolo Spurio 
> > > Signed-off-by: Vinay Belgaumkar 
> > > Signed-off-by: Badal Nilawar 
> > > Reviewed-by: Vinay Belgaumkar 
> > 
> > He is the author of the patch, no?!
> > or you can remove this or change the author to be you and keep his
> > reviewed-by...
> > 
> > or I can just remove his rv-b while merging.. just let me know..
> As he is original author I will prefer not to change it. You can
> remove 
> his rv-b while merging.

Thanks and pushed.

> 
> Regards,
> Badal
> > 
> > > Reviewed-by: Rodrigo Vivi 
> > > ---
> > >   drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18
> > > ++
> > >   drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 
> > >   2 files changed, 22 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > index b0a4a2dbe3ee..e971b153fda9 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > > @@ -15,6 +15,22 @@
> > >   #include "intel_rc6.h"
> > >   #include "intel_ring.h"
> > >   #include "shmem_utils.h"
> > > +#include "intel_gt_regs.h"
> > > +
> > > +static void intel_gsc_idle_msg_enable(struct intel_engine_cs
> > > *engine)
> > > +{
> > > +   struct drm_i915_private *i915 = engine->i915;
> > > +
> > > +   if (IS_METEORLAKE(i915) && engine->id == GSC0) {
> > > +   intel_uncore_write(engine->gt->uncore,
> > > +  RC_PSMI_CTRL_GSCCS,
> > > +
> > > _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));
> > > +   /* hysteresis 0xA=5us as recommended in spec*/
> > > +   intel_uncore_write(engine->gt->uncore,
> > > +  PWRCTX_MAXCNT_GSCCS,
> > > +  0xA);
> > > +   }
> > > +}
> > >   
> > >   static void dbg_poison_ce(struct intel_context *ce)
> > >   {
> > > @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct
> > > intel_engine_cs
> > > *engine)
> > >   
> > >  intel_wakeref_init(>wakeref, rpm, _ops);
> > >  intel_engine_init_heartbeat(engine);
> > > +
> > > +   intel_gsc_idle_msg_enable(engine);
> > >   }
> > >   
> > >   /**
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > index c3cd92691795..80a979e6f6be 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> > > @@ -917,6 +917,10 @@
> > >   #define  MSG_IDLE_FW_MASK  REG_GENMASK(13, 9)
> > >   #define  MSG_IDLE_FW_SHIFT 9
> > >   
> > > +#defineRC_PSMI_CTRL_GSCCS  _MMIO(0x11a050)
> > > +#define  IDLE_MSG_DISABLE  REG_BIT(0)
> > > +#definePWRCTX_MAXCNT_GSCCS _MMIO(0x11a054)
> > > +
> > >   #define FORCEWAKE_MEDIA_GEN9   _MMIO(0xa270)
> > >   #define FORCEWAKE_RENDER_GEN9  _MMIO(0xa278)
> > >   
> > 

Re: [PATCH v3] drm/i915/mtl: Enable Idle Messaging for GSC CS

[Vivi, Rodrigo]

On Fri, 2022-11-18 at 13:37 -0800, Dixit, Ashutosh wrote:
> On Fri, 18 Nov 2022 10:37:37 -0800, Vivi, Rodrigo wrote:
> > 
> > On Sat, 2022-11-19 at 00:03 +0530, Badal Nilawar wrote:
> > > From: Vinay Belgaumkar 
> > > 
> > > By defaut idle messaging is disabled for GSC CS so to unblock RC6
> > > entry on media tile idle messaging need to be enabled.
> > > 
> > > v2:
> > >  - Fix review comments (Vinay)
> > >  - Set GSC idle hysteresis as per spec (Badal)
> > > v3:
> > >  - Fix review comments (Rodrigo)
> > > 
> > > Bspec: 71496
> > > 
> > > Cc: Daniele Ceraolo Spurio 
> > > Signed-off-by: Vinay Belgaumkar 
> > > Signed-off-by: Badal Nilawar 
> > > Reviewed-by: Vinay Belgaumkar 
> > 
> > He is the author of the patch, no?!
> > or you can remove this or change the author to be you and keep his
> > reviewed-by...
> > 
> > or I can just remove his rv-b while merging.. just let me know..
> 
> Not sure if that is the case here,

yeap, this is too small patch for that to happen.

>  but when multiple people contribute to a
> patch, the original author can review changes by others and add his
> Reviewed-by, no? Or are we saying it is redundant for the author to
> add his
> R-b?
> 
> Similarly, are S-o-b and R-b by the same person ok? I add changes to
> someone's patch so add my S-o-b but also review other's changes so
> add my
> R-b? Sometimes finding a 3rd person to add a R-b is hard. But two
> poeple
> can contribute to a patch and review each other's changes so add both
> their
> S-o-b's and R-b's or no?

Definitely a case by case thing. If that happens it is good to have it
clearly written in the commit message to know who did what, who
reviewed what.

Although I'd say that I'd still prefer the co-authored-by and having a
third one doing the full review.

> 
> :)
> 
> Ashutosh
> 

Re: [PATCH v3] drm/i915/mtl: Enable Idle Messaging for GSC CS

[Vivi, Rodrigo]

On Sat, 2022-11-19 at 00:03 +0530, Badal Nilawar wrote:
> From: Vinay Belgaumkar 
> 
> By defaut idle messaging is disabled for GSC CS so to unblock RC6
> entry on media tile idle messaging need to be enabled.
> 
> v2:
>  - Fix review comments (Vinay)
>  - Set GSC idle hysteresis as per spec (Badal)
> v3:
>  - Fix review comments (Rodrigo)
> 
> Bspec: 71496
> 
> Cc: Daniele Ceraolo Spurio 
> Signed-off-by: Vinay Belgaumkar 
> Signed-off-by: Badal Nilawar 
> Reviewed-by: Vinay Belgaumkar 

He is the author of the patch, no?!
or you can remove this or change the author to be you and keep his
reviewed-by...

or I can just remove his rv-b while merging.. just let me know..

> Reviewed-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_pm.c | 18 ++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h   |  4 
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index b0a4a2dbe3ee..e971b153fda9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -15,6 +15,22 @@
>  #include "intel_rc6.h"
>  #include "intel_ring.h"
>  #include "shmem_utils.h"
> +#include "intel_gt_regs.h"
> +
> +static void intel_gsc_idle_msg_enable(struct intel_engine_cs
> *engine)
> +{
> +   struct drm_i915_private *i915 = engine->i915;
> +
> +   if (IS_METEORLAKE(i915) && engine->id == GSC0) {
> +   intel_uncore_write(engine->gt->uncore,
> +  RC_PSMI_CTRL_GSCCS,
> + 
> _MASKED_BIT_DISABLE(IDLE_MSG_DISABLE));
> +   /* hysteresis 0xA=5us as recommended in spec*/
> +   intel_uncore_write(engine->gt->uncore,
> +  PWRCTX_MAXCNT_GSCCS,
> +  0xA);
> +   }
> +}
>  
>  static void dbg_poison_ce(struct intel_context *ce)
>  {
> @@ -275,6 +291,8 @@ void intel_engine_init__pm(struct intel_engine_cs
> *engine)
>  
> intel_wakeref_init(>wakeref, rpm, _ops);
> intel_engine_init_heartbeat(engine);
> +
> +   intel_gsc_idle_msg_enable(engine);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index c3cd92691795..80a979e6f6be 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -917,6 +917,10 @@
>  #define  MSG_IDLE_FW_MASK  REG_GENMASK(13, 9)
>  #define  MSG_IDLE_FW_SHIFT 9
>  
> +#defineRC_PSMI_CTRL_GSCCS  _MMIO(0x11a050)
> +#define  IDLE_MSG_DISABLE  REG_BIT(0)
> +#definePWRCTX_MAXCNT_GSCCS _MMIO(0x11a054)
> +
>  #define FORCEWAKE_MEDIA_GEN9   _MMIO(0xa270)
>  #define FORCEWAKE_RENDER_GEN9  _MMIO(0xa278)
>  

Re: [Intel-gfx] [PULL] drm-intel-next

[Vivi, Rodrigo]

On Sat, 2022-10-29 at 02:41 +0300, Ville Syrjälä wrote:
> On Fri, Oct 28, 2022 at 02:22:33PM -0400, Rodrigo Vivi wrote:
> > Hi Dave and Daniel,
> > 
> > Here goes the first chunk of drm-intel-next targeting 6.2
> > 
> > The highlight goes to Ville with many display related clean-up
> > and improvement, some other MTL enabling work and many other
> > fixes and small clean-ups.
> > 
> > drm-intel-next-2022-10-28:
> ...
> > - ELD precompute and readout (Ville)
> 
> A slight clarification seems to be in order. The ELD
> precompute+readout is in fact not in yet. This was just a pile
> of cleanups and minor fixes. The real ELD stuff will come later,
> once we figure out how we actually want to do it.

Sorry for the confusion. I have just used the subject of your series
as summary: 
[Intel-gfx] [PATCH 00/22] drm/i915: ELD precompute and readout

Should I change that to ELD precompute and readout

> 

Re: [PULL] drm-intel-fixes

[Vivi, Rodrigo]

On Sun, 2022-09-11 at 19:22 +0200, Jason A. Donenfeld wrote:
> Hi Rodrigo,
> 
> On Thu, Sep 08, 2022 at 09:59:54AM -0400, Rodrigo Vivi wrote:
> > Hi Dave and Daniel,
> > 
> > A few fixes, but most targeting stable.
> > 
> > [...]
> > 
> > Ville Syrjälä (2):
> >   drm/i915: Implement WaEdpLinkRateDataReload
> 
> Don't you need to revert d5929835080a60f9119d024fa42f315913942f76 in
> order for "drm/i915: Implement WaEdpLinkRateDataReload" to actually
> be
> useful/interesting? Otherwise, what's the point?

Unfortunately there was no clear indication on the revert patch for
the tool to pick, and I hadn't followed that front myself closely.

Should 
483e3d87a37e ("Revert "drm/i915/display: Re-add check for low voltage
sku for max dp source rate"")
have a Fixes tag?

Or should dim consider all reverts as Fixes?

Anyway, I will cherry pick this to our fixes branch for propagation
this week.

Thanks for the heads up,
Rodrigo.

> 
> Regards,
> Jason

Re: [PATCH i-g-t] i915/guc: Disable i915_pm_rps when SLPC is enabled

[Vivi, Rodrigo]

On Thu, 2022-08-18 at 13:42 -0700, Vinay Belgaumkar wrote:
> These tests were specifically designed for host Turbo. Skip
> them when SLPC is enabled as they fail frequently. We will look
> to keep adding to SLPC test coverage with these scenarios.
> 
> Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/3963
> Bug: https://gitlab.freedesktop.org/drm/intel/issues/4016
> Bug: https://gitlab.freedesktop.org/drm/intel/issues/5468
> Bug: https://gitlab.freedesktop.org/drm/intel/issues/5831
> 
> Cc: Rodrigo Vivi 
> Signed-off-by: Vinay Belgaumkar 

Ideally we should add other api tests and cases to validate the
expected slpc flow.
But this could be done in a follow-up work since right now it is
important to merge
the fix for the performance regression without the ignore_efficient
freq.

So,
Reviewed-by: Rodrigo Vivi 

> ---
>  lib/igt_pm.c | 15 +++
>  lib/igt_pm.h |  1 +
>  tests/i915/i915_pm_rps.c | 29 -
>  3 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index 6ebbad33..79bd6e2a 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -1202,3 +1202,18 @@ void
> igt_pm_print_pci_card_runtime_status(void)
> igt_pm_print_pci_dev_runtime_status(__pci_dev_pwrattr
> [i].pci_dev);
> }
>  }
> +
> +bool i915_is_slpc_enabled(int fd)
> +{
> +   int debugfs_fd = igt_debugfs_dir(fd);
> +   char buf[4096];
> +   int len;
> +
> +   igt_require(debugfs_fd != -1);
> +
> +   len = igt_debugfs_simple_read(debugfs_fd,
> "gt/uc/guc_slpc_info", buf, sizeof(buf));
> +   if (len < 0)
> +   return false;
> +   else
> +   return strstr(buf, "SLPC state: running");
> +}
> diff --git a/lib/igt_pm.h b/lib/igt_pm.h
> index f28b6ebf..cbbde12b 100644
> --- a/lib/igt_pm.h
> +++ b/lib/igt_pm.h
> @@ -79,5 +79,6 @@ void igt_pm_enable_pci_card_runtime_pm(struct
> pci_device *root,
>  void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev);
>  void igt_pm_restore_pci_card_runtime_pm(void);
>  void igt_pm_print_pci_card_runtime_status(void);
> +bool i915_is_slpc_enabled(int fd);
>  
>  #endif /* IGT_PM_H */
> diff --git a/tests/i915/i915_pm_rps.c b/tests/i915/i915_pm_rps.c
> index d06ade27..cce07009 100644
> --- a/tests/i915/i915_pm_rps.c
> +++ b/tests/i915/i915_pm_rps.c
> @@ -914,35 +914,54 @@ igt_main
> igt_install_exit_handler(pm_rps_exit_handler);
> }
>  
> -   igt_subtest("basic-api")
> +   igt_subtest("basic-api") {
> +   igt_skip_on_f(i915_is_slpc_enabled(drm_fd),
> + "This subtest is not supported when
> SLPC is enabled");
> min_max_config(basic_check, false);
> +   }
>  
> /* Verify the constraints, check if we can reach idle */
> -   igt_subtest("min-max-config-idle")
> +   igt_subtest("min-max-config-idle") {
> +   igt_skip_on_f(i915_is_slpc_enabled(drm_fd),
> + "This subtest is not supported when
> SLPC is enabled");
> min_max_config(idle_check, true);
> +   }
>  
> /* Verify the constraints with high load, check if we can
> reach max */
> igt_subtest("min-max-config-loaded") {
> +   igt_skip_on_f(i915_is_slpc_enabled(drm_fd),
> + "This subtest is not supported when
> SLPC is enabled");
> load_helper_run(HIGH);
> min_max_config(loaded_check, false);
> load_helper_stop();
> }
>  
> /* Checks if we achieve boost using gem_wait */
> -   igt_subtest("waitboost")
> +   igt_subtest("waitboost") {
> +   igt_skip_on_f(i915_is_slpc_enabled(drm_fd),
> + "This subtest is not supported when
> SLPC is enabled");
> waitboost(drm_fd, false);
> +   }
>  
> igt_describe("Check if the order of fences does not affect
> waitboosting");
> -   igt_subtest("fence-order")
> +   igt_subtest("fence-order") {
> +   igt_skip_on_f(i915_is_slpc_enabled(drm_fd),
> + "This subtest is not supported when
> SLPC is enabled");
> fence_order(drm_fd);
> +   }
>  
> igt_describe("Check if context reuse does not affect
> waitboosting");
> -   igt_subtest("engine-order")
> +   igt_subtest("engine-order") {
> +   igt_skip_on_f(i915_is_slpc_enabled(drm_fd),
> + "This subtest is not supported when
> SLPC is enabled");
> engine_order(drm_fd);
> +   }
>  
> /* Test boost frequency after GPU reset */
> igt_subtest("reset") {
> igt_hang_t hang = igt_allow_hang(drm_fd, 0, 0);
> +   igt_skip_on_f(i915_is_slpc_enabled(drm_fd),
> + "This subtest is not supported when
> SLPC is enabled");
> waitboost(drm_fd, 

Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: pass a pointer for tlb seqno at vma_invalidate_tlb()

[Vivi, Rodrigo]

On Tue, 2022-08-09 at 01:09 +0200, Andi Shyti wrote:
> Hi Rodrigo,
> 
> On Mon, Aug 08, 2022 at 03:04:13PM -0400, Rodrigo Vivi wrote:
> > On Mon, Aug 08, 2022 at 06:37:58PM +0200, Andi Shyti wrote:
> > > Hi Mauro,
> > > 
> > > On Thu, Aug 04, 2022 at 09:37:22AM +0200, Mauro Carvalho Chehab
> > > wrote:
> > > > WRITE_ONCE() should happen at the original var, not on a local
> > > > copy of it.
> > > > 
> > > > Fixes: 5d36acb7198b ("drm/i915/gt: Batch TLB invalidations")
> > > > Signed-off-by: Mauro Carvalho Chehab 
> > > 
> > > Reviewed-by: Andi Shyti 
> > 
> > Thanks and pushed...
> 
> Thanks!
> 
> > > 
> > > Are you going to send it to the stable mailing list?
> > 
> > I added cc stable while pushing and the cherry-pick to drm-intel-
> > next-fixes
> > has the right sha, so I'd assume this would be automagically now.
> > But yeap, it would be good if Mauro can follow up whenever this
> > gets
> > to Linus tree and Greg's script start to pop up the heads-up
> > messages.
> 
> That's what I meant... does Mauro now need to send the e-mail
> again for the stable?
> 
> I thought there was some suspicion towards e-mails pushed without
> being first sent to both stable and upstream mailing lists
> because they can get lost or forgotten... maybe I'm wrong.

It doesn't help to send now to stable ml because it can only be merged
there after it reaches the Linus' master tree.
Right now with the right fixes and cc:stable it should be automatically
and he shouldn't worry.
But in case he notices that the first patch got in but the second
didn't then it is when we send the patch directly to the stable ml.


> 
> Andi
> 
> > Thanks,
> > Rodrigo.
> > 
> > > 
> > > Andi

Re: [PULL] drm-intel-gt-next

[Vivi, Rodrigo]

On Wed, 2022-07-13 at 17:31 -0400, Rodrigo Vivi wrote:
> Hi Dave and Daniel,
> 
> On behalf of Tvrtko, who is recovering from Covid,
> here goes the latest drm-intel-gt-next pull request
> targeting 5.20.

Hi Folks,

any particular issue with this pull request?

We just realized it is not yet part of the drm-next.


Thanks,
Rodrigo.

> 
> Thanks,
> Rodrigo.
> 
> Driver uAPI changes:
> - All related to the Small BAR support: (and all by Matt Auld)
>  * add probed_cpu_visible_size
>  * expose the avail memory region tracking
>  * apply ALLOC_GPU only by default
>  * add NEEDS_CPU_ACCESS hint
>  * tweak error capture on recoverable contexts
> 
> Driver highlights:
> - Add Small BAR support (Matt)
> - Add MeteorLake support (RK)
> - Add support for LMEM PCIe resizable BAR (Akeem)
> 
> Driver important fixes:
> - ttm related fixes (Matt Auld)
> - Fix a performance regression related to waitboost (Chris)
> - Fix GT resets (Chris)
> 
> Driver others:
> - Adding GuC SLPC selftest (Vinay)
> - Fix ADL-N GuC load (Daniele)
> - Add platform workaround (Gustavo, Matt Roper)
> - DG2 and ATS-M device ID updates (Matt Roper)
> - Add VM_BIND doc rfc with uAPI documentation (Niranjana)
> - Fix user-after-free in vma destruction (Thomas)
> - Async flush of GuC log regions (Alan)
> - Fixes in selftests (Chris, Dan, Andrzej)
> - Convert to drm_dbg (Umesh)
> - Disable OA sseu config param for newer hardware (Umesh)
> - Multi-cast register steering changes (Matt Roper)
> - Add lmem_bar_size modparam (Priyanka)
> 
> Thanks,
> Rodrigo.
> 
> The following changes since commit
> a06968563775181690125091f470a8655742dcbf:
> 
>   drm/i915: Fix a lockdep warning at error capture (2022-06-29
> 14:52:50 +0530)
> 
> are available in the Git repository at:
> 
>   git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-gt-next-
> 2022-07-13
> 
> for you to fetch changes up to
> 17cd10a44a8962860ff4ba351b2a290e752dbbde:
> 
>   drm/i915: Add lmem_bar_size modparam (2022-07-13 17:47:51 +0100)
> 
> 
> Driver uAPI changes:
> - All related to the Small BAR support: (and all by Matt Auld)
>  * add probed_cpu_visible_size
>  * expose the avail memory region tracking
>  * apply ALLOC_GPU only by default
>  * add NEEDS_CPU_ACCESS hint
>  * tweak error capture on recoverable contexts
> 
> Driver highlights:
> - Add Small BAR support (Matt)
> - Add MeteorLake support (RK)
> - Add support for LMEM PCIe resizable BAR (Akeem)
> 
> Driver important fixes:
> - ttm related fixes (Matt Auld)
> - Fix a performance regression related to waitboost (Chris)
> - Fix GT resets (Chris)
> 
> Driver others:
> - Adding GuC SLPC selftest (Vinay)
> - Fix ADL-N GuC load (Daniele)
> - Add platform workaround (Gustavo, Matt Roper)
> - DG2 and ATS-M device ID updates (Matt Roper)
> - Add VM_BIND doc rfc with uAPI documentation (Niranjana)
> - Fix user-after-free in vma destruction (Thomas)
> - Async flush of GuC log regions (Alan)
> - Fixes in selftests (Chris, Dan, Andrzej)
> - Convert to drm_dbg (Umesh)
> - Disable OA sseu config param for newer hardware (Umesh)
> - Multi-cast register steering changes (Matt Roper)
> - Add lmem_bar_size modparam (Priyanka)
> 
> 
> Akeem G Abodunrin (1):
>   drm/i915: Add support for LMEM PCIe resizable bar
> 
> Alan Previn (1):
>   drm/i915/guc: Asynchronous flush of GuC log regions
> 
> Andrzej Hajda (1):
>   drm/i915/selftests: fix subtraction overflow bug
> 
> Chris Wilson (6):
>   drm/i915/selftests: Grab the runtime pm in shrink_thp
>   drm/i915/gt: Serialize GRDOM access between multiple engine
> resets
>   drm/i915/gt: Serialize TLB invalidates with GT resets
>   drm/i915/gem: Look for waitboosting across the whole object
> prior to individual waits
>   drm/i915: Bump GT idling delay to 2 jiffies
>   drm/i915/gt: Only kick the signal worker if there's been an
> update
> 
> Dan Carpenter (1):
>   drm/i915/selftests: fix a couple IS_ERR() vs NULL tests
> 
> Daniele Ceraolo Spurio (1):
>   drm/i915/guc: ADL-N should use the same GuC FW as ADL-S
> 
> Gustavo Sousa (1):
>   drm/i915/pvc: Implement w/a 16016694945
> 
> Matt Roper (4):
>   drm/i915: DG2 and ATS-M device ID updates
>   drm/i915/gt: Add general DSS steering iterator to intel_gt_mcr
>   drm/i915/dg2: Add Wa_15010599737
>   drm/i915: Correct ss -> steering calculation for pre-Xe_HP
> platforms
> 
> Matthew Auld (15):
>   drm/doc: add rfc section for small BAR uapi
>   drm/i915/uapi: add probed_cpu_visible_size
>   drm/i915/uapi: expose the avail tracking
>   drm/i915: remove intel_memory_region avail
>   drm/i915/uapi: apply ALLOC_GPU_ONLY by default
>   drm/i915/uapi: add NEEDS_CPU_ACCESS hint
>   drm/i915/error: skip non-mappable pages
>   drm/i915/uapi: tweak error capture on recoverable contexts
>   drm/i915/selftests: skip the mman tests for 

Re: [PATCH] drm/i915: Fix 32-bit build

[Vivi, Rodrigo]

On Sun, 2022-07-17 at 09:20 -0700, Guenter Roeck wrote:
> Commit aff1e0b09b54 ("drm/i915/ttm: fix sg_table construction")
> introduces
> an additional parameter to i915_rsgt_from_mm_node(). The parameter is
> used
> to calculate segment_pages. This in turn is used in DIV_ROUND_UP() as
> divisor. So far segment_pages was a constant and handled without
> divide
> operation. Since it is no longer constant, a divide operation is now
> necessary. This results in build errors on 32-bit builds.
> 
> x86_64-linux-ld: drivers/gpu/drm/i915/i915_scatterlist.o:
> in function `i915_rsgt_from_mm_node':
> i915_scatterlist.c:(.text+0x196): undefined reference to `__udivdi3'
> 
> Fix the problem by using DIV_ROUND_UP_ULL() instead of
> DIV_ROUND_UP().
> 
> Fixes: aff1e0b09b54 ("drm/i915/ttm: fix sg_table construction")
> Cc: Matthew Auld 
> Cc: Nirmoy Das 
> Cc: Rodrigo Vivi 
> Signed-off-by: Guenter Roeck 
> ---
> I took a stab at the problem. Please ignore the noise if it has
> already
> been fixed with a different patch.

Thanks for your patch.

This was actually fixed already in our -next branches, but missed
on the fixes:

https://patchwork.freedesktop.org/patch/493637/?series=106260=1

Thanks,
Rodrigo.


> 
>  drivers/gpu/drm/i915/i915_scatterlist.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_scatterlist.c
> b/drivers/gpu/drm/i915/i915_scatterlist.c
> index f63b50b71e10..b81d5658c222 100644
> --- a/drivers/gpu/drm/i915/i915_scatterlist.c
> +++ b/drivers/gpu/drm/i915/i915_scatterlist.c
> @@ -96,7 +96,7 @@ struct i915_refct_sgt *i915_rsgt_from_mm_node(const
> struct drm_mm_node *node,
>  
> i915_refct_sgt_init(rsgt, node->size << PAGE_SHIFT);
> st = >table;
> -   if (sg_alloc_table(st, DIV_ROUND_UP(node->size,
> segment_pages),
> +   if (sg_alloc_table(st, DIV_ROUND_UP_ULL(node->size,
> segment_pages),
>    GFP_KERNEL)) {
> i915_refct_sgt_put(rsgt);
> return ERR_PTR(-ENOMEM);

Re: [git pull] drm fixes for 5.19-rc7

[Vivi, Rodrigo]

On Sat, 2022-07-16 at 15:08 -0700, Linus Torvalds wrote:
> On Sat, Jul 16, 2022 at 2:35 PM Linus Torvalds
>  wrote:
> > 
> > That said, even those type simplifications do not fix the
> > fundamental
> > issue. That "DIV_ROUND_UP()" still ends up being a 64-bit divide,
> > although now it's at least a "64-by-32" bit divide.
> 
> Hmm. The "DIV_ROUND_UP()" issue could be solved by just making the
> rule be that the max_segment size is always a power of two.
> 
> Then you don't need the (expensive!) DIV_ROUND_UP(), and can just use
> the regular "round_up()" that works on powers-of-two.
> 
> And the simplest way to do that is to just make "max_segments" be
> 2GB.
> 
> The whole "round_down(UINT_MAX, page_alignment)" seems entirely
> pointless. Do you really want segments that are some odd number just
> under the 4GB mark, and force expensive divides?

I fully agree with you that if we have only things at 32bit we could
use the round up and avoid the division.

> 
> For consistency, I used the same value in
> i915_rsgt_from_buddy_resource(). I have no idea if that makes sense.
> 
> Anyway, the attached patch is COMPLETELY UNTESTED. But it at least
> seems to compile. Maybe.

Thanks. We should check this.

Meanwhile I'd like to say that the team had worked already to fix the
horrible 32 vs 64 bits inconsistency and the build breakage already.

The fix [1] was merged Jul 13.

[1] https://patchwork.freedesktop.org/patch/493637/?series=106260=1

I'm the one to blame for not having
propagated this along with the latest drm-intel-fixes round.

Please accept my apologies.

I will check right now why this was missed on my side and check how to
propagate quickly.

Sorry,
Rodrigo.

> 
>   Linus

Re: [PULL] drm-intel-gt-next

[Vivi, Rodrigo]

On Tue, 2022-02-22 at 11:44 -0800, Lucas De Marchi wrote:
> On Mon, Feb 21, 2022 at 11:21:35AM +0200, Jani Nikula wrote:
> > On Mon, 21 Feb 2022, Dave Airlie  wrote:
> > > On Thu, 17 Feb 2022 at 20:26, Joonas Lahtinen
> > >  wrote:
> > > > 
> > > > Hi Dave & Daniel,
> > > > 
> > > > Here is the first drm-intel-gt-next feature PR towards v5.18.
> > > 
> > > Am I missing some previous drm-intel pull?
> > > 
> > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/i915/gt/intel_
> > > workarounds.c:
> > > In function ‘rcs_engine_wa_init’:
> > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/i915/gt/intel_
> > > workarounds.c:2051:40:
> > > error: ‘XEHP_DIS_BBL_SYSPIPE’ undeclared (first use in this
> > > function)
> > >  2051 |   wa_masked_en(wal, GEN9_ROW_CHICKEN4,
> > > XEHP_DIS_BBL_SYSPIPE);
> > >   |   
> > > ^~~~
> > > /home/airlied/devel/kernel/dim/src/drivers/gpu/drm/i915/gt/intel_
> > > workarounds.c:2051:40:
> > > note: each undeclared identifier is reported only once for each
> > > function it appears in
> > 
> > There's apparently a silent conflict between changes in drm-intel-
> > next
> > and drm-intel-gt-next. There's a fixup patch in drm-rerere:
> > fixups/drm-intel-gt-next.patch.

With ack from Dave on #dri-devel, I've applied this pull request to
drm-intel-next.

Then I used this big fixup on the merge resolution.

Now I'm going to prepare a drm-intel-next pull request towards drm-
next.

Then for the next rounds we check if we are doing cross merges,
or the other way around and get drm-intel-next into drm-intel-gt-next

Thanks,
Rodrigo.


> yeah, with all header refactors landing in drm-intel-next there were
> quite a few conflict lately. Just taking fixups/drm-intel-gt-
> next.patch
> doesn't fix it though as we'd need to follow the merge order drm-tip
> is
> doing, i.e. first get a pull request for drm-intel-next in, and then
> drm-intel-gt-next. Or the octopus merge
> 
> For this merge only I believe the fixup is:
> 
> git show 064030837c5b:fixups/drm-intel-gt-next.patch | patch
> -p1
> 
> with 064030837c5b being the commit in drm-rerere. Cc'ing Matt Roper
> 
> Lucas De Marchi
> 
> > 
> > We opted to sync the branches via drm-next pulls and backmerges,
> > but I'm
> > afraid that means you'd have to use the fixups when merging. I
> > guess we
> > failed to communicate that. The alternative would've been cross-
> > merges
> > within drm-intel.
> > 
> > 
> > BR,
> > Jani.
> > 
> > 
> > > 
> > > Dave.
> > > > 
> > > > For DG2 adds subplatform G12, missing workarounds and fixes GuC
> > > > loading on ARM64. C0/D0 stepping info added for RPL-S.
> > > > 
> > > > For uAPI enables support for simple parallel submission with
> > > > execlists which was previously enabled only for GuC.
> > > > 
> > > > Further fixes for PMU metrics when GuC is enabled, better error
> > > > reporting for GuC loading failures. Fix for PXP unbind splat.
> > > > Updates to GuC version 69.0.3 with improvements to GT reset
> > > > scenarios.
> > > > 
> > > > The rest is mostly refactoring of the memory management code,
> > > > as highlights introduction of async unbinding/migration and
> > > > removal of short-term pinning in execbuf.
> > > > 
> > > > Then a few selftest and coding style fixes.
> > > > 
> > > > Regards, Joonas
> > > > 
> > > > ***
> > > > 
> > > > drm-intel-gt-next-2022-02-17:
> > > > 
> > > > UAPI Changes:
> > > > 
> > > > - Weak parallel submission support for execlists
> > > > 
> > > >   Minimal implementation of the parallel submission support for
> > > >   execlists backend that was previously only implemented for
> > > > GuC.
> > > >   Support one sibling non-virtual engine.
> > > > 
> > > > Core Changes:
> > > > 
> > > > - Two backmerges of drm/drm-next for header file
> > > > renames/changes and
> > > >   i915_regs reorganization
> > > > 
> > > > Driver Changes:
> > > > 
> > > > - Add new DG2 subplatform: DG2-G12 (Matt R)
> > > > - Add new DG2 workarounds (Matt R, Ram, Bruce)
> > > > - Handle pre-programmed WOPCM registers for DG2+ (Daniele)
> > > > - Update guc shim control programming on XeHP SDV+ (Daniele)
> > > > - Add RPL-S C0/D0 stepping information (Anusha)
> > > > - Improve GuC ADS initialization to work on ARM64 on dGFX
> > > > (Lucas)
> > > > 
> > > > - Fix KMD and GuC race on accessing PMU busyness (Umesh)
> > > > - Use PM timestamp instead of RING TIMESTAMP for reference in
> > > > PMU with GuC (Umesh)
> > > > - Report error on invalid reset notification from GuC (John)
> > > > - Avoid WARN splat by holding RPM wakelock during PXP unbind
> > > > (Juston)
> > > > - Fixes to parallel submission implementation (Matt B.)
> > > > - Improve GuC loading status check/error reports (John)
> > > > - Tweak TTM LRU priority hint selection (Matt A.)
> > > > - Align the plane_vma to min_page_size of stolen mem (Ram)
> > > > 
> > > > - Introduce vma resources and implement async unbinding
> > > > (Thomas)
> > > > - Use struct 

Re: [PATCH 3/3] drm/i915/gt: Improve "race-to-idle" at low frequencies

[Vivi, Rodrigo]

On Tue, 2021-11-23 at 09:17 +, Tvrtko Ursulin wrote:
> 
> On 22/11/2021 18:44, Rodrigo Vivi wrote:
> > On Wed, Nov 17, 2021 at 02:49:55PM -0800, Vinay Belgaumkar wrote:
> > > From: Chris Wilson 
> > > 
> > > While the power consumption is proportional to the frequency,
> > > there is
> > > also a static draw for active gates. The longer we are able to
> > > powergate
> > > (rc6), the lower the static draw. Thus there is a sweetspot in
> > > the
> > > frequency/power curve where we run at higher frequency in order
> > > to sleep
> > > longer, aka race-to-idle. This is more evident at lower
> > > frequencies, so
> > > let's look to bump the frequency if we think we will benefit by
> > > sleeping
> > > longer at the higher frequency and so conserving power.
> > > 
> > > Signed-off-by: Chris Wilson 
> > > Cc: Vinay Belgaumkar 
> > > Cc: Tvrtko Ursulin 
> > 
> > Please let's not increase the complexity here, unless we have a
> > very good
> > and documented reason.
> > 
> > Before trying to implement anything smart like this in the driver
> > I'd like
> > to see data, power and performance results in different platforms
> > and with
> > different workloads.
> 
> Who has such test suite and test farm which isn't focused to
> workloads 
> from a single customer? ;(

Okay, maybe we don't need to cover the world here. But without seen any
data at all it is hard to make this call.

> 
> Regards,
> 
> Tvrtko

Re: [Intel-gfx] [PATCH 1/1] drm/i915/rpm: Enable runtime pm autosuspend by default

[Vivi, Rodrigo]

On Thu, 2021-11-11 at 14:42 +0200, Ville Syrjälä wrote:
> On Wed, Nov 10, 2021 at 05:24:22PM -0500, Rodrigo Vivi wrote:
> > On Wed, Nov 10, 2021 at 01:46:46PM +0200, Ville Syrjälä wrote:
> > > On Wed, Nov 10, 2021 at 10:59:26AM +0530, Tilak Tangudu wrote:
> > > > Enable runtime pm autosuspend by default for gen12 and
> > > > later versions.
> > > > 
> > > > Signed-off-by: Tilak Tangudu 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_runtime_pm.c | 4 
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > index eaf7688f517d..ef75f24288ef 100644
> > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > > @@ -600,6 +600,10 @@ void intel_runtime_pm_enable(struct
> > > > intel_runtime_pm *rpm)
> > > > pm_runtime_use_autosuspend(kdev);
> > > > }
> > > >  
> > > > +   /* XXX: Enable by default only for newer platforms for
> > > > now */
> > > > +   if (GRAPHICS_VER(i915) >= 12)
> > > > +   pm_runtime_allow(kdev);
> > > 
> > > If we change some default then we should just do it across the
> > > board.
> > > There is nothing special about tgl+.
> > 
> > Nothing special with tgl and newer platforms indeed. This is why we
> > have the XXX message here.
> > 
> > The problem in the last attempt was with the gen9 platforms.
> 
> What problem was that?

unfortunately it looks like the logs are not available anymore. :(

Tilak, could you please send this patch without the if?

so we can at
least make sure we spot the differences and see if there's something
quick that we can do about the gen9 or if we should take this path of
gen12, then fix gen9 , then enable eveywhere

> 
> > Apparently some special there, and I didn't want to block the
> > progress while we cannot get to the gen9 bugs.
> > 
> > > 
> > > > +
> > > > /*
> > > >  * The core calls the driver load handler with an RPM
> > > > reference held.
> > > >  * We drop that here and will reacquire it during
> > > > unloading in
> > > > -- 
> > > > 2.25.1
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel
> 

Re: [PATCH 1/2] drm/i915/dmabuf: fix broken build

[Vivi, Rodrigo]

On Wed, 2021-10-27 at 10:48 +0100, Matthew Auld wrote:
> On Wed, 27 Oct 2021 at 10:44, Jani Nikula
>  wrote:
> > 
> > On Wed, 27 Oct 2021, Matthew Auld 
> > wrote:
> > > On Wed, 27 Oct 2021 at 09:58, Jani Nikula
> > >  wrote:
> > > > 
> > > > On Wed, 27 Oct 2021, Matthew Auld
> > > >  wrote:
> > > > > On Thu, 21 Oct 2021 at 13:54, Matthew Auld
> > > > >  wrote:
> > > > > > 
> > > > > > wbinvd_on_all_cpus() is only defined on x86 it seems, plus
> > > > > > we need to
> > > > > > include asm/smp.h here.
> > > > > > 
> > > > > > Reported-by: kernel test robot 
> > > > > > Signed-off-by: Matthew Auld 
> > > > > > Cc: Thomas Hellström 
> > > > > 
> > > > > Jani, would it make sense to cherry-pick this to -fixes? The
> > > > > offending
> > > > > commit is in drm-next, and there have been a few reports
> > > > > around this.
> > > > > 
> > > > > Fixes: a035154da45d ("drm/i915/dmabuf: add paranoid flush-on-
> > > > > acquire")
> > > > 
> > > > If the Fixes: tag is in place, our tooling will cherry-pick it
> > > > where it
> > > > belongs. (In this case, drm-intel-next-fixes, not drm-intel-
> > > > fixes.)
> > > 
> > > Yeah, I forgot to add the fixes tag here unfortunately.
> > 
> > Already merged? What's the commit id to be cherry-picked? Rodrigo
> > can do
> > it manually.
> 
> Yeah, it was merged to gt-next:
> 
> 777226dac058 ("drm/i915/dmabuf: fix broken build")

picked up to drm-intel-next-fixes

thanks,
Rodrigo.

> 
> > 
> > Note to self, we should set up some way to check which maintainer
> > is
> > responsible for which branches and when.
> > 
> > BR,
> > Jani.
> > 
> > > 
> > > > 
> > > > Cc: Rodrigo who covers drm-intel-next-fixes atm.
> > > > 
> > > > BR,
> > > > Jani.
> > > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 7 +++
> > > > > >  1 file changed, 7 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> > > > > > b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> > > > > > index 1adcd8e02d29..a45d0ec2c5b6 100644
> > > > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> > > > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> > > > > > @@ -12,6 +12,13 @@
> > > > > >  #include "i915_gem_object.h"
> > > > > >  #include "i915_scatterlist.h"
> > > > > > 
> > > > > > +#if defined(CONFIG_X86)
> > > > > > +#include 
> > > > > > +#else
> > > > > > +#define wbinvd_on_all_cpus() \
> > > > > > +   pr_warn(DRIVER_NAME ": Missing cache flush in
> > > > > > %s\n", __func__)
> > > > > > +#endif
> > > > > > +
> > > > > >  I915_SELFTEST_DECLARE(static bool
> > > > > > force_different_devices;)
> > > > > > 
> > > > > >  static struct drm_i915_gem_object *dma_buf_to_obj(struct
> > > > > > dma_buf *buf)
> > > > > > --
> > > > > > 2.26.3
> > > > > > 
> > > > 
> > > > --
> > > > Jani Nikula, Intel Open Source Graphics Center
> > 
> > --
> > Jani Nikula, Intel Open Source Graphics Center

Re: [GIT PULL] drm-misc + drm-intel: Add support for out-of-band hotplug notification

[Vivi, Rodrigo]

On Thu, 2021-08-26 at 10:23 +0200, Maxime Ripard wrote:
> On Wed, Aug 25, 2021 at 04:03:43PM +0000, Vivi, Rodrigo wrote:
> > On Tue, 2021-08-24 at 18:48 +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 8/24/21 10:45 AM, Jani Nikula wrote:
> > > > On Fri, 20 Aug 2021, Hans de Goede  wrote:
> > > > > Hello drm-misc and drm-intel maintainers,
> > > > > 
> > > > > My "Add support for out-of-band hotplug notification"
> > > > > patchset:
> > > > > https://patchwork.freedesktop.org/series/93763/
> > > > > 
> > > > > Is ready for merging now, as discussed on IRC I based this
> > > > > series
> > > > > on top drm-tip and when trying to apply the i915 parts on top
> > > > > of drm-misc this fails due to conflict.
> > > > > 
> > > > > So as Jani suggested here is a pull-req for a topic-branch
> > > > > with
> > > > > the
> > > > > entire set, minus the troublesome i915 bits. Once this has
> > > > > been
> > > > > merged
> > > > > into both drm-misc-next and drm-intel-next I can push the 2
> > > > > i915
> > > > > patch do drm-intel-next on top of the merge.
> > > > > 
> > > > > Note there are also 2 drivers/usb/typec patches in here these
> > > > > have Greg KH's Reviewed-by for merging through the drm tree,
> > > > > Since this USB code does not change all that much. I also
> > > > > checked
> > > > > and the drm-misc-next-2021-08-12 base of this tree contains
> > > > > the
> > > > > same last commit to the modified file as usb-next.
> > > > > 
> > > > > Daniel Vetter mentioned on IRC that it might be better for
> > > > > you to
> > > > > simply
> > > > > pick-up the series directly from patchwork, that is fine too
> > > > > in
> > > > > that
> > > > > case don't forget to add:
> > > > > 
> > > > > Reviewed-by: Lyude Paul 
> > > > > 
> > > > > To the entire series (given in a reply to the cover-letter)
> > > > > 
> > > > > And:
> > > > > 
> > > > > Reviewed-by: Greg Kroah-Hartman 
> > > > > 
> > > > > To the usb/typec patches (patch 7/8), this was given in reply
> > > > > to a previous posting of the series and I forgot to add this
> > > > > in the resend.
> > > > 
> > > > Since this is mostly touching drm core, I think it should be
> > > > merged
> > > > to
> > > > drm-misc-next first, and drm-intel-next after. Please let us
> > > > know.
> > > 
> > > I agree this should go to drm-misc-next first.
> > > 
> > > (I was planning on pushing this to drm-misc-next myself,
> > > but then ended up going with the topic branch because of the
> > > conflict in the i915 bits.)
> > 
> > Just to be clear and avoid confusion: This pull request does apply
> > cleanly on drm-misc-next nd drm-intel-next right now.
> > 
> > I'm just waiting for drm-misc-next maintainers to pull this to drm-
> > misc-next so I can pull it to drm-intel-next.
> > 
> > Maxime, is that your round now?
> > or Thomas?
> 
> That's me, I just pushed it to drm-misc-next

Thank you!
I also pushed to drm-intel-next.

> 
> Thanks!
> Maxime

Re: [GIT PULL] drm-misc + drm-intel: Add support for out-of-band hotplug notification

[Vivi, Rodrigo]

On Tue, 2021-08-24 at 18:48 +0200, Hans de Goede wrote:
> Hi,
> 
> On 8/24/21 10:45 AM, Jani Nikula wrote:
> > On Fri, 20 Aug 2021, Hans de Goede  wrote:
> > > Hello drm-misc and drm-intel maintainers,
> > > 
> > > My "Add support for out-of-band hotplug notification" patchset:
> > > https://patchwork.freedesktop.org/series/93763/
> > > 
> > > Is ready for merging now, as discussed on IRC I based this series
> > > on top drm-tip and when trying to apply the i915 parts on top
> > > of drm-misc this fails due to conflict.
> > > 
> > > So as Jani suggested here is a pull-req for a topic-branch with
> > > the
> > > entire set, minus the troublesome i915 bits. Once this has been
> > > merged
> > > into both drm-misc-next and drm-intel-next I can push the 2 i915
> > > patch do drm-intel-next on top of the merge.
> > > 
> > > Note there are also 2 drivers/usb/typec patches in here these
> > > have Greg KH's Reviewed-by for merging through the drm tree,
> > > Since this USB code does not change all that much. I also checked
> > > and the drm-misc-next-2021-08-12 base of this tree contains the
> > > same last commit to the modified file as usb-next.
> > > 
> > > Daniel Vetter mentioned on IRC that it might be better for you to
> > > simply
> > > pick-up the series directly from patchwork, that is fine too in
> > > that
> > > case don't forget to add:
> > > 
> > > Reviewed-by: Lyude Paul 
> > > 
> > > To the entire series (given in a reply to the cover-letter)
> > > 
> > > And:
> > > 
> > > Reviewed-by: Greg Kroah-Hartman 
> > > 
> > > To the usb/typec patches (patch 7/8), this was given in reply
> > > to a previous posting of the series and I forgot to add this
> > > in the resend.
> > 
> > Since this is mostly touching drm core, I think it should be merged
> > to
> > drm-misc-next first, and drm-intel-next after. Please let us know.
> 
> I agree this should go to drm-misc-next first.
> 
> (I was planning on pushing this to drm-misc-next myself,
> but then ended up going with the topic branch because of the
> conflict in the i915 bits.)

Just to be clear and avoid confusion: This pull request does apply
cleanly on drm-misc-next nd drm-intel-next right now.

I'm just waiting for drm-misc-next maintainers to pull this to drm-
misc-next so I can pull it to drm-intel-next.

Maxime, is that your round now?
or Thomas?

Thanks,
Rodrigo.

> 
> Regards,
> 
> Hans
> 
> 
> 
> > > The following changes since commit
> > > c7782443a88926a4f938f0193041616328cf2db2:
> > > 
> > >   drm/bridge: ti-sn65dsi86: Avoid creating multiple connectors
> > > (2021-08-12 09:56:09 -0700)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git
> > > drm-misc-intel-oob-hotplug-v1
> > > 
> > > for you to fetch changes up to
> > > 7f811394878535ed9a6849717de8c2959ae38899:
> > > 
> > >   usb: typec: altmodes/displayport: Notify drm subsys of hotplug
> > > events (2021-08-20 12:35:59 +0200)
> > > 
> > > 
> > > Topic branch for drm-misc / drm-intel for OOB hotplug support for
> > > Type-C connectors
> > > 
> > > 
> > > Hans de Goede (6):
> > >   drm/connector: Give connector sysfs devices there own
> > > device_type
> > >   drm/connector: Add a fwnode pointer to drm_connector and
> > > register with ACPI (v2)
> > >   drm/connector: Add drm_connector_find_by_fwnode() function
> > > (v3)
> > >   drm/connector: Add support for out-of-band hotplug
> > > notification (v3)
> > >   usb: typec: altmodes/displayport: Make dp_altmode_notify()
> > > more generic
> > >   usb: typec: altmodes/displayport: Notify drm subsys of
> > > hotplug events
> > > 
> > >  drivers/gpu/drm/drm_connector.c  | 79
> > > +
> > >  drivers/gpu/drm/drm_crtc_internal.h  |  2 +
> > >  drivers/gpu/drm/drm_sysfs.c  | 87
> > > +++-
> > >  drivers/usb/typec/altmodes/Kconfig   |  1 +
> > >  drivers/usb/typec/altmodes/displayport.c | 58 +-
> > > ---
> > >  include/drm/drm_connector.h  | 25 +
> > >  6 files changed, 217 insertions(+), 35 deletions(-)
> > > 
> > 
> 

Re: [PULL] drm-intel-fixes v2

[Vivi, Rodrigo]

On Wed, 2020-12-09 at 14:11 +0200, Ville Syrjälä wrote:
> On Thu, Dec 03, 2020 at 05:47:05AM -0800, Rodrigo Vivi wrote:
> > Hi Dave and Daniel,
> > 
> > Please ignore the pull request I had sent yesterday and use only
> > this one.
> > 
> > I had missed one patch: 14d1eaf08845 ("drm/i915/gt: Protect context
> > lifetime with RCU").
> > 
> > Also, please notice that the commit 6db58901c2aa
> > ("drm/i915/display: return earlier from
> > +intel_modeset_init() without display") was not actually a crucial
> > fix, but it
> > +allowed a clean pick of the use-after-free one.
> > 
> > Here goes drm-intel-fixes-2020-12-03:
> > Fixes for GPU hang, null dereference, suspend-resume, power
> > consumption, and use-after-free.
> > 
> > - Program mocs:63 for cache eviction on gen9 (Chris)
> > - Protect context lifetime with RCU (Chris)
> > - Split the breadcrumb spinlock between global and contexts (Chris)
> > - Retain default context state across shrinking (Venkata)
> > - Limit frequency drop to RPe on parking (Chris)
> > - Return earlier from intel_modeset_init() without display (Jani)
> > - Defer initial modeset until after GGTT is initialized (Chris)
> > 
> > Thanks,
> > Rodrigo.
> > 
> > The following changes since commit
> > b65054597872ce3aefbc6a666385eabdf9e288da:
> > 
> >   Linux 5.10-rc6 (2020-11-29 15:50:50 -0800)
> > 
> > are available in the Git repository at:
> > 
> >   git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-
> > 2020-12-03
> > 
> > for you to fetch changes up to
> > ccc9e67ab26feda7e62749bb54c05d7abe07dca9:
> > 
> >   drm/i915/display: Defer initial modeset until after GGTT is
> > initialised (2020-12-02 17:05:58 -0800)
> > 
> > 
> > Fixes for GPU hang, null dereference, suspend-resume, power
> > consumption, and use-after-free.
> > 
> > - Program mocs:63 for cache eviction on gen9 (Chris)
> > - Protect context lifetime with RCU (Chris)
> > - Split the breadcrumb spinlock between global and contexts (Chris)
> > - Retain default context state across shrinking (Venkata)
> > - Limit frequency drop to RPe on parking (Chris)
> > - Return earlier from intel_modeset_init() without display (Jani)
> > - Defer initial modeset until after GGTT is initialized (Chris)
> > 
> > 
> > Chris Wilson (5):
> >   drm/i915/gt: Program mocs:63 for cache eviction on gen9
> 
> That also needs
> commit 444fbf5d7058 ("drm/i915/gt: Declare gen9 has 64 mocs
> entries!")
> which seems to have not made it into this pull.

yeap, I'm sorry I have missed that one.
dim doesn't deal very well with fixes for fixes and I end up missing
that.

It is now on dif and will be part of this week's pull request.

Thanks for the heads up,
Rodrigo.

> 
> >   drm/i915/gt: Protect context lifetime with RCU
> >   drm/i915/gt: Split the breadcrumb spinlock between global and
> > contexts
> >   drm/i915/gt: Limit frequency drop to RPe on parking
> >   drm/i915/display: Defer initial modeset until after GGTT is
> > initialised
> > 
> > Jani Nikula (1):
> >   drm/i915/display: return earlier from intel_modeset_init()
> > without display
> > 
> > Venkata Ramana Nayana (1):
> >   drm/i915/gt: Retain default context state across shrinking
> > 
> >  drivers/gpu/drm/i915/display/intel_display.c  |  24 ++--
> >  drivers/gpu/drm/i915/gt/intel_breadcrumbs.c   | 168
> > ++
> >  drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h |   6 +-
> >  drivers/gpu/drm/i915/gt/intel_context.c   |  15 +-
> >  drivers/gpu/drm/i915/gt/intel_context_types.h |  23 ++-
> >  drivers/gpu/drm/i915/gt/intel_mocs.c  |  14 +-
> >  drivers/gpu/drm/i915/gt/intel_rps.c   |   4 +
> >  drivers/gpu/drm/i915/gt/shmem_utils.c |   7 +-
> >  drivers/gpu/drm/i915/i915_request.h   |   6 +-
> >  9 files changed, 143 insertions(+), 124 deletions(-)
> > ___
> > dim-tools mailing list
> > dim-to...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dim-tools
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PULL] drm-intel-fixes

[Vivi, Rodrigo]

> On Nov 12, 2020, at 4:32 PM, Dave Airlie  wrote:
> 
> On Fri, 13 Nov 2020 at 09:08, Rodrigo Vivi  wrote:
>> 
>> Hi Dave and Daniel,
>> 
>> This is the same set as last week + couple new fixes targeting stable.
>> 
> 
> But I merged last weeks set and it's in rc3, maybe you can generate
> the pull request relative to origin/master or drm/drm-fixes because
> I'm not sure which bits to edit out here.

oh, of course... bad rebase on my part. Sorry...
Please ignore this one. I'm going to generate another one soon.

> 
> Dave.
> 
>> Thanks,
>> Rodrigo.
>> 
>> drm-intel-fixes-2020-11-12-1:
>> - GVT fixes including vGPU suspend/resume fixes and workaround for APL guest 
>> GPU hang.
>> - Fix set domain's cache coherency (Chris)
>> - Fixes around breadcrumbs (Chris)
>> - Fix encoder lookup during PSR atomic (Imre)
>> - Hold onto an explicit ref to i915_vma_work.pinned (Chris)
>> - Pull phys pread/pwrite implementations to the backend (Chris)
>> - Correctly set SFC capability for video engines
>> The following changes since commit 3cea11cd5e3b00d91caf0b4730194039b45c5891:
>> 
>>  Linux 5.10-rc2 (2020-11-01 14:43:51 -0800)
>> 
>> are available in the Git repository at:
>> 
>>  git://anongit.freedesktop.org/drm/drm-intel 
>> tags/drm-intel-fixes-2020-11-12-1
>> 
>> for you to fetch changes up to a4264790f4c2f0062d27d8173344c914bc7884e0:
>> 
>>  drm/i915: Correctly set SFC capability for video engines (2020-11-12 
>> 16:41:54 -0500)
>> 
>> 
>> - GVT fixes including vGPU suspend/resume fixes and workaround for APL guest 
>> GPU hang.
>> - Fix set domain's cache coherency (Chris)
>> - Fixes around breadcrumbs (Chris)
>> - Fix encoder lookup during PSR atomic (Imre)
>> - Hold onto an explicit ref to i915_vma_work.pinned (Chris)
>> - Pull phys pread/pwrite implementations to the backend (Chris)
>> - Correctly set SFC capability for video engines
>> 
>> 
>> Chris Wilson (6):
>>  drm/i915/gem: Flush coherency domains on first set-domain-ioctl
>>  drm/i915/gt: Use the local HWSP offset during submission
>>  drm/i915/gt: Expose more parameters for emitting writes into the ring
>>  drm/i915/gt: Flush xcs before tgl breadcrumbs
>>  drm/i915: Hold onto an explicit ref to i915_vma_work.pinned
>>  drm/i915/gem: Pull phys pread/pwrite implementations to the backend
>> 
>> Colin Xu (4):
>>  drm/i915/gvt: Allow zero out HWSP addr on hws_pga_write
>>  drm/i915/gvt: Set SNOOP for PAT3 on BXT/APL to workaround GPU BB hang
>>  drm/i915/gvt: Only pin/unpin intel_context along with workload
>>  drm/i915/gvt: Fix mmio handler break on BXT/APL.
>> 
>> Imre Deak (1):
>>  drm/i915: Fix encoder lookup during PSR atomic check
>> 
>> Matthew Auld (1):
>>  drm/i915/gem: Allow backends to override pread implementation
>> 
>> Venkata Sandeep Dhanalakota (1):
>>  drm/i915: Correctly set SFC capability for video engines
>> 
>> drivers/gpu/drm/i915/display/intel_psr.c |  2 +-
>> drivers/gpu/drm/i915/gem/i915_gem_domain.c   | 28 ++--
>> drivers/gpu/drm/i915/gem/i915_gem_object_types.h |  2 +
>> drivers/gpu/drm/i915/gem/i915_gem_phys.c | 55 
>> 
>> drivers/gpu/drm/i915/gt/intel_engine.h   | 55 
>> +++-
>> drivers/gpu/drm/i915/gt/intel_engine_cs.c|  3 +-
>> drivers/gpu/drm/i915/gt/intel_lrc.c  | 31 +
>> drivers/gpu/drm/i915/gt/intel_timeline.c | 18 
>> drivers/gpu/drm/i915/gt/intel_timeline_types.h   |  2 +
>> drivers/gpu/drm/i915/gvt/handlers.c  | 47 ++--
>> drivers/gpu/drm/i915/gvt/scheduler.c | 15 ---
>> drivers/gpu/drm/i915/i915_gem.c  | 32 +++---
>> drivers/gpu/drm/i915/i915_vma.c  |  6 ++-
>> 13 files changed, 204 insertions(+), 92 deletions(-)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH] drm/i915/ehl: Remove require_force_probe protection

[Vivi, Rodrigo]

> On Oct 27, 2020, at 11:49 PM, Pandey, Hariom  wrote:
> 
> Hi Chris,
>  
> Awaiting your kind response here…

https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9208/fi-ehl-1/igt@i915_selftest@live@gt_pm.html
"Did not enter RC6!"

Chris already told that we need to get RC6 working on CI.
If we need BIOS update or machine replacement there we need to work with CI 
team to make that happen.

>  
> Thanks
> Hariom Pandey
>  
> From: Pandey, Hariom 
> Sent: Tuesday, October 20, 2020 9:28 PM
> To: Chris Wilson 
> Cc: Ausmus, James ; Nikula, Jani 
> ; intel-gfx@  intel-...@lists.freedesktop.org>; Souza, Jose ; 
> dri-devel@ ; 
> Surendrakumar Upadhyay, TejaskumarX 
> ; K, SrinivasX 
> ; Vivi, Rodrigo ; Meena, 
> Mahesh 
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/ehl: Remove require_force_probe 
> protection
>  
> Hi Chris,
>  
> We have run RC6 test cases as recently as 4 days ago on EHL and they have 
> passed. Below are the pass log links & attached email has the DRM/IGT tag 
> where they have passed. We are finding that the “EHL BAT setup” is not upto 
> date in terms of Silicon & BIOS which we are working to upgrade. But just for 
> that, should we block this patch? Just trying to understand as there is no 
> inherent or latent RC6 issue anymore.
>  
>   • igt@i915_pm_rc6_residency@rc6-accuracy --- PASS - Log
>   • igt@i915_pm_rc6_residency@rc6-fence --- PASS – Log
>   • igt@i915_pm_rc6_residency@rc6-idle --- PASS - Log
>   • igt@perf@rc6-disable --- PASS - Log
>   • igt@perf_pmu@rc6 --- PASS - Log
>   • igt@perf_pmu@rc6-runtime-pm --- PASS - Log
>   • igt@perf_pmu@rc6-runtime-pm-long --- PASS – Log
>  
>  
> Thanks
> Hariom Pandey
>  
> > -Original Message-
> > From: Chris Wilson 
> > Sent: Tuesday, October 20, 2020 12:04 AM
> > To: K, SrinivasX ; Vivi, Rodrigo
> > 
> > Cc: Pandey, Hariom ; Ausmus, James
> > ; Nikula, Jani ; intel-gfx@
> > ; Souza, Jose
> > ; dri-devel@  > de...@lists.freedesktop.org>; Surendrakumar Upadhyay, TejaskumarX
> > 
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/ehl: Remove require_force_probe
> > protection
> > 
> > Quoting Rodrigo Vivi (2020-10-19 19:29:36)
> > >
> > > I just checked the CI picture and it looks much better indeed.
> > >
> > > Only bad case being the gt_pm, which is also failing on other platforms.
> > 
> > Not nearly in the same manner. CI is indicating that there is no RC6 entry 
> > and
> > no power saving at all; neither in the selftests nor visible from userspace.
> > That is a critical battery eating bug.
> > 
> > If there's a patch to fix it for ehl and jsl, send it to CI for proving.
> > -Chris
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915/dp: Tweak initial dpcd backlight.enabled value

[Vivi, Rodrigo]

> On Oct 12, 2020, at 2:47 PM, Lyude Paul  wrote:
> 
> Just pushed this, but it's not in drm-tip because it would seem that 
> rebuilding
> drm-tip has failed, and the conflict doesn't appear to be from any of the
> patches I pushed so I'm getting the feeling from the DRM maintainer docs I
> should probably let one of the drm-misc-folks handle the conflict.

conflicts solved. feel free to push now.

For the drm-misc I simply went with the drm-misc-next solution and for the 
drm-intel
I went with the drm-intel-next-queued one.

> 
> On Mon, 2020-10-12 at 13:50 -0400, Sean Paul wrote:
>> On Tue, Sep 22, 2020 at 11:36 AM Lyude Paul  wrote:
>>> On Tue, 2020-09-22 at 09:39 -0400, Sean Paul wrote:
 On Mon, Sep 21, 2020 at 6:35 PM Lyude Paul  wrote:
> So if I understand this correctly, it sounds like that some Pixelbooks
> boot up
> with DP_EDP_BACKLIGHT_BRIGHTNESS_MSB set to a non-zero value, without
> the
> panel actually having DPCD backlight controls enabled?
 
 It boots with DP_EDP_BACKLIGHT_BRIGHTNESS_MSB == 0, which used to set
 backlight.enabled = false. By changing backlight.level = max,
 backlight.enabled is now set to true. This results in losing backlight
 control on boot (since the enable routine is no longer invoked).
 
>>> Ahhh ok, I'm fine with that - review still stands :)
>> 
>> Pinging intel maintainers, could someone please apply this?
>> 
>> 
>> Sean
>> 
 Sean
 
> If I'm understanding that correctly, then this patch looks good to me:
> 
> Reviewed-by: Lyude Paul 
> 
> On Thu, 2020-09-17 at 20:28 -0400, Sean Paul wrote:
>> From: Sean Paul 
>> 
>> In commit 79946723092b ("drm/i915: Assume 100% brightness when not in
>> DPCD control mode"), we fixed the brightness level when DPCD control
>> was
>> not active to max brightness. This is as good as we can guess since
>> most
>> backlights go on full when uncontrolled.
>> 
>> However in doing so we changed the semantics of the initial
>> 'backlight.enabled' value. At least on Pixelbooks, they  were relying
>> on the brightness level in DP_EDP_BACKLIGHT_BRIGHTNESS_MSB to be 0 on
>> boot such that enabled would be false. This causes the device to be
>> enabled when the brightness is set. Without this, brightness control
>> doesn't work. So by changing brightness to max, we also flipped
>> enabled
>> to be true on boot.
>> 
>> To fix this, make enabled a function of brightness and backlight
>> control
>> mechanism.
>> 
>> Fixes: 79946723092b ("drm/i915: Assume 100% brightness when not in
>> DPCD
>> control mode")
>> Cc: Lyude Paul 
>> Cc: Jani Nikula 
>> Cc: Juha-Pekka Heikkila 
>> Cc: "Ville Syrjälä" 
>> Cc: Rodrigo Vivi 
>> Cc: Kevin Chowski >
>> Signed-off-by: Sean Paul 
>> ---
>> .../drm/i915/display/intel_dp_aux_backlight.c | 31 
>> ---
>> 1 file changed, 20 insertions(+), 11 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> index acbd7eb66cbe..036f504ac7db 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> @@ -52,17 +52,11 @@ static void set_aux_backlight_enable(struct
>> intel_dp
>> *intel_dp, bool enable)
>>  }
>> }
>> 
>> -/*
>> - * Read the current backlight value from DPCD register(s) based
>> - * on if 8-bit(MSB) or 16-bit(MSB and LSB) values are supported
>> - */
>> -static u32 intel_dp_aux_get_backlight(struct intel_connector
>> *connector)
>> +static bool intel_dp_aux_backlight_dpcd_mode(struct intel_connector
>> *connector)
>> {
>>  struct intel_dp *intel_dp = intel_attached_dp(connector);
>>  struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> - u8 read_val[2] = { 0x0 };
>>  u8 mode_reg;
>> - u16 level = 0;
>> 
>>  if (drm_dp_dpcd_readb(_dp->aux,
>>DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
>> @@ -70,15 +64,29 @@ static u32 intel_dp_aux_get_backlight(struct
>> intel_connector *connector)
>>  drm_dbg_kms(>drm,
>>  "Failed to read the DPCD register 0x%x\n",
>>  DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
>> - return 0;
>> + return false;
>>  }
>> 
>> + return (mode_reg & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) ==
>> +DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;
>> +}
>> +
>> +/*
>> + * Read the current backlight value from DPCD register(s) based
>> + * on if 8-bit(MSB) or 16-bit(MSB and LSB) values are supported
>> + */
>> +static u32 intel_dp_aux_get_backlight(struct intel_connector
>> *connector)

Re: [Intel-gfx] [PATCH] drm/i915/ehl: Remove require_force_probe protection

[Vivi, Rodrigo]

> On Oct 9, 2020, at 1:31 AM, K, SrinivasX  wrote:
> 
> Hi Rodrigo,
> 
> How do we get W/A and rc6 changes in, do you have any details?

I told based on what I was seeing on 
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip-alt.html?
focusing on the issues that are exclusively for ehl and not happening on other 
platforms.

It looks like workarounds are fine there now. so I'm not sure if it was 
sporadic thing that day.

for the rc6 there are a few testcases failing around it:
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_675/fi-ehl-1/igt@i915_pm_rc6_reside...@rc6-fence.html
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_675/fi-ehl-1/igt@i915_pm_rc6_reside...@rc6-idle.html
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_675/fi-ehl-1/igt@i915_selftest@live@gt_pm.html#dmesg-warnings415

> 
> Thanks,
> Srinivas
> 
> -Original Message-
> From: Souza, Jose 
> Sent: 06 October 2020 23:33
> To: Vivi, Rodrigo ; ch...@chris-wilson.co.uk
> Cc: Ausmus, James ; Nikula, Jani 
> ; Pandey, Hariom ; Roper, 
> Matthew D ; intel-...@lists.freedesktop.org; 
> dri-devel@lists.freedesktop.org; K, SrinivasX ; 
> Surendrakumar Upadhyay, TejaskumarX 
> 
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/ehl: Remove require_force_probe 
> protection
> 
> On Tue, 2020-10-06 at 10:55 -0700, Vivi, Rodrigo wrote:
>> 
>>> On Oct 6, 2020, at 10:48 AM, Chris Wilson  wrote:
>>> 
>>> Quoting Souza, Jose (2020-10-06 18:46:45)
>>>> +Rodrigo and Jani
>>>> 
>>>> On Tue, 2020-10-06 at 14:56 +, Kamati Srinivas wrote:
>>>>> Removing force probe protection from EHL platform. Did not
>>>>> observe warnings, errors, flickering or any visual defects while
>>>>> doing ordinary tasks like browsing and editing documents in a
>>>>> two monitor setup.
>>>> 
>>>> One of the requirements was also to have CI BAT all green and
>>>> shards as green is possible but EHL don't show up in CI results, we 
>>>> actually have one single EHL machine in CI but I guess it is not able to 
>>>> run all tests that shards do:
>>>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9097/filelist.html
>>> 
>>> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip-alt.html
>> 
>> we are really close to that point. We just need to fix some w/a and
>> rc6 issues before applying this change.
>> 
>>> -Chris
>> 
> 
> Huum okay we have drm-tip results for EHL but if someone sends a patch that 
> breaks EHL it will not be caught in pre-merge testing.
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH] drm/i915/ehl: Remove require_force_probe protection

[Vivi, Rodrigo]

> On Oct 6, 2020, at 10:48 AM, Chris Wilson  wrote:
> 
> Quoting Souza, Jose (2020-10-06 18:46:45)
>> +Rodrigo and Jani
>> 
>> On Tue, 2020-10-06 at 14:56 +, Kamati Srinivas wrote:
>>> Removing force probe protection from EHL platform. Did
>>> not observe warnings, errors, flickering or any visual
>>> defects while doing ordinary tasks like browsing and
>>> editing documents in a two monitor setup.
>> 
>> One of the requirements was also to have CI BAT all green and shards as 
>> green is possible but EHL don't show up in CI results, we actually have one
>> single EHL machine in CI but I guess it is not able to run all tests that 
>> shards do:
>> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9097/filelist.html
> 
> https://intel-gfx-ci.01.org/tree/drm-tip/drmtip-alt.html

we are really close to that point. We just need to fix some w/a and rc6 issues
before applying this change.

> -Chris

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: WTF: patch "[PATCH] drm/mgag200: Remove declaration of mgag200_mmap() from header" was seriously submitted to be applied to the 5.8-stable tree?

[Vivi, Rodrigo]

> On Aug 10, 2020, at 5:49 AM, Daniel Vetter  wrote:
> 
> On Sat, Aug 08, 2020 at 05:24:53PM +0200, Daniel Vetter wrote:
>> On Sat, Aug 8, 2020 at 1:28 PM Greg KH  wrote:
>>> 
>>> On Sat, Aug 08, 2020 at 01:02:34PM +0200, Daniel Vetter wrote:
 On Sat, Aug 8, 2020 at 12:24 PM Greg KH  wrote:
> 
> On Sat, Aug 08, 2020 at 11:13:54AM +0200, Daniel Vetter wrote:
>> On Fri, Aug 7, 2020 at 3:54 PM Thomas Zimmermann  
>> wrote:
>>> 
>>> Hi
>>> 
>>> Am 07.08.20 um 15:30 schrieb gre...@linuxfoundation.org:
 The patch below was submitted to be applied to the 5.8-stable tree.
 
 I fail to see how this patch meets the stable kernel rules as found at
 Documentation/process/stable-kernel-rules.rst.
 
 I could be totally wrong, and if so, please respond to
  and let me know why this patch should be
 applied.  Otherwise, it is now dropped from my patch queues, never to 
 be
 seen again.
>>> 
>>> Sorry for the noise. There's no reason this should go into stable.
>> 
>> We have a little script in our maintainer toolbox for bugfixes, which
>> generates the Fixes: line, adds everyone from the original commit to
>> the cc: list and also adds Cc: stable if that sha1 the patch fixes is
>> in a release already.
>> 
>> I guess we trained people a bit too much on using Fixes: tags like
>> that with the tooling, since they often do that for checkpatch stuff
>> and spelling fixes like this here too. I think the autoselect bot also
>> loves Fixes: tags a bit too much for its own good.
>> 
>> Not sure what to do, since telling people to "please sprinkle less
>> Fixes: tags" doesn't sound great either. I also don't want to tell
>> people to use the maintainer toolbox less, the autogenerated cc: list
>> is generally the right thing to do. Maybe best if the stable team
>> catches the obvious ones before adding them to the stable queue, if
>> you're ok with that Greg?
> 
> As I think this is the first time that I've had this problem for a DRM
> submission, I don't think it's a big issue yet at all, so whatever you
> are doing today is fine.
> 
> I do think that the number of patches submitted for stable for
> drm-related issues feels very very low given the rate of change and
> number of overall patches you all submit to the kernel, so if anything,
> you all should be increasing the number of times you tag stuff for
> stable, not reducing it :)
 
 Ok, sounds like we should encourage people to use the Fixes: tag and
 auto-cc tooling more, not less.
 
 I also crunched some quick numbers:
 commits with cc: stable in drm/amd: 2.6%
 ... in drm/i915: 2.5%
 ... drm overall: 2.3%
 drivers/ overall: 3.1%
 
 So from a quick look no big outliers at least, maybe not quite enough
 cc: stable from smaller drivers (i915+amd is about 60% of everything
 in drm). This is for the past year. Compared to drivers/ overall a bit
 lower, but not drastically so. At least if I didn't screw up my
 scripting.
>>> 
>>> Seems about right, so on those averages, you have missed about 40-50
>>> patches that should have been cc:ed stable.
>>> 
>>> However, you are comparing yourself against stuff like drivers/net/
>>> which shouldn't have cc: stable for most stuff (as per the networking
>>> workflow), and other subsystems that seem to never want to cc: stable
>>> for various reasons (offenders not mentioned to be nice...)
>>> 
>>> So let's bump that number up a bit, maybe you are missing 100 patches
>>> this past year that should have been backported?
>>> 
>>> Feels like you all could tag more, even if the number is only 40-50 :)
>>> 
>>> Oh wait, are you sure you don't count the horrid "double commits" where
>>> you backport something from your development branch to your "for linus"
>>> branch, and have cc: stable on both, so that during the -rc1 merge
>>> window I see a ton of commits that are already in the tree?  That would
>>> inflate your numbers a lot more so your real percentages might be a lot
>>> lower...
>>> 
>>> fun with math.
>> 
>> Even drivers/net has like 1.0% cc: stable or so, but yeah maybe a
>> third cc: stable might be missing overall in drm. The math aint more
>> accurate no matter what, but agrees with your "about 100 patches".
>> 
>> And yeah I didn't take out the cherry-picked ones. Trying to grep for
>> those (yay more fun with math) says there's 37 stable commits I
>> double-counted, leaving 1.4% left over for drm/i915. That seems indeed
>> a bit too low :-/
>> 
>> I guess time to add intel maintainers (kinda not my direct business anymore).
> 
> So for comparison I also looked at mesa3d, which at least compared to
> drivers/gpu is very similar:
> - 3 months release cycle, 1 month -rc
> - very low level C codebase dealing with gpu nonsense
> - same Cc: 

Re: [Nouveau] [PATCH] drm/nouveau: missing cases of rename ttm_mem_reg to ttm_resource.

[Vivi, Rodrigo]

> On Aug 6, 2020, at 9:30 PM, Dave Airlie  wrote:
> 
> On Fri, 7 Aug 2020 at 14:03, Dave Airlie  wrote:
>> 
>> On Fri, 7 Aug 2020 at 11:13, Rodrigo Vivi  wrote:
>>> 
>>> From: Rodrigo Vivi 
>>> 
>>> These are missed cases that I just identified with allyesconfig build.
>>> 
>> 
>> Is this against drm-tip? it's a merge problem, that I thought I'd
>> already addressed, but tip seems to have lost it.
> 
> I think I've fixed drm-tip rebuilds now.

perfect, thanks!

> 
> Dave.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Merge in the drm-intel tree

[Vivi, Rodrigo]

Outch, it reapared on the backmerge I did yesterday and I didn't noticed. Sorry

Chris has fixed this now.

Thank you both.

> On Aug 22, 2019, at 3:20 AM, Stephen Rothwell  wrote:
> 
> Hi all,
> 
> I noticed that the drm tree has been back merge into the drm-intel
> tree.  Unfortunately, it seems that the file
> drivers/gpu/drm/i915/i915_gem_batch_pool.c has been resurrected.
> 
> It was removed in commit
> 
>  b40d73784ffc ("drm/i915: Replace struct_mutex for batch pool serialisation")
> 
> but has come back due to a conflict with commit
> 
>  52791eeec1d9 ("dma-buf: rename reservation_object to dma_resv")
> 
> from the drm tree.
> -- 
> Cheers,
> Stephen Rothwell

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/lima: Fix broken compilation.

[Vivi, Rodrigo]

> On Apr 4, 2019, at 6:39 PM, Dave Airlie  wrote:
> 
>> On Fri, 5 Apr 2019 at 08:43, Rodrigo Vivi  wrote:
>> 
>>> On Thu, Apr 04, 2019 at 03:25:38PM -0700, Rodrigo Vivi wrote:
>>> From: Rodrigo Vivi 
>> 
>> And it seems that I don't know how to spell my own name anymore! :)
>> 
>> If you decide for this patch please let me know, so we can fix while
>> pushing to drm-misc-fixes or tell me to send a v2.
>> 
>>> 
>>> I'm not entirely sure about the limits we should use
>>> here on ARM based driver, but apparently the entry and limit
>>> are inverted and UINT_MAX cannot be used directly there.
>>> 
>>> So let's at least for now fix this compilation issue
>>> on a compilation only driver:
> 
> I ended up writing this as well and squashing it into the merge into drm-next.

Cool, thanks 

> 
> Thanks,
> Dave.
> 
>>> 
>>> CC [M]  drivers/gpu/drm/lima/lima_ctx.o
>>> In file included from ./include/linux/kernel.h:7,
>>> from ./include/asm-generic/bug.h:18,
>>> from ./arch/x86/include/asm/bug.h:83,
>>> from ./include/linux/bug.h:5,
>>> from ./include/linux/mmdebug.h:5,
>>> from ./include/linux/gfp.h:5,
>>> from ./include/linux/slab.h:15,
>>> from drivers/gpu/drm/lima/lima_ctx.c:4:
>>> drivers/gpu/drm/lima/lima_ctx.c: In function ‘lima_ctx_create’:
>>> ./include/linux/limits.h:13:18: error: incompatible type for argument 4 of 
>>> ‘xa_alloc’
>>> #define UINT_MAX (~0U)
>>>  ^
>>> drivers/gpu/drm/lima/lima_ctx.c:27:41: note: in expansion of macro 
>>> ‘UINT_MAX’
>>>  err = xa_alloc(>handles, id, ctx, UINT_MAX, GFP_KERNEL);
>>> ^~~~
>>> In file included from ./include/linux/radix-tree.h:31,
>>> from ./include/linux/idr.h:15,
>>> from ./include/drm/drm_device.h:7,
>>> from drivers/gpu/drm/lima/lima_device.h:7,
>>> from drivers/gpu/drm/lima/lima_ctx.c:7:
>>> ./include/linux/xarray.h:817:32: note: expected ‘struct xa_limit’ but 
>>> argument is of type ‘unsigned int’
>>>   void *entry, struct xa_limit limit, gfp_t gfp)
>>>^
>>> make[4]: *** [scripts/Makefile.build:276: drivers/gpu/drm/lima/lima_ctx.o] 
>>> Error 1
>>> make[3]: *** [scripts/Makefile.build:486: drivers/gpu/drm/lima] Error 2
>>> make[2]: *** [scripts/Makefile.build:486: drivers/gpu/drm] Error 2
>>> make[1]: *** [scripts/Makefile.build:486: drivers/gpu] Error 2
>>> make: *** [Makefile:1051: drivers] Error 2
>>> 
>>> Cc: Qiang Yu 
>>> Signed-off-by: Rodrigo Vivi 
>>> ---
>>> drivers/gpu/drm/lima/lima_ctx.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/gpu/drm/lima/lima_ctx.c 
>>> b/drivers/gpu/drm/lima/lima_ctx.c
>>> index c8d12f7c6894..22fff6caa961 100644
>>> --- a/drivers/gpu/drm/lima/lima_ctx.c
>>> +++ b/drivers/gpu/drm/lima/lima_ctx.c
>>> @@ -23,7 +23,7 @@ int lima_ctx_create(struct lima_device *dev, struct 
>>> lima_ctx_mgr *mgr, u32 *id)
>>>  goto err_out0;
>>>  }
>>> 
>>> - err = xa_alloc(>handles, id, UINT_MAX, ctx, GFP_KERNEL);
>>> + err = xa_alloc(>handles, id, ctx, xa_limit_32b, GFP_KERNEL);
>>>  if (err < 0)
>>>  goto err_out0;
>>> 
>>> --
>>> 2.20.1
>>> 
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [drm-tip:drm-tip 1/7] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:219:6: error: redefinition of 'dm_dp_mst_dc_sink_create'

[Vivi, Rodrigo]

Drm-tip rebuild created this function duplication in some merge resolution... 
not sure where that came from or started... But Lucas had warned me yesterday 
morning...

So, yesterday night I added a fix-up on drm-rerere that remove the duplication 
on drm-tip... and that should be fixed by now I think...

Could you please fetch and recheck?!

On Dec 30, 2017, at 9:10 AM, Wu, Fengguang 
> wrote:

tree:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
head:   16432d39f2cbdc7a8798df3ebb4f7c882fb23132
commit: a79b622ef5cad1b3a868a1d7250494e39bb04c05 [1/7] Merge remote-tracking 
branch 'airlied/drm-next' into drm-tip
config: i386-randconfig-b0-12302345 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
   git checkout a79b622ef5cad1b3a868a1d7250494e39bb04c05
   # save the attached .config to linux build tree
   make ARCH=i386

Note: the drm-tip/drm-tip HEAD 16432d39f2cbdc7a8798df3ebb4f7c882fb23132 builds 
fine.
 It only hurts bisectibility.

All errors (new ones prefixed by >>):

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:219:6: 
error: redefinition of 'dm_dp_mst_dc_sink_create'
   void dm_dp_mst_dc_sink_create(struct drm_connector *connector)
^
  drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:183:6: 
note: previous definition of 'dm_dp_mst_dc_sink_create' was here
   void dm_dp_mst_dc_sink_create(struct drm_connector *connector)
^

vim +/dm_dp_mst_dc_sink_create +219 
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c

54427651 Jerry Zuo   2017-09-20  218
becd0875 Jerry (Fangzhi  Zuo 2017-12-01 @219) void 
dm_dp_mst_dc_sink_create(struct drm_connector *connector)
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  220) {
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  221)struct amdgpu_dm_connector 
*aconnector = to_amdgpu_dm_connector(connector);
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  222)struct edid *edid;
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  223)struct dc_sink *dc_sink;
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  224)struct dc_sink_init_data 
init_params = {
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  225).link = 
aconnector->dc_link,
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  226).sink_signal = 
SIGNAL_TYPE_DISPLAY_PORT_MST };
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  227)
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  228)edid = 
drm_dp_mst_get_edid(connector, >mst_port->mst_mgr, 
aconnector->port);
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  229)
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  230)if (!edid) {
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  231)
drm_mode_connector_update_edid_property(
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  232)>base,
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  233)NULL);
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  234)return;
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  235)}
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  236)
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  237)aconnector->edid = edid;
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  238)
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  239)dc_sink = 
dc_link_add_remote_sink(
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  240)aconnector->dc_link,
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  241)(uint8_t 
*)aconnector->edid,
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  242)
(aconnector->edid->extensions + 1) * EDID_LENGTH,
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  243)_params);
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  244)
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  245)dc_sink->priv = aconnector;
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  246)aconnector->dc_sink = dc_sink;
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  247)
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  248)
amdgpu_dm_add_sink_to_freesync_module(
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  249)connector, 
aconnector->edid);
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  250)
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  251)
drm_mode_connector_update_edid_property(
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  252)
>base, aconnector->edid);
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  253) }
becd0875 Jerry (Fangzhi  Zuo 2017-12-01  254)

:: The code at line 219 was first introduced by commit
:: becd0875f4393a992afbf57aa323f7bf1a71c3ff drm/amd/display: Fix rehook MST 
display not light back on

:: TO: Jerry (Fangzhi) Zuo >
:: CC: Alex Deucher 
>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
<.config.gz>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/2] drm/i915/psr: Set frames before SU entry for psr2

[Vivi, Rodrigo]

> On Sep 20, 2017, at 7:33 AM, Nagaraju, Vathsala  
> wrote:
> 
> Set frames before SU entry value for max resync frame count of
> dpcd register 2009, bit field 0:3.
> 
> Cc: Rodrigo Vivi 
> CC: Puthikorn Voravootivat 
> Signed-off-by: Vathsala Nagaraju 
> ---
> drivers/gpu/drm/i915/intel_psr.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index acb5094..04b253f 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -327,6 +327,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
> */
>uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>uint32_t val;
> +uint8_t sink_latency;
> 
>val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
> 
> @@ -334,8 +335,15 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
> * mesh at all with our frontbuffer tracking. And the hw alone isn't
> * good enough. */
>val |= EDP_PSR2_ENABLE |
> -EDP_SU_TRACK_ENABLE |
> -EDP_FRAMES_BEFORE_SU_ENTRY;

Please also remove the definition of this su_entry since it was not following 
the new standards anyway...
Probably good to replace with function macro style for better use below...

> +EDP_SU_TRACK_ENABLE;
> +
> +if (drm_dp_dpcd_readb(_dp->aux, DP_SINK_SYNCHRONIZATION_LATENCY,
> +_latency)) {
> +sink_latency = sink_latency & DP_MAX_RESYNC_FRAME_CNT_MASK;
> +val |= (sink_latency + 1) << EDP_PSR2_FRAME_BEFORE_SU_SHIFT;

... so you could use
val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency +1);


> +} else {
> +val |= EDP_FRAMES_BEFORE_SU_ENTRY;
> +}
> 
>if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>val |= EDP_PSR2_TP2_TIME_2500;
> -- 
> 1.9.1
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Vivi, Rodrigo]

On Wed, 2017-01-18 at 10:12 +0200, Jani Nikula wrote:
> On Tue, 17 Jan 2017, Rodrigo Vivi  wrote:
> > On Mon, Jan 16, 2017 at 2:04 AM, Jani Nikula
> >  wrote:
> >> On Fri, 13 Jan 2017, Rodrigo Vivi  wrote:
> >>> This and all the remaining patches on this series (6,7,8 and 9) got
> >>> merged to dinq.
> >>
> >> Given that this patch series was not properly sent as a thread, I don't
> >> think our CI ran it as a whole, and it should not have been pushed
> >> before that.
> >
> > I assumed "[Intel-gfx] ✓ Fi.CI.BAT: success for enable psr2 for
> > idle_screen on y-cordinate panel"
> > was enough, giving the patches haven't drastically changed after.
> 
> The idea is to test the code that gets pushed...

Yep, this makes sense, although I'm sure that BAT doesn't test any of
the code touched...
But this is another problem.

Anyway, that won't happen again. But question: 
should it be resent to mailing list or try-bot is fine?

> 
> BR,
> Jani.
> 
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Vivi, Rodrigo]

Reviewed-by: Rodrigo Vivi 

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

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Vivi, Rodrigo]

Reviewed-by: Rodrigo Vivi 

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

[PATCH] drm/i915/psr: disable psr2 for resolution greater than 32X20

[Vivi, Rodrigo]

Reviewed-by: Rodrigo Vivi 

On Tue, 2017-01-10 at 12:32 +0530, vathsala nagaraju wrote:
> PSR2 is restricted to work with panel resolutions upto 3200x2000,
> move the check to intel_psr_match_conditions and fully block psr.
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Suggested-by: Rodrigo Vivi 
> Signed-off-by: Vathsala Nagaraju 
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 6aca8ff..f2ca2a9 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -387,6 +387,13 @@ static bool intel_psr_match_conditions(struct intel_dp 
> *intel_dp)
>   return false;
>   }
>  
> + /* PSR2 is restricted to work with panel resolutions upto 3200x2000 */
> + if (intel_crtc->config->pipe_src_w > 3200 ||
> + intel_crtc->config->pipe_src_h > 2000) {
> + dev_priv->psr.psr2_support = false;
> + return false;
> + }
> +
>   dev_priv->psr.source_ok = true;
>   return true;
>  }
> @@ -425,7 +432,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   struct drm_device *dev = intel_dig_port->base.base.dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
>  
>   if (!HAS_PSR(dev_priv)) {
>   DRM_DEBUG_KMS("PSR not supported on this platform\n");
> @@ -452,12 +458,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   hsw_psr_setup_vsc(intel_dp);
>  
>   if (dev_priv->psr.psr2_support) {
> - /* PSR2 is restricted to work with panel resolutions 
> upto 3200x2000 */
> - if (crtc->config->pipe_src_w > 3200 ||
> - crtc->config->pipe_src_h > 2000)
> - dev_priv->psr.psr2_support = false;
> - else
> - skl_psr_setup_su_vsc(intel_dp);
> + skl_psr_setup_su_vsc(intel_dp);
>   }
>  
>   /*

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

[Vivi, Rodrigo]

On Mon, 2017-01-09 at 18:26 +0530, vathsala nagaraju wrote:
> Psr1 and psr2 are mutually exclusive,ie when psr2 is enabled,
> psr1 should be disabled.When psr2 is exited , bit 31 of reg
> PSR2_CTL must be set to 0 but currently bit 31 of SRD_CTL
> (psr1 control register)is set to 0.
> Also ,PSR2_IDLE state is looked up from SRD_STATUS(psr1 register)
> instead of PSR2_STATUS register, which has wrong data, resulting
> in blankscreen.
> hsw_enable_source is split into hsw_enable_source_psr1 and
> hsw_enable_source_psr2 for easier code review and maintenance,
> as suggested by rodrigo and jim.
> 
> v2: (Rodrigo)
> - Rename hsw_enable_source_psr* to intel_enable_source_psr*
> 
> v3: (Rodrigo)
> - In hsw_psr_disable ,
>   1) for psr active case, handle psr2 followed by psr1.
>   2) psr inactive case, handle psr2 followed by psr1

much better, thanks, but still has one blocking issue imho:

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

[PATCH 08/10] drm/i915/psr: enable psr2 for y cordinate panels

[Vivi, Rodrigo]

On Fri, 2017-01-06 at 17:55 +, Nagaraju, Vathsala wrote:
> 1) I  am still not able to get psr1 working on y-cordinate panels. Only psr2 
> works.
> 2) I haven't got a GTC panel to test this scenario. Once we test psr1 on psr2 
>  GTC panel, we could add dev_priv->psr.psr2_support = false; till GTC is 
> implemented. 

Agree!

Reviewed-by: Rodrigo Vivi 

> 
> -Original Message-----
> From: Vivi, Rodrigo 
> Sent: Friday, January 6, 2017 11:08 PM
> To: Nagaraju, Vathsala 
> Cc: dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; 
> jim.bride at linux.intel.com; Patil, Deepti 
> Subject: Re: [PATCH 08/10] drm/i915/psr: enable psr2 for y cordinate panels
> 
> I was going to write the rv-b,
> but something came to my mind...
> 
> In this case where y_cord_support but we don't have gtc yet, couldn't we 
> enable PSR1 instead?
> in this case instead of return false we would do dev_priv->psr.psr2_support = 
> false;
> 
> what do you think/advise?
> 
> On Fri, 2017-01-06 at 23:01 +0530, vathsala nagaraju wrote:
> > Psr2 is enabled only for y cordinate panels.Once GTC (global time 
> > code) is implemented,this restriction is removed so that psr2 can work 
> > on panels without y cordinate support.
> > 
> > v2: (Rodrigo)
> > - Move the check to intel_psr_match_conditions
> > 
> > v3: (Rodrigo)
> > - add return false
> > 
> > Cc: Rodrigo Vivi 
> > Cc: Jim Bride 
> > Signed-off-by: Vathsala Nagaraju 
> > Signed-off-by: Patil Deepti 
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 9 +
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 05efd4e..fc32b04 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -433,6 +433,15 @@ static bool intel_psr_match_conditions(struct intel_dp 
> > *intel_dp)
> > return false;
> > }
> >  
> > +   /*
> > +* FIXME:enable psr2 only for y-cordinate psr2 panels
> > +* After gtc implementation , remove this restriction.
> > +*/
> > +   if (!dev_priv->psr.y_cord_support &&  dev_priv->psr.psr2_support) {
> > +   DRM_DEBUG_KMS("PSR2 disabled, panel does not support Y 
> > coordinate\n");
> > +   return false;
> > +   }
> > +
> > dev_priv->psr.source_ok = true;
> > return true;
> >  }
> 

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

[Vivi, Rodrigo]

On Sat, 2017-01-07 at 00:28 +0530, vathsala nagaraju wrote:
> As per bpsec, CHICKEN_TRANS_EDP bit 12 ,15
> must be programmed.
> Enable bit 12 for programmable header packet.
> Enable bit 15 for Y cordinate support.
> 
> v2: (Rodrigo)
> - move CHICKEN_TRANS_EDP bit set logic right after setup_vsc
> 
> v3:(Rodrigo)
> - initialize chicken_trans to CHICKEN_TRANS_BIT12 instead of 0
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: vathsala nagaraju 
> Signed-off-by: Patil Deepti 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 7 +++
>  drivers/gpu/drm/i915/intel_psr.c | 6 ++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7830e6e..5ca506a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6449,6 +6449,13 @@ enum {
>  #define  BDW_DPRS_MASK_VBLANK_SRD(1 << 0)
>  #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, 
> _CHICKEN_PIPESL_1_B)
>  
> +#define CHICKEN_TRANS_A 0x420c0
> +#define CHICKEN_TRANS_B 0x420c4
> +#define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, 
> CHICKEN_TRANS_B)
> +#define TRANS_EDP  3
> +#define CHICKEN_TRANS_BIT12(1<<12)
> +#define CHICKEN_TRANS_BIT15(1<<15)
> +
>  #define DISP_ARB_CTL _MMIO(0x45000)
>  #define  DISP_FBC_MEMORY_WAKE(1<<31)
>  #define  DISP_TILE_SURFACE_SWIZZLING (1<<13)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 7020f42..7573c2f 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -475,6 +475,8 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   struct drm_device *dev = intel_dig_port->base.base.dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
>   struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> + /* Set CHICKEN_TRANS_BIT12 for programable header */
> + uint32_t chicken_trans = CHICKEN_TRANS_BIT12;

+ uint32_t chicken_trans;

>  
>   if (!HAS_PSR(dev_priv)) {
>   DRM_DEBUG_KMS("PSR not supported on this platform\n");
> @@ -505,6 +507,10 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   dev_priv->psr.psr2_support = false;
>   else
>   skl_psr_setup_su_vsc(intel_dp);
+
+   /* Set CHICKEN_TRANS_BIT12 for programable header */
+   chicken_trans = CHICKEN_TRANS_BIT12;
> + /* Set CHICKEN_TRANS_BIT15 if Y coordinate is supported 
> */
> + if (dev_priv->psr.y_cord_support)
> + chicken_trans |= CHICKEN_TRANS_BIT15;
> + I915_WRITE(CHICKEN_TRANS(TRANS_EDP), chicken_trans);
>   } else {
>   /* set up vsc header for psr1 */
>   hsw_psr_setup_vsc(intel_dp);

[PATCH 08/10] drm/i915/psr: enable psr2 for y cordinate panels

[Vivi, Rodrigo]

I was going to write the rv-b,
but something came to my mind...

In this case where y_cord_support but we don't have gtc yet, couldn't we
enable PSR1 instead?
in this case instead of return false we would do
dev_priv->psr.psr2_support = false;

what do you think/advise?

On Fri, 2017-01-06 at 23:01 +0530, vathsala nagaraju wrote:
> Psr2 is enabled only for y cordinate panels.Once GTC (global time code)
> is implemented,this restriction is removed so that psr2
> can work on panels without y cordinate support.
> 
> v2: (Rodrigo)
> - Move the check to intel_psr_match_conditions
> 
> v3: (Rodrigo)
> - add return false
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: Vathsala Nagaraju 
> Signed-off-by: Patil Deepti 
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 05efd4e..fc32b04 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -433,6 +433,15 @@ static bool intel_psr_match_conditions(struct intel_dp 
> *intel_dp)
>   return false;
>   }
>  
> + /*
> +  * FIXME:enable psr2 only for y-cordinate psr2 panels
> +  * After gtc implementation , remove this restriction.
> +  */
> + if (!dev_priv->psr.y_cord_support &&  dev_priv->psr.psr2_support) {
> + DRM_DEBUG_KMS("PSR2 disabled, panel does not support Y 
> coordinate\n");
> + return false;
> + }
> +
>   dev_priv->psr.source_ok = true;
>   return true;
>  }

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

[Vivi, Rodrigo]

On Fri, 2017-01-06 at 21:59 +0530, vathsala nagaraju wrote:
> As per bpsec, CHICKEN_TRANS_EDP bit 12 ,15
> must be programmed.
> Enable bit 12 for programmable header packet.
> Enable bit 15 for Y cordinate support.
> 
> v2: (Rodrigo)
> - move CHICKEN_TRANS_EDP bit set logic right
>   after setup_vsc
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: vathsala nagaraju 
> Signed-off-by: Patil Deepti 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 7 +++
>  drivers/gpu/drm/i915/intel_psr.c | 9 -
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7830e6e..5ca506a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6449,6 +6449,13 @@ enum {
>  #define  BDW_DPRS_MASK_VBLANK_SRD(1 << 0)
>  #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, 
> _CHICKEN_PIPESL_1_B)
>  
> +#define CHICKEN_TRANS_A 0x420c0
> +#define CHICKEN_TRANS_B 0x420c4
> +#define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, 
> CHICKEN_TRANS_B)
> +#define TRANS_EDP  3
> +#define CHICKEN_TRANS_BIT12(1<<12)
> +#define CHICKEN_TRANS_BIT15(1<<15)
> +
>  #define DISP_ARB_CTL _MMIO(0x45000)
>  #define  DISP_FBC_MEMORY_WAKE(1<<31)
>  #define  DISP_TILE_SURFACE_SWIZZLING (1<<13)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 7020f42..bcfe0db 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -348,7 +348,6 @@ static void intel_enable_source_psr2(struct intel_dp 
> *intel_dp)
>   val |= EDP_PSR2_TP2_TIME_100;
>   else
>   val |= EDP_PSR2_TP2_TIME_50;
> -

I'm afraid you missed this space here and removed one extra line.

>   I915_WRITE(EDP_PSR2_CTL, val);
>  }
>  
> @@ -475,6 +474,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   struct drm_device *dev = intel_dig_port->base.base.dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
>   struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> + uint32_t chicken_trans = 0;

what about invert the order below so you don't need to initialize this
as 0 here.

chicken_trans = CHICKEN_TRANS_BIT12;
if (dev_priv->psr.y_cord_support)
chicken_trans |= CHICKEN_TRANS_BIT15;

>  
>   if (!HAS_PSR(dev_priv)) {
>   DRM_DEBUG_KMS("PSR not supported on this platform\n");
> @@ -505,6 +505,13 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   dev_priv->psr.psr2_support = false;
>   else
>   skl_psr_setup_su_vsc(intel_dp);
> +
> + /* Set CHICKEN_TRANS_BIT15 if Y coordinate is supported 
> */
> + if (dev_priv->psr.y_cord_support)
> + chicken_trans = CHICKEN_TRANS_BIT15;
> + /* Set CHICKEN_TRANS_BIT12 for programable header */
> + chicken_trans = chicken_trans | CHICKEN_TRANS_BIT12;

or at least use |= here...


> + I915_WRITE(CHICKEN_TRANS(TRANS_EDP), chicken_trans);
>   } else {
>   /* set up vsc header for psr1 */
>   hsw_psr_setup_vsc(intel_dp);

[PATCH 09/10] drm/i915/psr: report live PSR2 State

[Vivi, Rodrigo]

Reviewed-by: Rodrigo Vivi 

On Fri, 2017-01-06 at 22:02 +0530, vathsala nagaraju wrote:
> Reports  live state of PSR2 form PSR2_STATUS register.
> bit field 31:28 gives the live state of PSR2.
> It can be used to check if system is in deep sleep,
> selective update or selective update standby.
> During video play back, we can use this to check
> if system is entering SU mode or not.
> when system is in idle state, DEEP_SLEEP(8) must be entered.
> When video playback is happening, system must be in
> SLEEP(3 / selective update) or SU_STANDBY( 6 / selective update standby)
> 
> v2: (Rodrigo)
> - Remove EDP_PSR2_STATUS_TG_ON=a ,instead use ARRAY_SIZE
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: Vathsala Nagaraju 
> Signed-off-by: Patil Deepti 
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 24 
>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9d7b5a8..ec9013e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2606,6 +2606,30 @@ static int i915_edp_psr_status(struct seq_file *m, 
> void *data)
>  
>   seq_printf(m, "Performance_Counter: %u\n", psrperf);
>   }
> + if (dev_priv->psr.psr2_support) {
> + static const char * const live_status[] = {
> + "IDLE",
> + "CAPTURE",
> + "CAPTURE_FS",
> + "SLEEP",
> + "BUFON_FW",
> + "ML_UP",
> + "SU_STANDBY",
> + "FAST_SLEEP",
> + "DEEP_SLEEP",
> + "BUF_ON",
> + "TG_ON" };
> + u8 pos = (I915_READ(EDP_PSR2_STATUS_CTL) &
> + EDP_PSR2_STATUS_STATE_MASK) >>
> + EDP_PSR2_STATUS_STATE_SHIFT;
> +
> + seq_printf(m, "PSR2_STATUS_EDP: %x\n",
> + I915_READ(EDP_PSR2_STATUS_CTL));
> +
> + if (pos < ARRAY_SIZE(live_status))
> + seq_printf(m, "PSR2 live state %s\n",
> + live_status[pos]);
> + }
>   mutex_unlock(_priv->psr.lock);
>  
>   intel_runtime_pm_put(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 272a283..65fffc5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3621,6 +3621,7 @@ enum {
>  
>  #define EDP_PSR2_STATUS_CTL_MMIO(0x6f940)
>  #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
> +#define EDP_PSR2_STATUS_STATE_SHIFT28
>  
>  /* VGA port control */
>  #define ADPA _MMIO(0x61100)

[PATCH 08/10] drm/i915/psr: enable psr2 for y cordinate panels

[Vivi, Rodrigo]

On Fri, 2017-01-06 at 22:21 +0530, vathsala nagaraju wrote:
> Psr2 is enabled only for y cordinate panels.Once GTC (global time code)
> is implemented,this restriction is removed so that psr2
> can work on panels without y cordinate support.
> 
> v2: (Rodrigo)
> - Move the check to intel_psr_match_conditions
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: Vathsala Nagaraju 
> Signed-off-by: Patil Deepti 
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 05efd4e..1729128 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -433,6 +433,15 @@ static bool intel_psr_match_conditions(struct intel_dp 
> *intel_dp)
>   return false;
>   }
>  
> + /*
> +  * FIXME:enable psr2 only for y-cordinate psr2 panels
> +  * After gtc implementation , remove this restriction.
> +  */
> + if (!dev_priv->psr.y_cord_support &&  dev_priv->psr.psr2_support) {
> + DRM_DEBUG_KMS("PSR2 disabled, panel does not support Y 
> coordinate\n");
> + return;


return false;

> + }
> +
>   dev_priv->psr.source_ok = true;
>   return true;
>  }

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

[Vivi, Rodrigo]

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

Regression in v4.8-rc4: i915 flickering since commit 1c80c25fb6

[Vivi, Rodrigo]

On Wed, 2016-09-07 at 15:37 +0300, Jani Nikula wrote:
> On Sun, 04 Sep 2016, Dominik Brodowski  wrote:
> > Hi!
> >
> > Since commit 1c80c25fb6 (determined by git bisect, and confirmed by
> > reverting this patch on top of 9ca581b50d), the sceen on my DELL XPS 13
> > is flickering every once in a while (sometimes multiple times per
> > second, sometimes only every few seconds).
> 
> *sigh* another PSR issue.

https://patchwork.kernel.org/patch/9121361/ 

Rodrigo Vivi - May 25, 2016, 10:52 p.m. "Hm, I believe this is
dangerous..."


> commit 1c80c25fb622973dd135878e98d172be20859049
> Author: Daniel Vetter 
> Date:   Wed May 18 18:47:12 2016 +0200
> 
> drm/i915/psr: Make idle_frames sensible again
> 
> Please file a bug at [1].
> 
> BR,
> Jani.
> 
> [1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel
> 
> 
> >
> > That's for
> >
> > 00:02.0 VGA compatible controller: Intel Corporation Broadwell-U Integrated 
> > Graphics (rev 09) (prog-if 00 [VGA controller])
> > Subsystem: Dell Device 0665
> > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
> > Stepping- SERR- FastB2B- DisINTx+
> > Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- 
> > SERR-  > Latency: 0
> > Interrupt: pin A routed to IRQ 42
> > Region 0: Memory at f600 (64-bit, non-prefetchable) [size=16M]
> > Region 2: Memory at e000 (64-bit, prefetchable) [size=256M]
> > Region 4: I/O ports at f000 [size=64]
> > [virtual] Expansion ROM at 000c [disabled] [size=128K]
> > Capabilities: 
> > Kernel driver in use: i915
> >
> > and Debian Jessie userland.
> >
> > Best,
> > Dominik
> 

[PATCH] drm: Make sure drm_vblank_no_hw_counter isn't abused

[Vivi, Rodrigo]

Reviewed-by: Rodrigo Vivi 

On Mon, 2016-08-08 at 18:24 +0200, Daniel Vetter wrote:
> Shouldn't be possible since everyone kzallocs this, but better safe
> than sorry. Random drive-by-idea really.
> 
> Cc: Rodrigo Vivi 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_irq.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index f5a9f8cef360..10611a936059 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1795,6 +1795,7 @@ EXPORT_SYMBOL(drm_crtc_handle_vblank);
>   */
>  u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int
> pipe)
>  {
> + WARN_ON_ONCE(dev->max_vblank_count != 0);
>  return 0;
>  }
>  EXPORT_SYMBOL(drm_vblank_no_hw_counter);