Re: [PATCH v2 3/4] drm/i915/alpm: Calculate ALPM Entry check
On Mon, 2024-01-29 at 02:08 +, Murthy, Arun R wrote:
>
>
> > -Original Message-
> > From: Intel-gfx On Behalf
> > Of Jouni
> > Högander
> > Sent: Friday, January 5, 2024 7:45 PM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [PATCH v2 3/4] drm/i915/alpm: Calculate ALPM Entry check
> >
> > ALPM Entry Check represents the number of lines needed to put the
> > main link
> > to sleep and keep it in the sleep state before it can be taken out
> > of the SLEEP
> > state (eDP requires the main link to be in the SLEEP state for a
> > minimum of
> > 5us).
> >
> > Bspec: 71477
> >
> > Signed-off-by: Jouni Högander
> > ---
> > .../drm/i915/display/intel_display_types.h | 3 ++
> > drivers/gpu/drm/i915/display/intel_psr.c | 28
> > +++
> > 2 files changed, 31 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 889a8b34b7ac..7eddef859ff4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1680,6 +1680,9 @@ struct intel_pps { struct alpm_parameters {
> > u8 io_wake_lines;
> > u8 fast_wake_lines;
> > +
> > + /* LNL and beyond */
> > + u8 check_entry_lines;
> > };
> >
> > struct intel_psr {
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 1709ebb31215..7fbd18f21c3b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1099,6 +1099,28 @@ static bool
> > _compute_psr2_sdp_prior_scanline_indication(struct intel_dp
> > *intel_d
> > return true;
> > }
> >
> > +static bool _lnl_compute_alpm_params(struct intel_dp *intel_dp,
> > + struct intel_crtc_state
> > *crtc_state) {
> > + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > + struct alpm_parameters *alpm_params = _dp-
> > >psr.alpm_params;
> > + int check_entry_lines;
> > +
> > + /* ALPM Entry Check = 2 + CEILING( 5us /tline ) */
> > + check_entry_lines = 2 +
> > + intel_usecs_to_scanlines(_state-
> > >hw.adjusted_mode, 5);
> > +
> > + if (check_entry_lines > 15)
> > + return false;
> > +
> > + if (i915->display.params.psr_safest_params)
> > + check_entry_lines = 15;
> > +
> > + alpm_params->check_entry_lines = check_entry_lines;
> > +
> > + return true;
> > +}
> > +
> > static bool _compute_alpm_params(struct intel_dp *intel_dp,
> > struct intel_crtc_state
> > *crtc_state) { @@ -
> > 1114,6 +1136,8 @@ static bool _compute_alpm_params(struct intel_dp
> > *intel_dp,
> > * it is not enough -> use 45 us.
> > */
> > fast_wake_time = 45;
> > +
> > + /* TODO: Check how we can use ALPM_CTL fast wake
> > extended
> > field */
> > max_wake_lines = 12;
> > } else {
> > io_wake_time = 50;
> > @@ -1130,6 +1154,10 @@ static bool _compute_alpm_params(struct
> > intel_dp
> > *intel_dp,
> > fast_wake_lines > max_wake_lines)
> > return false;
> >
> > + if (DISPLAY_VER(i915) >= 20 &&
> > !_lnl_compute_alpm_params(intel_dp,
> > +
> > crtc_state))
> The function name _lnl_*** indicates it should be display ver 20. So
> can this display ver check be moved to _lnl_*** ?
Yes, I can do that.
BR,
Jouni Högander
>
> Thanks and Regards,
> Arun R Murthy
> ---
> > + return false;
> > +
> > if (i915->display.params.psr_safest_params)
> > io_wake_lines = fast_wake_lines = max_wake_lines;
> >
> > --
> > 2.34.1
>
Re: [PATCH] drm/i915/gvt: Fix uninitialized variable in handle_mmio()
On 2024.01.26 11:41:47 +0300, Dan Carpenter wrote:
> This code prints the wrong variable in the warning message. It should
> print "i" instead of "info->offset". On the first iteration "info" is
> uninitialized leading to a crash and on subsequent iterations it prints
> the previous offset instead of the current one.
>
> Fixes: e0f74ed4634d ("i915/gvt: Separate the MMIO tracking table from GVT-g")
> Signed-off-by: Dan Carpenter
> ---
> drivers/gpu/drm/i915/gvt/handlers.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
> b/drivers/gpu/drm/i915/gvt/handlers.c
> index 90f6c1ece57d..efcb00472be2 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -2849,8 +2849,7 @@ static int handle_mmio(struct intel_gvt_mmio_table_iter
> *iter, u32 offset,
> for (i = start; i < end; i += 4) {
> p = intel_gvt_find_mmio_info(gvt, i);
> if (p) {
> - WARN(1, "dup mmio definition offset %x\n",
> - info->offset);
> + WARN(1, "dup mmio definition offset %x\n", i);
>
> /* We return -EEXIST here to make GVT-g load fail.
>* So duplicated MMIO can be found as soon as
> --
> 2.43.0
>
Thanks for the fix.
Reviewed-by: Zhenyu Wang
signature.asc
Description: PGP signature
RE: [PATCH v2 3/4] drm/i915/alpm: Calculate ALPM Entry check
> -Original Message-
> From: Intel-gfx On Behalf Of Jouni
> Högander
> Sent: Friday, January 5, 2024 7:45 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [PATCH v2 3/4] drm/i915/alpm: Calculate ALPM Entry check
>
> ALPM Entry Check represents the number of lines needed to put the main link
> to sleep and keep it in the sleep state before it can be taken out of the
> SLEEP
> state (eDP requires the main link to be in the SLEEP state for a minimum of
> 5us).
>
> Bspec: 71477
>
> Signed-off-by: Jouni Högander
> ---
> .../drm/i915/display/intel_display_types.h| 3 ++
> drivers/gpu/drm/i915/display/intel_psr.c | 28 +++
> 2 files changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 889a8b34b7ac..7eddef859ff4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1680,6 +1680,9 @@ struct intel_pps { struct alpm_parameters {
> u8 io_wake_lines;
> u8 fast_wake_lines;
> +
> + /* LNL and beyond */
> + u8 check_entry_lines;
> };
>
> struct intel_psr {
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 1709ebb31215..7fbd18f21c3b 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1099,6 +1099,28 @@ static bool
> _compute_psr2_sdp_prior_scanline_indication(struct intel_dp *intel_d
> return true;
> }
>
> +static bool _lnl_compute_alpm_params(struct intel_dp *intel_dp,
> + struct intel_crtc_state *crtc_state) {
> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + struct alpm_parameters *alpm_params = _dp->psr.alpm_params;
> + int check_entry_lines;
> +
> + /* ALPM Entry Check = 2 + CEILING( 5us /tline ) */
> + check_entry_lines = 2 +
> + intel_usecs_to_scanlines(_state->hw.adjusted_mode, 5);
> +
> + if (check_entry_lines > 15)
> + return false;
> +
> + if (i915->display.params.psr_safest_params)
> + check_entry_lines = 15;
> +
> + alpm_params->check_entry_lines = check_entry_lines;
> +
> + return true;
> +}
> +
> static bool _compute_alpm_params(struct intel_dp *intel_dp,
>struct intel_crtc_state *crtc_state) { @@ -
> 1114,6 +1136,8 @@ static bool _compute_alpm_params(struct intel_dp
> *intel_dp,
>* it is not enough -> use 45 us.
>*/
> fast_wake_time = 45;
> +
> + /* TODO: Check how we can use ALPM_CTL fast wake extended
> field */
> max_wake_lines = 12;
> } else {
> io_wake_time = 50;
> @@ -1130,6 +1154,10 @@ static bool _compute_alpm_params(struct intel_dp
> *intel_dp,
> fast_wake_lines > max_wake_lines)
> return false;
>
> + if (DISPLAY_VER(i915) >= 20 && !_lnl_compute_alpm_params(intel_dp,
> + crtc_state))
The function name _lnl_*** indicates it should be display ver 20. So can this
display ver check be moved to _lnl_*** ?
Thanks and Regards,
Arun R Murthy
---
> + return false;
> +
> if (i915->display.params.psr_safest_params)
> io_wake_lines = fast_wake_lines = max_wake_lines;
>
> --
> 2.34.1
[drm-tip:drm-tip 1/7] drivers/gpu/drm/bridge/samsung-dsim.c:1504:3: error: implicit declaration of function 'samsung_dsim_set_stop_state' is invalid in C99
tree: git://anongit.freedesktop.org/drm/drm-tip drm-tip
head: 0f1b42b9d395bd4097b2846230a13869dc638216
commit: cd3a0e22e5de2867cd98b5223094a467a5b0993d [1/7] Merge remote-tracking
branch 'drm-misc/drm-misc-next' into drm-tip
config: arm-defconfig
(https://download.01.org/0day-ci/archive/20240129/202401291018.wgyuxgmh-...@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git
f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
(https://download.01.org/0day-ci/archive/20240129/202401291018.wgyuxgmh-...@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot
| Closes:
https://lore.kernel.org/oe-kbuild-all/202401291018.wgyuxgmh-...@intel.com/
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/bridge/samsung-dsim.c:1504:3: error: implicit declaration of
>> function 'samsung_dsim_set_stop_state' is invalid in C99
>> [-Werror,-Wimplicit-function-declaration]
samsung_dsim_set_stop_state(dsi, true);
^
drivers/gpu/drm/bridge/samsung-dsim.c:1504:3: note: did you mean
'samsung_dsim_set_phy_ctrl'?
drivers/gpu/drm/bridge/samsung-dsim.c:749:13: note:
'samsung_dsim_set_phy_ctrl' declared here
static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
^
drivers/gpu/drm/bridge/samsung-dsim.c:1629:22: error: use of undeclared
identifier 'samsung_dsim_atomic_disable'; did you mean
'samsung_dsim_atomic_enable'?
.atomic_disable = samsung_dsim_atomic_disable,
^~~
samsung_dsim_atomic_enable
drivers/gpu/drm/bridge/samsung-dsim.c:1487:13: note:
'samsung_dsim_atomic_enable' declared here
static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
^
2 errors generated.
vim +/samsung_dsim_set_stop_state +1504 drivers/gpu/drm/bridge/samsung-dsim.c
e7447128ca4a25 Jagan Teki 2023-03-08 1497
e7447128ca4a25 Jagan Teki 2023-03-08 1498 static void
samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
e7447128ca4a25 Jagan Teki 2023-03-08 1499
struct drm_bridge_state *old_bridge_state)
e7447128ca4a25 Jagan Teki 2023-03-08 1500 {
e7447128ca4a25 Jagan Teki 2023-03-08 1501 struct samsung_dsim
*dsi = bridge_to_dsi(bridge);
e7447128ca4a25 Jagan Teki 2023-03-08 1502
b2fe2292624ac4 Dario Binacchi 2023-12-18 1503 if
(!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
b2fe2292624ac4 Dario Binacchi 2023-12-18 @1504
samsung_dsim_set_stop_state(dsi, true);
e7447128ca4a25 Jagan Teki 2023-03-08 1505
e7447128ca4a25 Jagan Teki 2023-03-08 1506 dsi->state &=
~DSIM_STATE_ENABLED;
e7447128ca4a25 Jagan Teki 2023-03-08 1507
pm_runtime_put_sync(dsi->dev);
e7447128ca4a25 Jagan Teki 2023-03-08 1508 }
e7447128ca4a25 Jagan Teki 2023-03-08 1509
:: The code at line 1504 was first introduced by commit
:: b2fe2292624ac4fc98dcdaf76c983d3f6e8455e5 drm: bridge: samsung-dsim:
enter display mode in the enable() callback
:: TO: Dario Binacchi
:: CC: Robert Foss
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: Re: [PATCH] drm/atomic-helpers: remove legacy_cursor_update hacks
On Thu, Jan 25, 2024 at 07:17:21PM +0100, Daniel Vetter wrote: > On Tue, Jan 23, 2024 at 06:09:05AM +, Jason-JH Lin (林睿祥) wrote: > > Hi Maxime, Daniel, > > > > We encountered similar issue with mediatek SoCs. > > > > We have found that in drm_atomic_helper_commit_rpm(), when disabling > > the cursor plane, the old_state->legacy_cursor_update in > > drm_atomic_wait_for_vblank() is set to true. > > As the result, we are not actually waiting for a vlbank to wait for our > > hardware to close the cursor plane. Subsequently, the execution > > proceeds to drm_atomic_helper_cleanup_planes() to free the cursor > > buffer. This can lead to use-after-free issues with our hardware. > > > > Could you please apply this patch to fix our problem? > > Or are there any considerations for not applying this patch? > > Mostly it needs someone to collect a pile of acks/tested-by and then land > it. > > I'd be _very_ happy if someone else can take care of that ... > > There's also the potential issue that it might slow down some of the > legacy X11 use-cases that really needed a non-blocking cursor, but I think > all the drivers where this matters have switched over to the async plane > update stuff meanwhile. So hopefully that's good. I think there was also a regression with msm no one really figured out? Maxime signature.asc Description: PGP signature
