| Message ID | 20250106165111.1672722-4-mitulkumar.ajitkumar.golani@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Check Scaler and DSC Prefill Latency Against Vblank | expand |
On 1/6/2025 10:21 PM, Mitul Golani wrote: > Compute scaling factors and scaler user for pipe scaler if > particular scaler user is pipe scaler. > > --v2: > - Update typo 'plane scaling' to 'pipe scaling'. [Ankit] > - Remove FIXME tag which was added to check for pipe scaling > factors as well. [Ankit] > - Should be common hscale, vscale instead of local one to > avoid garbage overwritten. > - Typo in commit message 'perticular' to 'particular'. > > Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> > --- > drivers/gpu/drm/i915/display/skl_scaler.c | 34 ++++++++++++++++++----- > 1 file changed, 27 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c > index 4f838220f880..294493e5ee53 100644 > --- a/drivers/gpu/drm/i915/display/skl_scaler.c > +++ b/drivers/gpu/drm/i915/display/skl_scaler.c > @@ -320,6 +320,7 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state, > &crtc_state->scaler_state; > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > u32 mode; > + int hscale, vscale; > > if (*scaler_id < 0) > *scaler_id = intel_allocate_scaler(scaler_state, crtc); > @@ -368,15 +369,11 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state, > mode = SKL_PS_SCALER_MODE_DYN; > } > > - /* > - * FIXME: we should also check the scaler factors for pfit, so > - * this shouldn't be tied directly to planes. > - */ > if (plane_state && plane_state->hw.fb) { > const struct drm_framebuffer *fb = plane_state->hw.fb; > const struct drm_rect *src = &plane_state->uapi.src; > const struct drm_rect *dst = &plane_state->uapi.dst; > - int hscale, vscale, max_vscale, max_hscale; > + int max_vscale, max_hscale; > > /* > * FIXME: When two scalers are needed, but only one of > @@ -426,11 +423,34 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state, > > return -EINVAL; > } > + } > + > + if (crtc_state->pch_pfit.enabled) { > + struct drm_rect src; > + > + drm_rect_init(&src, 0, 0, > + drm_rect_width(&crtc_state->pipe_src) << 16, > + drm_rect_height(&crtc_state->pipe_src) << 16); > + > + hscale = drm_rect_calc_hscale(&src, &crtc_state->pch_pfit.dst, > + 0, INT_MAX); > + vscale = drm_rect_calc_vscale(&src, &crtc_state->pch_pfit.dst, > + 0, INT_MAX); We need to do similar to plane scaler case, to check if hscale/vscale are less than the max scaling values. I think it would be better to make a function to get max_hscale and vscale for a platform. With 420 encoding, there is different limit so need to account that too. e.g Bspec:50441 Regards, Ankit > > - scaler_state->scalers[*scaler_id].hscale = hscale; > - scaler_state->scalers[*scaler_id].vscale = vscale; > + if (hscale < 0 || vscale < 0) { > + drm_dbg_kms(display->drm, > + "Scaler %d doesn't support required pipe scaling\n", > + *scaler_id); > + drm_rect_debug_print("src: ", &src, true); > + drm_rect_debug_print("dst: ", &crtc_state->pch_pfit.dst, false); > + > + return -EINVAL; > + } > } > > + scaler_state->scalers[*scaler_id].hscale = hscale; > + scaler_state->scalers[*scaler_id].vscale = vscale; > + > drm_dbg_kms(display->drm, "Attached scaler id %u.%u to %s:%d\n", > crtc->pipe, *scaler_id, name, idx); > scaler_state->scalers[*scaler_id].mode = mode;
Hi Mitul,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Mitul-Golani/drm-i915-scaler-Add-and-compute-scaling-factors/20250107-005708
base: git://anongit.freedesktop.org/drm-intel for-linux-next
patch link: https://lore.kernel.org/r/20250106165111.1672722-4-mitulkumar.ajitkumar.golani%40intel.com
patch subject: [PATCH v3 3/5] drm/i915/scaler: Compute scaling factors for pipe scaler
config: x86_64-randconfig-161-20250107 (https://download.01.org/0day-ci/archive/20250108/202501080440.Cxe0ZHXJ-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
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 <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202501080440.Cxe0ZHXJ-lkp@intel.com/
smatch warnings:
drivers/gpu/drm/i915/display/skl_scaler.c:451 intel_atomic_setup_scaler() error: uninitialized symbol 'hscale'.
drivers/gpu/drm/i915/display/skl_scaler.c:452 intel_atomic_setup_scaler() error: uninitialized symbol 'vscale'.
vim +/hscale +451 drivers/gpu/drm/i915/display/skl_scaler.c
61e24d4de0edbc Mitul Golani 2025-01-06 312 static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state,
7488f64e89b02f Ville Syrjälä 2024-11-07 313 int num_scalers_need, struct intel_crtc *crtc,
8976b18249407d Ville Syrjälä 2023-04-18 314 const char *name, int idx,
8976b18249407d Ville Syrjälä 2023-04-18 315 struct intel_plane_state *plane_state,
8976b18249407d Ville Syrjälä 2023-04-18 316 int *scaler_id)
8976b18249407d Ville Syrjälä 2023-04-18 317 {
3360d4d6d42ab3 Ville Syrjälä 2024-11-07 318 struct intel_display *display = to_intel_display(crtc);
61e24d4de0edbc Mitul Golani 2025-01-06 319 struct intel_crtc_scaler_state *scaler_state =
61e24d4de0edbc Mitul Golani 2025-01-06 320 &crtc_state->scaler_state;
7488f64e89b02f Ville Syrjälä 2024-11-07 321 struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
8976b18249407d Ville Syrjälä 2023-04-18 322 u32 mode;
7c369018e87617 Mitul Golani 2025-01-06 323 int hscale, vscale;
8976b18249407d Ville Syrjälä 2023-04-18 324
ecf2afc59c1ea3 Ville Syrjälä 2024-11-07 325 if (*scaler_id < 0)
ecf2afc59c1ea3 Ville Syrjälä 2024-11-07 326 *scaler_id = intel_allocate_scaler(scaler_state, crtc);
8976b18249407d Ville Syrjälä 2023-04-18 327
3360d4d6d42ab3 Ville Syrjälä 2024-11-07 328 if (drm_WARN(display->drm, *scaler_id < 0,
8976b18249407d Ville Syrjälä 2023-04-18 329 "Cannot find scaler for %s:%d\n", name, idx))
8976b18249407d Ville Syrjälä 2023-04-18 330 return -EINVAL;
8976b18249407d Ville Syrjälä 2023-04-18 331
8976b18249407d Ville Syrjälä 2023-04-18 332 /* set scaler mode */
8976b18249407d Ville Syrjälä 2023-04-18 333 if (plane_state && plane_state->hw.fb &&
8976b18249407d Ville Syrjälä 2023-04-18 334 plane_state->hw.fb->format->is_yuv &&
8976b18249407d Ville Syrjälä 2023-04-18 335 plane_state->hw.fb->format->num_planes > 1) {
8976b18249407d Ville Syrjälä 2023-04-18 336 struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
8976b18249407d Ville Syrjälä 2023-04-18 337
3360d4d6d42ab3 Ville Syrjälä 2024-11-07 338 if (DISPLAY_VER(display) == 9) {
8976b18249407d Ville Syrjälä 2023-04-18 339 mode = SKL_PS_SCALER_MODE_NV12;
8976b18249407d Ville Syrjälä 2023-04-18 340 } else if (icl_is_hdr_plane(dev_priv, plane->id)) {
8976b18249407d Ville Syrjälä 2023-04-18 341 /*
8976b18249407d Ville Syrjälä 2023-04-18 342 * On gen11+'s HDR planes we only use the scaler for
8976b18249407d Ville Syrjälä 2023-04-18 343 * scaling. They have a dedicated chroma upsampler, so
8976b18249407d Ville Syrjälä 2023-04-18 344 * we don't need the scaler to upsample the UV plane.
8976b18249407d Ville Syrjälä 2023-04-18 345 */
8976b18249407d Ville Syrjälä 2023-04-18 346 mode = PS_SCALER_MODE_NORMAL;
8976b18249407d Ville Syrjälä 2023-04-18 347 } else {
8976b18249407d Ville Syrjälä 2023-04-18 348 struct intel_plane *linked =
8976b18249407d Ville Syrjälä 2023-04-18 349 plane_state->planar_linked_plane;
8976b18249407d Ville Syrjälä 2023-04-18 350
8976b18249407d Ville Syrjälä 2023-04-18 351 mode = PS_SCALER_MODE_PLANAR;
8976b18249407d Ville Syrjälä 2023-04-18 352
8976b18249407d Ville Syrjälä 2023-04-18 353 if (linked)
ebb4e2b03acc3a Ville Syrjälä 2023-04-26 354 mode |= PS_BINDING_Y_PLANE(linked->id);
8976b18249407d Ville Syrjälä 2023-04-18 355 }
3360d4d6d42ab3 Ville Syrjälä 2024-11-07 356 } else if (DISPLAY_VER(display) >= 10) {
8976b18249407d Ville Syrjälä 2023-04-18 357 mode = PS_SCALER_MODE_NORMAL;
7488f64e89b02f Ville Syrjälä 2024-11-07 358 } else if (num_scalers_need == 1 && crtc->num_scalers > 1) {
8976b18249407d Ville Syrjälä 2023-04-18 359 /*
8976b18249407d Ville Syrjälä 2023-04-18 360 * when only 1 scaler is in use on a pipe with 2 scalers
8976b18249407d Ville Syrjälä 2023-04-18 361 * scaler 0 operates in high quality (HQ) mode.
8976b18249407d Ville Syrjälä 2023-04-18 362 * In this case use scaler 0 to take advantage of HQ mode
8976b18249407d Ville Syrjälä 2023-04-18 363 */
3de26c81e9ae4a Ville Syrjälä 2024-11-07 364 scaler_state->scalers[*scaler_id].in_use = false;
8976b18249407d Ville Syrjälä 2023-04-18 365 *scaler_id = 0;
3de26c81e9ae4a Ville Syrjälä 2024-11-07 366 scaler_state->scalers[0].in_use = true;
8976b18249407d Ville Syrjälä 2023-04-18 367 mode = SKL_PS_SCALER_MODE_HQ;
8976b18249407d Ville Syrjälä 2023-04-18 368 } else {
8976b18249407d Ville Syrjälä 2023-04-18 369 mode = SKL_PS_SCALER_MODE_DYN;
8976b18249407d Ville Syrjälä 2023-04-18 370 }
8976b18249407d Ville Syrjälä 2023-04-18 371
8976b18249407d Ville Syrjälä 2023-04-18 372 if (plane_state && plane_state->hw.fb) {
8976b18249407d Ville Syrjälä 2023-04-18 373 const struct drm_framebuffer *fb = plane_state->hw.fb;
8976b18249407d Ville Syrjälä 2023-04-18 374 const struct drm_rect *src = &plane_state->uapi.src;
8976b18249407d Ville Syrjälä 2023-04-18 375 const struct drm_rect *dst = &plane_state->uapi.dst;
7c369018e87617 Mitul Golani 2025-01-06 376 int max_vscale, max_hscale;
8976b18249407d Ville Syrjälä 2023-04-18 377
8976b18249407d Ville Syrjälä 2023-04-18 378 /*
8976b18249407d Ville Syrjälä 2023-04-18 379 * FIXME: When two scalers are needed, but only one of
8976b18249407d Ville Syrjälä 2023-04-18 380 * them needs to downscale, we should make sure that
8976b18249407d Ville Syrjälä 2023-04-18 381 * the one that needs downscaling support is assigned
8976b18249407d Ville Syrjälä 2023-04-18 382 * as the first scaler, so we don't reject downscaling
8976b18249407d Ville Syrjälä 2023-04-18 383 * unnecessarily.
8976b18249407d Ville Syrjälä 2023-04-18 384 */
8976b18249407d Ville Syrjälä 2023-04-18 385
3360d4d6d42ab3 Ville Syrjälä 2024-11-07 386 if (DISPLAY_VER(display) >= 14) {
8976b18249407d Ville Syrjälä 2023-04-18 387 /*
8976b18249407d Ville Syrjälä 2023-04-18 388 * On versions 14 and up, only the first
8976b18249407d Ville Syrjälä 2023-04-18 389 * scaler supports a vertical scaling factor
8976b18249407d Ville Syrjälä 2023-04-18 390 * of more than 1.0, while a horizontal
8976b18249407d Ville Syrjälä 2023-04-18 391 * scaling factor of 3.0 is supported.
8976b18249407d Ville Syrjälä 2023-04-18 392 */
8976b18249407d Ville Syrjälä 2023-04-18 393 max_hscale = 0x30000 - 1;
8976b18249407d Ville Syrjälä 2023-04-18 394 if (*scaler_id == 0)
8976b18249407d Ville Syrjälä 2023-04-18 395 max_vscale = 0x30000 - 1;
8976b18249407d Ville Syrjälä 2023-04-18 396 else
8976b18249407d Ville Syrjälä 2023-04-18 397 max_vscale = 0x10000;
8976b18249407d Ville Syrjälä 2023-04-18 398
3360d4d6d42ab3 Ville Syrjälä 2024-11-07 399 } else if (DISPLAY_VER(display) >= 10 ||
8976b18249407d Ville Syrjälä 2023-04-18 400 !intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier)) {
8976b18249407d Ville Syrjälä 2023-04-18 401 max_hscale = 0x30000 - 1;
8976b18249407d Ville Syrjälä 2023-04-18 402 max_vscale = 0x30000 - 1;
8976b18249407d Ville Syrjälä 2023-04-18 403 } else {
8976b18249407d Ville Syrjälä 2023-04-18 404 max_hscale = 0x20000 - 1;
8976b18249407d Ville Syrjälä 2023-04-18 405 max_vscale = 0x20000 - 1;
8976b18249407d Ville Syrjälä 2023-04-18 406 }
8976b18249407d Ville Syrjälä 2023-04-18 407
8976b18249407d Ville Syrjälä 2023-04-18 408 /*
8976b18249407d Ville Syrjälä 2023-04-18 409 * FIXME: We should change the if-else block above to
8976b18249407d Ville Syrjälä 2023-04-18 410 * support HQ vs dynamic scaler properly.
8976b18249407d Ville Syrjälä 2023-04-18 411 */
8976b18249407d Ville Syrjälä 2023-04-18 412
8976b18249407d Ville Syrjälä 2023-04-18 413 /* Check if required scaling is within limits */
8976b18249407d Ville Syrjälä 2023-04-18 414 hscale = drm_rect_calc_hscale(src, dst, 1, max_hscale);
8976b18249407d Ville Syrjälä 2023-04-18 415 vscale = drm_rect_calc_vscale(src, dst, 1, max_vscale);
8976b18249407d Ville Syrjälä 2023-04-18 416
8976b18249407d Ville Syrjälä 2023-04-18 417 if (hscale < 0 || vscale < 0) {
3360d4d6d42ab3 Ville Syrjälä 2024-11-07 418 drm_dbg_kms(display->drm,
8976b18249407d Ville Syrjälä 2023-04-18 419 "Scaler %d doesn't support required plane scaling\n",
8976b18249407d Ville Syrjälä 2023-04-18 420 *scaler_id);
8976b18249407d Ville Syrjälä 2023-04-18 421 drm_rect_debug_print("src: ", src, true);
8976b18249407d Ville Syrjälä 2023-04-18 422 drm_rect_debug_print("dst: ", dst, false);
8976b18249407d Ville Syrjälä 2023-04-18 423
8976b18249407d Ville Syrjälä 2023-04-18 424 return -EINVAL;
8976b18249407d Ville Syrjälä 2023-04-18 425 }
7c369018e87617 Mitul Golani 2025-01-06 426 }
7c369018e87617 Mitul Golani 2025-01-06 427
7c369018e87617 Mitul Golani 2025-01-06 428 if (crtc_state->pch_pfit.enabled) {
7c369018e87617 Mitul Golani 2025-01-06 429 struct drm_rect src;
7c369018e87617 Mitul Golani 2025-01-06 430
7c369018e87617 Mitul Golani 2025-01-06 431 drm_rect_init(&src, 0, 0,
7c369018e87617 Mitul Golani 2025-01-06 432 drm_rect_width(&crtc_state->pipe_src) << 16,
7c369018e87617 Mitul Golani 2025-01-06 433 drm_rect_height(&crtc_state->pipe_src) << 16);
7c369018e87617 Mitul Golani 2025-01-06 434
7c369018e87617 Mitul Golani 2025-01-06 435 hscale = drm_rect_calc_hscale(&src, &crtc_state->pch_pfit.dst,
7c369018e87617 Mitul Golani 2025-01-06 436 0, INT_MAX);
7c369018e87617 Mitul Golani 2025-01-06 437 vscale = drm_rect_calc_vscale(&src, &crtc_state->pch_pfit.dst,
7c369018e87617 Mitul Golani 2025-01-06 438 0, INT_MAX);
7c369018e87617 Mitul Golani 2025-01-06 439
7c369018e87617 Mitul Golani 2025-01-06 440 if (hscale < 0 || vscale < 0) {
7c369018e87617 Mitul Golani 2025-01-06 441 drm_dbg_kms(display->drm,
7c369018e87617 Mitul Golani 2025-01-06 442 "Scaler %d doesn't support required pipe scaling\n",
7c369018e87617 Mitul Golani 2025-01-06 443 *scaler_id);
7c369018e87617 Mitul Golani 2025-01-06 444 drm_rect_debug_print("src: ", &src, true);
7c369018e87617 Mitul Golani 2025-01-06 445 drm_rect_debug_print("dst: ", &crtc_state->pch_pfit.dst, false);
7c369018e87617 Mitul Golani 2025-01-06 446
7c369018e87617 Mitul Golani 2025-01-06 447 return -EINVAL;
7c369018e87617 Mitul Golani 2025-01-06 448 }
7c369018e87617 Mitul Golani 2025-01-06 449 }
0641adb4624b5e Mitul Golani 2025-01-06 450
0641adb4624b5e Mitul Golani 2025-01-06 @451 scaler_state->scalers[*scaler_id].hscale = hscale;
0641adb4624b5e Mitul Golani 2025-01-06 @452 scaler_state->scalers[*scaler_id].vscale = vscale;
vscale and hscale are initialized if crtc_state->pch_pfit.enabled or
plane_state->hw.fb is true, but otherwise they aren't.
8976b18249407d Ville Syrjälä 2023-04-18 453
3360d4d6d42ab3 Ville Syrjälä 2024-11-07 454 drm_dbg_kms(display->drm, "Attached scaler id %u.%u to %s:%d\n",
7488f64e89b02f Ville Syrjälä 2024-11-07 455 crtc->pipe, *scaler_id, name, idx);
8976b18249407d Ville Syrjälä 2023-04-18 456 scaler_state->scalers[*scaler_id].mode = mode;
8976b18249407d Ville Syrjälä 2023-04-18 457
8976b18249407d Ville Syrjälä 2023-04-18 458 return 0;
8976b18249407d Ville Syrjälä 2023-04-18 459 }
Hi Dan, Sure I will add patch fix and suggested tags. Regards, Mitul
diff --git a/drivers/gpu/drm/i915/display/skl_scaler.c b/drivers/gpu/drm/i915/display/skl_scaler.c index 4f838220f880..294493e5ee53 100644 --- a/drivers/gpu/drm/i915/display/skl_scaler.c +++ b/drivers/gpu/drm/i915/display/skl_scaler.c @@ -320,6 +320,7 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state, &crtc_state->scaler_state; struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); u32 mode; + int hscale, vscale; if (*scaler_id < 0) *scaler_id = intel_allocate_scaler(scaler_state, crtc); @@ -368,15 +369,11 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state, mode = SKL_PS_SCALER_MODE_DYN; } - /* - * FIXME: we should also check the scaler factors for pfit, so - * this shouldn't be tied directly to planes. - */ if (plane_state && plane_state->hw.fb) { const struct drm_framebuffer *fb = plane_state->hw.fb; const struct drm_rect *src = &plane_state->uapi.src; const struct drm_rect *dst = &plane_state->uapi.dst; - int hscale, vscale, max_vscale, max_hscale; + int max_vscale, max_hscale; /* * FIXME: When two scalers are needed, but only one of @@ -426,11 +423,34 @@ static int intel_atomic_setup_scaler(struct intel_crtc_state *crtc_state, return -EINVAL; } + } + + if (crtc_state->pch_pfit.enabled) { + struct drm_rect src; + + drm_rect_init(&src, 0, 0, + drm_rect_width(&crtc_state->pipe_src) << 16, + drm_rect_height(&crtc_state->pipe_src) << 16); + + hscale = drm_rect_calc_hscale(&src, &crtc_state->pch_pfit.dst, + 0, INT_MAX); + vscale = drm_rect_calc_vscale(&src, &crtc_state->pch_pfit.dst, + 0, INT_MAX); - scaler_state->scalers[*scaler_id].hscale = hscale; - scaler_state->scalers[*scaler_id].vscale = vscale; + if (hscale < 0 || vscale < 0) { + drm_dbg_kms(display->drm, + "Scaler %d doesn't support required pipe scaling\n", + *scaler_id); + drm_rect_debug_print("src: ", &src, true); + drm_rect_debug_print("dst: ", &crtc_state->pch_pfit.dst, false); + + return -EINVAL; + } } + scaler_state->scalers[*scaler_id].hscale = hscale; + scaler_state->scalers[*scaler_id].vscale = vscale; + drm_dbg_kms(display->drm, "Attached scaler id %u.%u to %s:%d\n", crtc->pipe, *scaler_id, name, idx); scaler_state->scalers[*scaler_id].mode = mode;
Compute scaling factors and scaler user for pipe scaler if particular scaler user is pipe scaler. --v2: - Update typo 'plane scaling' to 'pipe scaling'. [Ankit] - Remove FIXME tag which was added to check for pipe scaling factors as well. [Ankit] - Should be common hscale, vscale instead of local one to avoid garbage overwritten. - Typo in commit message 'perticular' to 'particular'. Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> --- drivers/gpu/drm/i915/display/skl_scaler.c | 34 ++++++++++++++++++----- 1 file changed, 27 insertions(+), 7 deletions(-)