Re: [Intel-gfx] [PATCH i-g-t] tests/i915/kms_big_fb: trigger async flip with a dummy flip

[Juha-Pekka Heikkilä]

Hi,

On 5.7.2022 12.49, Karthik B S wrote:

On 7/5/2022 3:08 PM, Murthy, Arun R wrote:

On 6/28/2022 4:34 PM, Arun R Murthy wrote:

In oder to trigger the async flip, a dummy flip is required after sync
flip so as to update the watermarks for async in KMD which happens as
part of this dummy flip. Thereafter async memory update will act as a
trigger register.
Capturing the CRC is done after the async flip as async flip at some
times can consume fairly a vblank period time.

Signed-off-by: Arun R Murthy 
---
   tests/i915/kms_big_fb.c | 29 +++--
   1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/tests/i915/kms_big_fb.c b/tests/i915/kms_big_fb.c index
d50fde45..6caf3c31 100644
--- a/tests/i915/kms_big_fb.c
+++ b/tests/i915/kms_big_fb.c
@@ -465,7 +465,7 @@ static bool test_pipe(data_t *data)
   static bool
   max_hw_stride_async_flip_test(data_t *data)
   {
-    uint32_t ret, startframe;
+    uint32_t ret, frame;
   const uint32_t w = data->output->config.default_mode.hdisplay,
  h = data->output->config.default_mode.vdisplay;
   igt_plane_t *primary;
@@ -519,7 +519,19 @@ max_hw_stride_async_flip_test(data_t *data)


DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);

   igt_wait_for_vblank(data->drm_fd, data-
display.pipes[primary->pipe->pipe].crtc_offset);
-    startframe = kmstest_get_vblank(data->drm_fd, data->pipe,

0) + 1;

+    /*
+ * In older platforms (<= Gen10), async address update bit is

double buffered.
+ * So flip timestamp can be verified only from the second 
flip.

+ * The first async flip just enables the async address update.
+ * In platforms greater than DISPLAY13 the first async flip 
will

be discarded

+ * in order to change the watermark levels as per the

optimization. Hence the

+ * subsequent async flips will actually do the asynchronous

flips.

+ */
+    ret = drmModePageFlip(data->drm_fd, data->output-
config.crtc->crtc_id,
+  data->big_fb_flip[i].fb_id,
+

DRM_MODE_PAGE_FLIP_ASYNC, NULL);

+    igt_wait_for_vblank(data->drm_fd, data-
display.pipes[primary->pipe->pipe].crtc_offset);
+    frame = kmstest_get_vblank(data->drm_fd, data->pipe, 0) +

1;

Hi,

Should this be added inside a gen check similar to kms_async_flips?

Yes sorry missed it!


   for (int j = 0; j < 2; j++) {
   do {
@@ -528,23 +540,20 @@ max_hw_stride_async_flip_test(data_t *data)


DRM_MODE_PAGE_FLIP_ASYNC, NULL);

   } while (ret == -EBUSY);
   igt_assert(ret == 0);
+    igt_pipe_crc_get_for_frame(data->drm_fd, data-
pipe_crc,
+   frame, &compare_crc);

+    frame = kmstest_get_vblank(data->drm_fd, data-
pipe, 0) + 1;
   do {
   ret = drmModePageFlip(data->drm_fd, data-
output->config.crtc->crtc_id,
 data->big_fb.fb_id,


DRM_MODE_PAGE_FLIP_ASYNC, NULL);

   } while (ret == -EBUSY);
   igt_assert(ret == 0);
+    igt_pipe_crc_get_for_frame(data->drm_fd, data-
pipe_crc,
+   frame, &async_crc);
We should be moving this IMHO. By waiting for vblank after each async 
flip

to capture CRC, what is the intended result? Seems to be completely
changing the existing test logic.
We are getting the vblank count and based on that getting the crc. Now 
we know for async flip at
certain times it can consume a time equivalent to a vblank period. So 
in those scenarios getting crc
based on the vblank goes for a toss. Hence capturing the vblank start 
time stamp at the beginning

of each flip.


Hi,

But what is the the reference CRC we're expecting? The original logic is 
designed to match on one iteration and mismatch on the next using a 
combination of fb's by using async flips. But with these changes that 
whole logic goes for a toss?




Also, seems like we are overwriting the CRC captured for j = 0, as 
comparison

is done outside this loop. Is this done on purpose?
Now with the changing the vblank start frame capture being added 
before the async flip, CRC can
be captured outside the loop as well, but makes no sense. To maintain 
this order moving the CRC

Capture also after each frame.


The CRC comparison is still outside the loop, so no point capturing 
CRC's if we finally discard it anyway?




I think generally the idea Arun is changing is correct but he's missed 
how the test work.


I see there's Ville's change on kernel side for display_ver >=13 first 
async_flip is unconditionally (intentionally) missed, this is at 
intel_plane_do_async_flip(..) so this test will need adjustment


What Arun seem to have missed is on test those nested loops how they 
work, that part probably should've originally been commented in code bit 
better.


On original code there's after loop for j two time 
igt_pipe_crc_get_for_frame(..), first will capture crc from duration of 
loop of 

Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/display: Add smem fallback allocation for dpt (rev4)

[Juha-Pekka Heikkilä]

Hi Lakshmi,

Here would be another false positive, I don't see how my changes would
affect debugfs_test@read_all_entries test on kbl.

/Juha-Pekka

to 16. kesäk. 2022 klo 19.31 Patchwork 
kirjoitti:

> *Patch Details*
> *Series:* series starting with [1/3] drm/i915/display: Add smem fallback
> allocation for dpt (rev4)
> *URL:* https://patchwork.freedesktop.org/series/104983/
> *State:* failure
> *Details:*
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104983v4/index.html CI
> Bug Log - changes from CI_DRM_11768 -> Patchwork_104983v4 Summary
>
> *FAILURE*
>
> Serious unknown changes coming with Patchwork_104983v4 absolutely need to
> be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in Patchwork_104983v4, please notify your bug team to allow them
> to document this new failure mode, which will reduce false positives in CI.
>
> External URL:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_104983v4/index.html
> Participating hosts (43 -> 41)
>
> Additional (2): bat-adlm-1 bat-atsm-1
> Missing (4): bat-dg2-8 bat-jsl-2 bat-dg2-9 fi-bdw-samus
> Possible new issues
>
> Here are the unknown changes that may have been introduced in
> Patchwork_104983v4:
> IGT changes Possible regressions
>
>- igt@debugfs_test@read_all_entries:
>   - fi-kbl-guc: PASS
>   
> 
>   -> FAIL
>   
> 
>
> Known issues
>
> Here are the changes found in Patchwork_104983v4 that come from known
> issues:
> IGT changes Issues hit
>
>-
>
>igt@i915_selftest@live@requests:
>- fi-blb-e6850: PASS
>   
> 
>   -> DMESG-FAIL
>   
> 
>   (i915#4528 )
>-
>
>igt@kms_busy@basic@flip:
>- fi-tgl-u2: PASS
>   
> 
>   -> DMESG-WARN
>   
> 
>   (i915#402 ) +1
>   similar issue
>-
>
>igt@kms_chamelium@common-hpd-after-suspend:
>-
>
>   fi-hsw-g3258: NOTRUN -> SKIP
>   
> 
>   (fdo#109271  /
>   fdo#111827 )
>   -
>
>   fi-hsw-4770: NOTRUN -> SKIP
>   
> 
>   (fdo#109271  /
>   fdo#111827 )
>   -
>
>igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
>- bat-adlp-4: PASS
>   
> 
>   -> DMESG-WARN
>   
> 
>   (i915#3576 )
>
> Possible fixes
>
>-
>
>igt@i915_pm_rpm@module-reload:
>- bat-adlp-4: DMESG-WARN
>   
> 
>   (i915#3576 )
>   -> PASS
>   
> 
>   +3 similar issues
>-
>
>igt@i915_selftest@live@hangcheck:
>-
>
>   fi-hsw-4770: INCOMPLETE
>   
> 
>   (i915#3303  /
>   i915#4785 )
>   -> PASS
>   
> 
>   -
>
>   fi-hsw-g3258: INCOMPLETE
>   
> 
>   (i915#4785 )
>   -> PASS
>   
> 

Re: [Intel-gfx] [PATCH 1/2] drm/i915/display: Add smem fallback allocation for dpt

[Juha-Pekka Heikkilä]

Matthew Auld kirjoitti 11.5.2022 klo 13.41:

On Fri, 6 May 2022 at 14:11, Juha-Pekka Heikkila
 wrote:


Add fallback smem allocation for dpt if stolen memory allocation failed.

Signed-off-by: Juha-Pekka Heikkila 
---
  drivers/gpu/drm/i915/display/intel_dpt.c | 16 
  1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c 
b/drivers/gpu/drm/i915/display/intel_dpt.c
index fb0e7e79e0cd..10008699656e 100644
--- a/drivers/gpu/drm/i915/display/intel_dpt.c
+++ b/drivers/gpu/drm/i915/display/intel_dpt.c
@@ -10,6 +10,7 @@
  #include "intel_display_types.h"
  #include "intel_dpt.h"
  #include "intel_fb.h"
+#include "gem/i915_gem_internal.h"


Nit: Keep these ordered.



  struct i915_dpt {
 struct i915_address_space vm;
@@ -128,6 +129,10 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space 
*vm)
 void __iomem *iomem;
 struct i915_gem_ww_ctx ww;
 int err;
+   u64 pin_flags = 0;


Nit: Christmas tree-ish. Move this above the err.


+
+   if (!i915_gem_object_is_lmem(dpt->obj))
+   pin_flags |= PIN_MAPPABLE;


If we do this then we don't need the second patch ;)

I guess the second patch was meant to make this is_stolen? Maybe just
move the second patch to be the first in the series?



Hi Matthew, thanks for the comments. I think I'm still missing some 
essential part. Without marking PIN_MAPPABLE when !lmem I was hitting 
WARN_ON() in gem code when doing this pinning. Weird thing with it was I 
got everything working with y-tile but x-tile was 100% fail. I'll need 
to repro those results and see why I got that. There was recently fixed 
regression on igt side which may have affected my results but I'll 
probably anyway need to have another round for these patches including 
fixes to those parts you pointed out.


/Juha-Pekka



 wakeref = intel_runtime_pm_get(&i915->runtime_pm);
 atomic_inc(&i915->gpu_error.pending_fb_pin);
@@ -138,7 +143,7 @@ struct i915_vma *intel_dpt_pin(struct i915_address_space 
*vm)
 continue;

 vma = i915_gem_object_ggtt_pin_ww(dpt->obj, &ww, NULL, 0, 4096,
- HAS_LMEM(i915) ? 0 : 
PIN_MAPPABLE);
+ pin_flags);
 if (IS_ERR(vma)) {
 err = PTR_ERR(vma);
 continue;
@@ -248,10 +253,13 @@ intel_dpt_create(struct intel_framebuffer *fb)

 size = round_up(size * sizeof(gen8_pte_t), I915_GTT_PAGE_SIZE);

-   if (HAS_LMEM(i915))
-   dpt_obj = i915_gem_object_create_lmem(i915, size, 
I915_BO_ALLOC_CONTIGUOUS);
-   else
+   dpt_obj = i915_gem_object_create_lmem(i915, size, 
I915_BO_ALLOC_CONTIGUOUS);
+   if (IS_ERR(dpt_obj) && i915_ggtt_has_aperture(to_gt(i915)->ggtt))
 dpt_obj = i915_gem_object_create_stolen(i915, size);
+   if (IS_ERR(dpt_obj) && !HAS_LMEM(i915)) {
+   drm_dbg_kms(&i915->drm, "Allocating dpt from smem\n");


Hopefully this is not too noisy?

With the s/is_lmem/is_stolen/, or however you want to deal with that,
Reviewed-by: Matthew Auld 


+   dpt_obj = i915_gem_object_create_internal(i915, size);
+   }
 if (IS_ERR(dpt_obj))
 return ERR_CAST(dpt_obj);

--
2.25.1

Re: [Intel-gfx] [PATCH 1/4] drm/fourcc: Introduce format modifiers for DG2 render and media compression

[Juha-Pekka Heikkilä]

Seems my first mail didn't come through so here's second time for this patch:

Reviewed-by: Juha-Pekka Heikkila 

On Mon, Apr 4, 2022 at 4:39 PM Imre Deak  wrote:
>
> From: Matt Roper 
>
> The render/media engines on DG2 unify render compression and media
> compression into a single format for the first time, using the Tile 4
> layout for main surfaces. The compression algorithm is different from
> any previous platform and the display engine must still be configured to
> decompress either a render or media compressed surface; as such, we
> need new RC and MC framebuffer modifiers to represent buffers in this
> format.
>
> v2: Clarify modifier layout description.
>
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Matt Roper 
> Signed-off-by: Imre Deak 
> Acked-by: Nanley Chery 
> ---
>  include/uapi/drm/drm_fourcc.h | 22 ++
>  1 file changed, 22 insertions(+)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index b73fe6797fc37..4a5117715db3c 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -583,6 +583,28 @@ extern "C" {
>   */
>  #define I915_FORMAT_MOD_4_TILED fourcc_mod_code(INTEL, 9)
>
> +/*
> + * Intel color control surfaces (CCS) for DG2 render compression.
> + *
> + * The main surface is Tile 4 and at plane index 0. The CCS data is stored
> + * outside of the GEM object in a reserved memory area dedicated for the
> + * storage of the CCS data for all RC/RC_CC/MC compressible GEM objects. The
> + * main surface pitch is required to be a multiple of four Tile 4 widths.
> + */
> +#define I915_FORMAT_MOD_4_TILED_DG2_RC_CCS fourcc_mod_code(INTEL, 10)
> +
> +/*
> + * Intel color control surfaces (CCS) for DG2 media compression.
> + *
> + * The main surface is Tile 4 and at plane index 0. For semi-planar formats
> + * like NV12, the Y and UV planes are Tile 4 and are located at plane indices
> + * 0 and 1, respectively. The CCS for all planes are stored outside of the
> + * GEM object in a reserved memory area dedicated for the storage of the
> + * CCS data for all RC/RC_CC/MC compressible GEM objects. The main surface
> + * pitch is required to be a multiple of four Tile 4 widths.
> + */
> +#define I915_FORMAT_MOD_4_TILED_DG2_MC_CCS fourcc_mod_code(INTEL, 11)
> +
>  /*
>   * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks
>   *
> --
> 2.30.2
>

Re: [Intel-gfx] [PATCH 8/8] drm/i915/fbc: Allow higher compression limits on FBC1

[Juha-Pekka Heikkilä]

Look ok to me.

Reviewed-by: Juha-Pekka Heikkila 

On 2.7.2021 23.46, Ville Syrjala wrote:

From: Ville Syrjälä 

On FBC1 we can specify an arbitrary cfb stride. The hw will
simply throw away any compressed line that would exceed the
specified limit and keep using the uncompressed data instead.
Thus we can allow arbitrary compression limits.

The one thing we have to keep in mind though is that the cfb
stride is specified in units of 32B (gen2) or 64B (gen3+).
Fortunately X-tile is already 128B (gen2) or 512B (gen3+) wide
so as long as we limit outselves to the same 4x compression
limit that FBC2 has we are guaranteed to have a sufficiently
aligned cfb stride.

Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/display/intel_fbc.c | 20 +++-
  1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c 
b/drivers/gpu/drm/i915/display/intel_fbc.c
index daf2191dd3f6..d46ee7b49d68 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -144,15 +144,13 @@ static void i8xx_fbc_deactivate(struct drm_i915_private 
*dev_priv)
  
  static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)

  {
-   struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
+   struct intel_fbc *fbc = &dev_priv->fbc;
+   const struct intel_fbc_reg_params *params = &fbc->params;
int cfb_pitch;
int i;
u32 fbc_ctl;
  
-	/* Note: fbc.limit == 1 for i8xx */

-   cfb_pitch = params->cfb_size / FBC_LL_SIZE;
-   if (params->fb.stride < cfb_pitch)
-   cfb_pitch = params->fb.stride;
+   cfb_pitch = params->cfb_stride / fbc->limit;
  
  	/* FBC_CTL wants 32B or 64B units */

if (DISPLAY_VER(dev_priv) == 2)
@@ -498,18 +496,14 @@ static int intel_fbc_min_limit(int fb_cpp)
  
  static int intel_fbc_max_limit(struct drm_i915_private *dev_priv)

  {
-   /*
-* FIXME: FBC1 can have arbitrary cfb stride,
-* so we could support different compression ratios.
-*/
-   if (DISPLAY_VER(dev_priv) < 5 && !IS_G4X(dev_priv))
-   return 1;
-
/* WaFbcOnly1to1Ratio:ctg */
if (IS_G4X(dev_priv))
return 1;
  
-	/* FBC2 can only do 1:1, 1:2, 1:4 */

+   /*
+* FBC2 can only do 1:1, 1:2, 1:4, we limit
+* FBC1 to the same out of convenience.
+*/
return 4;
  }
  


-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 


This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Fix wm params for ccs

[Juha-Pekka Heikkilä]

Hi Lakshmi,

Here would be again one false positive result.

/Juha-Pekka

On Wed, Jul 14, 2021 at 7:38 AM Patchwork 
wrote:

> *Patch Details*
> *Series:* drm/i915: Fix wm params for ccs
> *URL:* https://patchwork.freedesktop.org/series/92491/
> *State:* failure
> *Details:*
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20589/index.html CI
> Bug Log - changes from CI_DRM_10342_full -> Patchwork_20589_full Summary
>
> *FAILURE*
>
> Serious unknown changes coming with Patchwork_20589_full absolutely need
> to be
> verified manually.
>
> If you think the reported changes have nothing to do with the changes
> introduced in Patchwork_20589_full, please notify your bug team to allow
> them
> to document this new failure mode, which will reduce false positives in CI.
> Possible new issues
>
> Here are the unknown changes that may have been introduced in
> Patchwork_20589_full:
> IGT changes Possible regressions
>
>- igt@dumb_buffer@map-invalid-size:
>   - shard-apl: NOTRUN -> DMESG-WARN
>   
> 
>
> Suppressed
>
> The following results come from untrusted machines, tests, or statuses.
> They do not affect the overall result.
>
>-
>
>igt@kms_dither@fb-8bpc-vs-panel-6bpc:
>- {shard-rkl}: NOTRUN -> SKIP
>   
> 
>-
>
>igt@runner@aborted:
>- {shard-rkl}: (FAIL
>   
> ,
>   FAIL
>   
> ,
>   FAIL
>   
> ,
>   FAIL
>   
> ,
>   FAIL
>   
> )
>   ([i915#3002] / [i915#3728]) -> (FAIL
>   
> ,
>   FAIL
>   
> ,
>   FAIL
>   
> ,
>   FAIL
>   
> )
>   ([i915#3002])
>
> Known issues
>
> Here are the changes found in Patchwork_20589_full that come from known
> issues:
> IGT changes Issues hit
>
>-
>
>igt@gem_create@create-massive:
>- shard-snb: NOTRUN -> DMESG-WARN
>   
> 
>   ([i915#3002])
>-
>
>igt@gem_ctx_persistence@legacy-engines-cleanup:
>- shard-snb: NOTRUN -> SKIP
>   
> 
>   ([fdo#109271] / [i915#1099]) +3 similar issues
>-
>
>igt@gem_ctx_persistence@legacy-engines-hang@blt:
>- shard-skl: NOTRUN -> SKIP
>   
> 
>   ([fdo#109271]) +119 similar issues
>-
>
>igt@gem_exec_fair@basic-deadline:
>- shard-apl: NOTRUN -> FAIL
>   
> 
>   ([i915#2846])
>-
>
>igt@gem_exec_fair@basic-pace-share@rcs0:
>- shard-tglb: PASS
>   
> 
>   -> FAIL
>   
> 
>   ([i915#2842])
>-
>
>igt@gem_exec_fair@basic-throttle@rcs0:
>-
>
>   shard-glk: PASS
>   
> 
>   -> FAIL
>   
> 
>   ([i915#2842])
>   -
>
>   shard-iclb: PASS
>   
> 
>   -> FAIL
>   
> 
>   ([i915#2849])
>   -
>
>igt@gem_exec_reloc@basic-wide-active@rcs0:
>- shard-snb: NOTRUN -> FAIL
>   
> 

Re: [Intel-gfx] [PATCH 15/19] drm/i915: Simplify skl_max_scale()

[Juha-Pekka Heikkilä]

Patches 13, 14 and this 15 look ok to me. Those num/den combos in 13 I 
cannot bet my head on but the plumbing look all ok.


Also if on 1..8 some patch wasn't pushed yet, those are all

Reviewed-by: Juha-Pekka Heikkila 

Ville Syrjala kirjoitti 8.7.2019 klo 15.53:

From: Ville Syrjälä 

Now that the planes declare their minimum cdclk requirements properly
we don't need to check the cdclk in skl_max_scale() anymore. Just check
against the maximum downscale ratio, and move the code next to it's
only caller.

Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/display/intel_display.c | 38 
  drivers/gpu/drm/i915/display/intel_sprite.c  | 12 ++-
  drivers/gpu/drm/i915/intel_drv.h |  2 --
  3 files changed, 11 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 1e67fbe50476..489620ef476b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -14525,44 +14525,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
mutex_unlock(&dev_priv->drm.struct_mutex);
  }
  
-int

-skl_max_scale(const struct intel_crtc_state *crtc_state,
- const struct drm_format_info *format)
-{
-   struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-   int max_scale;
-   int crtc_clock, max_dotclk, tmpclk1, tmpclk2;
-
-   if (!crtc_state->base.enable)
-   return DRM_PLANE_HELPER_NO_SCALING;
-
-   crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
-   max_dotclk = 
to_intel_atomic_state(crtc_state->base.state)->cdclk.logical.cdclk;
-
-   if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
-   max_dotclk *= 2;
-
-   if (WARN_ON_ONCE(!crtc_clock || max_dotclk < crtc_clock))
-   return DRM_PLANE_HELPER_NO_SCALING;
-
-   /*
-* skl max scale is lower of:
-*close to 3 but not 3, -1 is for that purpose
-*or
-*cdclk/crtc_clock
-*/
-   if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
-   !drm_format_info_is_yuv_semiplanar(format))
-   tmpclk1 = 0x3 - 1;
-   else
-   tmpclk1 = 0x2 - 1;
-   tmpclk2 = (1 << 8) * ((max_dotclk << 8) / crtc_clock);
-   max_scale = min(tmpclk1, tmpclk2);
-
-   return max_scale;
-}
-
  static void intel_begin_crtc_commit(struct intel_atomic_state *state,
struct intel_crtc *crtc)
  {
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c 
b/drivers/gpu/drm/i915/display/intel_sprite.c
index a07887279e1a..0ffbec8291ee 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -2089,6 +2089,16 @@ static int skl_plane_check_nv12_rotation(const struct 
intel_plane_state *plane_s
return 0;
  }
  
+static int skl_plane_max_scale(struct drm_i915_private *dev_priv,

+  const struct drm_framebuffer *fb)
+{
+   if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
+   !drm_format_info_is_yuv_semiplanar(fb->format))
+   return 0x3 - 1;
+   else
+   return 0x2 - 1;
+}
+
  static int skl_plane_check(struct intel_crtc_state *crtc_state,
   struct intel_plane_state *plane_state)
  {
@@ -2106,7 +2116,7 @@ static int skl_plane_check(struct intel_crtc_state 
*crtc_state,
/* use scaler when colorkey is not required */
if (!plane_state->ckey.flags && intel_fb_scalable(fb)) {
min_scale = 1;
-   max_scale = skl_max_scale(crtc_state, fb->format);
+   max_scale = skl_plane_max_scale(dev_priv, fb);
}
  
  	ret = drm_atomic_helper_check_plane_state(&plane_state->base,

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 999ad3166cd1..02eeaec86997 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1620,8 +1620,6 @@ void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
  
  u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);

  int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
-int skl_max_scale(const struct intel_crtc_state *crtc_state,
- const struct drm_format_info *format);
  
  static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)

  {


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/6] drm: Add Y2xx and Y4xx (xx:10/12/16) format definitions and fourcc

[Juha-Pekka Heikkilä]

Swati Sharma kirjoitti 13.2.2019 klo 15.25:

The following pixel formats are packed format that follows 4:2:2
chroma sampling. For memory represenation each component is
allocated 16 bits each. Thus each pixel occupies 32bit.

Y210:   For each component, valid data occupies MSB 10 bits.
LSB 6 bits are filled with zeroes.
Y212:   For each component, valid data occupies MSB 12 bits.
LSB 4 bits are filled with zeroes.
Y216:   For each component valid data occupies 16 bits,
doesn't require any padding bits.

First 16 bits stores the Y value and the next 16 bits stores one
of the chroma samples alternatively. The first luma sample will
be accompanied by first U sample and second luma sample is
accompanied by the first V sample.

The following pixel formats are packed format that follows 4:4:4
chroma sampling. Channels are arranged in the order UYVA in
increasing memory order.

Y410:   Each color component occupies 10 bits and X component
takes 2 bits, thus each pixel occupies 32 bits.
Y412:   Each color component is 16 bits where valid data
occupies MSB 12 bits. LSB 4 bits are filled with zeroes.
Thus, each pixel occupies 64 bits.
Y416:   Each color component occupies 16 bits for valid data,
doesn't require any padding bits. Thus, each pixel
occupies 64 bits.

Signed-off-by: Swati Sharma 
Signed-off-by: Vidya Srinivas 
---
  drivers/gpu/drm/drm_fourcc.c  |  6 ++
  include/uapi/drm/drm_fourcc.h | 18 +-
  2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index ba7e19d..45c9882 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -226,6 +226,12 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_VYUY,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_XYUV,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_AYUV,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, 
.is_yuv = true },
+   { .format = DRM_FORMAT_Y210,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
+   { .format = DRM_FORMAT_Y212,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
+   { .format = DRM_FORMAT_Y216,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
+   { .format = DRM_FORMAT_Y410,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
+   { .format = DRM_FORMAT_Y412,.depth = 0,  
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
+   { .format = DRM_FORMAT_Y416,.depth = 0,  
.num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_Y0L0,.depth = 0,  
.num_planes = 1,
  .char_per_block = { 8, 0, 0 }, .block_w = { 2, 0, 0 }, 
.block_h = { 2, 0, 0 },
  .hsub = 2, .vsub = 2, .has_alpha = true, .is_yuv = true },
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index bab2029..6e20ced 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -151,7 +151,23 @@
  #define DRM_FORMAT_VYUY   fourcc_code('V', 'Y', 'U', 'Y') /* 
[31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */
  
  #define DRM_FORMAT_AYUV		fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */

-#define DRM_FORMAT_XYUVfourcc_code('X', 'Y', 'U', 'V') /* 
[31:0] X:Y:Cb:Cr 8:8:8:8 little endian */
+#define DRM_FORMAT_XYUVfourcc_code('X', 'Y', 'U', 'V') /* [31:0] 
X:Y:Cb:Cr 8:8:8:8 little endian */

^^
one tab removed?

With that fixed this is
Reviewed-by: Juha-Pekka Heikkila 


+
+/*
+ * packed Y2xx indicate for each component, xx valid data occupy msb
+ * 16-xx padding occupy lsb
+ */
+#define DRM_FORMAT_Y210 fourcc_code('Y', '2', '1', '0') /* [63:0] 
Y0:x:Cb0:x:Y1:x:Cr1:x 10:6:10:6:10:6:10:6 little endian per 2 Y pixels */
+#define DRM_FORMAT_Y212 fourcc_code('Y', '2', '1', '2') /* [63:0] 
Y0:x:Cb0:x:Y1:x:Cr1:x 12:4:12:4:12:4:12:4 little endian per 2 Y pixels */
+#define DRM_FORMAT_Y216 fourcc_code('Y', '2', '1', '6') /* [63:0] 
Y0:Cb0:Y1:Cr1 16:16:16:16 little endian per 2 Y pixels */
+
+/*
+ * packed Y4xx indicate for each component, xx valid data occupy msb
+ * 16-xx padding occupy lsb except Y410
+ */
+#define DRM_FORMAT_Y410 fourcc_code('Y', '4', '1', '0') /* [31:0] 
X:V:Y:U 2:10:10:10 little endian */
+#define DRM_FORMAT_Y412 fourcc_code('Y', '4', '1', '2') 

Re: [Intel-gfx] [PATCH v3 4/4] drm/i915/icl: Enable Y210, Y212, Y216 format for primary and sprite planes

[Juha-Pekka Heikkilä]

Swati Sharma kirjoitti 22.10.2018 klo 8.31:

From: Vidya Srinivas 

In this patch, a list for icl specific pixel formats is created
in which Y210, Y212 and Y216 pixel formats are added along with
legacy pixel formats for primary and sprite plane.

v3: since support for planar formats on ICL was getting totally
 skipped, added support for the same in intel_display.c and
 intel_sprite.c. (juha)

Signed-off-by: Swati Sharma 
Signed-off-by: Vidya Srinivas 
---
  drivers/gpu/drm/i915/intel_display.c | 58 
  drivers/gpu/drm/i915/intel_sprite.c  | 42 --
  2 files changed, 92 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 98f2939..f83fbb4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -105,6 +105,42 @@
DRM_FORMAT_NV12,
  };
  
+static const uint32_t icl_primary_formats[] = {

+   DRM_FORMAT_C8,
+   DRM_FORMAT_RGB565,
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_ARGB,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_XRGB2101010,
+   DRM_FORMAT_XBGR2101010,
+   DRM_FORMAT_YUYV,
+   DRM_FORMAT_YVYU,
+   DRM_FORMAT_UYVY,
+   DRM_FORMAT_VYUY,
+   DRM_FORMAT_Y210,
+   DRM_FORMAT_Y212,
+   DRM_FORMAT_Y216,
+};
+
+static const uint32_t icl_pri_planar_formats[] = {
+   DRM_FORMAT_C8,
+   DRM_FORMAT_RGB565,
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_ARGB,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_XRGB2101010,
+   DRM_FORMAT_XBGR2101010,
+   DRM_FORMAT_YUYV,
+   DRM_FORMAT_YVYU,
+   DRM_FORMAT_UYVY,
+   DRM_FORMAT_VYUY,
+   DRM_FORMAT_Y210,
+   DRM_FORMAT_Y212,
+   DRM_FORMAT_Y216,
+};
+
  static const uint64_t skl_format_modifiers_noccs[] = {
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
@@ -13788,16 +13824,26 @@ bool skl_plane_has_planar(struct drm_i915_private 
*dev_priv,
fbc->possible_framebuffer_bits |= primary->frontbuffer_bit;
}
  
+

^^ stray newline?

The next if(){}else{} I would write separating gen11 from gen9 because 
you need both tables, primary_formats as well as pri_planar_formats, for 
gen11.


So, instead of having

if(>=gen9) {
  if(>=gen11) {
  ..
  } else {
  ..
  }
}

I'd write it like

if(>=gen11) {
  ..
} else if(>=gen9) {
  ..
} els...

I think it will be much easier to read. Same below in intel_sprite.c

/Juha-Pekka


if (INTEL_GEN(dev_priv) >= 9) {
primary->has_ccs = skl_plane_has_ccs(dev_priv, pipe,
-PLANE_PRIMARY);
-
-   if (skl_plane_has_planar(dev_priv, pipe, PLANE_PRIMARY)) {
+   PLANE_PRIMARY);
+
+   if (skl_plane_has_planar(dev_priv, pipe,
+   PLANE_PRIMARY)) {
+   if (INTEL_GEN(dev_priv) >= 11) {
+   intel_primary_formats = icl_primary_formats;
+   num_formats = ARRAY_SIZE(icl_primary_formats);
+   } else {
+   intel_primary_formats = skl_primary_formats;
+   num_formats = ARRAY_SIZE(skl_primary_formats);
+   }
+   } else if (INTEL_GEN(dev_priv) >= 11) {
+   intel_primary_formats = icl_pri_planar_formats;
+   num_formats = ARRAY_SIZE(icl_pri_planar_formats);
+   } else {
intel_primary_formats = skl_pri_planar_formats;
num_formats = ARRAY_SIZE(skl_pri_planar_formats);
-   } else {
-   intel_primary_formats = skl_primary_formats;
-   num_formats = ARRAY_SIZE(skl_primary_formats);
}
  
  		if (primary->has_ccs)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index c831360..7d9b3e4 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1564,6 +1564,36 @@ int intel_sprite_set_colorkey_ioctl(struct drm_device 
*dev, void *data,
DRM_FORMAT_NV12,
  };
  
+static uint32_t icl_plane_formats[] = {

+   DRM_FORMAT_RGB565,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_ARGB,
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_YUYV,
+   DRM_FORMAT_YVYU,
+   DRM_FORMAT_UYVY,
+   DRM_FORMAT_VYUY,
+   DRM_FORMAT_Y210,
+   DRM_FORMAT_Y212,
+   DRM_FORMAT_Y216,
+};
+
+static uint32_t icl_planar_formats[] = {
+   DRM_FORMAT_RGB565,
+   DRM_FORMAT_ABGR,
+   DRM_FORMAT_ARGB,
+   DRM_FORMAT_XBGR,
+   DRM_FORMAT_XRGB,
+   DRM_FORMAT_YUYV,
+   DRM_FORMAT_YVYU,
+   DRM_FORMAT_UYVY,
+   DRM_FORMAT_VYUY,
+  

Re: [Intel-gfx] [PATCH v3 2/4] drm/i915/icl: Add Y210, Y212, Y216 plane control definitions

[Juha-Pekka Heikkilä]

I did earlier give R-b for this patch. The patch anyway hasn't changed 
as those defines have not changed.


/Juha-Pekka

Swati Sharma kirjoitti 22.10.2018 klo 8.31:

From: Vidya Srinivas 

Added needed plane control flag definitions for Y210, Y212 and
Y216 formats.

v3: no change

Signed-off-by: Swati Sharma 
Signed-off-by: Vidya Srinivas 
---
  drivers/gpu/drm/i915/i915_reg.h | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a71c507..cbb2917 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6506,6 +6506,9 @@ enum {
  #define   PLANE_CTL_FORMAT_RGB_565(14 << 24)
  #define   ICL_PLANE_CTL_FORMAT_MASK   (0x1f << 23)
  #define   PLANE_CTL_PIPE_CSC_ENABLE   (1 << 23) /* Pre-GLK */
+#define   PLANE_CTL_FORMAT_Y210(1 << 23)
+#define   PLANE_CTL_FORMAT_Y212(3 << 23)
+#define   PLANE_CTL_FORMAT_Y216(5 << 23)
  #define   PLANE_CTL_KEY_ENABLE_MASK   (0x3 << 21)
  #define   PLANE_CTL_KEY_ENABLE_SOURCE (1 << 21)
  #define   PLANE_CTL_KEY_ENABLE_DESTINATION(2 << 21)


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/4] drm: Add P010, P012, P016 format definitions and fourcc

[Juha-Pekka Heikkilä]

Alexandru-Cosmin Gheorghe kirjoitti 3.10.2018 klo 20.18:

On Wed, Oct 03, 2018 at 02:31:08PM +0300, Juha-Pekka Heikkila wrote:

Hi Alex,

For my patches there seems limited interest to get them merged before IGT
support these modes..I'm not holding my breath for this.


I'm interested if that counts.

I asked the same question on the DRM_FORMAT_XYUV thread, do we need to
wait for userspace to get new fourcc merged.


I'd say yes. Why would otherwise clutter headers which affect what other 
guys are doing for different drivers?


If it makes any difference for you I made KMS video output plug-in for 
VLC media player as part of my enablement of P01x formats. My plug-in 
didn't make it to any VLC release yet but you can find it in VLC master 
branch. Through my plug-in you can ask any fourcc for setting up KMS 
planes, then you'll need to find which VLC fourcc matches your KMS plane 
and tell VLC about it on commandline. If you have driver implementation 
of P010 format which I saw earlier in your patch you can compile VLC 
with P010 included in DRM headers and then you'll be able to watch 
videos using your new format. For P01x formats recompile is needed as 
their setup is different from other formats, packed formats should work 
without anything special.


/Juha-Pekka





https://lists.freedesktop.org/archives/intel-gfx/2018-September/174877.html

/Juha-Pekka

On 02.10.2018 18:00, Alexandru-Cosmin Gheorghe wrote:

Hi,

How is this going on, anything holding it back from getting merged ?
I'm interested in adding/using P010, [1]

Thank you,
Alex Gheorghe

[1] https://lists.freedesktop.org/archives/dri-devel/2018-August/186963.html

On Thu, Aug 30, 2018 at 03:41:11PM +0300, Juha-Pekka Heikkila wrote:

Add P010 definition, semi-planar yuv format where each component
is 16 bits 10 msb containing color value. First come Y plane [10:6]
followed by 2x2 subsampled Cr:Cb plane [10:6:10:6]

Add P012 definition, semi-planar yuv format where each component
is 16 bits 12 msb containing color value. First come Y plane [12:4]
followed by 2x2 subsampled Cr:Cb plane [12:4:12:4]

Add P016 definition, semi-planar yuv format where each component
is 16 bits. First come Y plane followed by 2x2 subsampled Cr:Cb
plane [16:16]

Signed-off-by: Juha-Pekka Heikkila 
---
  drivers/gpu/drm/drm_fourcc.c  |  3 +++
  include/uapi/drm/drm_fourcc.h | 10 ++
  2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 35c1e27..32e07a2 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -173,6 +173,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_UYVY,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_VYUY,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_AYUV,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, 
.is_yuv = true },
+   { .format = DRM_FORMAT_P010,.depth = 0,  
.num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true  },
+   { .format = DRM_FORMAT_P012,.depth = 0,  
.num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true  },
+   { .format = DRM_FORMAT_P016,.depth = 0,  
.num_planes = 2, .cpp = { 2, 4, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true  },
};
unsigned int i;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 2ed46e9..daaabb1 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -178,6 +178,16 @@ extern "C" {
  #define DRM_FORMAT_NV42   fourcc_code('N', 'V', '4', '2') /* 
non-subsampled Cb:Cr plane */
  /*
+ * 2 plane YCbCr
+ * index 0 = Y plane, [15:0] Y little endian where Pxxx indicate
+ * component xxx msb Y [xxx:16-xxx]
+ * index 1 = Cr:Cb plane, [31:0] Cr:Cb little endian [xxx:16-xxx:xxx:16-xxx]
+ */
+#define DRM_FORMAT_P010fourcc_code('P', '0', '1', '0') /* 2x2 
subsampled Cr:Cb plane, 10 bit per channel */
+#define DRM_FORMAT_P012fourcc_code('P', '0', '1', '2') /* 2x2 
subsampled Cr:Cb plane, 12 bit per channel */
+#define DRM_FORMAT_P016fourcc_code('P', '0', '1', '6') /* 2x2 
subsampled Cr:Cb plane, 16 bit per channel */
+
+/*
   * 3 plane YCbCr
   * index 0: Y plane, [7:0] Y
   * index 1: Cb plane, [7:0] Cb
--
2.7.4

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





___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Fix ILK-IVB sprite enable delays

[Juha-Pekka Heikkilä]

Look ok to me. I will try this on my HSW box to see will this affect 
those issues which look really similar as seen on IVB/SNB


Reviewed-by: Juha-Pekka Heikkila 

Ville Syrjala kirjoitti 28.9.2018 klo 16.24:

From: Ville Syrjälä 

Sprite enable on ILK-IVB may take two frames to complete
when the hardware is in big FIFO mode (LP1+). That is
not entirely great as it means the sprite enable may
actually happen one frame after we've already signalled
flip completion. At the very least crc checks may fail
due to the sprite not yet being visible when we expect it.

We already have code to deal with big FIFO mode when it
comes to the sprite scaling on IVB
(WaCxSRDisabledForSpriteScaling:ivb). Let's extend that
workaround to kick in whenever the sprite is in the process
of being enabled. Also ILK/SNB bspec has some notes to
indicate that we should most likely also do the sprite
scaling w/a on all three platforms, so let's do that as well.

Pretty easy to reproduce on SNB/IVB. ILK has proved more
elusive, but let's trust the spec and include it as well.

Cc: Juha-Pekka Heikkila 
Testcase: igt/kms_plane/pixel-format-pipe-*-planes
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107749
Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/intel_display.c | 36 +++-
  1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 4c5c2b39e65c..4710f3aab552 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10696,14 +10696,40 @@ int intel_plane_atomic_calc_changes(const struct 
intel_crtc_state *old_crtc_stat
pipe_config->fb_bits |= plane->frontbuffer_bit;
  
  	/*

+* ILK/SNB DVSACNTR/Sprite Enable
+* IVB SPR_CTL/Sprite Enable
+* "When in Self Refresh Big FIFO mode, a write to enable the
+*  plane will be internally buffered and delayed while Big FIFO
+*  mode is exiting."
+*
+* Which means that enabling the sprite can take an extra frame
+* when we start in big FIFO mode (LP1+). Thus we need to drop
+* down to LP0 and wait for vblank in order to make sure the
+* sprite gets enabled on the next vblank after the register write.
+* Doing otherwise would risk enabling the sprite one frame after
+* we've already signalled flip completion. We can resume LP1+
+* once the sprite has been enabled.
+*
+*
 * WaCxSRDisabledForSpriteScaling:ivb
+* IVB SPR_SCALE/Scaling Enable
+* "Low Power watermarks must be disabled for at least one
+*  frame before enabling sprite scaling, and kept disabled
+*  until sprite scaling is disabled."
+*
+* ILK/SNB DVSASCALE/Scaling Enable
+* "When in Self Refresh Big FIFO mode, scaling enable will be
+*  masked off while Big FIFO mode is exiting."
 *
-* cstate->update_wm was already set above, so this flag will
-* take effect when we commit and program watermarks.
+* Despite the w/a only being listed for IVB we assume that
+* the ILK/SNB note has similar ramifications, hence we apply
+* the w/a on all three platforms.
 */
-   if (plane->id == PLANE_SPRITE0 && IS_IVYBRIDGE(dev_priv) &&
-   needs_scaling(to_intel_plane_state(plane_state)) &&
-   !needs_scaling(old_plane_state))
+   if (plane->id == PLANE_SPRITE0 &&
+   (IS_GEN5(dev_priv) || IS_GEN6(dev_priv) ||
+IS_IVYBRIDGE(dev_priv)) &&
+   (turn_on || (!needs_scaling(old_plane_state) &&
+needs_scaling(to_intel_plane_state(plane_state)
pipe_config->disable_lp_wm = true;
  
  	return 0;



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v10 0/2] Add XYUV format support

[Juha-Pekka Heikkilä]

Lisovskiy, Stanislav kirjoitti 14.9.2018 klo 17.30:

On Fri, 2018-09-14 at 16:47 +0300, Ville Syrjälä wrote:

On Fri, Sep 14, 2018 at 01:36:32PM +, Lisovskiy, Stanislav wrote:

On Fri, 2018-09-07 at 11:45 +0300, Stanislav Lisovskiy wrote:

Introduced new XYUV scan-in format for framebuffer and
added support for it to i915(SkyLake+).

Stanislav Lisovskiy (2):
   drm: Introduce new DRM_FORMAT_XYUV
   drm/i915: Adding YUV444 packed format support for skl+

  drivers/gpu/drm/drm_fourcc.c |  1 +
  drivers/gpu/drm/i915/i915_reg.h  |  2 +-
  drivers/gpu/drm/i915/intel_display.c | 15 +++
  drivers/gpu/drm/i915/intel_sprite.c  |  2 ++
  include/uapi/drm/drm_fourcc.h|  1 +
  5 files changed, 20 insertions(+), 1 deletion(-)



Ping. There is an IGT patch which got Reviewed-by Ville.
This one left in order to get XYUV support.


Did we figure out userspace for this?

Was the conflict solved with the other guy (forgot who it is)
trying to add the same format with a different name?


Currently for userspace we have VLC(checked with Juha-Pekka) and
also IGT test case.


Hei, no. VLC was *not* showing the mode correctly back then when we 
tested. VLC kms plugin is able to initialize the mode and it's all 
stable but VLC didn't have correct video converter to reach matching 
xYUV output. You remember Stan we tried to increase the recursion for 
decoding in case it would help? VLC doesn't have codec which produce 
correct output, one of those xxx -> YUV plug-ins in VLC should be 
patched before VLC can be considered as userspace for this mode.


/Juha-Pekka



I think those guys were from ARM and they were adding also support for
XYUV. The only difference was that they called XYUV, like XYUV
something.
Our patches were otherwise identical regarding drm_fourcc.c part, I
don't see their stuff merged, but I guess it really shouldn't matter,
who does this first. i915 specific part doesn't conflict with their
stuff. To be honest, I forgot the guy's name neither could find his
email in my mailbox.
So hopefully somebody shows up here.




___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 3/4] drm/i915/icl: Preparations for enabling Y210, Y212, Y216 formats

[Juha-Pekka Heikkilä]

Swati Sharma kirjoitti 12.9.2018 klo 13.32:

From: Vidya Srinivas 

Signed-off-by: Swati Sharma 
Signed-off-by: Vidya Srinivas 
---
  drivers/gpu/drm/i915/intel_display.c | 15 +++
  drivers/gpu/drm/i915/intel_sprite.c  |  3 +++
  2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2b77d93..7c68a0d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3512,6 +3512,12 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
case DRM_FORMAT_NV12:
return PLANE_CTL_FORMAT_NV12;
+   case DRM_FORMAT_Y210:
+   return PLANE_CTL_FORMAT_Y210;
+   case DRM_FORMAT_Y212:
+   return PLANE_CTL_FORMAT_Y212;
+   case DRM_FORMAT_Y216:
+   return PLANE_CTL_FORMAT_Y216;
default:
MISSING_CASE(pixel_format);
}
@@ -4960,6 +4966,9 @@ static int skl_update_scaler_plane(struct 
intel_crtc_state *crtc_state,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_NV12:
+   case DRM_FORMAT_Y210:
+   case DRM_FORMAT_Y212:
+   case DRM_FORMAT_Y216:
break;
default:
DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 
0x%x\n",
@@ -13422,6 +13431,9 @@ static bool skl_plane_format_mod_supported(struct 
drm_plane *_plane,
case DRM_FORMAT_YVYU:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
+   case DRM_FORMAT_Y210:
+   case DRM_FORMAT_Y212:
+   case DRM_FORMAT_Y216:
case DRM_FORMAT_NV12:
if (modifier == I915_FORMAT_MOD_Yf_TILED)
return true;
@@ -14551,6 +14563,9 @@ static int intel_framebuffer_init(struct 
intel_framebuffer *intel_fb,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_YVYU:
case DRM_FORMAT_VYUY:
+   case DRM_FORMAT_Y210:
+   case DRM_FORMAT_Y212:
+   case DRM_FORMAT_Y216:
if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) {
DRM_DEBUG_KMS("unsupported pixel format: %s\n",
  drm_get_format_name(mode_cmd->pixel_format, 
&format_name));


This is wrong place for these cases. These three new YUV formats are 
available on ICL+. You'll need case handling which checks if 
INTEL_GEN(dev_priv) < 11.


/Juha-Pekka


diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 9600ccf..f7e2768 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1419,6 +1419,9 @@ static bool skl_plane_format_mod_supported(struct 
drm_plane *_plane,
case DRM_FORMAT_YVYU:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
+   case DRM_FORMAT_Y210:
+   case DRM_FORMAT_Y212:
+   case DRM_FORMAT_Y216:
case DRM_FORMAT_NV12:
if (modifier == I915_FORMAT_MOD_Yf_TILED)
return true;


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx