| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The UHBR check was using > instead of >=. Use the helper instead to
avoid mistakes. Also always use the non-UHBR values for HDMI.
v2: Use intel_crtc_has_dp_encoder() && intel_dp_is_uhbr() (Ville)
Fixes: 2817efaeb608 ("drm/i915/dg2: add SNPS PHY translations for UHBR link rates")
Reported-by: Ville Syrjälä <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
As per the comment on top of acpi_evaluate_dsm():
| * Evaluate device's _DSM method with specified GUID, revision id and
| * function number. Caller needs to free the returned object.
We should free the returned object of acpi_evaluate_dsm() to avoid memory
leakage. Otherwise the kmemleak splat will be triggered at boot time (if we
compile kernel with CONFIG_DEBUG_TEST_DRIVER_REMOVE=y).
Fixes: 8e55f99c510f ("drm/i915: Invoke another _DSM to enable MUX on HP Workstation laptops")
Cc: Kai-Heng Feng <[email protected]>
Signed-off-by: Zenghui Yu <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
I suppose intel_dp_dump_link_status() might be useful for diagnosing
link training failures. Hoever we only call from the channel EQ phase
currently. Let's call it from the CR phase as well.
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Imre Deak <[email protected]>
|
|
Unify all debug prints during link training to include information
on both the encoder and the LTTPR. We unify the format to something
like "[ENCODER:1:FOO][LTTPR 1] Something something". Though not
sure if those brackets around the dp_phy just make it look like
line noise? I'll accept suggestions on better formatting.
I'm slightly on the fence about also including the connector,
but technically only the DPRX is the SST connector (ie.
intel_dp->attached_connector). I suppose you could think of it
as the branch device/whatever in the topology, and we're training
the link leading to it. So that could argue for its inclusion.
But it's all getting a bit long alrady, so not going to do it
I think.
v2: Keep the connector name in the final passed/failed debug print
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Imre Deak <[email protected]>
|
|
Print out each DP vswing adjustment request we got from the RX.
Could help in diagnosing what's going on during link training.
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Imre Deak <[email protected]>
|
|
Indicate which LTTPR we're currently attempting to train when
we print which training pattern we're using.
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Imre Deak <[email protected]>
|
|
Currently we consider the max vswing reached when we transmit a
the max voltage level, but we don't consider pre-emphasis at all.
This kinda matches older DP specs that only had some vague text
about transmitting the maximum voltage swing. Latest versions
now say something vague about consider the sum of the vswing
and pre-emphasis fields in the ADJUST_REQUEST_LANE registers.
Very vague, and super confusing especially the fact that it
talks about transmitted voltgage swing in the same sentence
as it say to look at the requested values.
Also glanced at the link CTS spec, and that one seems to have
tests that assume contradicting behaviour. Some say to consider
just the vswing level we transmit, others say to check for
sum of transmitted vswing+preemph being 3.
So let's try to take some kind of sane middle ground here.
I think what could make sense is only consider max vswing
reached if MAX_SWING_REACHED==1 _and_ vswing+preemph==3.
That will allow things to go all the way up to vswing 3 +
pre-emph 0 or vswing 2 + pre-emph 1, depending on what
the maximum supported vswing is. Only considering the sum
of vswing+pre-emph doesn't make much sense to me since
we could terminate too early if the sink requests eg.
vswing 0 + pre-emph 3. And if we'd stick to the current
code we could terminate too early of the sink asks for
vswing 2 + pre-emph 0 when vswing level 3 is not supported.
Side note: I don't really understand why any of this stuff is
"specified" at all. There is already a limit of 5 attempts at
the same vswing+pre-emph level, and a total limit of 10
attempts. So might as well stick to the same max 5 attempts
across the board IMO.
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Imre Deak <[email protected]>
|
|
The "digi_port" pointer can't be NULL and we have already dereferenced
it so checking for NULL is not necessary. Delete the check.
Signed-off-by: Dan Carpenter <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/20211004103737.GC25015@kili
|
|
Since the VT-d vs. async flip issues are plaguing a wider range
of supported hw let's try to minimize the impact on normal
operation by flipping the relevant chicken bits on and off
as needed. I presume there is some power/perf impact on since
this is reducing some prefetching I think.
Cc: Karthik B S <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Matt Roper <[email protected]>
|
|
Looks like skl/bxt/derivatives also need the plane stride
stretch w/a when using async flips and VT-d is enabled, or
else we get corruption on screen. To my surprise this was
even documented in bspec, but only as a note on the
CHICHKEN_PIPESL register description rather than on the
w/a list.
So very much the same thing as on HSW/BDW, except the bits
moved yet again.
Cc: [email protected]
Cc: Karthik B S <[email protected]>
Fixes: 55ea1cb178ef ("drm/i915: Enable async flips in i915")
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Matt Roper <[email protected]>
|
|
LTTPRs should support per-lane drive settings I think, and even if
they don't they should implement their own fallback logic to determine
suitable common drive settings to use for all the lanes.
v2: Actually check the correct thing
Reviewed-by: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Adjust the link training code to accommodate per-lane drive settings,
if supported by the platform. Actually enabling this will involve
some changes to each platform's .set_signal_level() implementation,
so for the moment all supported platforms will keep using the current
codepath that just uses the same drive settings for all the lanes.
v2: Fix min() vs. max() fumble
v3: Compact the debug print to a single line
Reviewed-by: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
In order to have per-lane drive settings we need intel_ddi_level()
to accept the lane as a parameter. That is, the eventual goal is to
call intel_ddi_level() once for each lane. For now we just pass in
a hardcoded 0 and use the same settings for every lane. Ie. no
change in behaviour yet.
Reviewed-by: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Since intel_ddi_level() now looks at the buf_trans table there's
no point in having intel_ddi_hdmi_num_entries() around. Just
roll the necessary bits of locic into
intel_ddi_hdmi_level()/intel_ddi_level().
Reviewed-by: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
All callers of intel_ddi_level() duplicate the check+WARN
to make sure the returned level is actually present in the
appropriate buf_trans table. Let's push that stuff into
intel_ddi_level() so the callers don't have to worry about it.
Reviewed-by: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Convert bxt_ddi_phy_set_signal_levels() to act as the full
.set_signal_levels() hook instead of going through a pointless wrapper.
Reviewed-by: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Now that .set_signal_levels() is used for HDMI as well, we can
remove the extra level of indirection and just plug the correct
stuff straight into .set_signal_levels().
Reviewed-by: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Currently .set_signal_levels() is only used by encoders in DP mode.
For most modern platforms there is no essential difference between
DP and HDMI, and both codepaths just end up calling the same function
under the hood. Let's get remove the need for that extra indirection
by moving .set_signal_levels() into the encoder from intel_dp.
Since we already plumb the crtc_state/etc. into .set_signal_levels()
the code will do the right thing for both DP and HDMI.
HSW/BDW/SKL are the only platforms that need a bit of care on
account of having to preload the hardware buf_trans register
with the full set of values. So we must still remember to call
hsw_prepare_{dp,hdmi}_ddi_buffers() to do said preloading, and
.set_signal_levels() will just end up selecting the correct entry
for DP, and also setting up the iboost magic for both DP and HDMI.
Note that previously on HSW/BDW/SKL we did write to DDI_BUF_CTL to
select the correct entry until link training started, now that we
call .set_signal_levels() already from hsw_ddi_pre_enable_dp() that
is no longer the case. But it's all safe now that the
intel_ddi_init_dp_buf_reg() call was hoisted up and it no longer
sets up the DDI_BUF_CTL_ENABLE bit (that is still deferred until
link training).
v2: Rebase due to has_{iboost,buf_trans_select}()
Add some notes about the DDI_BUF_CTL situation on HSW/BDW/SKL (Imre)
Reviewed-by: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Add a small helper to determine if DDI_BUF_CTL uses the
DDI_BUF_TRANS_SELECT field, and whether we have the
accompanying DDI_BUF_TRANS table in the hardware.
Cc: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Imre Deak <[email protected]>
|
|
Suck the "do we have iboost?" platform checks into a small helper.
Cc: Imre Deak <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Imre Deak <[email protected]>
|
|
The DP spec says:
"If the receiver keeps the same value in the ADJUST_REQUEST_LANEx_y
register(s) while the LANEx_CR_DONE bits remain unset, the transmitter
must loop four times with the same voltage swing. On the fifth time,
the transmitter must down-shift to the lower bit rate and must repeat
the CR-lock training sequence as described below."
Lets fix the code to follow that instead of terminating after five
times of transmitting the same signal levels. The text in spec feels
a little bit ambiguous still, but this is my best guess at its meaning.
As a bonus this also gets rid of the train_set[0] stuff which
would not work for per-lane drive settings anyway.
Cc: Imre Deak <[email protected]>
CC: Jani Nikula <[email protected]>
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Imre Deak <[email protected]>
|
|
For controlling the audio SDP split.
Bspec: 63837
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
This was causing infinite recursion on snb/ivb.
Fixes: 5716c8c6f4b6 ("drm/i915/uncore: split the fw get function into separate vfunc")
Signed-off-by: Dave Airlie <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
While sanitizing the hardware state we're currently forcing
the pipe bottom color legacy csc/gamma bits on. That is not a
good idea as BIOSen are likely to leave gabage in the LUTs and
so doing this causes ugly visual glitches if and when the
planes covering the background get disabled. This was exactly
the case on this Dell Precision 5560 tgl laptop.
On icl+ we don't normally even use these legacy bits
anymore and instead use their GAMMA_MODE counterparts.
On earlier platforms the bits are used, but we still
shouldn't force them on without knowing what's in the LUT.
So two options, get rid of the whole thing, or do what
intel_color_commit() does to make sure the bottom color state
matches whatever out hardware readout produced. I chose the
latter since it'll match what happens on older platforms when
the primary plane gets turned off. In fact let's just call
intel_color_commit(). It'll also do some CSC programming but
since we don't have readout for that it'll actually just set
to all zeros. So in the unlikely case of CSC actually being
enabld by the BIOS we'll end up with all black until the first
atomic commit happens.
Still not totally sure what we should do about color management
features here in general. Probably the safest thing would be to
force everything off exactly at the same time when we disable
the primary plane as there is no guarantees that whatever the
LUTs/CSCs contain make any sense whatsoever without the
specific pixel data in the BIOS fb. And if we preserve the
primary plane then we should disable the color management
features exactly when the primary plane fb contents first
changes since the new content assumes more or less no
transformations. But of course synchronizing front buffer
rendering with anything else is a bit hard...
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3534
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Uma Shankar <[email protected]>
|
|
Check for the zero length front porch already in intel_mode_valid()
so that we get the same validation for both get_modes() and setcrtc()/etc.
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Jani Nikula <[email protected]>
|
|
"CRTC fixup failed" is probably leftovers from pre-atomic days
when there was an actual fixup() function. Let's unify the debug
messages between encoder vs. crtc compute_config() calls.
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Jani Nikula <[email protected]>
|
|
Unify how we check for -EDEADLK vs. other errors from
crtc vs. encoder compute_config() calls.
Signed-off-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Reviewed-by: Jani Nikula <[email protected]>
|
|
Prefer the intel_ types. No functional changes.
v2: Fix build.
Reviewed-by: Rodrigo Vivi <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Failures to register debugfs should be ignored anyway, so stop
propagating errors altogether for clarity and simplicity. No functional
changes.
Reviewed-by: Rodrigo Vivi <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/346562ccef2282ccdbdea54409fab1d2b48f313c.1630327990.git.jani.nikula@intel.com
|
|
The debugfs file shows it's not capable, don't duplicate the info.
Reviewed-by: Rodrigo Vivi <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/939453050a5a5175a12a08f16542c1b40bd726dc.1630327990.git.jani.nikula@intel.com
|
|
Prefer i915 over drm pointer.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Using standard -EAGAIN should be perfectly fine instead of using a
special case value.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
Avoid using the incidental -EPERM.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/e2f79220ed2558f615c051e2533275a5dae1a04f.1633000838.git.jani.nikula@intel.com
|
|
Avoid using the incidental -EPERM. Return the -EIO directly from
i915_get_bridge_dev() instead of converting return values later.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/1ee72c31963d8be98490cd78f7c1182ba4f54c13.1633000838.git.jani.nikula@intel.com
|
|
Avoid using the incidental -EPERM.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/8acf7ffe9222d23c7f47dbd95ff1f737221ff72c.1633000838.git.jani.nikula@intel.com
|
|
Avoid using the incidental -EPERM. Also remove useless comment.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/37df1edc6d3745997cec2dfe41520d9f704e14b4.1633000838.git.jani.nikula@intel.com
|
|
Having two functions for this seems like excess duplication and
parameter juggling. Merge them together.
While at it, drop the extra error message, as wait_for_payload_credits()
already prints an error, and switch from incidental -EPERM (i.e. -1) to
actual error codes.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/f74f7462a36e76070db6b4c01616d0eb663b9938.1633000838.git.jani.nikula@intel.com
|
|
Pass a const pointer instead of passing 32 bytes of struct
mipi_dsi_packet by value.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/c67d2fa0d97bf336a321497775b9717d85d44a51.1633000838.git.jani.nikula@intel.com
|
|
Keep the functionality and the assert code together.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/0a5fa9b8d4d4615d4e6503b6bb33541c0bccffbb.1632992608.git.jani.nikula@intel.com
|
|
Keep the functionality and the assert code together.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/0229659fb8af6c91c774408c6f7bb8c4ff8735e3.1632992608.git.jani.nikula@intel.com
|
|
Move assert_panel_unlocked() to intel_pps.c and rename
assert_pps_unlocked(). Keep the functionality and the assert code
together.
There's still a bit of a split between the eDP PPS usage in intel_pps.c
and all the other PPS usage, and assert_pps_unlocked() is arguably more
related to the latter. However, intel_pps.c is the best fit for anything
touching the PPS registers.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/a9b77692a145891789eefb0447e082cfc22aaa85.1632992608.git.jani.nikula@intel.com
|
|
Keep the functionality and the assert code together.
Reviewed-by: Ville Syrjälä <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/427d27eb4e5daca208d496d6c2ffc91ed90ba714.1632992608.git.jani.nikula@intel.com
|
|
With all the past fixes now this feature is functional and can be
enabled by default in desktop enviroments that uses compositor.
Cc: Gwan-gyeong Mun <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Reviewed-by: Gwan-gyeong Mun <[email protected]>
Signed-off-by: José Roberto de Souza <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
With all the recent fixes PSR2 is properly working in Alderlake-P but
due to some issues that don't have software workarounds it will not be
supported in display steppings older than B0.
Even with this patch PSR2 will no be enabled by default in ADL-P, it
still requires enable_psr2_sel_fetch to be set to true, what some
of our tests does.
Cc: Gwan-gyeong Mun <[email protected]>
Reviewed-by: Gwan-gyeong Mun <[email protected]>
Signed-off-by: José Roberto de Souza <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
The Wa_14014971508 is required to fix scanout when a feature that i915
do not support is enabled and this feature is not planned to be enabled
for adlp.
Keeping this workaround enabled can badly hurt power-savings when
a full frame fetch is required(see psr2_sel_fetch_plane_state_supported()
and psr2_sel_fetch_pipe_state_supported()).
Here a example that could badly hurt power-savings, userspace does
a page flip to a rotated plane, so CONTINUOS_FULL_FRAME set.
But then for a whole 30 seconds nothing in the screen requires updates
but because CONTINUOS_FULL_FRAME is set, it will not go into DC5/DC6.
Reverting Wa_14014971508 fixes that, as only a single frame will be
sent and then display can go to DC5/DC6 for those 30 seconds of
idleness.
BSpec: 54369
Cc: Gwan-gyeong Mun <[email protected]>
Signed-off-by: José Roberto de Souza <[email protected]>
Reviewed-by: Gwan-gyeong Mun <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
enabled
Legacy cursor APIs are handled by intel_legacy_cursor_update(), that
calls drm_atomic_helper_update_plane() when going through the
slow/atomic path to update cursor, what was the case for PSR2
selective fetch.
drm_atomic_helper_update_plane() sets
drm_atomic_state->legacy_cursor_update to true when updating the
cursor plane, to allow several cursor updates to happen within the
same frame, as userspace does that.
If drivers waited for a vblank increment at the end of every cursor
movement that would cause a visible lag in the cursor.
But this optimization do not properly work with PSR2 selective fetch
dirt area calculation, for example if within a single frame the cursor
had 3 moves the final dirt area programmed to PSR2_MAN_TRK_CTL would
be based in the second movement as old state and third movement as new
state, not updating the area where cursor was in the first state.
So here switching back to the fast path approach in
intel_legacy_cursor_update() and handling cursor movements as
frontbuffer rendering(psr_force_hw_tracking_exit()), that is not the
most optimal for power-savings but is the solution that we have until
mailbox style updates is implemented.
Also removing the cursor workaround as not it is properly undestand
the issue and is know that it will never cover all the cases.
Cc: Ville Syrjälä <[email protected]>
Cc: Gwan-gyeong Mun <[email protected]>
Signed-off-by: José Roberto de Souza <[email protected]>
Acked-by: Ville Syrjälä <[email protected]>
Reviewed-by: Gwan-gyeong Mun <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
enabled
When PSR2 selective fetch is enabled writes to CURSURFLIVE alone do
not causes the panel to be updated when doing frontbuffer rendering.
From what I was able to figure from experiments the writes to
CURSURFLIVE takes PSR2 from deep sleep but panel is not updated
because PSR2_MAN_TRK_CTL has no start and end region set.
As we don't have the dirt area from current flush and invalidate API
and even if we did userspace could do several draws to frontbuffer and
we would need a way to append all the damaged areas of all the draws
that need to be part of next frame.
So here only programing PSR2_MAN_TRK_CTL to do a single full frame
fetch.
It is a safe approach as if scanout is in the visible area
the single full frame will only be visible for hardware in the next
frame because of the double buffering, and if scanout is in vblank
area it will be draw in the current frame.
No need to disable PSR and wait a few miliseconds to enable it again.
Cc: Gwan-gyeong Mun <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Signed-off-by: José Roberto de Souza <[email protected]>
Reviewed-by: Gwan-gyeong Mun <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
This unnecessary flushes are hurting power-savings are it causes
features like PSR, FBC and DRRS to disable it self to handle
frontbuffer rendering, below some explanation of why each removed
call is not necessary.
The flush in intel_prepare_plane_fb() is not required as framebuffer
will be flipped and power-saving features do the proper flip handling
in hardware.
intel_find_initial_plane_obj() flush is not required because it is
only executed during driver load and at this point the power-saving
features are not even enabled.
And the last one intelfb_create(), is also not required as at this
point the fbdev was just allocated, userspace will draw on
it what will trigger frontbuffer invalidates and flushes later on.
Cc: Ville Syrjälä <[email protected]>
Cc: Gwan-gyeong Mun <[email protected]>
Signed-off-by: José Roberto de Souza <[email protected]>
Reviewed-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|
|
We are still missing the PSR2 selective fetch handling of multi-planar
formats but until proper handle is added we can workaround it by
doing full frames fetch when state has such formats.
Cc: Ville Syrjälä <[email protected]>
Signed-off-by: Gwan-gyeong Mun <[email protected]>
Reviewed-by: Ville Syrjälä <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
Signed-off-by: José Roberto de Souza <[email protected]>
|
|
PSR2 selective is not supported over rotated and scaled planes.
We had the rotation check in intel_psr2_sel_fetch_config_valid()
but that code path is only execute when a modeset is needed and
those plane parameters can change without a modeset.
Pipe selective fetch restrictions are also needed, it could be added
in intel_psr_compute_config() but pippe scaling is computed after
it is executed, so leaving as is for now.
There is no much loss in this approach as it would cause selective
fetch to not enabled as for alderlake-P and newer will cause it to
switch to PSR1 that will have the same power-savings as do full pipe
fetch.
Also need to check those restricions in the second
for_each_oldnew_intel_plane_in_state() loop because the state could
only have a plane that is not affected by those restricitons but
the damaged area intersect with planes that has those restrictions,
so a full pipe fetch is required.
v2:
- also handling pipe restrictions
BSpec: 55229
Reviewed-by: Gwan-gyeong Mun <[email protected]> # v1
Cc: Ville Syrjälä <[email protected]>
Cc: Gwan-gyeong Mun <[email protected]>
Signed-off-by: José Roberto de Souza <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
|