Currently we are observing mouse cursor stuttering when using
xrandr --scaling=1.2x1.2. X scaling/transformation seems to be
doing fronbuffer rendering. When moving mouse cursor X seems to
perform several invalidates and only one DirtyFB. I.e. it seems
to be assuming updates are sent to panel while drawing is done.
Earlier we were disabling PSR in frontbuffer invalidate call back
(when drawing in X started). PSR was re-enabled in frontbuffer
flush callback (dirtyfb ioctl). This was working fine with X
scaling/transformation. Now we are just enabling continuous full
frame (cff) in PSR invalidate callback. Enabling cff doesn't
trigger any updates. It just configures PSR to send full frame
when updates are sent. I.e. there are no updates on screen before
PSR flush callback is made. X seems to be doing several updates
in frontbuffer before doing dirtyfb ioctl.
Fix this by sending single update on every invalidate callback.
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Fixes: 805f04d42a ("drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations")
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6679
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
Reported-by: Brian J. Tarricone <brian@tarricone.org>
Tested-by: Brian J. Tarricone <brian@tarricone.org>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221024054649.31299-1-jouni.hogander@intel.com
Not all Dekel PHY registers have a lane instance, so having to specify
this when using them is awkward. It makes more sense to define each PHY
register with its full internal PHY offset where bits 15:12 is the lane
for lane-instanced PHY registers and just a register bank index for other
PHY registers. This way lane-instanced registers can be referred to with
the (tc_port, lane) parameters, while other registers just with a tc_port
parameter.
An additional benefit of this change is to prevent passing a Dekel
register to a generic MMIO access function or vice versa.
v2:
- Fix parameter reuse in the DKL_REG_MMIO definition.
v3:
- Rebase on latest patchset version.
Cc: Jani Nikula <jani.nikula@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221025114457.2191004-3-imre.deak@intel.com
Accessing the TypeC DKL PHY registers during modeset-commit,
-verification, DP link-retraining and AUX power well toggling is racy
due to these code paths being concurrent and the PHY register bank
selection register (HIP_INDEX_REG) being shared between PHY instances
(aka TC ports) and the bank selection being not atomic wrt. the actual
PHY register access.
Add the required locking around each PHY register bank selection->
register access sequence.
Kudos to Ville for noticing the race conditions.
v2:
- Add the DKL PHY register accessors to intel_dkl_phy.[ch]. (Jani)
- Make the DKL_REG_TC_PORT macro independent of PHY internals.
- Move initing the DKL PHY lock to a more logical place.
v3:
- Fix parameter reuse in the DKL_REG_TC_PORT definition.
- Document the usage of phy_lock.
v4:
- Fix adding TC_PORT_1 offset in the DKL_REG_TC_PORT definition.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: <stable@vger.kernel.org> # v5.5+
Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221025114457.2191004-1-imre.deak@intel.com
Make glk_load_luts() a bit lighter for the common case
where neither the degamma LUT nor pipe CSC are enabled
by not loading the linear degamma LUT. Making .load_luts()
as lightweight as possible is a good idea since it may need
to execute from a vblank worker under tight deadlines.
My earlier reasoning for always loading the linear degamma LUT
was to avoid an extra LUT load when just enabling/disabling the
pipe CSC, but that is nonsense since we load the LUTs on every
flagged color management change/modeset anyway (either of which
is needed for a pipe CSC toggle).
We can also get rid of the glk_can_preload_luts() special
case since the presence of the degamma LUT will now always
match csc_enable.
v2: Fix typos (Uma)
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221024161514.5340-6-ville.syrjala@linux.intel.com
Add an extra remapping step between the logical state of the LUTs
(hw.(de)gamma_lut) as specified via uapi/bigjoiner copy vs.
the actual state of the LUTs programmed into the hardware.
With this we should be finally able finish the (de)gamma
readout/state checker support for the remaining platforms
(ilk-skl) where the same hardware LUT can be positioned
either before or after the pipe CSC unit. Where we position
it depends on factors such as presence of the logical degamma
LUT, RGB vs. YCbCr output, full vs. limited RGB quantization
range.
Without the extra remapping step the state readout doesn't
really know whether the LUT read from the hardware is the
degamma or gamma LUT, and so we is unable to accurately store
it into our crtc state. With the remapping step we know
exactly where to put it given the order of the LUT vs. CSC
in the hardware state.
Only the initial hw->uapi state readout done during driver
load/resume still has the problem of not really knowing
what to do with the LUT(s). But we can just assume 1:1
mapping there and let subsequent commits fix things up.
Another benefit is that we now have a place for purely
internal LUTs, without complicating the bigjoiner uapi->hw
copy logic. This should prove useful for streamlining
glk degamma LUT handling.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221024161514.5340-3-ville.syrjala@linux.intel.com
Be consistent in whether we flag a full modeset or a
fastset for the pipe. intel_modeset_all_pipes() would
seem to be the only codepath not getting this right.
The other case is when we flag the fastset initially,
currently we just clear the mode_changed flag and set
the update_pipe flag. But we could still have
connectors_changed==true or active_changed==true forcing
a full modeset anyway. So check for that after clearing
the mode_changed flag.
And let's add a WARN to make sure we did get it right.
v2: Deal with {connectors,active}_changed
Reviewed-by: Jani Nikula <jani.nikula@intel.com> #v1
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221021162442.27283-4-ville.syrjala@linux.intel.com
On BDW+ we have just the one set of DP M/N registers. The
values we write into said registers depends on whether we
want DRRS to be in high or low gear. This causes issues
for the state checker which currently has to assume either
set of M/N (high or low refresh rate) values may appear there.
That sort of works for M/N itself, but all other values
derived from the M/N (dotclock, pixel rate) are not handled
correctly, leading to potential for state checker mismatches.
Let's avoid all those problems by simply keeping DRRS in
high gear until the state checker has done its hardware
state readout.
Note that hitting this issue presumable became very hard
after commit 1b333c679a ("drm/i915: Do DRRS disable/enable
during pre/post_plane_update()") since the state check would
have to laze about for one full second (delay used by
intel_drrs_schedule_work()) to see the low refresh rate.
But it is still theoretically possible.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221020120706.25728-1-ville.syrjala@linux.intel.com
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
DGFX platforms has lmem and cpu can access the lmem objects
via mmap and i915 internal i915_gem_object_pin_map() for
i915 own usages. Both of these methods has pre-requisite
requirement to keep GFX PCI endpoint in D0 for a supported
iomem transaction over PCI link. (Refer PCIe specs 5.3.1.4.1)
Both DG1/DG2 have a known hardware bug that violates the PCIe specs
and support the iomem read write transaction over PCIe bus despite
endpoint is D3 state.
Due to above H/W bug, we had never observed any issue with i915 runtime
PM versus lmem access.
But this issue becomes visible when PCIe gfx endpoint's upstream
bridge enters to D3, at this point any lmem read/write access will be
returned as unsupported request. But again this issue is not observed
on every platform because it has been observed on few host machines
DG1/DG2 endpoint's upstream bridge does not bind with pcieport driver.
which really disables the PCIe power savings and leaves the bridge
at D0 state.
We need a unique interface to read/write from lmem with runtime PM
wakeref protection something similar to intel_uncore_{read, write},
keep autosuspend control to 'on' on all discrete platforms,
until we have a unique interface to read/write from lmem.
This just change the default autosuspend setting of i915 on dGPU,
user can still change it to 'auto'.
v2:
- Modified the commit message and subject with more information.
- Changed the Fixes tag to LMEM support commit. [Joonas]
- Changed !HAS_LMEM() Cond to !IS_DGFX(). [Rodrigo]
Fixes: b908be543e ("drm/i915: support creating LMEM objects")
Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221014113258.1284226-1-anshuman.gupta@intel.com
