Re: [Intel-gfx] [PATCH 3/8] drm/i915: Use new atomic helpers in intel_plane_atomic_check

[Mahesh Kumar]

Hi,


On Thursday 20 July 2017 06:45 PM, Maarten Lankhorst wrote:

Remove the use of plane->state and drm_atomic_get_existing_state,
instead use the new helpers, and also add
intel_atomic_get_new_crtc_state as it's needed.

Signed-off-by: Maarten Lankhorst 
---
  drivers/gpu/drm/i915/intel_atomic_plane.c | 22 +-
  drivers/gpu/drm/i915/intel_drv.h  | 14 ++
  2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index ee76fab7bb6f..1081d0b63b6e 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -198,12 +198,17 @@ int intel_plane_atomic_check_with_state(struct 
intel_crtc_state *crtc_state,
  }
  
  static int intel_plane_atomic_check(struct drm_plane *plane,

-   struct drm_plane_state *state)
+   struct drm_plane_state *new_plane_state)
  {
-   struct drm_crtc *crtc = state->crtc;
-   struct drm_crtc_state *drm_crtc_state;
+   struct drm_atomic_state *state = new_plane_state->state;
+   struct intel_crtc *crtc;
+   struct intel_crtc_state *new_crtc_state;
+   struct drm_plane_state *old_plane_state;
+   struct intel_plane_state *new_intel_plane_state;
  
-	crtc = crtc ? crtc : plane->state->crtc;

+   old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+
+   crtc = to_intel_crtc(new_plane_state->crtc ?: old_plane_state->crtc);
  
  	/*

 * Both crtc and plane->crtc could be NULL if we're updating a

you may want to update comment as well for better understanding. :)

@@ -214,12 +219,11 @@ static int intel_plane_atomic_check(struct drm_plane 
*plane,
if (!crtc)
return 0;
  
-	drm_crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc);

-   if (WARN_ON(!drm_crtc_state))
-   return -EINVAL;
+   new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
+   new_intel_plane_state = to_intel_plane_state(new_plane_state);
  
-	return intel_plane_atomic_check_with_state(to_intel_crtc_state(drm_crtc_state),

-  to_intel_plane_state(state));
+   return intel_plane_atomic_check_with_state(new_crtc_state,
+  new_intel_plane_state);
  }
  
  static void intel_plane_atomic_update(struct drm_plane *plane,

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3723ee443faa..98851ed5ec2b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1957,6 +1957,20 @@ intel_atomic_get_old_crtc_state(struct drm_atomic_state 
*state,
return NULL;
  }
  
+static inline struct intel_crtc_state *

+intel_atomic_get_new_crtc_state(struct drm_atomic_state *state,
+   struct intel_crtc *crtc)
+{
+   struct drm_crtc_state *crtc_state;
+
+   crtc_state = drm_atomic_get_new_crtc_state(state, &crtc->base);
+
+   if (crtc_state)
+   return to_intel_crtc_state(crtc_state);
+   else
+   return NULL;
+}
+
With previous patch comment addressed, This wrapper will be already 
available.

otherwise patch looks ok to me.

-Mahesh

  static inline struct intel_plane_state *
  intel_atomic_get_new_plane_state(struct drm_atomic_state *state,
  struct intel_plane *plane)


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

Re: [Intel-gfx] [PATCH 2/8] drm/i915: Change get_existing_crtc_state to old state

[Maarten Lankhorst]

Hey,

Op 01-08-17 om 08:46 schreef Mahesh Kumar:
> Hi,
>
> As per my understanding "get_existing_*_state == get_new_*_state"
>
> It looks like you are trying to use currently available unused wrapper for 
> completely different purpose. But IMHO you should create a new wrapper for 
> get_old_state
>
> & this should be renamed to get_new_state. :) 

This really depends, and is the reason why we have to rename this.

Before commit:
get_existing_state == get_new_state

After commit:
get_existing_state == get_old_state.

Perhaps I can add both in this patch, I'll clean the series up slightly.

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

Re: [Intel-gfx] [PATCH 1/8] drm/i915: Change use get_new_plane_state instead of existing plane state

[Maarten Lankhorst]

Op 01-08-17 om 08:36 schreef Mahesh Kumar:
> Hi,
>
>
> On Thursday 20 July 2017 06:45 PM, Maarten Lankhorst wrote:
>> The get_existing macros are deprecated and should be replaced by
>> get_old/new_state for clarity.
>>
>> Signed-off-by: Maarten Lankhorst 
>> ---
>>   drivers/gpu/drm/i915/intel_atomic.c | 4 ++--
>>   drivers/gpu/drm/i915/intel_drv.h| 4 ++--
>>   drivers/gpu/drm/i915/intel_pm.c | 5 ++---
>>   3 files changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
>> b/drivers/gpu/drm/i915/intel_atomic.c
>> index 36d4e635e4ce..5d9a26fa5f6d 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -301,8 +301,8 @@ int intel_atomic_setup_scalers(struct drm_i915_private 
>> *dev_priv,
>>   continue;
>>   }
>>   -plane_state = intel_atomic_get_existing_plane_state(drm_state,
>> -intel_plane);
>> +plane_state = intel_atomic_get_new_plane_state(drm_state,
>> +   intel_plane);
>>   scaler_id = &plane_state->scaler_id;
>>   }
>>   diff --git a/drivers/gpu/drm/i915/intel_drv.h 
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 4f9775a05df7..f59507137347 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1958,12 +1958,12 @@ intel_atomic_get_existing_crtc_state(struct 
>> drm_atomic_state *state,
>>   }
>> static inline struct intel_plane_state *
>> -intel_atomic_get_existing_plane_state(struct drm_atomic_state *state,
>> +intel_atomic_get_new_plane_state(struct drm_atomic_state *state,
>> struct intel_plane *plane)
> please fix indentation of arguments as well.
>
>>   {
>>   struct drm_plane_state *plane_state;
>>   -plane_state = drm_atomic_get_existing_plane_state(state, 
>> &plane->base);
>> +plane_state = drm_atomic_get_new_plane_state(state, &plane->base);
>> return to_intel_plane_state(plane_state);
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c 
>> b/drivers/gpu/drm/i915/intel_pm.c
>> index 48785ef75d33..0cdb9453e0e2 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -3028,8 +3028,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc_state 
>> *cstate)
>>   for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
>>   struct intel_plane_state *ps;
>>   -ps = intel_atomic_get_existing_plane_state(state,
>> -   intel_plane);
>> +ps = intel_atomic_get_new_plane_state(state, intel_plane);
>>   if (!ps)
>>   continue;
>>   @@ -4766,7 +4765,7 @@ skl_ddb_add_affected_planes(struct intel_crtc_state 
>> *cstate)
>>   struct drm_plane *plane;
>>   enum pipe pipe = intel_crtc->pipe;
>>   -WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
>> +WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc));
> Patch talks about changing get_existing_plane_state, but you are changing 
> get_existing_crtc_state here.
> IMO it should be part of different patch or patch subject should be changed. 
Yeah ok, this hunk should be separate.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/8] drm/i915: Use intel_atomic_get_new_crtc_state in intel_fbc.c

[Mahesh Kumar]

Hi,

patch looks ok to me.

Reviewed-by: Mahesh Kumar 


On Thursday 20 July 2017 06:45 PM, Maarten Lankhorst wrote:

The previous commit added intel_atomic_get_new_crtc_state,
convert intel_fbc.c to the new helper.

Signed-off-by: Maarten Lankhorst 
---
  drivers/gpu/drm/i915/intel_fbc.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 860b8c26d29b..81d156f04db9 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -1046,7 +1046,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private 
*dev_priv,
  
  	/* Does this atomic commit involve the CRTC currently tied to FBC? */

if (fbc->crtc &&
-   !drm_atomic_get_existing_crtc_state(state, &fbc->crtc->base))
+   !intel_atomic_get_new_crtc_state(state, fbc->crtc))
goto out;
  
  	if (!intel_fbc_can_enable(dev_priv))

@@ -1071,8 +1071,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private 
*dev_priv,
if (fbc_on_plane_a_only(dev_priv) && crtc->plane != PLANE_A)
continue;
  
-		intel_crtc_state = to_intel_crtc_state(

-   drm_atomic_get_existing_crtc_state(state, &crtc->base));
+   intel_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
  
  		intel_crtc_state->enable_fbc = true;

crtc_chosen = true;


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

Re: [Intel-gfx] [PATCH 2/8] drm/i915: Change get_existing_crtc_state to old state

[Mahesh Kumar]

On Tuesday 01 August 2017 01:23 PM, Maarten Lankhorst wrote:

Hey,

Op 01-08-17 om 08:46 schreef Mahesh Kumar:

Hi,

As per my understanding "get_existing_*_state == get_new_*_state"

It looks like you are trying to use currently available unused wrapper for 
completely different purpose. But IMHO you should create a new wrapper for 
get_old_state

& this should be renamed to get_new_state. :)

This really depends, and is the reason why we have to rename this.

Before commit:
get_existing_state == get_new_state

After commit:
get_existing_state == get_old_state.

yeah, right :)


Perhaps I can add both in this patch, I'll clean the series up slightly.

sounds good
-Mahesh




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

Re: [Intel-gfx] [PATCH i-g-t] tests: kms_pipe_color: only test existing properties

[Arkadiusz Hiler]

On Thu, Jul 27, 2017 at 05:45:28PM +0100, Lionel Landwerlin wrote:
> Some platforms might not have degamma or ctm support.
> We can only verify whether those properties behave properly
> if they're available.
> 
> Fixes: aa55641d4 ("tests/kms_color: New test for pipe level color management")
> Signed-off-by: Lionel Landwerlin 
Reviewed-by: Arkadiusz Hiler 

Pushed with the beginning of the commit message changed to
"test/kms_pipe_color: Only..." and a note that I did the change.

Thanks!

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

[Intel-gfx] [PATCH] drm/i915: Update the status of connector when receiving MST unplug event

[Ethan Hsieh]

We do not update the status of connector when receiving MST unplug event.
Call detect function to get latest status and then update status of connector.

Before applying the patch:
[313.665321] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 
0x0020, dig 0x10101012, pins 0x0020
[313.665383] [drm:intel_hpd_irq_handler [i915]] digital hpd port B - long
[313.665436] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN 5 
- cnt: 0
[313.665539] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B - long
[313.944743] [drm:intel_dp_destroy_mst_connector [i915]]
[313.944848] [drm:intel_dp_destroy_mst_connector [i915]]

After applying the patch:
[43.175798] [drm:intel_dp_destroy_mst_connector [i915]] [CONNECTOR:70:DP-4] 
status updated from connected to disconnected
[43.175870] [drm:intel_dp_destroy_mst_connector [i915]]
[43.177675] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] 
[CONNECTOR:70:DP-4]
[43.177679] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] 
[CONNECTOR:70:DP-4] disconnected

Signed-off-by: Ethan Hsieh 
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
b/drivers/gpu/drm/i915/intel_dp_mst.c
index e4ea968..b02a9a8 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -492,6 +492,20 @@ static void intel_dp_destroy_mst_connector(struct 
drm_dp_mst_topology_mgr *mgr,
 {
struct intel_connector *intel_connector = to_intel_connector(connector);
struct drm_i915_private *dev_priv = to_i915(connector->dev);
+   enum drm_connector_status old_status;
+
+   mutex_lock(&connector->dev->mode_config.mutex);
+   old_status = connector->status;
+   connector->status = connector->funcs->detect(connector, false);
+
+   if (old_status != connector->status)
+   DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to 
%s\n",
+ connector->base.id,
+ connector->name,
+ drm_get_connector_status_name(old_status),
+ drm_get_connector_status_name(connector->status));
+
+   mutex_unlock(&connector->dev->mode_config.mutex);
 
drm_connector_unregister(connector);
 
-- 
2.7.4

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Update the status of connector when receiving MST unplug event

[Patchwork]

== Series Details ==

Series: drm/i915: Update the status of connector when receiving MST unplug event
URL   : https://patchwork.freedesktop.org/series/28181/
State : success

== Summary ==

Series 28181v1 drm/i915: Update the status of connector when receiving MST 
unplug event
https://patchwork.freedesktop.org/api/1.0/series/28181/revisions/1/mbox/

Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-atomic:
fail   -> PASS   (fi-snb-2600) fdo#100215
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass   -> SKIP   (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
dmesg-warn -> PASS   (fi-pnv-d510) fdo#101597
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  
time:446s
fi-bdw-gvtdvmtotal:280  pass:266  dwarn:0   dfail:0   fail:0   skip:14  
time:440s
fi-blb-e6850 total:280  pass:225  dwarn:1   dfail:0   fail:0   skip:54  
time:353s
fi-bsw-n3050 total:280  pass:244  dwarn:0   dfail:0   fail:0   skip:36  
time:544s
fi-bxt-j4205 total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:513s
fi-byt-j1900 total:280  pass:255  dwarn:1   dfail:0   fail:0   skip:24  
time:497s
fi-byt-n2820 total:280  pass:251  dwarn:1   dfail:0   fail:0   skip:28  
time:489s
fi-glk-2atotal:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:605s
fi-hsw-4770  total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:437s
fi-hsw-4770r total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:411s
fi-ilk-650   total:280  pass:230  dwarn:0   dfail:0   fail:0   skip:50  
time:417s
fi-ivb-3520m total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:507s
fi-ivb-3770  total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:473s
fi-kbl-7500u total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:469s
fi-kbl-7560u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:576s
fi-kbl-r total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:594s
fi-pnv-d510  total:280  pass:224  dwarn:1   dfail:0   fail:0   skip:55  
time:567s
fi-skl-6260u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:462s
fi-skl-6700hqtotal:280  pass:263  dwarn:0   dfail:0   fail:0   skip:17  
time:586s
fi-skl-6700k total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:471s
fi-skl-6770hqtotal:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:482s
fi-skl-gvtdvmtotal:280  pass:267  dwarn:0   dfail:0   fail:0   skip:13  
time:434s
fi-skl-x1585ltotal:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  
time:481s
fi-snb-2520m total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  
time:548s
fi-snb-2600  total:280  pass:251  dwarn:0   dfail:0   fail:0   skip:29  
time:409s

118657cafa1ebca90cd3f6d3e0fd491a1f0ac38c drm-tip: 2017y-08m-01d-07h-50m-12s UTC 
integration manifest
df84431dd0f8 drm/i915: Update the status of connector when receiving MST unplug 
event

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5302/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t] lib/igt_debugfs: Prevent compiler warning on unchecked printf format

[Arkadiusz Hiler]

On Thu, Jul 27, 2017 at 01:51:28AM -0300, Gabriel Krisman Bertazi wrote:
> Commit 34a54192e1fb ("lib/igt_debugfs: Add extended helper to format
> crc to string") introduced the following compiler warning:
> 
>   igt_debugfs.c: In function ‘igt_crc_to_string_extended’:
>   igt_debugfs.c:373:4: warning: format not a string literal, argument
>   types not checked [-Wformat-nonliteral]
>   i == (crc->n_words - 1) ? '\0' : delimiter);
> 
> This patch addresses the warning while also preventing a possible bad
> memory access access if n_words is > 64.  I have no clue why we
> care about the padding size (crc_size), but since this was added
> recently, I'd rather leave it alone.
> 
> Signed-off-by: Gabriel Krisman Bertazi 
Reviewed-by: Arkadiusz Hiler 

and pushed, thanks!

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

Re: [Intel-gfx] [PATCH i-g-t] lib/igt_debugfs: Prevent compiler warning on unchecked printf format

[Paul Kocialkowski]

On Thu, 2017-07-27 at 01:51 -0300, Gabriel Krisman Bertazi wrote:
> Commit 34a54192e1fb ("lib/igt_debugfs: Add extended helper to format
> crc to string") introduced the following compiler warning:
> 
>   igt_debugfs.c: In function ‘igt_crc_to_string_extended’:
>   igt_debugfs.c:373:4: warning: format not a string literal, argument
>   types not checked [-Wformat-nonliteral]
>   i == (crc->n_words - 1) ? '\0' : delimiter);

Thanks for the fix, I wasn't even aware that the printf format you used
("%0*x%c") existed! This is definitely what I should have done. And it's
definitely better to get rid of the fixed-length allocated buffer.

> This patch addresses the warning while also preventing a possible bad
> memory access access if n_words is > 64.  I have no clue why we
> care about the padding size (crc_size), but since this was added
> recently, I'd rather leave it alone.

That's because the Chamelium CRCs are actually 2 bytes, not the full 4
that the type allows, so this avoids printing heading zeros all the
time.

> Signed-off-by: Gabriel Krisman Bertazi 
> ---
>  lib/igt_debugfs.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 2aa335866066..ee1f0f544824 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -351,7 +351,7 @@ bool igt_check_crc_equal(const igt_crc_t *a, const
> igt_crc_t *b)
>   * igt_crc_to_string_extended:
>   * @crc: pipe CRC value to print
>   * @delimiter: The delimiter to use between crc words
> - * @crc_size: the number of bytes to print per crc word (either 4 or
> 2)
> + * @crc_size: the number of bytes to print per crc word (between 1
> and 4)
>   *
>   * This function allocates a string and formats @crc into it,
> depending on
>   * @delimiter and @crc_size.
> @@ -362,16 +362,19 @@ bool igt_check_crc_equal(const igt_crc_t *a,
> const igt_crc_t *b)
>  char *igt_crc_to_string_extended(igt_crc_t *crc, char delimiter, int
> crc_size)
>  {
>   int i;
> - char *buf = calloc(128, sizeof(char));
> - const char *format[2] = { "%08x%c", "%04x%c" };
> + int len = 0;
> + int field_width = 2 * crc_size; /* Two chars per byte. */
> + char *buf = malloc((field_width+1) * crc->n_words *
> sizeof(char));
>  
>   if (!buf)
>   return NULL;
>  
>   for (i = 0; i < crc->n_words; i++)
> - sprintf(buf + strlen(buf), format[crc_size == 2],
> crc->crc[i],
> - i == (crc->n_words - 1) ? '\0' : delimiter);
> + len += sprintf(buf + len, "%0*x%c", field_width,
> +crc->crc[i], delimiter);
>  
> + /* Eat the last delimiter */
> + buf[len - 1] = '\0';
>   return buf;
>  }
>  
-- 
Paul Kocialkowski 
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing command stream based OA reports and ctx id info.

[Kamble, Sagar A]

-Original Message-
From: Landwerlin, Lionel G 
Sent: Monday, July 31, 2017 9:16 PM
To: Kamble, Sagar A ; intel-gfx@lists.freedesktop.org
Cc: Sourab Gupta 
Subject: Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing 
command stream based OA reports and ctx id info.

On 31/07/17 08:59, Sagar Arun Kamble wrote:
> From: Sourab Gupta 
>
> This patch introduces a framework to capture OA counter reports associated
> with Render command stream. We can then associate the reports captured
> through this mechanism with their corresponding context id's. This can be
> further extended to associate any other metadata information with the
> corresponding samples (since the association with Render command stream
> gives us the ability to capture these information while inserting the
> corresponding capture commands into the command stream).
>
> The OA reports generated in this way are associated with a corresponding
> workload, and thus can be used the delimit the workload (i.e. sample the
> counters at the workload boundaries), within an ongoing stream of periodic
> counter snapshots.
>
> There may be usecases wherein we need more than periodic OA capture mode
> which is supported currently. This mode is primarily used for two usecases:
>  - Ability to capture system wide metrics, alongwith the ability to map
>the reports back to individual contexts (particularly for HSW).
>  - Ability to inject tags for work, into the reports. This provides
>visibility into the multiple stages of work within single context.
>
> The userspace will be able to distinguish between the periodic and CS based
> OA reports by the virtue of source_info sample field.
>
> The command MI_REPORT_PERF_COUNT can be used to capture snapshots of OA
> counters, and is inserted at BB boundaries.
> The data thus captured will be stored in a separate buffer, which will
> be different from the buffer used otherwise for periodic OA capture mode.
> The metadata information pertaining to snapshot is maintained in a list,
> which also has offsets into the gem buffer object per captured snapshot.
> In order to track whether the gpu has completed processing the node,
> a field pertaining to corresponding gem request is added, which is tracked
> for completion of the command.
>
> Both periodic and CS based reports are associated with a single stream
> (corresponding to render engine), and it is expected to have the samples
> in the sequential order according to their timestamps. Now, since these
> reports are collected in separate buffers, these are merge sorted at the
> time of forwarding to userspace during the read call.
>
> v2: Aligning with the non-perf interface (custom drm ioctl based). Also,
> few related patches are squashed together for better readability
>
> v3: Updated perf sample capture emit hook name. Reserving space upfront
> in the ring for emitting sample capture commands and using
> req->fence.seqno for tracking samples. Added SRCU protection for streams.
> Changed the stream last_request tracking to resv object. (Chris)
> Updated perf.sample_lock spin_lock usage to avoid softlockups. Moved
> stream to global per-engine structure. (Sagar)
> Update unpin and put in the free routines to i915_vma_unpin_and_release.
> Making use of perf stream cs_buffer vma resv instead of separate resv obj.
> Pruned perf stream vma resv during gem_idle. (Chris)
> Changed payload field ctx_id to u64 to keep all sample data aligned at 8
> bytes. (Lionel)
> stall/flush prior to sample capture is not added. Do we need to give this
> control to user to select whether to stall/flush at each sample?
>
> Signed-off-by: Sourab Gupta 
> Signed-off-by: Robert Bragg 
> Signed-off-by: Sagar Arun Kamble 
> ---
>   drivers/gpu/drm/i915/i915_drv.h|  101 ++-
>   drivers/gpu/drm/i915/i915_gem.c|1 +
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |8 +
>   drivers/gpu/drm/i915/i915_perf.c   | 1185 
> ++--
>   drivers/gpu/drm/i915/intel_engine_cs.c |4 +
>   drivers/gpu/drm/i915/intel_ringbuffer.c|2 +
>   drivers/gpu/drm/i915/intel_ringbuffer.h|5 +
>   include/uapi/drm/i915_drm.h|   15 +
>   8 files changed, 1073 insertions(+), 248 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2c7456f..8b1cecf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1985,6 +1985,24 @@ struct i915_perf_stream_ops {
>* The stream will always be disabled before this is called.
>*/
>   void (*destroy)(struct i915_perf_stream *stream);
> +
> + /*
> +  * @emit_sample_capture: Emit the commands in the command streamer
> +  * for a particular gpu engine.
> +  *
> +  * The commands are inserted to capture the perf sample data at
> +  * specific points during workload execution, such as before and after
> +  * the batch buffer.

Re: [Intel-gfx] [PATCH v8 6/6] drm/i915/perf: Implement I915_PERF_ADD/REMOVE_CONFIG interface

[Datczuk, Andrzej]

Hi Lionel,

Despite the fact the current whitelist covers less registers than we sent, it 
looks like it covers all we need. 
One comment, according to Marek, whitelisting 0x20CC (selective slice 
programming) makes sense only since gen8.

Just to make sure, are these changes intended also for Gen9 (whitelist the same 
as for gen8)?

Regards,
Andrzej

-Original Message-
From: Landwerlin, Lionel G 
Sent: Friday, July 28, 2017 7:10 PM
To: intel-gfx@lists.freedesktop.org
Cc: Landwerlin, Lionel G ; Auld, Matthew 
; Datczuk, Andrzej 
Subject: [PATCH v8 6/6] drm/i915/perf: Implement I915_PERF_ADD/REMOVE_CONFIG 
interface

The motivation behind this new interface is expose at runtime the creation of 
new OA configs which can be used as part of the i915 perf open interface. This 
will enable the kernel to learn new configs which may be experimental, or 
otherwise not part of the core set currently available through the i915 perf 
interface.

v2: Drop DRM_ERROR for userspace errors (Matthew)
Add padding to userspace structure (Matthew)
s/guid/uuid/ (Matthew)

v3: Use u32 instead of int to iterate through registers (Matthew)

v4: Lock access to dynamic config list (Lionel)

v5: by Matthew:
Fix uninitialized error values
Fix incorrect unwiding when opening perf stream
Use kmalloc_array() to store register
Use uuid_is_valid() to valid config uuids
Declare ioctls as write only
Check padding members are set to 0
by Lionel:
Return ENOENT rather than EINVAL when trying to remove non
existing config

v6: by Chris:
Use ref counts for OA configs
Store UUID in drm_i915_perf_oa_config rather then using pointer
Shuffle fields of drm_i915_perf_oa_config to avoid padding

v7: by Chris
Rename uapi pointers fields to end with '_ptr'

v8: by Andrzej, Marek, Sebastian
Update register whitelisting
by Lionel
Add more register names for documentation
Allow configuration programming in non-paranoid mode
Add support for value filter for a couple of registers already
programmed in other part of the kernel

Signed-off-by: Matthew Auld 
Signed-off-by: Lionel Landwerlin 
Signed-off-by: Andrzej Datczuk 
---
 Documentation/gpu/i915.rst   |   4 +
 drivers/gpu/drm/i915/i915_drv.c  |   2 +
 drivers/gpu/drm/i915/i915_drv.h  |  47   drivers/gpu/drm/i915/i915_perf.c 
| 468 +--
 drivers/gpu/drm/i915/i915_reg.h  |  70 +-
 include/uapi/drm/i915_drm.h  |  20 ++
 6 files changed, 594 insertions(+), 17 deletions(-)

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst index 
9c7ed3e3f1e9..46875c2bcc31 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -417,6 +417,10 @@ integrate with drm/i915 and to handle the 
`DRM_I915_PERF_OPEN` ioctl.
:functions: i915_perf_open_ioctl
 .. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
:functions: i915_perf_release
+.. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
+   :functions: i915_perf_add_config_ioctl .. kernel-doc:: 
+drivers/gpu/drm/i915/i915_perf.c
+   :functions: i915_perf_remove_config_ioctl
 
 i915 Perf Stream
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c 
index 214555e813f1..cc25115c2db7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2729,6 +2729,8 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, 
i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, 
i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, 
DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, 
+i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 };
 
 static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h 
index 771069e14aff..4b84c85f3cc2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1935,6 +1935,8 @@ struct i915_oa_config {
struct attribute_group sysfs_metric;
struct attribute *attrs[2];
struct device_attribute sysfs_metric_id;
+
+   atomic_t ref_count;
 };
 
 struct i915_perf_stream;
@@ -2061,6 +2063,25 @@ struct i915_perf_stream {
  */
 struct i915_oa_ops {
/**
+* @is_valid_b_counter_reg: Validates register's address for
+* programming boolean counters for a particular platform.
+*/
+   bool (*is_valid_b_counter_reg)(struct drm_i915_private *dev_priv,
+  u32 addr);
+
+   /**
+* @is_valid_mux_reg: Validates register's address for programming mux
+* for a particular platform.
+*/
+   bool (*is_valid_mux_reg)(struct drm_i915_private 

Re: [Intel-gfx] [PATCH v2 4/7] drm/omapdrm: Fix omap_atomic_wait_for_completion

[Maarten Lankhorst]

Op 25-07-17 om 10:27 schreef Daniel Vetter:
> On Wed, Jul 19, 2017 at 04:39:17PM +0200, Maarten Lankhorst wrote:
>> Use the new iterator macro and look for crtc_state->active instead of
>> enable, only crtc_state->enable implies that vblanks will happen.
> s/enable/active/, since enable only means logically enabled (aka resources
> reserved). With that my r-b holds.
> -Daniel
Ok thanks. I've pushed patch 1, 4, 5 and 6.

I'll wait a bit longer for the conflict in patch 3, and for a better solution 
on async in patch 2.

It seems the nouveau patches caused a bit of a conflict in 
nv50_disp_atomic_commit_tail
because of better event handling compared to drm-misc, I've fixed up drm-tip.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Remove unnecessary scaler output max size check

[Nabendu Maiti]

There is no upper limit of maximum size of scaler output. Check is
removed.

Signed-off-by: Nabendu Maiti 
---
 drivers/gpu/drm/i915/intel_display.c | 4 +---
 drivers/gpu/drm/i915/intel_drv.h | 2 --
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5a89db1..8e8b1be 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4529,9 +4529,7 @@ static void cpt_verify_modeset(struct drm_device *dev, 
int pipe)
/* range checks */
if (src_w < SKL_MIN_SRC_W || src_h < SKL_MIN_SRC_H ||
dst_w < SKL_MIN_DST_W || dst_h < SKL_MIN_DST_H ||
-
-   src_w > SKL_MAX_SRC_W || src_h > SKL_MAX_SRC_H ||
-   dst_w > SKL_MAX_DST_W || dst_h > SKL_MAX_DST_H) {
+   src_w > SKL_MAX_SRC_W || src_h > SKL_MAX_SRC_H) {
DRM_DEBUG_KMS("scaler_user index %u.%u: src %ux%u dst %ux%u "
"size is out of scaler range\n",
intel_crtc->pipe, scaler_user, src_w, src_h, dst_w, 
dst_h);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ee0daec..f31f16a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -453,9 +453,7 @@ struct intel_initial_plane_config {
 #define SKL_MIN_SRC_H 8
 #define SKL_MAX_SRC_H 4096
 #define SKL_MIN_DST_W 8
-#define SKL_MAX_DST_W 4096
 #define SKL_MIN_DST_H 8
-#define SKL_MAX_DST_H 4096
 
 struct intel_scaler {
int in_use;
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH v8 6/6] drm/i915/perf: Implement I915_PERF_ADD/REMOVE_CONFIG interface

[Lionel Landwerlin]

Hi Andrzej,

Okay, I'll remove 0x20CC from the Haswell whitelist.
Those changes are supposed to cover gen7.5->gen9.

The difference from your initial request should be Gen10 registers.

Cheers,

-
Lionel

On 01/08/17 10:33, Datczuk, Andrzej wrote:

Hi Lionel,

Despite the fact the current whitelist covers less registers than we sent, it 
looks like it covers all we need.
One comment, according to Marek, whitelisting 0x20CC (selective slice 
programming) makes sense only since gen8.

Just to make sure, are these changes intended also for Gen9 (whitelist the same 
as for gen8)?

Regards,
Andrzej

-Original Message-
From: Landwerlin, Lionel G
Sent: Friday, July 28, 2017 7:10 PM
To: intel-gfx@lists.freedesktop.org
Cc: Landwerlin, Lionel G ; Auld, Matthew 
; Datczuk, Andrzej 
Subject: [PATCH v8 6/6] drm/i915/perf: Implement I915_PERF_ADD/REMOVE_CONFIG 
interface

The motivation behind this new interface is expose at runtime the creation of 
new OA configs which can be used as part of the i915 perf open interface. This 
will enable the kernel to learn new configs which may be experimental, or 
otherwise not part of the core set currently available through the i915 perf 
interface.

v2: Drop DRM_ERROR for userspace errors (Matthew)
 Add padding to userspace structure (Matthew)
 s/guid/uuid/ (Matthew)

v3: Use u32 instead of int to iterate through registers (Matthew)

v4: Lock access to dynamic config list (Lionel)

v5: by Matthew:
 Fix uninitialized error values
 Fix incorrect unwiding when opening perf stream
 Use kmalloc_array() to store register
 Use uuid_is_valid() to valid config uuids
 Declare ioctls as write only
 Check padding members are set to 0
 by Lionel:
 Return ENOENT rather than EINVAL when trying to remove non
 existing config

v6: by Chris:
 Use ref counts for OA configs
 Store UUID in drm_i915_perf_oa_config rather then using pointer
 Shuffle fields of drm_i915_perf_oa_config to avoid padding

v7: by Chris
 Rename uapi pointers fields to end with '_ptr'

v8: by Andrzej, Marek, Sebastian
 Update register whitelisting
 by Lionel
 Add more register names for documentation
 Allow configuration programming in non-paranoid mode
 Add support for value filter for a couple of registers already
 programmed in other part of the kernel

Signed-off-by: Matthew Auld 
Signed-off-by: Lionel Landwerlin 
Signed-off-by: Andrzej Datczuk 
---
  Documentation/gpu/i915.rst   |   4 +
  drivers/gpu/drm/i915/i915_drv.c  |   2 +
  drivers/gpu/drm/i915/i915_drv.h  |  47   drivers/gpu/drm/i915/i915_perf.c 
| 468 +--
  drivers/gpu/drm/i915/i915_reg.h  |  70 +-
  include/uapi/drm/i915_drm.h  |  20 ++
  6 files changed, 594 insertions(+), 17 deletions(-)

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst index 
9c7ed3e3f1e9..46875c2bcc31 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -417,6 +417,10 @@ integrate with drm/i915 and to handle the 
`DRM_I915_PERF_OPEN` ioctl.
 :functions: i915_perf_open_ioctl
  .. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
 :functions: i915_perf_release
+.. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
+   :functions: i915_perf_add_config_ioctl .. kernel-doc::
+drivers/gpu/drm/i915/i915_perf.c
+   :functions: i915_perf_remove_config_ioctl
  
  i915 Perf Stream

  
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c 
index 214555e813f1..cc25115c2db7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2729,6 +2729,8 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, 
i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, 
i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, 
DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG,
+i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
  };
  
  static struct drm_driver driver = {

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h 
index 771069e14aff..4b84c85f3cc2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1935,6 +1935,8 @@ struct i915_oa_config {
struct attribute_group sysfs_metric;
struct attribute *attrs[2];
struct device_attribute sysfs_metric_id;
+
+   atomic_t ref_count;
  };
  
  struct i915_perf_stream;

@@ -2061,6 +2063,25 @@ struct i915_perf_stream {
   */
  struct i915_oa_ops {
/**
+* @is_valid_b_counter_reg: Validates register's address for
+* programming boolean counters for a particular platform.
+*/
+   bool (*is_valid_b_counter_

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Remove unnecessary scaler output max size check

[Patchwork]

== Series Details ==

Series: drm/i915: Remove unnecessary scaler output max size check
URL   : https://patchwork.freedesktop.org/series/28187/
State : success

== Summary ==

Series 28187v1 drm/i915: Remove unnecessary scaler output max size check
https://patchwork.freedesktop.org/api/1.0/series/28187/revisions/1/mbox/

Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1

fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  
time:446s
fi-bdw-gvtdvmtotal:280  pass:266  dwarn:0   dfail:0   fail:0   skip:14  
time:433s
fi-blb-e6850 total:280  pass:225  dwarn:1   dfail:0   fail:0   skip:54  
time:361s
fi-bsw-n3050 total:280  pass:244  dwarn:0   dfail:0   fail:0   skip:36  
time:532s
fi-bxt-j4205 total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:509s
fi-glk-2atotal:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:604s
fi-hsw-4770  total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:434s
fi-hsw-4770r total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:418s
fi-ilk-650   total:280  pass:230  dwarn:0   dfail:0   fail:0   skip:50  
time:417s
fi-ivb-3520m total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:508s
fi-ivb-3770  total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:472s
fi-kbl-7500u total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:470s
fi-kbl-7560u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:572s
fi-kbl-r total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:588s
fi-pnv-d510  total:280  pass:223  dwarn:2   dfail:0   fail:0   skip:55  
time:564s
fi-skl-6260u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:464s
fi-skl-6700hqtotal:280  pass:263  dwarn:0   dfail:0   fail:0   skip:17  
time:596s
fi-skl-6700k total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:475s
fi-skl-6770hqtotal:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:475s
fi-skl-gvtdvmtotal:280  pass:267  dwarn:0   dfail:0   fail:0   skip:13  
time:434s
fi-skl-x1585ltotal:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:488s
fi-snb-2520m total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  
time:545s
fi-snb-2600  total:280  pass:250  dwarn:0   dfail:0   fail:1   skip:29  
time:403s

f9cb5a18db70d51c9f5b3db8253d6c42255cab35 drm-tip: 2017y-08m-01d-09h-46m-24s UTC 
integration manifest
0e0c1e63c243 drm/i915: Remove unnecessary scaler output max size check

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5303/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v8 6/6] drm/i915/perf: Implement I915_PERF_ADD/REMOVE_CONFIG interface

[Lionel Landwerlin]

On 31/07/17 18:36, Matthew Auld wrote:

On 28 July 2017 at 18:10, Lionel Landwerlin
 wrote:

The motivation behind this new interface is expose at runtime the
creation of new OA configs which can be used as part of the i915 perf
open interface. This will enable the kernel to learn new configs which
may be experimental, or otherwise not part of the core set currently
available through the i915 perf interface.

v2: Drop DRM_ERROR for userspace errors (Matthew)
 Add padding to userspace structure (Matthew)
 s/guid/uuid/ (Matthew)

v3: Use u32 instead of int to iterate through registers (Matthew)

v4: Lock access to dynamic config list (Lionel)

v5: by Matthew:
 Fix uninitialized error values
 Fix incorrect unwiding when opening perf stream
 Use kmalloc_array() to store register
 Use uuid_is_valid() to valid config uuids
 Declare ioctls as write only
 Check padding members are set to 0
 by Lionel:
 Return ENOENT rather than EINVAL when trying to remove non
 existing config

v6: by Chris:
 Use ref counts for OA configs
 Store UUID in drm_i915_perf_oa_config rather then using pointer
 Shuffle fields of drm_i915_perf_oa_config to avoid padding

v7: by Chris
 Rename uapi pointers fields to end with '_ptr'

v8: by Andrzej, Marek, Sebastian
 Update register whitelisting
 by Lionel
 Add more register names for documentation
 Allow configuration programming in non-paranoid mode
 Add support for value filter for a couple of registers already
 programmed in other part of the kernel

Signed-off-by: Matthew Auld 
Signed-off-by: Lionel Landwerlin 
Signed-off-by: Andrzej Datczuk 
---
  Documentation/gpu/i915.rst   |   4 +
  drivers/gpu/drm/i915/i915_drv.c  |   2 +
  drivers/gpu/drm/i915/i915_drv.h  |  47 
  drivers/gpu/drm/i915/i915_perf.c | 468 +--
  drivers/gpu/drm/i915/i915_reg.h  |  70 +-
  include/uapi/drm/i915_drm.h  |  20 ++
  6 files changed, 594 insertions(+), 17 deletions(-)

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index 9c7ed3e3f1e9..46875c2bcc31 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -417,6 +417,10 @@ integrate with drm/i915 and to handle the 
`DRM_I915_PERF_OPEN` ioctl.
 :functions: i915_perf_open_ioctl
  .. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
 :functions: i915_perf_release
+.. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
+   :functions: i915_perf_add_config_ioctl
+.. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
+   :functions: i915_perf_remove_config_ioctl

While you're here, fancy fixing the kernel doc for
i915_perf_ioctl_locked and gen7_append_oa_reports as a follow up
patch?

Also I don't think I can r-b this one since it has my s-o-b, so we'll
have to rope someone else in to giving it an r-b.



Should I add all of the utility functions too? (like append_oa_status, 
oa_buffer_check_unlocked, append_oa_sample, etc...)


I feel like gen7_*/gen8_* are kind of internal things, not sure how that 
should surface in the documentation...


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

Re: [Intel-gfx] [PATCH] Add drm ioctl DRM_IOCTL_MODE_GETFB2 & associated helpers.

[Laurent Pinchart]

Hi Joe,

Thank you for the patch.

On Monday 31 Jul 2017 11:29:13 Joe Kniss wrote:
> New getfb2 functionality uses drm_mode_fb_cmd2 struct to be symmetric
> with addfb2.

What's the use case for this ? We haven't needed such an ioctl for so long 
that it seemed to me that userspace doesn't really need it, but I could be 
wrong.

> Also modifies *_fb_create_handle() calls to accept a
> format_plane_index so that handles for each plane can be generated.
> Previously, many *_fb_create_handle() calls simply defaulted to plane 0
> only.

And with this patch the amd/amdgpu, armada, gma500, i915, mediatek, msm, 
nouveau and radeon drivers still do. Do none of them support multi-planar 
formats ?

> Signed-off-by: Joe Kniss 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  5 +-
>  drivers/gpu/drm/armada/armada_fb.c  |  1 +
>  drivers/gpu/drm/drm_crtc_internal.h |  2 +
>  drivers/gpu/drm/drm_fb_cma_helper.c | 11 ++--
>  drivers/gpu/drm/drm_framebuffer.c   | 79 +-
>  drivers/gpu/drm/drm_ioctl.c |  1 +
>  drivers/gpu/drm/exynos/exynos_drm_fb.c  |  7 ++-
>  drivers/gpu/drm/gma500/framebuffer.c|  2 +
>  drivers/gpu/drm/i915/intel_display.c|  1 +
>  drivers/gpu/drm/mediatek/mtk_drm_fb.c   |  1 +
>  drivers/gpu/drm/msm/msm_fb.c|  5 +-
>  drivers/gpu/drm/nouveau/nouveau_display.c   |  1 +
>  drivers/gpu/drm/omapdrm/omap_fb.c   |  5 +-
>  drivers/gpu/drm/radeon/radeon_display.c |  5 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  6 ++-
>  drivers/gpu/drm/tegra/fb.c  |  9 +++-
>  include/drm/drm_framebuffer.h   |  1 +
>  include/uapi/drm/drm.h  |  2 +
>  18 files changed, 127 insertions(+), 17 deletions(-)

[snip]

> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> b/drivers/gpu/drm/drm_framebuffer.c index 28a0108a1ab8..67b3be1bedbc 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include "drm_crtc_internal.h"

[snip]

> +/**
> + * drm_mode_getfb2 - get FB info
> + * @dev: drm device for the ioctl
> + * @data: data pointer for the ioctl
> + * @file_priv: drm file for the ioctl call
> + *
> + * Lookup the FB given its ID and return info about it.
> + *
> + * Called by the user via ioctl.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_mode_getfb2(struct drm_device *dev,
> +void *data, struct drm_file *file_priv)
> +{
> + struct drm_mode_fb_cmd2 *r = data;
> + struct drm_framebuffer *fb;
> + int ret, i;
> +
> + if (!drm_core_check_feature(dev, DRIVER_MODESET))
> + return -EINVAL;
> +
> + fb = drm_framebuffer_lookup(dev, r->fb_id);
> + if (!fb)
> + return -ENOENT;
> +
> + r->height = fb->height;
> + r->width = fb->width;
> + r->pixel_format = fb->format->format;
> + for (i = 0; i < 4; ++i) {
> + r->pitches[i] = fb->pitches[i];
> + r->offsets[i] = fb->offsets[i];
> + r->modifier[i] = fb->modifier;
> + r->handles[i] = 0;
> + }
> +
> + for (i = 0; i < fb->format->num_planes; ++i) {
> + if (fb->funcs->create_handle) {
> + if (drm_is_current_master(file_priv) ||
> + capable(CAP_SYS_ADMIN) ||
> + drm_is_control_client(file_priv)) {
> + ret = fb->funcs->create_handle(fb, i,
> file_priv,
> +
> &r->handles[i]);
> + if (ret)
> + break;
> + } else {
> + /* GET_FB() is an unprivileged ioctl so we
> must
> +  * not return a buffer-handle to non-master
> +  * processes! For backwards-compatibility
> +  * reasons, we cannot make GET_FB()
> privileged,
> +  * so just return an invalid handle for
> +  * non-masters. */

There's no backward compatibility to handle here, just make it privileged if 
it has to be.

> + r->handles[i] = 0;
> + ret = 0;
> + }
> + } else {
> + ret = -ENODEV;
> + break;
> + }
> + }
> +
> + /* If handle creation failed, delete/dereference any that were made. 
*/
> + if (ret) {
> + for (i = 0; i < 4; ++i) {
> + if (r->handles[i])
> + drm_gem_handle_delete(file_priv, r-
>handles[i]);

My initial reaction to this was to reply that not all drivers use GEM, but 
after some investigation it seems they actually do. If that's really th

Re: [Intel-gfx] [PATCH v8 6/6] drm/i915/perf: Implement I915_PERF_ADD/REMOVE_CONFIG interface

[Matthew Auld]

On 1 August 2017 at 11:58, Lionel Landwerlin
 wrote:
> On 31/07/17 18:36, Matthew Auld wrote:
>>
>> On 28 July 2017 at 18:10, Lionel Landwerlin
>>  wrote:
>>>
>>> The motivation behind this new interface is expose at runtime the
>>> creation of new OA configs which can be used as part of the i915 perf
>>> open interface. This will enable the kernel to learn new configs which
>>> may be experimental, or otherwise not part of the core set currently
>>> available through the i915 perf interface.
>>>
>>> v2: Drop DRM_ERROR for userspace errors (Matthew)
>>>  Add padding to userspace structure (Matthew)
>>>  s/guid/uuid/ (Matthew)
>>>
>>> v3: Use u32 instead of int to iterate through registers (Matthew)
>>>
>>> v4: Lock access to dynamic config list (Lionel)
>>>
>>> v5: by Matthew:
>>>  Fix uninitialized error values
>>>  Fix incorrect unwiding when opening perf stream
>>>  Use kmalloc_array() to store register
>>>  Use uuid_is_valid() to valid config uuids
>>>  Declare ioctls as write only
>>>  Check padding members are set to 0
>>>  by Lionel:
>>>  Return ENOENT rather than EINVAL when trying to remove non
>>>  existing config
>>>
>>> v6: by Chris:
>>>  Use ref counts for OA configs
>>>  Store UUID in drm_i915_perf_oa_config rather then using pointer
>>>  Shuffle fields of drm_i915_perf_oa_config to avoid padding
>>>
>>> v7: by Chris
>>>  Rename uapi pointers fields to end with '_ptr'
>>>
>>> v8: by Andrzej, Marek, Sebastian
>>>  Update register whitelisting
>>>  by Lionel
>>>  Add more register names for documentation
>>>  Allow configuration programming in non-paranoid mode
>>>  Add support for value filter for a couple of registers already
>>>  programmed in other part of the kernel
>>>
>>> Signed-off-by: Matthew Auld 
>>> Signed-off-by: Lionel Landwerlin 
>>> Signed-off-by: Andrzej Datczuk 
>>> ---
>>>   Documentation/gpu/i915.rst   |   4 +
>>>   drivers/gpu/drm/i915/i915_drv.c  |   2 +
>>>   drivers/gpu/drm/i915/i915_drv.h  |  47 
>>>   drivers/gpu/drm/i915/i915_perf.c | 468
>>> +--
>>>   drivers/gpu/drm/i915/i915_reg.h  |  70 +-
>>>   include/uapi/drm/i915_drm.h  |  20 ++
>>>   6 files changed, 594 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
>>> index 9c7ed3e3f1e9..46875c2bcc31 100644
>>> --- a/Documentation/gpu/i915.rst
>>> +++ b/Documentation/gpu/i915.rst
>>> @@ -417,6 +417,10 @@ integrate with drm/i915 and to handle the
>>> `DRM_I915_PERF_OPEN` ioctl.
>>>  :functions: i915_perf_open_ioctl
>>>   .. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
>>>  :functions: i915_perf_release
>>> +.. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
>>> +   :functions: i915_perf_add_config_ioctl
>>> +.. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c
>>> +   :functions: i915_perf_remove_config_ioctl
>>
>> While you're here, fancy fixing the kernel doc for
>> i915_perf_ioctl_locked and gen7_append_oa_reports as a follow up
>> patch?
>>
>> Also I don't think I can r-b this one since it has my s-o-b, so we'll
>> have to rope someone else in to giving it an r-b.
>>
>
> Should I add all of the utility functions too? (like append_oa_status,
> oa_buffer_check_unlocked, append_oa_sample, etc...)
>
> I feel like gen7_*/gen8_* are kind of internal things, not sure how that
> should surface in the documentation...
>

I guess that would be up to you, fwiw I would be fine with it either
way, just so long as anything non-static has some kernel doc.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] fbdev: Nuke FBINFO_MODULE

[Bartlomiej Zolnierkiewicz]

On Thursday, July 13, 2017 04:01:50 PM Bartlomiej Zolnierkiewicz wrote:
> On Wednesday, July 12, 2017 05:07:42 PM Daniel Vetter wrote:
> > On Wed, Jul 12, 2017 at 2:54 PM, Bartlomiej Zolnierkiewicz
> >  wrote:
> > > On Wednesday, July 12, 2017 02:42:14 PM Daniel Vetter wrote:
> > >> On Wed, Jul 12, 2017 at 12:41:34PM +0200, Bartlomiej Zolnierkiewicz 
> > >> wrote:
> > >> > On Tuesday, July 11, 2017 04:52:19 PM Daniel Vetter wrote:
> > >> > > Instead check info->ops->owner, which amounts to the same.
> > >> > >
> > >> > > Spotted because I want to remove the pile of broken and cargo-culted
> > >> > > fb_info->flags assignments in drm drivers.
> > >> > >
> > >> > > v2: Fixup matrox (reported by kbuild). Also nuke FBINFO_FLAG_* 
> > >> > > defines
> > >> > > that I've failed to spot.
> > >> > >
> > >> > > v3: Don't nuke FBINFO_FLAG_DEFAULT, that's used all over the place.
> > >> > >
> > >> > > Cc: Bartlomiej Zolnierkiewicz 
> > >> > > Cc: linux-fb...@vger.kernel.org
> > >> > > Signed-off-by: Daniel Vetter 
> > >> >
> > >> > Acked-by: Bartlomiej Zolnierkiewicz 
> > >>
> > >> Do you plan to pick these two patches up yourself, or do you expect me to
> > >> merge them?
> > >
> > > Since the original patchset contained DRM changes (two last patches)
> > > depending on fbdev changes (two first patches, the patch being discussed
> > > was the second one) I assumed that you would like to take them all
> > > through DRM tree. If this is not what is preferred, please tell me.
> > 
> > There's no direct depency between 1&2 and 3&4, the only effect of
> > merging them through separate trees is that the bootup logo might not
> > show up when it's expected, until the trees are merged together. We
> > could merge them through separate trees if you prefer that (I forgot
> > to mention that in the cover letter), but I'm fine with putting them
> > all into drm-misc with your ack for 4.14.
> > 
> > Whatever you prefer, I don't mind either way.
> 
> Then I will merge patches 1&2 for v4.14 through fbdev tree (there are
> some other changes pending touching fbdev core and I would like to avoid
> conflicts between fbdev & drm-misc trees). Thanks!

Patch queued for 4.14, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

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

Re: [Intel-gfx] [PATCH 1/4] fbcon: Make fbcon a built-time depency for fbdev

[Bartlomiej Zolnierkiewicz]

On Wednesday, July 12, 2017 12:40:45 PM Bartlomiej Zolnierkiewicz wrote:
> On Thursday, July 06, 2017 02:57:32 PM Daniel Vetter wrote:
> > There's a bunch of folks who're trying to make printk less
> > contended and faster, but there's a problem: printk uses the
> > console_lock, and the console lock has become the BKL for all things
> > fbdev/fbcon, which in turn pulled in half the drm subsystem under that
> > lock. That's awkward.
> > 
> > There reasons for that is probably just a historical accident:
> > 
> > - fbcon is a runtime option of fbdev, i.e. at runtime you can pick
> >   whether your fbdev driver instances are used as kernel consoles.
> >   Unfortunately this wasn't implemented with some module option, but
> >   through some module loading magic: As long as you don't load
> >   fbcon.ko, there's no fbdev console support, but loading it (in any
> >   order wrt fbdev drivers) will create console instances for all fbdev
> >   drivers.
> > 
> > - This was implemented through a notifier chain. fbcon.ko enumerates
> >   all fbdev instances at load time and also registers itself as
> >   listener in the fbdev notifier. The fbdev core tries to register new
> >   fbdev instances with fbcon using the notifier.
> > 
> > - On top of that the modifier chain is also used at runtime by the
> >   fbdev subsystem to e.g. control backlights for panels.
> > 
> > - The problem is that the notifier puts a mutex locking context
> >   between fbdev and fbcon, which mixes up the locking contexts for
> >   both the runtime usage and the register time usage to notify fbcon.
> >   And at runtime fbcon (through the fbdev core) might call into the
> >   notifier from a printk critical section while console_lock is held.
> > 
> > - This means console_lock must be an outer lock for the entire fbdev
> >   subsystem, which also means it must be acquired when registering a
> >   new framebuffer driver as the outermost lock since we might call
> >   into fbcon (through the notifier) which would result in a locking
> >   inversion if fbcon would acquire the console_lock from its notifier
> >   callback (which it needs to register the console).
> > 
> > - console_lock can be held anywhere, since printk can be called
> >   anywhere, and through the above story, plus drm/kms being an fbdev
> >   driver, we pull in a shocking amount of locking hiercharchy
> >   underneath the console_lock. Which makes cleaning up printk really
> >   hard (not even splitting console_lock into an rwsem is all that
> >   useful due to this).
> > 
> > There's various ways to address this, but the cleanest would be to
> > make fbcon a compile-time option, where fbdev directly calls the fbcon
> > register functions from register_framebuffer, or dummy static inline
> > versions if fbcon is disabled. Maybe augmented with a runtime knob to
> > disable fbcon, if that's needed (for debugging perhaps).
> > 
> > But this could break some users who rely on the magic "loading
> > fbcon.ko enables/disables fbdev framebuffers at runtime" thing, even
> > if that's unlikely. Hence we must be careful:
> > 
> > 1. Create a compile-time dependency between fbcon and fbdev in the
> > least minimal way. This is what this patch does.
> > 
> > 2. Wait at least 1 year to give possible users time to scream about
> > how we broke their setup. Unlikely, since all distros make fbcon
> > compile-in, and embedded platforms only compile stuff they know they
> > need anyway. But still.
> > 
> > 3. Convert the notifier to direct functions calls, with dummy static
> > inlines if fbcon is disabled. We'll still need the fb notifier for the
> > other uses (like backlights), but we can probably move it into the fb
> > core (atm it must be built-into vmlinux).
> > 
> > 4. Push console_lock down the call-chain, until it is down in
> > console_register again.
> > 
> > 5. Finally start to clean up and rework the printk/console locking.
> > 
> > For context of this saga see
> > 
> > commit 50e244cc793d511b86adea24972f3a7264cae114
> > Author: Alan Cox 
> > Date:   Fri Jan 25 10:28:15 2013 +1000
> > 
> > fb: rework locking to fix lock ordering on takeover
> > 
> > plus the pile of commits on top that tried to make this all work
> > without terminally upsetting lockdep. We've uncovered all this when
> > console_lock lockdep annotations where added in
> > 
> > commit daee779718a319ff9f83e1ba3339334ac650bb22
> > Author: Daniel Vetter 
> > Date:   Sat Sep 22 19:52:11 2012 +0200
> > 
> > console: implement lockdep support for console_lock
> > 
> > On the patch itself:
> > - Switch CONFIG_FRAMEBUFFER_CONSOLE to be a boolean, using the overall
> >   CONFIG_FB tristate to decided whether it should be a module or
> >   built-in.
> > 
> > - At first I thought I could force the build depency with just a dummy
> >   symbol that fbcon.ko exports and fb.ko uses. But that leads to a
> >   module depency cycle (it works fine when built-in).
> > 
> >   Since this tight binding is the entire goal the simple

Re: [Intel-gfx] [PATCH 6/6] drm/i915: Add support for CCS modifiers

[Ben Widawsky]

On 17-07-31 10:29:55, Daniel Vetter wrote:

On Sat, Jul 29, 2017 at 09:25:50AM -0700, Ben Widawsky wrote:

On 17-07-29 13:53:10, Daniel Stone wrote:
> Hi Ben,
>
> On 26 July 2017 at 19:08, Ben Widawsky  wrote:
> > +   } else if (INTEL_GEN(dev_priv) >= 9) {
> > intel_primary_formats = skl_primary_formats;
> > num_formats = ARRAY_SIZE(skl_primary_formats);
> > -   modifiers = skl_format_modifiers;
> > +   if (pipe >= PIPE_C)
> > +   modifiers = skl_format_modifiers_ccs;
> > +   else
> > +   modifiers = skl_format_modifiers_noccs;
>
> Shouldn't that be (pipe < PIPE_C)?
>
> Cheers,
> Daniel

Yep. It was wrong in v7 too :/. I have it fixed locally.


Shouldn't the igt catch this, or does it not try to exercise all the
plane/crtc combos there are?
-Daniel


I don't know whether or not IGT should have caught this, but it wouldn't have
because I had been sending these without Ville's patches, so my patches alone
don't even build (since his patches defined the modifiers).


--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Intel-gfx] [PATCH] drm/i915/cnl: DDIA Lane capability bit not set in clone mode

[clinton . a . taylor]

From: Clint Taylor 

DDIA Lane capability control 4 lane bit is not being set by firmware during
clone mode boot. This occurs when multiple monitors are connected during
boot. The driver will configure the port for 2 lane maximum width if this
bit is not set.

Once DDIA/E lane split is supported in vbt and the i915 driver we will need
to revisit this code.

Cc: Rodrigo Vivi 
Signed-off-by: Clint Taylor 
---
 drivers/gpu/drm/i915/intel_ddi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 494fbe0..e7644b4 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2713,9 +2713,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
enum port port)
 * configuration so that we use the proper lane count for our
 * calculations.
 */
-   if (IS_GEN9_LP(dev_priv) && port == PORT_A) {
+   if ((IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10) &&
+   port == PORT_A) {
if (!(intel_dig_port->saved_port_bits & DDI_A_4_LANES)) {
-   DRM_DEBUG_KMS("BXT BIOS forgot to set DDI_A_4_LANES for 
port A; fixing\n");
+   DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for 
port A\n");
intel_dig_port->saved_port_bits |= DDI_A_4_LANES;
max_lanes = 4;
}
-- 
1.9.1

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

[Intel-gfx] [PATCH 1/6] drm/i915: Implement .get_format_info() hook for CCS

[Ben Widawsky]

From: Ville Syrjälä 

SKL+ display engine can scan out certain kinds of compressed surfaces
produced by the render engine. This involved telling the display engine
the location of the color control surfae (CCS) which describes which
parts of the main surface are compressed and which are not. The location
of CCS is provided by userspace as just another plane with its own offset.

By providing our own format information for the CCS formats, we should
be able to make framebuffer_check() do the right thing for the CCS
surface as well.

Note that we'll return the same format info for both Y and Yf tiled
format as that's what happens with the non-CCS Y vs. Yf as well. If
desired, we could potentially return a unique pointer for each
pixel_format+tiling+ccs combination, in which case we immediately be
able to tell if any of that stuff changed by just comparing the
pointers. But that does sound a bit wasteful space wise.

v2: Drop the 'dev' argument from the hook
v3: Include the description of the CCS surface layout
v4: Pretend CCS tiles are regular 128 byte wide Y tiles (Jason)

Cc: Daniel Vetter 
Cc: Ben Widawsky 
Cc: Jason Ekstrand 
Reviewed-by: Ben Widawsky  (v3)
Signed-off-by: Ville Syrjä 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/drm_fourcc.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 37 
 include/drm/drm_mode_config.h|  3 ++-
 include/uapi/drm/drm_fourcc.h|  3 +++
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 9c0152df45ad..50da6180c495 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -222,7 +222,7 @@ drm_get_format_info(struct drm_device *dev,
const struct drm_format_info *info = NULL;
 
if (dev->mode_config.funcs->get_format_info)
-   info = dev->mode_config.funcs->get_format_info(mode_cmd);
+   info = dev->mode_config.funcs->get_format_info(dev, mode_cmd);
 
if (!info)
info = drm_format_info(mode_cmd->pixel_format);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e92fd14c06c7..6b00689ef6e0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2433,6 +2433,42 @@ static unsigned int intel_fb_modifier_to_tiling(uint64_t 
fb_modifier)
}
 }
 
+static const struct drm_format_info ccs_formats[] = {
+   { .format = DRM_FORMAT_XRGB, .depth = 24, .num_planes = 2, .cpp = { 
4, 1, }, .hsub = 16, .vsub = 8, },
+   { .format = DRM_FORMAT_XBGR, .depth = 24, .num_planes = 2, .cpp = { 
4, 1, }, .hsub = 16, .vsub = 8, },
+   { .format = DRM_FORMAT_ARGB, .depth = 32, .num_planes = 2, .cpp = { 
4, 1, }, .hsub = 16, .vsub = 8, },
+   { .format = DRM_FORMAT_ABGR, .depth = 32, .num_planes = 2, .cpp = { 
4, 1, }, .hsub = 16, .vsub = 8, },
+};
+
+static const struct drm_format_info *
+lookup_format_info(const struct drm_format_info formats[],
+  int num_formats, u32 format)
+{
+   int i;
+
+   for (i = 0; i < num_formats; i++) {
+   if (formats[i].format == format)
+   return &formats[i];
+   }
+
+   return NULL;
+}
+
+static const struct drm_format_info *
+intel_get_format_info(struct drm_device *dev,
+ const struct drm_mode_fb_cmd2 *cmd)
+{
+   switch (cmd->modifier[0]) {
+   case I915_FORMAT_MOD_Y_TILED_CCS:
+   case I915_FORMAT_MOD_Yf_TILED_CCS:
+   return lookup_format_info(ccs_formats,
+ ARRAY_SIZE(ccs_formats),
+ cmd->pixel_format);
+   default:
+   return NULL;
+   }
+}
+
 static int
 intel_fill_fb_info(struct drm_i915_private *dev_priv,
   struct drm_framebuffer *fb)
@@ -14630,6 +14666,7 @@ static void intel_atomic_state_free(struct 
drm_atomic_state *state)
 
 static const struct drm_mode_config_funcs intel_mode_funcs = {
.fb_create = intel_user_framebuffer_create,
+   .get_format_info = intel_get_format_info,
.output_poll_changed = intel_fbdev_output_poll_changed,
.atomic_check = intel_atomic_check,
.atomic_commit = intel_atomic_commit,
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 42981711189b..f0d3d3857ae2 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -81,7 +81,8 @@ struct drm_mode_config_funcs {
 * The format information specific to the given fb metadata, or
 * NULL if none is found.
 */
-   const struct drm_format_info *(*get_format_info)(const struct 
drm_mode_fb_cmd2 *mode_cmd);
+   const struct drm_format_info *(*get_format_info)(struct drm_device *dev,
+   const struct drm_mode_fb_cmd2 *mode_cmd);
 
/**
 * @output_poll_changed:
diff --git a/in

[Intel-gfx] [PATCH 3/6] [v7] drm: Plumb modifiers through plane init

[Ben Widawsky]

This is the plumbing for supporting fb modifiers on planes. Modifiers
have already been introduced to some extent, but this series will extend
this to allow querying modifiers per plane. Based on this, the client to
enable optimal modifications for framebuffers.

This patch simply allows the DRM drivers to initialize their list of
supported modifiers upon initializing the plane.

v2: A minor addition from Daniel

v3:
* Updated commit message
* s/INVALID/DRM_FORMAT_MOD_INVALID (Liviu)
* Remove some excess newlines (Liviu)
* Update comment for > 64 modifiers (Liviu)

v4: Minor comment adjustments (Liviu)

v5: Some new platforms added due to rebase

v6: Add some missed plane inits (or maybe they're new - who knows at
this point) (Daniel)

v7: Add sun8i (Daniel)

Signed-off-by: Ben Widawsky 
Reviewed-by: Daniel Stone  (v2)
Reviewed-by: Liviu Dudau 
Acked-by: Philippe Cornu  (for stm)
Tested-by: Philippe Cornu  (for stm)
---
 drivers/gpu/drm/arc/arcpgu_crtc.c   |  1 +
 drivers/gpu/drm/arm/hdlcd_crtc.c|  1 +
 drivers/gpu/drm/arm/malidp_planes.c |  2 +-
 drivers/gpu/drm/armada/armada_crtc.c|  1 +
 drivers/gpu/drm/armada/armada_overlay.c |  1 +
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c |  3 ++-
 drivers/gpu/drm/drm_modeset_helper.c|  1 +
 drivers/gpu/drm/drm_plane.c | 36 -
 drivers/gpu/drm/drm_simple_kms_helper.c |  3 +++
 drivers/gpu/drm/exynos/exynos_drm_plane.c   |  2 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c |  2 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c  |  1 +
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c|  5 +++-
 drivers/gpu/drm/i915/intel_sprite.c |  4 +--
 drivers/gpu/drm/imx/ipuv3-plane.c   |  4 +--
 drivers/gpu/drm/mediatek/mtk_drm_plane.c|  2 +-
 drivers/gpu/drm/meson/meson_plane.c |  1 +
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c   |  2 +-
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c   |  4 +--
 drivers/gpu/drm/mxsfb/mxsfb_drv.c   |  2 +-
 drivers/gpu/drm/nouveau/nv50_display.c  |  5 ++--
 drivers/gpu/drm/omapdrm/omap_plane.c|  2 +-
 drivers/gpu/drm/pl111/pl111_display.c   |  2 +-
 drivers/gpu/drm/qxl/qxl_display.c   |  2 +-
 drivers/gpu/drm/rcar-du/rcar_du_plane.c |  4 +--
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c   |  4 +--
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  4 +--
 drivers/gpu/drm/sti/sti_cursor.c|  2 +-
 drivers/gpu/drm/sti/sti_gdp.c   |  2 +-
 drivers/gpu/drm/sti/sti_hqvdp.c |  2 +-
 drivers/gpu/drm/stm/ltdc.c  |  2 +-
 drivers/gpu/drm/sun4i/sun4i_layer.c |  2 +-
 drivers/gpu/drm/sun4i/sun8i_layer.c |  2 +-
 drivers/gpu/drm/tegra/dc.c  | 12 -
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  2 +-
 drivers/gpu/drm/vc4/vc4_plane.c |  2 +-
 drivers/gpu/drm/virtio/virtgpu_plane.c  |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c|  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c|  4 +--
 drivers/gpu/drm/zte/zx_plane.c  |  2 +-
 include/drm/drm_plane.h | 22 ++-
 include/drm/drm_simple_kms_helper.h |  1 +
 include/uapi/drm/drm_fourcc.h   | 11 
 45 files changed, 131 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c 
b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 1859dd3ad622..799416651f2f 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -217,6 +217,7 @@ static struct drm_plane *arc_pgu_plane_init(struct 
drm_device *drm)
 
ret = drm_universal_plane_init(drm, plane, 0xff, &arc_pgu_plane_funcs,
   formats, ARRAY_SIZE(formats),
+  NULL,
   DRM_PLANE_TYPE_PRIMARY, NULL);
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 16e1e20cf04c..72b22b805412 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -315,6 +315,7 @@ static struct drm_plane *hdlcd_plane_init(struct drm_device 
*drm)
 
ret = drm_universal_plane_init(drm, plane, 0xff, &hdlcd_plane_funcs,
   formats, ARRAY_SIZE(formats),
+  NULL,
   DRM_PLANE_TYPE_PRIMARY, NULL);
if (ret) {
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/arm/malidp_planes.c 
b/drivers/gpu/drm/arm/malidp_planes.c
index 600fa7bd7f52..60402e27882f 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
++

[Intel-gfx] [PATCH 2/6] drm/i915: Add render decompression support

[Ben Widawsky]

From: Ville Syrjälä 

SKL+ display engine can scan out certain kinds of compressed surfaces
produced by the render engine. This involved telling the display engine
the location of the color control surfae (CCS) which describes
which parts of the main surface are compressed and which are not. The
location of CCS is provided by userspace as just another plane with its
own offset.

Add the required stuff to validate the user provided AUX plane metadata
and convert the user provided linear offset into something the hardware
can consume.

Due to hardware limitations we require that the main surface and
the AUX surface (CCS) be part of the same bo. The hardware also
makes life hard by not allowing you to provide separate x/y offsets
for the main and AUX surfaces (excpet with NV12), so finding suitable
offsets for both requires a bit of work. Assuming we still want keep
playing tricks with the offsets. I've just gone with a dumb "search
backward for suitable offsets" approach, which is far from optimal,
 but it works.

 Also not all planes will be capable of scanning out compressed 
surfaces,
 and eg. 90/270 degree rotation is not supported in combination 
with
 decompression either.

 This patch may contain work from at least the following people:
 * Vandana Kannan 
 * Daniel Vetter 
 * Ben Widawsky 

v2: Deal with display workarounds 0390, 0531, 1125 (Paulo)
v3: Pretend CCS tiles are regular 128 byte wide Y tiles (Jason)
Put the AUX register defines to the correct place
Fix up the slightly bogus rotation check
v4: Use I915_WRITE_FW() due to plane update locking changes
s/return -EINVAL/goto err/ in intel_framebuffer_init()
Eliminate a bunch hardcoded numbers in CCS code

v5: (By Ben)
conflict resolution +
-   res_blocks += fixed_16_16_to_u32_round_up(y_tile_minimum);
+   res_blocks += fixed16_to_u32_round_up(y_tile_minimum);

Cc: Paulo Zanoni 
Cc: Daniel Vetter 
Cc: Ben Widawsky 
Cc: Jason Ekstrand 
Signed-off-by: Ville Syrjä 
Reviewed-by: Ben Widawsky  (v1)
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_reg.h  |  23 
 drivers/gpu/drm/i915/intel_display.c | 233 ---
 drivers/gpu/drm/i915/intel_pm.c  |  29 -
 drivers/gpu/drm/i915/intel_sprite.c  |   5 +
 4 files changed, 272 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c712d01f92ab..cea4f941a56e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6106,6 +6106,10 @@ enum {
 #define _PLANE_KEYMSK_2_A  0x70298
 #define _PLANE_KEYMAX_1_A  0x701a0
 #define _PLANE_KEYMAX_2_A  0x702a0
+#define _PLANE_AUX_DIST_1_A0x701c0
+#define _PLANE_AUX_DIST_2_A0x702c0
+#define _PLANE_AUX_OFFSET_1_A  0x701c4
+#define _PLANE_AUX_OFFSET_2_A  0x702c4
 #define _PLANE_COLOR_CTL_1_A   0x701CC /* GLK+ */
 #define _PLANE_COLOR_CTL_2_A   0x702CC /* GLK+ */
 #define _PLANE_COLOR_CTL_3_A   0x703CC /* GLK+ */
@@ -6212,6 +6216,24 @@ enum {
 #define PLANE_NV12_BUF_CFG(pipe, plane)\
_MMIO_PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe), 
_PLANE_NV12_BUF_CFG_2(pipe))
 
+#define _PLANE_AUX_DIST_1_B0x711c0
+#define _PLANE_AUX_DIST_2_B0x712c0
+#define _PLANE_AUX_DIST_1(pipe) \
+   _PIPE(pipe, _PLANE_AUX_DIST_1_A, _PLANE_AUX_DIST_1_B)
+#define _PLANE_AUX_DIST_2(pipe) \
+   _PIPE(pipe, _PLANE_AUX_DIST_2_A, _PLANE_AUX_DIST_2_B)
+#define PLANE_AUX_DIST(pipe, plane) \
+   _MMIO_PLANE(plane, _PLANE_AUX_DIST_1(pipe), _PLANE_AUX_DIST_2(pipe))
+
+#define _PLANE_AUX_OFFSET_1_B  0x711c4
+#define _PLANE_AUX_OFFSET_2_B  0x712c4
+#define _PLANE_AUX_OFFSET_1(pipe)   \
+   _PIPE(pipe, _PLANE_AUX_OFFSET_1_A, _PLANE_AUX_OFFSET_1_B)
+#define _PLANE_AUX_OFFSET_2(pipe)   \
+   _PIPE(pipe, _PLANE_AUX_OFFSET_2_A, _PLANE_AUX_OFFSET_2_B)
+#define PLANE_AUX_OFFSET(pipe, plane)   \
+   _MMIO_PLANE(plane, _PLANE_AUX_OFFSET_1(pipe), _PLANE_AUX_OFFSET_2(pipe))
+
 #define _PLANE_COLOR_CTL_1_B   0x711CC
 #define _PLANE_COLOR_CTL_2_B   0x712CC
 #define _PLANE_COLOR_CTL_3_B   0x713CC
@@ -6695,6 +6717,7 @@ enum {
 # define CHICKEN3_DGMG_DONE_FIX_DISABLE(1 << 2)
 
 #define CHICKEN_PAR1_1 _MMIO(0x42080)
+#define  SKL_RC_HASH_OUTSIDE   (1 << 15)
 #define  DPA_MASK_VBLANK_SRD   (1 << 15)
 #define  FORCE_ARB_IDLE_PLANES (1 << 14)
 #define  SKL_EDP_PSR_FIX_RDWRAP(1 << 3)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6b00689ef6e0..f45f55998a5f 100644
--- a

[Intel-gfx] [PATCH 4/6] [v5] drm: Create a format/modifier blob

[Ben Widawsky]

Updated blob layout (Rob, Daniel, Kristian, xerpi)

v2:
* Removed __packed, and alignment (.+)
* Fix indent in drm_format_modifier fields (Liviu)
* Remove duplicated modifier > 64 check (Liviu)
* Change comment about modifier (Liviu)
* Remove arguments to blob creation, use plane instead (Liviu)
* Fix data types (Ben)
* Make the blob part of uapi (Daniel)

v3:
Remove unused ret field.
Change i, and j to unsigned int (Emil)

v4:
Use plane->modifier_count instead of recounting (Daniel)

v5:
Rename modifiers to modifiers_property (Ville)
Use sizeof(__u32) instead to reflect UAPI nature (Ville)
Make BUILD_BUG_ON for blob header size

Cc: Rob Clark 
Cc: Kristian H. Kristensen 
Signed-off-by: Ben Widawsky 
Reviewed-by: Daniel Stone  (v2)
Reviewed-by: Liviu Dudau  (v2)
Reviewed-by: Emil Velikov  (v3)
---
 drivers/gpu/drm/drm_mode_config.c |  7 
 drivers/gpu/drm/drm_plane.c   | 84 +++
 include/drm/drm_mode_config.h |  6 +++
 include/uapi/drm/drm_mode.h   | 50 +++
 4 files changed, 147 insertions(+)

diff --git a/drivers/gpu/drm/drm_mode_config.c 
b/drivers/gpu/drm/drm_mode_config.c
index d9862259a2a7..74f6ff5df656 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -337,6 +337,13 @@ static int drm_mode_create_standard_properties(struct 
drm_device *dev)
return -ENOMEM;
dev->mode_config.gamma_lut_size_property = prop;
 
+   prop = drm_property_create(dev,
+  DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB,
+  "IN_FORMATS", 0);
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.modifiers_property = prop;
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index d3fc561d7b48..5c14beee52ff 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -62,6 +62,87 @@ static unsigned int drm_num_planes(struct drm_device *dev)
return num;
 }
 
+static inline u32 *
+formats_ptr(struct drm_format_modifier_blob *blob)
+{
+   return (u32 *)(((char *)blob) + blob->formats_offset);
+}
+
+static inline struct drm_format_modifier *
+modifiers_ptr(struct drm_format_modifier_blob *blob)
+{
+   return (struct drm_format_modifier *)(((char *)blob) + 
blob->modifiers_offset);
+}
+
+static int create_in_format_blob(struct drm_device *dev, struct drm_plane 
*plane)
+{
+   const struct drm_mode_config *config = &dev->mode_config;
+   struct drm_property_blob *blob;
+   struct drm_format_modifier *mod;
+   size_t blob_size, formats_size, modifiers_size;
+   struct drm_format_modifier_blob *blob_data;
+   unsigned int i, j;
+
+   formats_size = sizeof(__u32) * plane->format_count;
+   if (WARN_ON(!formats_size)) {
+   /* 0 formats are never expected */
+   return 0;
+   }
+
+   modifiers_size =
+   sizeof(struct drm_format_modifier) * plane->modifier_count;
+
+   blob_size = sizeof(struct drm_format_modifier_blob);
+   /* Modifiers offset is a pointer to a struct with a 64 bit field so it
+* should be naturally aligned to 8B.
+*/
+   BUILD_BUG_ON(sizeof(struct drm_format_modifier_blob) % 8);
+   blob_size += ALIGN(formats_size, 8);
+   blob_size += modifiers_size;
+
+   blob = drm_property_create_blob(dev, blob_size, NULL);
+   if (IS_ERR(blob))
+   return -1;
+
+   blob_data = (struct drm_format_modifier_blob *)blob->data;
+   blob_data->version = FORMAT_BLOB_CURRENT;
+   blob_data->count_formats = plane->format_count;
+   blob_data->formats_offset = sizeof(struct drm_format_modifier_blob);
+   blob_data->count_modifiers = plane->modifier_count;
+
+   blob_data->modifiers_offset =
+   ALIGN(blob_data->formats_offset + formats_size, 8);
+
+   memcpy(formats_ptr(blob_data), plane->format_types, formats_size);
+
+   /* If we can't determine support, just bail */
+   if (!plane->funcs->format_mod_supported)
+   goto done;
+
+   mod = modifiers_ptr(blob_data);
+   for (i = 0; i < plane->modifier_count; i++) {
+   for (j = 0; j < plane->format_count; j++) {
+   if (plane->funcs->format_mod_supported(plane,
+  
plane->format_types[j],
+  
plane->modifiers[i])) {
+
+   mod->formats |= 1 << j;
+   }
+   }
+
+   mod->modifier = plane->modifiers[i];
+   mod->offset = 0;
+   mod->pad = 0;
+   mod++;
+   }
+
+done:
+   drm_object_attach_property(&plane->base, config->modifiers_property,
+  blob->base.id);
+
+   return 0;
+}
+
 /**
  * drm_universal_plane_init - 

[Intel-gfx] [PATCH 5/6] [v10] drm/i915: Add format modifiers for Intel

[Ben Widawsky]

This was based on a patch originally by Kristian. It has been modified
pretty heavily to use the new callbacks from the previous patch.

v2:
  - Add LINEAR and Yf modifiers to list (Ville)
  - Combine i8xx and i965 into one list of formats (Ville)
  - Allow 1010102 formats for Y/Yf tiled (Ville)

v3:
  - Handle cursor formats (Ville)
  - Put handling for LINEAR in the mod_support functions (Ville)

v4:
  - List each modifier explicitly in supported modifiers (Ville)
  - Handle the CURSOR plane (Ville)

v5:
  - Split out cursor and sprite handling (Ville)

v6:
  - Actually use the sprite funcs (Emil)
  - Use unreachable (Emil)

v7:
  - Only allow Intel modifiers and LINEAR (Ben)

v8
  - Fix spite assert introduced in v6 (Daniel)

v9
  - Change vendor check logic to avoid magic 56 (Emil)
  - Reorder skl_mod_support (Ville)
  - make intel_plane_funcs static, could be done as of v5 (Ville)
  - rename local variable intel_format_modifiers to modifiers (Ville)
- actually use sprite modifiers
  - split out modifier/formats by platform (Ville)

v10:
  - Undo vendor check from v9

Cc: Ville Syrjälä 
Cc: Kristian H. Kristensen 
Reviewed-by: Emil Velikov  (v8)
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/intel_display.c | 127 +--
 drivers/gpu/drm/i915/intel_drv.h |   1 -
 drivers/gpu/drm/i915/intel_sprite.c  | 141 ++-
 3 files changed, 259 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b9dda03678c0..ad49b99ef25f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -72,6 +72,12 @@ static const uint32_t i965_primary_formats[] = {
DRM_FORMAT_XBGR2101010,
 };
 
+static const uint64_t i9xx_format_modifiers[] = {
+   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID
+};
+
 static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_C8,
DRM_FORMAT_RGB565,
@@ -87,11 +93,24 @@ static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_VYUY,
 };
 
+static const uint64_t skl_format_modifiers[] = {
+   I915_FORMAT_MOD_Yf_TILED,
+   I915_FORMAT_MOD_Y_TILED,
+   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID
+};
+
 /* Cursor formats */
 static const uint32_t intel_cursor_formats[] = {
DRM_FORMAT_ARGB,
 };
 
+static const uint64_t cursor_format_modifiers[] = {
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID
+};
+
 static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
 static void ironlake_pch_clock_get(struct intel_crtc *crtc,
@@ -13797,7 +13816,98 @@ void intel_plane_destroy(struct drm_plane *plane)
kfree(to_intel_plane(plane));
 }
 
-const struct drm_plane_funcs intel_plane_funcs = {
+static bool i8xx_mod_supported(uint32_t format, uint64_t modifier)
+{
+   switch (format) {
+   case DRM_FORMAT_C8:
+   case DRM_FORMAT_RGB565:
+   case DRM_FORMAT_XRGB1555:
+   case DRM_FORMAT_XRGB:
+   return modifier == DRM_FORMAT_MOD_LINEAR ||
+   modifier == I915_FORMAT_MOD_X_TILED;
+   default:
+   return false;
+   }
+}
+
+static bool i965_mod_supported(uint32_t format, uint64_t modifier)
+{
+   switch (format) {
+   case DRM_FORMAT_C8:
+   case DRM_FORMAT_RGB565:
+   case DRM_FORMAT_XRGB:
+   case DRM_FORMAT_XBGR:
+   case DRM_FORMAT_XRGB2101010:
+   case DRM_FORMAT_XBGR2101010:
+   return modifier == DRM_FORMAT_MOD_LINEAR ||
+   modifier == I915_FORMAT_MOD_X_TILED;
+   default:
+   return false;
+   }
+}
+
+static bool skl_mod_supported(uint32_t format, uint64_t modifier)
+{
+   switch (format) {
+   case DRM_FORMAT_XRGB:
+   case DRM_FORMAT_XBGR:
+   case DRM_FORMAT_ARGB:
+   case DRM_FORMAT_ABGR:
+   case DRM_FORMAT_RGB565:
+   case DRM_FORMAT_XRGB2101010:
+   case DRM_FORMAT_XBGR2101010:
+   case DRM_FORMAT_YUYV:
+   case DRM_FORMAT_YVYU:
+   case DRM_FORMAT_UYVY:
+   case DRM_FORMAT_VYUY:
+   if (modifier == I915_FORMAT_MOD_Yf_TILED)
+   return true;
+   /* fall through */
+   case DRM_FORMAT_C8:
+   if (modifier == DRM_FORMAT_MOD_LINEAR ||
+   modifier == I915_FORMAT_MOD_X_TILED ||
+   modifier == I915_FORMAT_MOD_Y_TILED)
+   return true;
+   /* fall through */
+   default:
+   return false;
+   }
+}
+
+static bool intel_primary_plane_format_mod_supported(struct drm_plane *plane,
+uint32_t format,
+uint64_t modifier)
+{
+   struct drm_i915

[Intel-gfx] [PATCH 6/6] [v4] drm/i915: Add support for CCS modifiers

[Ben Widawsky]

v2:
  - Support sprite plane.
  - Support pipe C/D limitation on GEN9.

v3:
  - Rename structure (Ville)
  - Handle GLK (Ville)

v4:
  - Fix PIPE_C check, introduced in v2 (Daniel)
  - Whitespace fix (Daniel)

Cc: Daniel Stone 
Cc: Kristian Høgsberg 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/intel_display.c | 30 +++---
 drivers/gpu/drm/i915/intel_sprite.c  | 28 +++-
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ad49b99ef25f..0dc9f40edc7e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -93,7 +93,17 @@ static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_VYUY,
 };
 
-static const uint64_t skl_format_modifiers[] = {
+static const uint64_t skl_format_modifiers_noccs[] = {
+   I915_FORMAT_MOD_Yf_TILED,
+   I915_FORMAT_MOD_Y_TILED,
+   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID
+};
+
+static const uint64_t skl_format_modifiers_ccs[] = {
+   I915_FORMAT_MOD_Yf_TILED_CCS,
+   I915_FORMAT_MOD_Y_TILED_CCS,
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
I915_FORMAT_MOD_X_TILED,
@@ -13853,6 +13863,10 @@ static bool skl_mod_supported(uint32_t format, 
uint64_t modifier)
case DRM_FORMAT_XBGR:
case DRM_FORMAT_ARGB:
case DRM_FORMAT_ABGR:
+   if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
+   modifier == I915_FORMAT_MOD_Y_TILED_CCS)
+   return true;
+   /* fall through */
case DRM_FORMAT_RGB565:
case DRM_FORMAT_XRGB2101010:
case DRM_FORMAT_XBGR2101010:
@@ -14099,10 +14113,20 @@ intel_primary_plane_create(struct drm_i915_private 
*dev_priv, enum pipe pipe)
primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
primary->check_plane = intel_check_primary_plane;
 
-   if (INTEL_GEN(dev_priv) >= 9) {
+   if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+   intel_primary_formats = skl_primary_formats;
+   num_formats = ARRAY_SIZE(skl_primary_formats);
+   modifiers = skl_format_modifiers_ccs;
+
+   primary->update_plane = skylake_update_primary_plane;
+   primary->disable_plane = skylake_disable_primary_plane;
+   } else if (INTEL_GEN(dev_priv) >= 9) {
intel_primary_formats = skl_primary_formats;
num_formats = ARRAY_SIZE(skl_primary_formats);
-   modifiers = skl_format_modifiers;
+   if (pipe < PIPE_C)
+   modifiers = skl_format_modifiers_ccs;
+   else
+   modifiers = skl_format_modifiers_noccs;
 
primary->update_plane = skylake_update_primary_plane;
primary->disable_plane = skylake_disable_primary_plane;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index b1cc4835b963..5a2b3f3693a6 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1085,7 +1085,17 @@ static uint32_t skl_plane_formats[] = {
DRM_FORMAT_VYUY,
 };
 
+static const uint64_t skl_plane_format_modifiers_noccs[] = {
+   I915_FORMAT_MOD_Yf_TILED,
+   I915_FORMAT_MOD_Y_TILED,
+   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID
+};
+
 static const uint64_t skl_plane_format_modifiers[] = {
+   I915_FORMAT_MOD_Yf_TILED_CCS,
+   I915_FORMAT_MOD_Y_TILED_CCS,
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
I915_FORMAT_MOD_X_TILED,
@@ -1148,6 +1158,9 @@ static bool skl_sprite_plane_format_mod_supported(struct 
drm_plane *plane,
case DRM_FORMAT_XBGR:
case DRM_FORMAT_ARGB:
case DRM_FORMAT_ABGR:
+   if (modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
+   modifier == I915_FORMAT_MOD_Yf_TILED_CCS)
+   return true;
case DRM_FORMAT_RGB565:
case DRM_FORMAT_XRGB2101010:
case DRM_FORMAT_XBGR2101010:
@@ -1230,7 +1243,7 @@ intel_sprite_plane_create(struct drm_i915_private 
*dev_priv,
}
intel_plane->base.state = &state->base;
 
-   if (INTEL_GEN(dev_priv) >= 9) {
+   if (INTEL_GEN(dev_priv) >= 10) {
intel_plane->can_scale = true;
state->scaler_id = -1;
 
@@ -1240,6 +1253,19 @@ intel_sprite_plane_create(struct drm_i915_private 
*dev_priv,
plane_formats = skl_plane_formats;
num_plane_formats = ARRAY_SIZE(skl_plane_formats);
modifiers = skl_plane_format_modifiers;
+   } else if (INTEL_GEN(dev_priv) >= 9) {
+   intel_plane->can_scale = true;
+   state->scaler_id = -1;
+
+   intel_plane->update_plane = skl_upd

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/cnl: DDIA Lane capability bit not set in clone mode

[Patchwork]

== Series Details ==

Series: drm/i915/cnl: DDIA Lane capability bit not set in clone mode
URL   : https://patchwork.freedesktop.org/series/28204/
State : success

== Summary ==

Series 28204v1 drm/i915/cnl: DDIA Lane capability bit not set in clone mode
https://patchwork.freedesktop.org/api/1.0/series/28204/revisions/1/mbox/

Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass   -> SKIP   (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass   -> DMESG-WARN (fi-pnv-d510) fdo#101597

fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  
time:451s
fi-bdw-gvtdvmtotal:280  pass:266  dwarn:0   dfail:0   fail:0   skip:14  
time:435s
fi-blb-e6850 total:280  pass:225  dwarn:1   dfail:0   fail:0   skip:54  
time:355s
fi-bsw-n3050 total:280  pass:244  dwarn:0   dfail:0   fail:0   skip:36  
time:530s
fi-bxt-j4205 total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:511s
fi-byt-n2820 total:280  pass:251  dwarn:1   dfail:0   fail:0   skip:28  
time:516s
fi-glk-2atotal:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:603s
fi-hsw-4770  total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:441s
fi-hsw-4770r total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:414s
fi-ilk-650   total:280  pass:230  dwarn:0   dfail:0   fail:0   skip:50  
time:417s
fi-ivb-3520m total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:499s
fi-ivb-3770  total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:472s
fi-kbl-7500u total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:469s
fi-kbl-7560u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:572s
fi-kbl-r total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:581s
fi-pnv-d510  total:280  pass:222  dwarn:3   dfail:0   fail:0   skip:55  
time:561s
fi-skl-6260u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:467s
fi-skl-6700hqtotal:280  pass:263  dwarn:0   dfail:0   fail:0   skip:17  
time:588s
fi-skl-6700k total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:470s
fi-skl-6770hqtotal:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:481s
fi-skl-gvtdvmtotal:280  pass:267  dwarn:0   dfail:0   fail:0   skip:13  
time:442s
fi-skl-x1585ltotal:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  
time:473s
fi-snb-2520m total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  
time:544s
fi-snb-2600  total:280  pass:250  dwarn:0   dfail:0   fail:1   skip:29  
time:407s
fi-byt-j1900 failed to connect after reboot

f9cb5a18db70d51c9f5b3db8253d6c42255cab35 drm-tip: 2017y-08m-01d-09h-46m-24s UTC 
integration manifest
eb1904239980 drm/i915/cnl: DDIA Lane capability bit not set in clone mode

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5304/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] Add drm ioctl DRM_IOCTL_MODE_GETFB2 & associated helpers.

[Joe Kniss]

Thanks for the quick review!

On Tue, Aug 1, 2017 at 5:09 AM, Laurent Pinchart  wrote:

> Hi Joe,
>
> Thank you for the patch.
>
> On Monday 31 Jul 2017 11:29:13 Joe Kniss wrote:
> > New getfb2 functionality uses drm_mode_fb_cmd2 struct to be symmetric
> > with addfb2.
>
> What's the use case for this ? We haven't needed such an ioctl for so long
> that it seemed to me that userspace doesn't really need it, but I could be
> wrong.
>
> Sorry, I failed to reference the original email.  Here it is:

 I am a recent addition to Google's ChromeOS gfx team.   I am currently
working on display testing and reporting.   An important part of this is
our screen capture tool, which works by querying drm for crtcs, planes, and
fbs.  Unfortunately, there is only limited information available via
drmModeGetFB(), which often wrong information when drmModeAddFB2() was used
to create the fbs.   For example, if the pixel format is NV12 or YUV420,
the fb returned knows nothing about the additional buffer planes required
by these formats.   Ideally, we would like a function (e.g. drmModeGetFB2)
to return information symmetric with drmModeAddFB2 including the pixel
format id, buffer plane information etc.

ChromeOS has needed this functionality from the start, for both testing and
error reporting.  We got away with guessing the buffer's format (32bit
xrgb) until now.  We are now enabling overlays and more formats including
multi-planar (e.g. NV12).  Current getfb() reports neither the pixel format
nor  planar information.  Without this information, going forward, our gfx
testing is going to break.  It would be great if we had access to higher
level buffer structs (like gbm), but we generally don't since they would be
created by other apps (chrome browser, android apps, etc...).


> > Also modifies *_fb_create_handle() calls to accept a
> > format_plane_index so that handles for each plane can be generated.
> > Previously, many *_fb_create_handle() calls simply defaulted to plane 0
> > only.
>
> And with this patch the amd/amdgpu, armada, gma500, i915, mediatek, msm,
> nouveau and radeon drivers still do. Do none of them support multi-planar
> formats ?
>
> I would imagine that some of these do support multi-planar formats, but
they don't appear to have them implemented yet (except perhaps as offsets
into a single buffer).  I will certainly be looking into this soon, but any
changes will come in future patches.


> > Signed-off-by: Joe Kniss 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  5 +-
> >  drivers/gpu/drm/armada/armada_fb.c  |  1 +
> >  drivers/gpu/drm/drm_crtc_internal.h |  2 +
> >  drivers/gpu/drm/drm_fb_cma_helper.c | 11 ++--
> >  drivers/gpu/drm/drm_framebuffer.c   | 79
> +-
> >  drivers/gpu/drm/drm_ioctl.c |  1 +
> >  drivers/gpu/drm/exynos/exynos_drm_fb.c  |  7 ++-
> >  drivers/gpu/drm/gma500/framebuffer.c|  2 +
> >  drivers/gpu/drm/i915/intel_display.c|  1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_fb.c   |  1 +
> >  drivers/gpu/drm/msm/msm_fb.c|  5 +-
> >  drivers/gpu/drm/nouveau/nouveau_display.c   |  1 +
> >  drivers/gpu/drm/omapdrm/omap_fb.c   |  5 +-
> >  drivers/gpu/drm/radeon/radeon_display.c |  5 +-
> >  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  6 ++-
> >  drivers/gpu/drm/tegra/fb.c  |  9 +++-
> >  include/drm/drm_framebuffer.h   |  1 +
> >  include/uapi/drm/drm.h  |  2 +
> >  18 files changed, 127 insertions(+), 17 deletions(-)
>
> [snip]
>
> > diff --git a/drivers/gpu/drm/drm_framebuffer.c
> > b/drivers/gpu/drm/drm_framebuffer.c index 28a0108a1ab8..67b3be1bedbc
> 100644
> > --- a/drivers/gpu/drm/drm_framebuffer.c
> > +++ b/drivers/gpu/drm/drm_framebuffer.c
> > @@ -24,6 +24,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include "drm_crtc_internal.h"
>
> [snip]
>
> > +/**
> > + * drm_mode_getfb2 - get FB info
> > + * @dev: drm device for the ioctl
> > + * @data: data pointer for the ioctl
> > + * @file_priv: drm file for the ioctl call
> > + *
> > + * Lookup the FB given its ID and return info about it.
> > + *
> > + * Called by the user via ioctl.
> > + *
> > + * Returns:
> > + * Zero on success, negative errno on failure.
> > + */
> > +int drm_mode_getfb2(struct drm_device *dev,
> > +void *data, struct drm_file *file_priv)
> > +{
> > + struct drm_mode_fb_cmd2 *r = data;
> > + struct drm_framebuffer *fb;
> > + int ret, i;
> > +
> > + if (!drm_core_check_feature(dev, DRIVER_MODESET))
> > + return -EINVAL;
> > +
> > + fb = drm_framebuffer_lookup(dev, r->fb_id);
> > + if (!fb)
> > + return -ENOENT;
> > +
> > + r->height = fb->height;
> > + r->width = fb->width;
> > + r->pixel_format = fb->format->format;
> > + for (i = 0; i < 4; ++i) {
> > + r->pitches[i] = fb->pitches[i];
> > + r->offsets[i] = 

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/6] drm/i915: Implement .get_format_info() hook for CCS

[Patchwork]

== Series Details ==

Series: series starting with [1/6] drm/i915: Implement .get_format_info() hook 
for CCS
URL   : https://patchwork.freedesktop.org/series/28205/
State : success

== Summary ==

Series 28205v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/28205/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail   -> PASS   (fi-snb-2600) fdo#17
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS   (fi-byt-n2820) fdo#101705

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  
time:446s
fi-bdw-gvtdvmtotal:280  pass:266  dwarn:0   dfail:0   fail:0   skip:14  
time:431s
fi-blb-e6850 total:280  pass:225  dwarn:1   dfail:0   fail:0   skip:54  
time:356s
fi-bsw-n3050 total:280  pass:244  dwarn:0   dfail:0   fail:0   skip:36  
time:536s
fi-bxt-j4205 total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:513s
fi-byt-n2820 total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  
time:508s
fi-glk-2atotal:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:607s
fi-hsw-4770  total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:444s
fi-hsw-4770r total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:418s
fi-ilk-650   total:280  pass:230  dwarn:0   dfail:0   fail:0   skip:50  
time:415s
fi-ivb-3520m total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:501s
fi-ivb-3770  total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:475s
fi-kbl-7500u total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:463s
fi-kbl-7560u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:683s
fi-kbl-r total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:586s
fi-pnv-d510  total:280  pass:223  dwarn:2   dfail:0   fail:0   skip:55  
time:565s
fi-skl-6260u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:462s
fi-skl-6700hqtotal:280  pass:263  dwarn:0   dfail:0   fail:0   skip:17  
time:589s
fi-skl-6700k total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:472s
fi-skl-6770hqtotal:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:476s
fi-skl-gvtdvmtotal:280  pass:267  dwarn:0   dfail:0   fail:0   skip:13  
time:438s
fi-skl-x1585ltotal:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:500s
fi-snb-2520m total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  
time:549s
fi-snb-2600  total:280  pass:251  dwarn:0   dfail:0   fail:0   skip:29  
time:408s

f9cb5a18db70d51c9f5b3db8253d6c42255cab35 drm-tip: 2017y-08m-01d-09h-46m-24s UTC 
integration manifest
04a6fe64a0b8 drm/i915: Add support for CCS modifiers
68d6d8e4dc2f drm/i915: Add format modifiers for Intel
a77220de1b3f drm: Create a format/modifier blob
08272711c40b drm: Plumb modifiers through plane init
b582189f81f9 drm/i915: Add render decompression support
2c091a4eb36c drm/i915: Implement .get_format_info() hook for CCS

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5305/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 0/8] Adding NV12 support

[Bob Paauwe]

On Mon, 31 Jul 2017 10:41:48 +0200
Daniel Vetter  wrote:

> On Mon, Jul 31, 2017 at 12:34:45PM +0530, Vidya Srinivas wrote:
> > This patch series is adding NV12 support for Broxton display after
> > rebasing on latest drm-intel-nightly. Initial series of the patches
> > can be found here:
> > https://lists.freedesktop.org/archives/intel-gfx/2015-May/066786.html
> > 
> > Feature has been currently tested with custom linux based test tool
> > IGT test development is under progress. Floating these patches for
> > initial review. These NV12 patches are dependent on Ville's patches
> > mentioned below.
> > 
> > Update from last rev:
> > Patches were initial reviewed last when floated but
> > currently there was a design change with respect to
> > - the way fb offset is handled
> > - the way rotation is handled
> > Rebase of the current NV12 patch series has been done as per the
> > current changes on drm-intel-nightly.
> > Review comments from Ville (12th June 2017) have been addressed
> > Review comments from Clinton A Taylor (7th July 2017) have been
> > addressed
> > Review comments from Clinton A Taylor (10th July 2017) have been
> > addressed. Had missed out tested-by/reviewed-by in the patches.
> > Fixed that error in this series.
> > Review comments from Ville (11th July 2017) addressed.
> > Review comments from Paauwe, Bob (29th July 2017) addressed.  
> 
> Do we have a solid set of igt testcases for this (including against stuff
> like rotation, checking all the planes/crtc combos, scaling and
> placement)? Plus making sure that it all works with atomic updates too?
> 
> Thanks, Daniel

In IOTG, we have an IGT test case that does do
scaling/rotating/placement on the various combinations. But I'm seeing
a lot of problems both with scaling and Yf tiling.

Our IOTG branch has some differences in the NV12 support so I have
spent some time comparing this series with our tree and trying to make
our tree match this series.

At this point I'm still trying to determine if the problems are in the
test program or the driver.

Vidya, do you have Yf tiling working using the current igt test_nv12
program?  

Bob
> 
> > 
> > Chandra Konduru (6):
> >   drm/i915: Set scaler mode for NV12
> >   drm/i915: Update format_is_yuv() to include NV12
> >   drm/i915: Upscale scaler max scale for NV12
> >   drm/i915: Add NV12 as supported format for primary plane
> >   drm/i915: Add NV12 as supported format for sprite plane
> >   drm/i915: Add NV12 support to intel_framebuffer_init
> > 
> > Ville Syrjälä (2):
> >   drm/i915: Implement .get_format_info() hook for CCS
> >   SKL+ display engine can scan out certain kinds of compressed surfaces
> > produced by the render engine. This involved telling the display
> > engine the location of the color control surfae (CCS) which
> > describes which parts of the main surface are compressed and which
> > are not. The location of CCS is provided by userspace as just
> > another plane with its own offset.
> > 
> >  drivers/gpu/drm/i915/i915_reg.h  |  24 +++
> >  drivers/gpu/drm/i915/intel_atomic.c  |   8 +-
> >  drivers/gpu/drm/i915/intel_display.c | 345 
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h |   3 +-
> >  drivers/gpu/drm/i915/intel_pm.c  |  29 ++-
> >  drivers/gpu/drm/i915/intel_sprite.c  |  38 +++-
> >  include/uapi/drm/drm_fourcc.h|  20 ++
> >  7 files changed, 428 insertions(+), 39 deletions(-)
> > 
> > -- 
> > 1.9.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx  
> 



-- 
--
Bob Paauwe  
bob.j.paa...@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193

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

Re: [Intel-gfx] [PATCH 0/4] Userptr bo slab use optimization

[Ben Widawsky]

On 17-07-27 10:25:51, Chris Wilson wrote:

Quoting Tvrtko Ursulin (2017-07-27 10:05:00)

From: Tvrtko Ursulin 

Yet another attempt to get this series reviewed and merged...

I've heard Vulkan might be creating a lot of userptr objects so might be
interesting to check what benefit it brings to those use cases.


Optimist :) My thinking is that this should only impact get_pages ->
vma_bind, which is supposed a rare operation, and if should happen as
part of the steady state that we have too many sg in a chain is just one
of the myriad little paper cuts :)



Vulkan is critically dependent on userptr, but I don't believe we create many
usrptr BOs as the implementation and API reduce the number of BOs in general.

I don't see any reason not to do any of this though. Series is
Acked-by: Ben Widawsky 


As an introduction, this allows i915 to create fewer sg table entries for the bo
backing store representation. As such it primarily saves kernel slab memory.

When we added this optimisation to normal i915 bos, the savings were as far as
I remember around 1-2MiB of slab after booting to KDE desktop, and 2-4Mib on
neverball (game) main screen (or maybe it was while playing).


I think we also want to think about the aspect where we are creating
objects of multiple 1G huge pages, so we are going to run into the sg
limits very quickly.
-Chris


--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/cnl: DDIA Lane capability bit not set in clone mode

[Vivi, Rodrigo]

+ Art, couple questions below

On Tue, 2017-08-01 at 09:56 -0700, clinton.a.tay...@intel.com wrote:
> From: Clint Taylor 
> 
> DDIA Lane capability control 4 lane bit is not being set by firmware during
> clone mode boot. This occurs when multiple monitors are connected during
> boot. The driver will configure the port for 2 lane maximum width if this
> bit is not set.
> 
> Once DDIA/E lane split is supported in vbt and the i915 driver we will need
> to revisit this code.
> 
> Cc: Rodrigo Vivi 
> Signed-off-by: Clint Taylor 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 494fbe0..e7644b4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2713,9 +2713,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, 
> enum port port)

we would need to fix more comment lines:

/*
* Bspec says that DDI_A_4_LANES is the only supported
configuration   

 * for Broxton.  Yet some BIOS fail to set this bit on port A if
eDP   
 * wasn't lit up at boot.  Force this bit on in our internal 
>* configuration so that we use the proper lane count for our
>* calculations.
>*/

But I believe the approach we currently have with this might not be
optimal. If BIOS is not bringing the port A up why should we expect it
to set this bit ever? We probably need to be able to do it by ourselves,
without expecting others to do...

However, I've never seen a production BIOS that never brings port A when
it is present. And it is also true that spec tells: "This field must be
programmed at system boot based on board configuration and may not be
changed afterwards."

So I have kind of mixed feelings here on this bit.

For BXT it is so clear: "Not supported on Broxton."

Also few other future cases the 0 won't be valid, but not entirely sure
if this is always true. Art?

I just noticed on spec that DDI E for CNL-U and CNL-Y are marked as "not
supported", what means that for that SKU we should be able to add this
workaround here, but at same time means that we need to remove the power
wells support for DDI-E for these SKUs probably.

Art, could/should we set this bit blindly for all CNL? 

and if yes:

Art, will it always be a decision based by SKUs now on? Or should we
really rely on BIOS?

Is there another way to detect the port E presence? I don't believe VBT
has that info in a reliable way, does it? Otherwise we would be using it
already for the missed straps...


Thanks,
Rodrigo.

> - if (IS_GEN9_LP(dev_priv) && port == PORT_A) {
> + if ((IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10) &&
> + port == PORT_A) {
>   if (!(intel_dig_port->saved_port_bits & DDI_A_4_LANES)) {
> - DRM_DEBUG_KMS("BXT BIOS forgot to set DDI_A_4_LANES for 
> port A; fixing\n");
> + DRM_DEBUG_KMS("BIOS forgot to set DDI_A_4_LANES for 
> port A\n");
>   intel_dig_port->saved_port_bits |= DDI_A_4_LANES;
>   max_lanes = 4;
>   }

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

Re: [Intel-gfx] [PATCH] Add drm ioctl DRM_IOCTL_MODE_GETFB2 & associated helpers.

[Laurent Pinchart]

Hi Joe,

On Tuesday 01 Aug 2017 10:24:25 Joe Kniss wrote:
> On Tue, Aug 1, 2017 at 5:09 AM, Laurent Pinchart wrote:
> > On Monday 31 Jul 2017 11:29:13 Joe Kniss wrote:
> >> New getfb2 functionality uses drm_mode_fb_cmd2 struct to be symmetric
> >> with addfb2.
> > 
> > What's the use case for this ? We haven't needed such an ioctl for so long
> > that it seemed to me that userspace doesn't really need it, but I could be
> > wrong.
> 
> Sorry, I failed to reference the original email.  Here it is:
> 
> 
> I am a recent addition to Google's ChromeOS gfx team.   I am currently
> working on display testing and reporting.   An important part of this is
> our screen capture tool, which works by querying drm for crtcs, planes, and
> fbs.  Unfortunately, there is only limited information available via
> drmModeGetFB(), which often wrong information when drmModeAddFB2() was used
> to create the fbs.   For example, if the pixel format is NV12 or YUV420,
> the fb returned knows nothing about the additional buffer planes required
> by these formats.   Ideally, we would like a function (e.g. drmModeGetFB2)
> to return information symmetric with drmModeAddFB2 including the pixel
> format id, buffer plane information etc.
> 
> 
> ChromeOS has needed this functionality from the start, for both testing and
> error reporting.  We got away with guessing the buffer's format (32bit
> xrgb) until now.  We are now enabling overlays and more formats including
> multi-planar (e.g. NV12).  Current getfb() reports neither the pixel format
> nor  planar information.  Without this information, going forward, our gfx
> testing is going to break.  It would be great if we had access to higher
> level buffer structs (like gbm), but we generally don't since they would be
> created by other apps (chrome browser, android apps, etc...).

How is screen capture implemented ? Do you enumerate the framebuffers being 
scanned out, dump their contents and compose them manually based on the active 
plane configuration ? If so, isn't this very race-prone ?

> >> Also modifies *_fb_create_handle() calls to accept a
> >> format_plane_index so that handles for each plane can be generated.
> >> Previously, many *_fb_create_handle() calls simply defaulted to plane 0
> >> only.
> > 
> > And with this patch the amd/amdgpu, armada, gma500, i915, mediatek, msm,
> > nouveau and radeon drivers still do. Do none of them support multi-planar
> > formats ?
>  
> I would imagine that some of these do support multi-planar formats, but
> they don't appear to have them implemented yet (except perhaps as offsets
> into a single buffer).  I will certainly be looking into this soon, but any
> changes will come in future patches.
> 
> >> Signed-off-by: Joe Kniss 
> >> ---
> >> 
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  5 +-
> >>  drivers/gpu/drm/armada/armada_fb.c  |  1 +
> >>  drivers/gpu/drm/drm_crtc_internal.h |  2 +
> >>  drivers/gpu/drm/drm_fb_cma_helper.c | 11 ++--
> >>  drivers/gpu/drm/drm_framebuffer.c   | 79 +++-
> >>  drivers/gpu/drm/drm_ioctl.c |  1 +
> >>  drivers/gpu/drm/exynos/exynos_drm_fb.c  |  7 ++-
> >>  drivers/gpu/drm/gma500/framebuffer.c|  2 +
> >>  drivers/gpu/drm/i915/intel_display.c|  1 +
> >>  drivers/gpu/drm/mediatek/mtk_drm_fb.c   |  1 +
> >>  drivers/gpu/drm/msm/msm_fb.c|  5 +-
> >>  drivers/gpu/drm/nouveau/nouveau_display.c   |  1 +
> >>  drivers/gpu/drm/omapdrm/omap_fb.c   |  5 +-
> >>  drivers/gpu/drm/radeon/radeon_display.c |  5 +-
> >>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  6 ++-
> >>  drivers/gpu/drm/tegra/fb.c  |  9 +++-
> >>  include/drm/drm_framebuffer.h   |  1 +
> >>  include/uapi/drm/drm.h  |  2 +
> >>  18 files changed, 127 insertions(+), 17 deletions(-)
> > 
> > [snip]
> > 
> >> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> >> b/drivers/gpu/drm/drm_framebuffer.c index 28a0108a1ab8..67b3be1bedbc
> >> 100644
> >> --- a/drivers/gpu/drm/drm_framebuffer.c
> >> +++ b/drivers/gpu/drm/drm_framebuffer.c
> >> @@ -24,6 +24,7 @@

[snip]

> >> +/**
> >> + * drm_mode_getfb2 - get FB info
> >> + * @dev: drm device for the ioctl
> >> + * @data: data pointer for the ioctl
> >> + * @file_priv: drm file for the ioctl call
> >> + *
> >> + * Lookup the FB given its ID and return info about it.
> >> + *
> >> + * Called by the user via ioctl.
> >> + *
> >> + * Returns:
> >> + * Zero on success, negative errno on failure.
> >> + */
> >> +int drm_mode_getfb2(struct drm_device *dev,
> >> +void *data, struct drm_file *file_priv)
> >> +{
> >> + struct drm_mode_fb_cmd2 *r = data;
> >> + struct drm_framebuffer *fb;
> >> + int ret, i;
> >> +
> >> + if (!drm_core_check_feature(dev, DRIVER_MODESET))
> >> + return -EINVAL;
> >> +
> >> + fb = drm_framebuffer_lookup(dev, r->fb_id);
> >> + if (!fb)
> >> + return -ENOENT;

Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing command stream based OA reports and ctx id info.

[Lionel Landwerlin]

On 01/08/17 19:05, sourab gupta wrote:



On Tue, Aug 1, 2017 at 2:59 PM, Kamble, Sagar A 
mailto:sagar.a.kam...@intel.com>> wrote:




-Original Message-
From: Landwerlin, Lionel G
Sent: Monday, July 31, 2017 9:16 PM
To: Kamble, Sagar A mailto:sagar.a.kam...@intel.com>>;
intel-gfx@lists.freedesktop.org

Cc: Sourab Gupta mailto:sourab.gu...@intel.com>>
Subject: Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for
capturing command stream based OA reports and ctx id info.

On 31/07/17 08:59, Sagar Arun Kamble wrote:
> From: Sourab Gupta mailto:sourab.gu...@intel.com>>
>
> This patch introduces a framework to capture OA counter reports
associated
> with Render command stream. We can then associate the reports
captured
> through this mechanism with their corresponding context id's.
This can be
> further extended to associate any other metadata information
with the
> corresponding samples (since the association with Render command
stream
> gives us the ability to capture these information while
inserting the
> corresponding capture commands into the command stream).
>
> The OA reports generated in this way are associated with a
corresponding
> workload, and thus can be used the delimit the workload (i.e.
sample the
> counters at the workload boundaries), within an ongoing stream
of periodic
> counter snapshots.
>
> There may be usecases wherein we need more than periodic OA
capture mode
> which is supported currently. This mode is primarily used for
two usecases:
>  - Ability to capture system wide metrics, alongwith the
ability to map
>the reports back to individual contexts (particularly for
HSW).
>  - Ability to inject tags for work, into the reports. This
provides
>visibility into the multiple stages of work within single
context.
>
> The userspace will be able to distinguish between the periodic
and CS based
> OA reports by the virtue of source_info sample field.
>
> The command MI_REPORT_PERF_COUNT can be used to capture
snapshots of OA
> counters, and is inserted at BB boundaries.
> The data thus captured will be stored in a separate buffer,
which will
> be different from the buffer used otherwise for periodic OA
capture mode.
> The metadata information pertaining to snapshot is maintained in
a list,
> which also has offsets into the gem buffer object per captured
snapshot.
> In order to track whether the gpu has completed processing the node,
> a field pertaining to corresponding gem request is added, which
is tracked
> for completion of the command.
>
> Both periodic and CS based reports are associated with a single
stream
> (corresponding to render engine), and it is expected to have the
samples
> in the sequential order according to their timestamps. Now,
since these
> reports are collected in separate buffers, these are merge
sorted at the
> time of forwarding to userspace during the read call.
>
> v2: Aligning with the non-perf interface (custom drm ioctl
based). Also,
> few related patches are squashed together for better readability
>
> v3: Updated perf sample capture emit hook name. Reserving space
upfront
> in the ring for emitting sample capture commands and using
> req->fence.seqno for tracking samples. Added SRCU protection for
streams.
> Changed the stream last_request tracking to resv object. (Chris)
> Updated perf.sample_lock spin_lock usage to avoid softlockups. Moved
> stream to global per-engine structure. (Sagar)
> Update unpin and put in the free routines to
i915_vma_unpin_and_release.
> Making use of perf stream cs_buffer vma resv instead of separate
resv obj.
> Pruned perf stream vma resv during gem_idle. (Chris)
> Changed payload field ctx_id to u64 to keep all sample data
aligned at 8
> bytes. (Lionel)
> stall/flush prior to sample capture is not added. Do we need to
give this
> control to user to select whether to stall/flush at each sample?
>
> Signed-off-by: Sourab Gupta mailto:sourab.gu...@intel.com>>
> Signed-off-by: Robert Bragg mailto:rob...@sixbynine.org>>
> Signed-off-by: Sagar Arun Kamble mailto:sagar.a.kam...@intel.com>>
> ---
>   drivers/gpu/drm/i915/i915_drv.h |  101 ++-
>   drivers/gpu/drm/i915/i915_gem.c |1 +
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |8 +
>   drivers/gpu/drm/i915/i915_perf.c  | 1185
++--
>   drivers/gpu/drm/i915/intel_engine_cs.c  |4 +
>   drivers/gpu/drm/i915/intel_ringbuffer.c |2 +
>   drivers/gpu/drm/i915/intel_ringbuffer.h |5 +
>   include/uapi/drm/i915_drm.h|

Re: [Intel-gfx] [PATCH v13 5/7] vfio: ABI for mdev display dma-buf operation

[Alex Williamson]

On Tue, 25 Jul 2017 17:28:18 +0800
Tina Zhang  wrote:

> Add VFIO_DEVICE_QUERY_GFX_PLANE ioctl command to let user mode query and
> get the plan and its related information.
> 
> The dma-buf's life cycle is handled by user mode and tracked by kernel.
> The returned fd in struct vfio_device_query_gfx_plane can be a new
> fd or an old fd of a re-exported dma-buf. Host User mode can check the
> value of fd and to see if it needs to create new resource according to
> the new fd or just use the existed resource related to the old fd.
> 
> Signed-off-by: Tina Zhang 
> ---
>  include/uapi/linux/vfio.h | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index ae46105..827a230 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -502,6 +502,34 @@ struct vfio_pci_hot_reset {
>  
>  #define VFIO_DEVICE_PCI_HOT_RESET_IO(VFIO_TYPE, VFIO_BASE + 13)
>  
> +/**
> + * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE, VFIO_BASE + 14, struct 
> vfio_device_query_gfx_plane)
> + *
> + * Set the drm_plane_type and retrieve information about the gfx plane.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +struct vfio_device_gfx_plane_info {
> + __u32 argsz;
> + __u32 flags;
> + /* in */
> + __u32 drm_plane_type;   /* type of plane: DRM_PLANE_TYPE_* */
> + /* out */
> + __u32 drm_format;   /* drm format of plane */
> + __u64 drm_format_mod;   /* tiled mode */
> + __u32 width;/* width of plane */
> + __u32 height;   /* height of plane */
> + __u32 stride;   /* stride of plane */
> + __u32 size; /* size of plane in bytes, align on page*/
> + __u32 x_pos;/* horizontal position of cursor plane, upper left 
> corner in pixels */
> + __u32 y_pos;/* vertical position of cursor plane, upper left corner 
> in lines*/
> + __u32 region_index;
> + __s32 fd;   /* dma-buf fd */

How do I know which of these is valid, region_index or fd?  Can I ask
for one vs the other?  What are the errno values to differentiate
unsupported vs not initialized?  Is there a "probe" flag that I can use
to determine what the device supports without allocating those
resources yet?

Kirti, does this otherwise meet your needs?

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

Re: [Intel-gfx] [PATCH 6/6] [v4] drm/i915: Add support for CCS modifiers

[Ben Widawsky]

On 17-08-01 15:43:50, Kenneth Graunke wrote:

On Tuesday, August 1, 2017 9:58:17 AM PDT Ben Widawsky wrote:

v2:
  - Support sprite plane.
  - Support pipe C/D limitation on GEN9.

v3:
  - Rename structure (Ville)
  - Handle GLK (Ville)

v4:
  - Fix PIPE_C check, introduced in v2 (Daniel)
  - Whitespace fix (Daniel)

Cc: Daniel Stone 
Cc: Kristian Høgsberg 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/intel_display.c | 30 +++---
 drivers/gpu/drm/i915/intel_sprite.c  | 28 +++-
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ad49b99ef25f..0dc9f40edc7e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -93,7 +93,17 @@ static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_VYUY,
 };

-static const uint64_t skl_format_modifiers[] = {
+static const uint64_t skl_format_modifiers_noccs[] = {
+   I915_FORMAT_MOD_Yf_TILED,
+   I915_FORMAT_MOD_Y_TILED,
+   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID
+};
+
+static const uint64_t skl_format_modifiers_ccs[] = {
+   I915_FORMAT_MOD_Yf_TILED_CCS,
+   I915_FORMAT_MOD_Y_TILED_CCS,
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
I915_FORMAT_MOD_X_TILED,
@@ -13853,6 +13863,10 @@ static bool skl_mod_supported(uint32_t format, 
uint64_t modifier)
case DRM_FORMAT_XBGR:
case DRM_FORMAT_ARGB:
case DRM_FORMAT_ABGR:
+   if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
+   modifier == I915_FORMAT_MOD_Y_TILED_CCS)
+   return true;
+   /* fall through */
case DRM_FORMAT_RGB565:
case DRM_FORMAT_XRGB2101010:
case DRM_FORMAT_XBGR2101010:
@@ -14099,10 +14113,20 @@ intel_primary_plane_create(struct drm_i915_private 
*dev_priv, enum pipe pipe)
primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
primary->check_plane = intel_check_primary_plane;

-   if (INTEL_GEN(dev_priv) >= 9) {
+   if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+   intel_primary_formats = skl_primary_formats;
+   num_formats = ARRAY_SIZE(skl_primary_formats);
+   modifiers = skl_format_modifiers_ccs;
+
+   primary->update_plane = skylake_update_primary_plane;
+   primary->disable_plane = skylake_disable_primary_plane;
+   } else if (INTEL_GEN(dev_priv) >= 9) {
intel_primary_formats = skl_primary_formats;
num_formats = ARRAY_SIZE(skl_primary_formats);
-   modifiers = skl_format_modifiers;
+   if (pipe < PIPE_C)
+   modifiers = skl_format_modifiers_ccs;
+   else
+   modifiers = skl_format_modifiers_noccs;

primary->update_plane = skylake_update_primary_plane;
primary->disable_plane = skylake_disable_primary_plane;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index b1cc4835b963..5a2b3f3693a6 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1085,7 +1085,17 @@ static uint32_t skl_plane_formats[] = {
DRM_FORMAT_VYUY,
 };

+static const uint64_t skl_plane_format_modifiers_noccs[] = {
+   I915_FORMAT_MOD_Yf_TILED,
+   I915_FORMAT_MOD_Y_TILED,
+   I915_FORMAT_MOD_X_TILED,
+   DRM_FORMAT_MOD_LINEAR,
+   DRM_FORMAT_MOD_INVALID
+};
+
 static const uint64_t skl_plane_format_modifiers[] = {
+   I915_FORMAT_MOD_Yf_TILED_CCS,
+   I915_FORMAT_MOD_Y_TILED_CCS,
I915_FORMAT_MOD_Yf_TILED,
I915_FORMAT_MOD_Y_TILED,
I915_FORMAT_MOD_X_TILED,
@@ -1148,6 +1158,9 @@ static bool skl_sprite_plane_format_mod_supported(struct 
drm_plane *plane,
case DRM_FORMAT_XBGR:
case DRM_FORMAT_ARGB:
case DRM_FORMAT_ABGR:
+   if (modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
+   modifier == I915_FORMAT_MOD_Yf_TILED_CCS)
+   return true;
case DRM_FORMAT_RGB565:
case DRM_FORMAT_XRGB2101010:
case DRM_FORMAT_XBGR2101010:
@@ -1230,7 +1243,7 @@ intel_sprite_plane_create(struct drm_i915_private 
*dev_priv,
}
intel_plane->base.state = &state->base;

-   if (INTEL_GEN(dev_priv) >= 9) {
+   if (INTEL_GEN(dev_priv) >= 10) {


I think this should be INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv).

With that fixed, this patch would be:
Reviewed-by: Kenneth Graunke 

for what it's worth (I'm not that familiar with display).



Thanks. Here is what I've changed locally which didn't match the primary
support:

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 5a2b3f3693a6..1c484195fd0f 100644
--- a/d

Re: [Intel-gfx] [PATCH 6/6] [v4] drm/i915: Add support for CCS modifiers

[Kenneth Graunke]

On Tuesday, August 1, 2017 9:58:17 AM PDT Ben Widawsky wrote:
> v2:
>   - Support sprite plane.
>   - Support pipe C/D limitation on GEN9.
> 
> v3:
>   - Rename structure (Ville)
>   - Handle GLK (Ville)
> 
> v4:
>   - Fix PIPE_C check, introduced in v2 (Daniel)
>   - Whitespace fix (Daniel)
> 
> Cc: Daniel Stone 
> Cc: Kristian Høgsberg 
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 30 +++---
>  drivers/gpu/drm/i915/intel_sprite.c  | 28 +++-
>  2 files changed, 54 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index ad49b99ef25f..0dc9f40edc7e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -93,7 +93,17 @@ static const uint32_t skl_primary_formats[] = {
>   DRM_FORMAT_VYUY,
>  };
>  
> -static const uint64_t skl_format_modifiers[] = {
> +static const uint64_t skl_format_modifiers_noccs[] = {
> + I915_FORMAT_MOD_Yf_TILED,
> + I915_FORMAT_MOD_Y_TILED,
> + I915_FORMAT_MOD_X_TILED,
> + DRM_FORMAT_MOD_LINEAR,
> + DRM_FORMAT_MOD_INVALID
> +};
> +
> +static const uint64_t skl_format_modifiers_ccs[] = {
> + I915_FORMAT_MOD_Yf_TILED_CCS,
> + I915_FORMAT_MOD_Y_TILED_CCS,
>   I915_FORMAT_MOD_Yf_TILED,
>   I915_FORMAT_MOD_Y_TILED,
>   I915_FORMAT_MOD_X_TILED,
> @@ -13853,6 +13863,10 @@ static bool skl_mod_supported(uint32_t format, 
> uint64_t modifier)
>   case DRM_FORMAT_XBGR:
>   case DRM_FORMAT_ARGB:
>   case DRM_FORMAT_ABGR:
> + if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
> + modifier == I915_FORMAT_MOD_Y_TILED_CCS)
> + return true;
> + /* fall through */
>   case DRM_FORMAT_RGB565:
>   case DRM_FORMAT_XRGB2101010:
>   case DRM_FORMAT_XBGR2101010:
> @@ -14099,10 +14113,20 @@ intel_primary_plane_create(struct drm_i915_private 
> *dev_priv, enum pipe pipe)
>   primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
>   primary->check_plane = intel_check_primary_plane;
>  
> - if (INTEL_GEN(dev_priv) >= 9) {
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> + intel_primary_formats = skl_primary_formats;
> + num_formats = ARRAY_SIZE(skl_primary_formats);
> + modifiers = skl_format_modifiers_ccs;
> +
> + primary->update_plane = skylake_update_primary_plane;
> + primary->disable_plane = skylake_disable_primary_plane;
> + } else if (INTEL_GEN(dev_priv) >= 9) {
>   intel_primary_formats = skl_primary_formats;
>   num_formats = ARRAY_SIZE(skl_primary_formats);
> - modifiers = skl_format_modifiers;
> + if (pipe < PIPE_C)
> + modifiers = skl_format_modifiers_ccs;
> + else
> + modifiers = skl_format_modifiers_noccs;
>  
>   primary->update_plane = skylake_update_primary_plane;
>   primary->disable_plane = skylake_disable_primary_plane;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index b1cc4835b963..5a2b3f3693a6 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1085,7 +1085,17 @@ static uint32_t skl_plane_formats[] = {
>   DRM_FORMAT_VYUY,
>  };
>  
> +static const uint64_t skl_plane_format_modifiers_noccs[] = {
> + I915_FORMAT_MOD_Yf_TILED,
> + I915_FORMAT_MOD_Y_TILED,
> + I915_FORMAT_MOD_X_TILED,
> + DRM_FORMAT_MOD_LINEAR,
> + DRM_FORMAT_MOD_INVALID
> +};
> +
>  static const uint64_t skl_plane_format_modifiers[] = {
> + I915_FORMAT_MOD_Yf_TILED_CCS,
> + I915_FORMAT_MOD_Y_TILED_CCS,
>   I915_FORMAT_MOD_Yf_TILED,
>   I915_FORMAT_MOD_Y_TILED,
>   I915_FORMAT_MOD_X_TILED,
> @@ -1148,6 +1158,9 @@ static bool 
> skl_sprite_plane_format_mod_supported(struct drm_plane *plane,
>   case DRM_FORMAT_XBGR:
>   case DRM_FORMAT_ARGB:
>   case DRM_FORMAT_ABGR:
> + if (modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> + modifier == I915_FORMAT_MOD_Yf_TILED_CCS)
> + return true;
>   case DRM_FORMAT_RGB565:
>   case DRM_FORMAT_XRGB2101010:
>   case DRM_FORMAT_XBGR2101010:
> @@ -1230,7 +1243,7 @@ intel_sprite_plane_create(struct drm_i915_private 
> *dev_priv,
>   }
>   intel_plane->base.state = &state->base;
>  
> - if (INTEL_GEN(dev_priv) >= 9) {
> + if (INTEL_GEN(dev_priv) >= 10) {

I think this should be INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv).

With that fixed, this patch would be:
Reviewed-by: Kenneth Graunke 

for what it's worth (I'm not that familiar with display).

>   intel_plane->can_scale = true;
>   state->scaler_id = -1;
>  
> @@ -1240,6 +1253,19 @@ intel_sprite_plane_create(struct dr

Re: [Intel-gfx] [PATCH 6/6] [v4] drm/i915: Add support for CCS modifiers

[Kenneth Graunke]

On Tuesday, August 1, 2017 3:47:53 PM PDT Ben Widawsky wrote:
> On 17-08-01 15:43:50, Kenneth Graunke wrote:
> >On Tuesday, August 1, 2017 9:58:17 AM PDT Ben Widawsky wrote:
> >> v2:
> >>   - Support sprite plane.
> >>   - Support pipe C/D limitation on GEN9.
> >>
> >> v3:
> >>   - Rename structure (Ville)
> >>   - Handle GLK (Ville)
> >>
> >> v4:
> >>   - Fix PIPE_C check, introduced in v2 (Daniel)
> >>   - Whitespace fix (Daniel)
> >>
> >> Cc: Daniel Stone 
> >> Cc: Kristian Høgsberg 
> >> Signed-off-by: Ben Widawsky 
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 30 +++---
> >>  drivers/gpu/drm/i915/intel_sprite.c  | 28 +++-
> >>  2 files changed, 54 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index ad49b99ef25f..0dc9f40edc7e 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -93,7 +93,17 @@ static const uint32_t skl_primary_formats[] = {
> >>DRM_FORMAT_VYUY,
> >>  };
> >>
> >> -static const uint64_t skl_format_modifiers[] = {
> >> +static const uint64_t skl_format_modifiers_noccs[] = {
> >> +  I915_FORMAT_MOD_Yf_TILED,
> >> +  I915_FORMAT_MOD_Y_TILED,
> >> +  I915_FORMAT_MOD_X_TILED,
> >> +  DRM_FORMAT_MOD_LINEAR,
> >> +  DRM_FORMAT_MOD_INVALID
> >> +};
> >> +
> >> +static const uint64_t skl_format_modifiers_ccs[] = {
> >> +  I915_FORMAT_MOD_Yf_TILED_CCS,
> >> +  I915_FORMAT_MOD_Y_TILED_CCS,
> >>I915_FORMAT_MOD_Yf_TILED,
> >>I915_FORMAT_MOD_Y_TILED,
> >>I915_FORMAT_MOD_X_TILED,
> >> @@ -13853,6 +13863,10 @@ static bool skl_mod_supported(uint32_t format, 
> >> uint64_t modifier)
> >>case DRM_FORMAT_XBGR:
> >>case DRM_FORMAT_ARGB:
> >>case DRM_FORMAT_ABGR:
> >> +  if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
> >> +  modifier == I915_FORMAT_MOD_Y_TILED_CCS)
> >> +  return true;
> >> +  /* fall through */
> >>case DRM_FORMAT_RGB565:
> >>case DRM_FORMAT_XRGB2101010:
> >>case DRM_FORMAT_XBGR2101010:
> >> @@ -14099,10 +14113,20 @@ intel_primary_plane_create(struct 
> >> drm_i915_private *dev_priv, enum pipe pipe)
> >>primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
> >>primary->check_plane = intel_check_primary_plane;
> >>
> >> -  if (INTEL_GEN(dev_priv) >= 9) {
> >> +  if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> >> +  intel_primary_formats = skl_primary_formats;
> >> +  num_formats = ARRAY_SIZE(skl_primary_formats);
> >> +  modifiers = skl_format_modifiers_ccs;
> >> +
> >> +  primary->update_plane = skylake_update_primary_plane;
> >> +  primary->disable_plane = skylake_disable_primary_plane;
> >> +  } else if (INTEL_GEN(dev_priv) >= 9) {
> >>intel_primary_formats = skl_primary_formats;
> >>num_formats = ARRAY_SIZE(skl_primary_formats);
> >> -  modifiers = skl_format_modifiers;
> >> +  if (pipe < PIPE_C)
> >> +  modifiers = skl_format_modifiers_ccs;
> >> +  else
> >> +  modifiers = skl_format_modifiers_noccs;
> >>
> >>primary->update_plane = skylake_update_primary_plane;
> >>primary->disable_plane = skylake_disable_primary_plane;
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> >> b/drivers/gpu/drm/i915/intel_sprite.c
> >> index b1cc4835b963..5a2b3f3693a6 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -1085,7 +1085,17 @@ static uint32_t skl_plane_formats[] = {
> >>DRM_FORMAT_VYUY,
> >>  };
> >>
> >> +static const uint64_t skl_plane_format_modifiers_noccs[] = {
> >> +  I915_FORMAT_MOD_Yf_TILED,
> >> +  I915_FORMAT_MOD_Y_TILED,
> >> +  I915_FORMAT_MOD_X_TILED,
> >> +  DRM_FORMAT_MOD_LINEAR,
> >> +  DRM_FORMAT_MOD_INVALID
> >> +};
> >> +
> >>  static const uint64_t skl_plane_format_modifiers[] = {
> >> +  I915_FORMAT_MOD_Yf_TILED_CCS,
> >> +  I915_FORMAT_MOD_Y_TILED_CCS,
> >>I915_FORMAT_MOD_Yf_TILED,
> >>I915_FORMAT_MOD_Y_TILED,
> >>I915_FORMAT_MOD_X_TILED,
> >> @@ -1148,6 +1158,9 @@ static bool 
> >> skl_sprite_plane_format_mod_supported(struct drm_plane *plane,
> >>case DRM_FORMAT_XBGR:
> >>case DRM_FORMAT_ARGB:
> >>case DRM_FORMAT_ABGR:
> >> +  if (modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
> >> +  modifier == I915_FORMAT_MOD_Yf_TILED_CCS)
> >> +  return true;
> >>case DRM_FORMAT_RGB565:
> >>case DRM_FORMAT_XRGB2101010:
> >>case DRM_FORMAT_XBGR2101010:
> >> @@ -1230,7 +1243,7 @@ intel_sprite_plane_create(struct drm_i915_private 
> >> *dev_priv,
> >>}
> >>intel_plane->base.state = &state->base;
> >>
> >> -  if (INTEL_GEN(dev_priv) >= 9) {
> >> +  if (INTEL_GEN(dev_priv) >= 10) {
> >
> >I think this should be INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv).
> >

Re: [Intel-gfx] [PATCH] drm/i915: Use AUX for backlight only if eDP 1.4 or later

[Pandiyan, Dhinakaran]

On Mon, 2017-07-31 at 15:41 -0700, Puthikorn Voravootivat wrote:
> > But now you're suggesting another arbitrary narrow selection of panels
> > based on limited evidence.
> 
> I understand your point that the panel I observe is not the
> representative of the real world.
> 
> My point is that we don't know that the panel will work or not unless
> we test all panel in the world.
> And blacklist would be too much work to maintain and whitelist would
> make this code too limited.
> As standard adoption should be better over time, I suggest that the
> newer panel should have
> better implement of the standard than older panel. And I suggest that
> eDP 1.4 should be a good
> heuristic for the "newer panel" based on these 2 reasons
> 
> 1. Even though it is a limited evident, David and I independently saw
> unrelated eDP 1.3 panel that
> implement this feature incorrectly.
> 2. eDP 1.4 is the first version that support AUX backlight enablement.
> TCON vendor probably also
> make sure the AUX backlight brightness ajustment works when testing
> that feature.
> 
> Is this make sense?
> 
> Thanks.
> 

I tried to investigate this a little bit and found a device that
reproduces the issue. The backlight does not come back up after a
suspend-resume cycle because the PWM controller does not get enabled at
resume. However, things just work at boot because the BIOS happens to
enable PCH PWM at boot and the panel lights up via the BL_PWM_PIN. Like
you said, this could be because some eDP 1.3 panels have a broken
implementation and eDP 1.4 panels are better. Or, with the BL_PWM_PIN
wired to the board, it simply overrides the DPCD settings. I decided to
not disconnect the PWM pin and test this theory since this was a
development laptop. In summary, I am not really sure blacklisting all
eDP 1.3 panels is the best idea. Also, I don't know how many eDP 1.4
panels this has been tested to correctly work on.

Anyway, since we have four panels that do not work, we could check if
these are the same model/make etc. and blacklist them if there's a
common thread.   

-DK

> On Mon, Jul 31, 2017 at 3:55 AM, Jani Nikula
>  wrote:
> > On Mon, 24 Jul 2017, Puthikorn Voravootivat  wrote:
> >> I saw a DP 1.3 panel that advertise AUX backlight brightness control
> >> but not working properly. So it should work but not in real world.
> >> I think that is good reason enough to add this as a heuristic.
> >
> > Are you suggesting the one panel you mention is representative of the
> > real world?
> >
> > Granted, the original aux backlight implementation supported a very
> > narrow selection of panels. I believe this is the very reason you are
> > working on this patch series.
> >
> > But now you're suggesting another arbitrary narrow selection of panels
> > based on limited evidence.
> >
> >
> > BR,
> > Jani.
> >
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] Add drm ioctl DRM_IOCTL_MODE_GETFB2 & associated helpers.

[Joe Kniss]

On Tue, Aug 1, 2017 at 1:46 PM, Laurent Pinchart <
laurent.pinch...@ideasonboard.com> wrote:

> Hi Joe,
>
> On Tuesday 01 Aug 2017 10:24:25 Joe Kniss wrote:
> > On Tue, Aug 1, 2017 at 5:09 AM, Laurent Pinchart wrote:
> > > On Monday 31 Jul 2017 11:29:13 Joe Kniss wrote:
> > >> New getfb2 functionality uses drm_mode_fb_cmd2 struct to be symmetric
> > >> with addfb2.
> > >
> > > What's the use case for this ? We haven't needed such an ioctl for so
> long
> > > that it seemed to me that userspace doesn't really need it, but I
> could be
> > > wrong.
> >
> > Sorry, I failed to reference the original email.  Here it is:
> >
> > 
> > I am a recent addition to Google's ChromeOS gfx team.   I am currently
> > working on display testing and reporting.   An important part of this is
> > our screen capture tool, which works by querying drm for crtcs, planes,
> and
> > fbs.  Unfortunately, there is only limited information available via
> > drmModeGetFB(), which often wrong information when drmModeAddFB2() was
> used
> > to create the fbs.   For example, if the pixel format is NV12 or YUV420,
> > the fb returned knows nothing about the additional buffer planes required
> > by these formats.   Ideally, we would like a function (e.g.
> drmModeGetFB2)
> > to return information symmetric with drmModeAddFB2 including the pixel
> > format id, buffer plane information etc.
> > 
> >
> > ChromeOS has needed this functionality from the start, for both testing
> and
> > error reporting.  We got away with guessing the buffer's format (32bit
> > xrgb) until now.  We are now enabling overlays and more formats including
> > multi-planar (e.g. NV12).  Current getfb() reports neither the pixel
> format
> > nor  planar information.  Without this information, going forward, our
> gfx
> > testing is going to break.  It would be great if we had access to higher
> > level buffer structs (like gbm), but we generally don't since they would
> be
> > created by other apps (chrome browser, android apps, etc...).
>
> How is screen capture implemented ? Do you enumerate the framebuffers being
> scanned out, dump their contents and compose them manually based on the
> active
> plane configuration ? If so, isn't this very race-prone ?
>
>
Yes, this is basically it.  We identify the crtc to capture, query the
planes associated with it and their properties.  For each plane, we get the
fb, then a FD that we use to import a GBM buffer, which we map and
composite.  It's not ideal, but it's the only thing that will work across
all of our platforms and configs.   We either wait or sleep for consistent
testing captures.  I'm not sure what we can do about the race condition
without a much more substantial change...  We are currently looking into
some platform specific methods, but the current approach won't be going
away anytime soon.


> > >> Also modifies *_fb_create_handle() calls to accept a
> > >> format_plane_index so that handles for each plane can be generated.
> > >> Previously, many *_fb_create_handle() calls simply defaulted to plane
> 0
> > >> only.
> > >
> > > And with this patch the amd/amdgpu, armada, gma500, i915, mediatek,
> msm,
> > > nouveau and radeon drivers still do. Do none of them support
> multi-planar
> > > formats ?
> >
> > I would imagine that some of these do support multi-planar formats, but
> > they don't appear to have them implemented yet (except perhaps as offsets
> > into a single buffer).  I will certainly be looking into this soon, but
> any
> > changes will come in future patches.
> >
> > >> Signed-off-by: Joe Kniss 
> > >> ---
> > >>
> > >>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c |  5 +-
> > >>  drivers/gpu/drm/armada/armada_fb.c  |  1 +
> > >>  drivers/gpu/drm/drm_crtc_internal.h |  2 +
> > >>  drivers/gpu/drm/drm_fb_cma_helper.c | 11 ++--
> > >>  drivers/gpu/drm/drm_framebuffer.c   | 79
> +++-
> > >>  drivers/gpu/drm/drm_ioctl.c |  1 +
> > >>  drivers/gpu/drm/exynos/exynos_drm_fb.c  |  7 ++-
> > >>  drivers/gpu/drm/gma500/framebuffer.c|  2 +
> > >>  drivers/gpu/drm/i915/intel_display.c|  1 +
> > >>  drivers/gpu/drm/mediatek/mtk_drm_fb.c   |  1 +
> > >>  drivers/gpu/drm/msm/msm_fb.c|  5 +-
> > >>  drivers/gpu/drm/nouveau/nouveau_display.c   |  1 +
> > >>  drivers/gpu/drm/omapdrm/omap_fb.c   |  5 +-
> > >>  drivers/gpu/drm/radeon/radeon_display.c |  5 +-
> > >>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  6 ++-
> > >>  drivers/gpu/drm/tegra/fb.c  |  9 +++-
> > >>  include/drm/drm_framebuffer.h   |  1 +
> > >>  include/uapi/drm/drm.h  |  2 +
> > >>  18 files changed, 127 insertions(+), 17 deletions(-)
> > >
> > > [snip]
> > >
> > >> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> > >> b/drivers/gpu/drm/drm_framebuffer.c index 28a0108a1ab8..67b3be1bedbc
> > >> 100644
> > >> --- a/drivers/gpu/drm/drm_framebuffer.c
> > >> +++ b/drivers/gpu/drm/drm_frame

[Intel-gfx] [PATCH] drm/i915: Get alternate aux for port E from VBT.

[Rodrigo Vivi]

If VBT states that it uses an alternate aux for port E,
let's use it since even on new platforms there is no
dedicated aux for port E and it is being used for VGA
with DP-to-VGA converter by some OEMs.

Let's assume that port A is in use for eDP so let's
keep the default to port D to minimize the changes
on the behaviour.

Cc: Ville Syrjälä 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_dp.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2d42d09428c9..c856567d4529 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5905,6 +5905,9 @@ intel_dp_init_connector_port_info(struct 
intel_digital_port *intel_dig_port)
 {
struct intel_encoder *encoder = &intel_dig_port->base;
struct intel_dp *intel_dp = &intel_dig_port->dp;
+   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+   const struct ddi_vbt_port_info *info =
+   &dev_priv->vbt.ddi_port_info[intel_dig_port->port];
 
switch (intel_dig_port->port) {
case PORT_A:
@@ -5925,9 +5928,9 @@ intel_dp_init_connector_port_info(struct 
intel_digital_port *intel_dig_port)
break;
case PORT_E:
encoder->hpd_pin = HPD_PORT_E;
-
-   /* FIXME: Check VBT for actual wiring of PORT E */
-   intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
+   intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A +
+   info->alternate_aux_channel ?
+   info->alternate_aux_channel : POWER_DOMAIN_AUX_D;
break;
default:
MISSING_CASE(intel_dig_port->port);
-- 
2.13.2

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

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Get alternate aux for port E from VBT.

[Patchwork]

== Series Details ==

Series: drm/i915: Get alternate aux for port E from VBT.
URL   : https://patchwork.freedesktop.org/series/28216/
State : warning

== Summary ==

Series 28216v1 drm/i915: Get alternate aux for port E from VBT.
https://patchwork.freedesktop.org/api/1.0/series/28216/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass   -> FAIL   (fi-snb-2600) fdo#17
Test kms_flip:
Subgroup basic-flip-vs-modeset:
skip   -> PASS   (fi-skl-x1585l) fdo#101781
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass   -> DMESG-WARN (fi-skl-6700k) fdo#100367 +2
pass   -> DMESG-WARN (fi-skl-x1585l)
Subgroup suspend-read-crc-pipe-b:
pass   -> DMESG-WARN (fi-skl-x1585l)
Subgroup suspend-read-crc-pipe-c:
pass   -> DMESG-WARN (fi-skl-x1585l)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass   -> DMESG-WARN (fi-skl-6700k)
pass   -> DMESG-WARN (fi-skl-x1585l)
Subgroup basic-rte:
pass   -> DMESG-WARN (fi-skl-6700k)
pass   -> DMESG-WARN (fi-skl-x1585l)
Test drv_module_reload:
Subgroup basic-reload:
pass   -> DMESG-WARN (fi-skl-6700k)
pass   -> DMESG-WARN (fi-skl-x1585l)
Subgroup basic-reload-inject:
pass   -> DMESG-WARN (fi-skl-6700k)
pass   -> DMESG-WARN (fi-skl-x1585l)
Subgroup basic-reload-final:
pass   -> DMESG-WARN (fi-skl-6700k)
pass   -> DMESG-WARN (fi-skl-x1585l)

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367

fi-bdw-5557u total:280  pass:269  dwarn:0   dfail:0   fail:0   skip:11  
time:449s
fi-bdw-gvtdvmtotal:280  pass:266  dwarn:0   dfail:0   fail:0   skip:14  
time:431s
fi-blb-e6850 total:280  pass:225  dwarn:1   dfail:0   fail:0   skip:54  
time:357s
fi-bsw-n3050 total:280  pass:244  dwarn:0   dfail:0   fail:0   skip:36  
time:537s
fi-bxt-j4205 total:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:512s
fi-byt-n2820 total:280  pass:251  dwarn:1   dfail:0   fail:0   skip:28  
time:505s
fi-glk-2atotal:280  pass:261  dwarn:0   dfail:0   fail:0   skip:19  
time:602s
fi-hsw-4770  total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:438s
fi-hsw-4770r total:280  pass:264  dwarn:0   dfail:0   fail:0   skip:16  
time:420s
fi-ilk-650   total:280  pass:230  dwarn:0   dfail:0   fail:0   skip:50  
time:410s
fi-ivb-3520m total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:510s
fi-ivb-3770  total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:469s
fi-kbl-7500u total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:475s
fi-kbl-7560u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:582s
fi-kbl-r total:280  pass:262  dwarn:0   dfail:0   fail:0   skip:18  
time:582s
fi-pnv-d510  total:280  pass:224  dwarn:1   dfail:0   fail:0   skip:55  
time:566s
fi-skl-6260u total:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:466s
fi-skl-6700hqtotal:280  pass:263  dwarn:0   dfail:0   fail:0   skip:17  
time:590s
fi-skl-6700k total:280  pass:254  dwarn:8   dfail:0   fail:0   skip:18  
time:476s
fi-skl-6770hqtotal:280  pass:270  dwarn:0   dfail:0   fail:0   skip:10  
time:481s
fi-skl-gvtdvmtotal:280  pass:267  dwarn:0   dfail:0   fail:0   skip:13  
time:441s
fi-skl-x1585ltotal:280  pass:262  dwarn:8   dfail:0   fail:0   skip:10  
time:501s
fi-snb-2520m total:280  pass:252  dwarn:0   dfail:0   fail:0   skip:28  
time:556s
fi-snb-2600  total:280  pass:250  dwarn:0   dfail:0   fail:1   skip:29  
time:406s

fcb630a80579faf6d12ee62cb49bd7a4acff41e6 drm-tip: 2017y-08m-01d-17h-14m-57s UTC 
integration manifest
36203b7d1cd8 drm/i915: Get alternate aux for port E from VBT.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5306/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Update the status of connector when receiving MST unplug event

[Pandiyan, Dhinakaran]

On Tue, 2017-08-01 at 16:51 +0800, Ethan Hsieh wrote:
> We do not update the status of connector when receiving MST unplug event.
> Call detect function to get latest status and then update status of connector.
> 

Cc'ing Daniel and Chris.

Thanks for sending this to the list. 

The issue is connector ref count is not zero when
destroy_mst_connector() is called, which results in the connector not
being freed until the userspace shuts down the crtc tied to the
connector. But the kernel needs to update the connector status for the
user space to shut it down.


> Before applying the patch:
> [313.665321] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 
> 0x0020, dig 0x10101012, pins 0x0020
> [313.665383] [drm:intel_hpd_irq_handler [i915]] digital hpd port B - long
> [313.665436] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on PIN 
> 5 - cnt: 0
> [313.665539] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B - long
> [313.944743] [drm:intel_dp_destroy_mst_connector [i915]]
> [313.944848] [drm:intel_dp_destroy_mst_connector [i915]]
> 
> After applying the patch:
> [43.175798] [drm:intel_dp_destroy_mst_connector [i915]] [CONNECTOR:70:DP-4] 
> status updated from connected to disconnected
> [43.175870] [drm:intel_dp_destroy_mst_connector [i915]]
> [43.177675] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] 
> [CONNECTOR:70:DP-4]
> [43.177679] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] 
> [CONNECTOR:70:DP-4] disconnected
> 
> Signed-off-by: Ethan Hsieh 


This patch needs a Fixes: tag as it does fix a fdo bug.


> ---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> index e4ea968..b02a9a8 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -492,6 +492,20 @@ static void intel_dp_destroy_mst_connector(struct 
> drm_dp_mst_topology_mgr *mgr,
>  {
>   struct intel_connector *intel_connector = to_intel_connector(connector);
>   struct drm_i915_private *dev_priv = to_i915(connector->dev);
> + enum drm_connector_status old_status;
> +
> + mutex_lock(&connector->dev->mode_config.mutex);
> + old_status = connector->status;
> + connector->status = connector->funcs->detect(connector, false);

Isn't detect already done by this point? destroy_connector() should be
called after we know the display is disconnected. 

> +
> + if (old_status != connector->status)
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to 
> %s\n",
> +   connector->base.id,
> +   connector->name,
> +   drm_get_connector_status_name(old_status),
> +   drm_get_connector_status_name(connector->status));
> +
> + mutex_unlock(&connector->dev->mode_config.mutex);
>  
>   drm_connector_unregister(connector);

The connector is unregistered unconditionally here, so you might as well
set
connector->status = connector_status_disconnected;

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

Re: [Intel-gfx] [PATCH] drm/i915: Update the status of connector when receiving MST unplug event

[Pandiyan, Dhinakaran]

On Wed, 2017-08-02 at 01:49 +, Pandiyan, Dhinakaran wrote:
> On Tue, 2017-08-01 at 16:51 +0800, Ethan Hsieh wrote:
> > We do not update the status of connector when receiving MST unplug event.
> > Call detect function to get latest status and then update status of 
> > connector.
> > 
> 
> Cc'ing Daniel and Chris.
> 
> Thanks for sending this to the list. 
> 
> The issue is connector ref count is not zero when
> destroy_mst_connector() is called, which results in the connector not
> being freed until the userspace shuts down the crtc tied to the
> connector. But the kernel needs to update the connector status for the
> user space to shut it down.
> 
> 
> > Before applying the patch:
> > [313.665321] [drm:intel_get_hpd_pins [i915]] hotplug event received, stat 
> > 0x0020, dig 0x10101012, pins 0x0020
> > [313.665383] [drm:intel_hpd_irq_handler [i915]] digital hpd port B - long
> > [313.665436] [drm:intel_hpd_irq_handler [i915]] Received HPD interrupt on 
> > PIN 5 - cnt: 0
> > [313.665539] [drm:intel_dp_hpd_pulse [i915]] got hpd irq on port B - long
> > [313.944743] [drm:intel_dp_destroy_mst_connector [i915]]
> > [313.944848] [drm:intel_dp_destroy_mst_connector [i915]]
> > 
> > After applying the patch:
> > [43.175798] [drm:intel_dp_destroy_mst_connector [i915]] [CONNECTOR:70:DP-4] 
> > status updated from connected to disconnected
> > [43.175870] [drm:intel_dp_destroy_mst_connector [i915]]
> > [43.177675] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] 
> > [CONNECTOR:70:DP-4]
> > [43.177679] [drm:drm_helper_probe_single_connector_modes [drm_kms_helper]] 
> > [CONNECTOR:70:DP-4] disconnected
> > 
> > Signed-off-by: Ethan Hsieh 
> 
> 
> This patch needs a Fixes: tag as it does fix a fdo bug.

Sorry, s/Fixes/Bugzilla

> 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 14 ++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index e4ea968..b02a9a8 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -492,6 +492,20 @@ static void intel_dp_destroy_mst_connector(struct 
> > drm_dp_mst_topology_mgr *mgr,
> >  {
> > struct intel_connector *intel_connector = to_intel_connector(connector);
> > struct drm_i915_private *dev_priv = to_i915(connector->dev);
> > +   enum drm_connector_status old_status;
> > +
> > +   mutex_lock(&connector->dev->mode_config.mutex);
> > +   old_status = connector->status;
> > +   connector->status = connector->funcs->detect(connector, false);
> 
> Isn't detect already done by this point? destroy_connector() should be
> called after we know the display is disconnected. 
> 
> > +
> > +   if (old_status != connector->status)
> > +   DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to 
> > %s\n",
> > + connector->base.id,
> > + connector->name,
> > + drm_get_connector_status_name(old_status),
> > + drm_get_connector_status_name(connector->status));
> > +
> > +   mutex_unlock(&connector->dev->mode_config.mutex);
> >  
> > drm_connector_unregister(connector);
> 
> The connector is unregistered unconditionally here, so you might as well
> set
> connector->status = connector_status_disconnected;
> 
> >  
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] linux-next: manual merge of the drm-misc tree with Linus' tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/nouveau/nv50_display.c

between commit:

  4a5431af19bc ("drm/nouveau/kms/nv50: update vblank state in response to 
modeset actions")

from Linus' tree and commit:

  3c847d6cdadb ("drm/nouveau: Convert nouveau to use new iterator macros, v2.")

from the drm-misc tree.

I fixed it up (I think - see below) and can carry the fix as necessary.
This is now fixed as far as linux-next is concerned, but any non
trivial conflicts should be mentioned to your upstream maintainer when
your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/nouveau/nv50_display.c
index 9d40b2a8be4d,bd1199b67eb4..
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@@ -3941,8 -3933,6 +3942,8 @@@ nv50_disp_atomic_commit_tail(struct drm
  
NV_ATOMIC(drm, "%s: clr %04x (set %04x)\n", crtc->name,
  asyh->clr.mask, asyh->set.mask);
-   if (crtc_state->active && !asyh->state.active)
++  if (new_crtc_state->active && !asyh->state.active)
 +  drm_crtc_vblank_off(crtc);
  
if (asyh->clr.mask) {
nv50_head_flush_clr(head, asyh, atom->flush_disable);
@@@ -4028,13 -4018,11 +4029,13 @@@
nv50_head_flush_set(head, asyh);
interlock_core = 1;
}
 -  }
  
 -  for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
 -  if (new_crtc_state->event)
 -  drm_crtc_vblank_get(crtc);
 +  if (asyh->state.active) {
-   if (!crtc_state->active)
++  if (!new_crtc_state->active)
 +  drm_crtc_vblank_on(crtc);
 +  if (asyh->state.event)
 +  drm_crtc_vblank_get(crtc);
 +  }
}
  
/* Update plane(s). */
@@@ -4077,18 -4065,16 +4078,18 @@@
NV_ERROR(drm, "%s: timeout\n", plane->name);
}
  
-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
-   if (crtc->state->event) {
+   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
+   if (new_crtc_state->event) {
unsigned long flags;
/* Get correct count/ts if racing with vblank irq */
 -  drm_crtc_accurate_vblank_count(crtc);
 +  if (crtc->state->active)
 +  drm_crtc_accurate_vblank_count(crtc);
spin_lock_irqsave(&crtc->dev->event_lock, flags);
-   drm_crtc_send_vblank_event(crtc, crtc->state->event);
+   drm_crtc_send_vblank_event(crtc, new_crtc_state->event);
spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
-   crtc->state->event = NULL;
+   new_crtc_state->event = NULL;
 -  drm_crtc_vblank_put(crtc);
 +  if (crtc->state->active)
 +  drm_crtc_vblank_put(crtc);
}
}
  
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing command stream based OA reports and ctx id info.

[sourab gupta]

On Wed, Aug 2, 2017 at 2:28 AM, Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> On 01/08/17 19:05, sourab gupta wrote:
>
>
>
> On Tue, Aug 1, 2017 at 2:59 PM, Kamble, Sagar A 
> wrote:
>
>>
>>
>> -Original Message-
>> From: Landwerlin, Lionel G
>> Sent: Monday, July 31, 2017 9:16 PM
>> To: Kamble, Sagar A ;
>> intel-gfx@lists.freedesktop.org
>> Cc: Sourab Gupta 
>> Subject: Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing
>> command stream based OA reports and ctx id info.
>>
>> On 31/07/17 08:59, Sagar Arun Kamble wrote:
>> > From: Sourab Gupta 
>> >
>> > This patch introduces a framework to capture OA counter reports
>> associated
>> > with Render command stream. We can then associate the reports captured
>> > through this mechanism with their corresponding context id's. This can
>> be
>> > further extended to associate any other metadata information with the
>> > corresponding samples (since the association with Render command stream
>> > gives us the ability to capture these information while inserting the
>> > corresponding capture commands into the command stream).
>> >
>> > The OA reports generated in this way are associated with a corresponding
>> > workload, and thus can be used the delimit the workload (i.e. sample the
>> > counters at the workload boundaries), within an ongoing stream of
>> periodic
>> > counter snapshots.
>> >
>> > There may be usecases wherein we need more than periodic OA capture mode
>> > which is supported currently. This mode is primarily used for two
>> usecases:
>> >  - Ability to capture system wide metrics, alongwith the ability to
>> map
>> >the reports back to individual contexts (particularly for HSW).
>> >  - Ability to inject tags for work, into the reports. This provides
>> >visibility into the multiple stages of work within single
>> context.
>> >
>> > The userspace will be able to distinguish between the periodic and CS
>> based
>> > OA reports by the virtue of source_info sample field.
>> >
>> > The command MI_REPORT_PERF_COUNT can be used to capture snapshots of OA
>> > counters, and is inserted at BB boundaries.
>> > The data thus captured will be stored in a separate buffer, which will
>> > be different from the buffer used otherwise for periodic OA capture
>> mode.
>> > The metadata information pertaining to snapshot is maintained in a list,
>> > which also has offsets into the gem buffer object per captured snapshot.
>> > In order to track whether the gpu has completed processing the node,
>> > a field pertaining to corresponding gem request is added, which is
>> tracked
>> > for completion of the command.
>> >
>> > Both periodic and CS based reports are associated with a single stream
>> > (corresponding to render engine), and it is expected to have the samples
>> > in the sequential order according to their timestamps. Now, since these
>> > reports are collected in separate buffers, these are merge sorted at the
>> > time of forwarding to userspace during the read call.
>> >
>> > v2: Aligning with the non-perf interface (custom drm ioctl based). Also,
>> > few related patches are squashed together for better readability
>> >
>> > v3: Updated perf sample capture emit hook name. Reserving space upfront
>> > in the ring for emitting sample capture commands and using
>> > req->fence.seqno for tracking samples. Added SRCU protection for
>> streams.
>> > Changed the stream last_request tracking to resv object. (Chris)
>> > Updated perf.sample_lock spin_lock usage to avoid softlockups. Moved
>> > stream to global per-engine structure. (Sagar)
>> > Update unpin and put in the free routines to i915_vma_unpin_and_release.
>> > Making use of perf stream cs_buffer vma resv instead of separate resv
>> obj.
>> > Pruned perf stream vma resv during gem_idle. (Chris)
>> > Changed payload field ctx_id to u64 to keep all sample data aligned at 8
>> > bytes. (Lionel)
>> > stall/flush prior to sample capture is not added. Do we need to give
>> this
>> > control to user to select whether to stall/flush at each sample?
>> >
>> > Signed-off-by: Sourab Gupta 
>> > Signed-off-by: Robert Bragg 
>> > Signed-off-by: Sagar Arun Kamble 
>> > ---
>> >   drivers/gpu/drm/i915/i915_drv.h|  101 ++-
>> >   drivers/gpu/drm/i915/i915_gem.c|1 +
>> >   drivers/gpu/drm/i915/i915_gem_execbuffer.c |8 +
>> >   drivers/gpu/drm/i915/i915_perf.c   | 1185
>> ++--
>> >   drivers/gpu/drm/i915/intel_engine_cs.c |4 +
>> >   drivers/gpu/drm/i915/intel_ringbuffer.c|2 +
>> >   drivers/gpu/drm/i915/intel_ringbuffer.h|5 +
>> >   include/uapi/drm/i915_drm.h|   15 +
>> >   8 files changed, 1073 insertions(+), 248 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> > index 2c7456f..8b1cecf 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -1985,6 

Re: [Intel-gfx] [PATCH 0/8] Adding NV12 support

[Srinivas, Vidya]

> -Original Message-
> From: Paauwe, Bob J
> Sent: Tuesday, August 1, 2017 11:04 PM
> To: Daniel Vetter 
> Cc: Srinivas, Vidya ; intel-
> g...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 0/8] Adding NV12 support
> 
> On Mon, 31 Jul 2017 10:41:48 +0200
> Daniel Vetter  wrote:
> 
> > On Mon, Jul 31, 2017 at 12:34:45PM +0530, Vidya Srinivas wrote:
> > > This patch series is adding NV12 support for Broxton display after
> > > rebasing on latest drm-intel-nightly. Initial series of the patches
> > > can be found here:
> > > https://lists.freedesktop.org/archives/intel-gfx/2015-May/066786.htm
> > > l
> > >
> > > Feature has been currently tested with custom linux based test tool
> > > IGT test development is under progress. Floating these patches for
> > > initial review. These NV12 patches are dependent on Ville's patches
> > > mentioned below.
> > >
> > > Update from last rev:
> > > Patches were initial reviewed last when floated but currently there
> > > was a design change with respect to
> > > - the way fb offset is handled
> > > - the way rotation is handled
> > > Rebase of the current NV12 patch series has been done as per the
> > > current changes on drm-intel-nightly.
> > > Review comments from Ville (12th June 2017) have been addressed
> > > Review comments from Clinton A Taylor (7th July 2017) have been
> > > addressed Review comments from Clinton A Taylor (10th July 2017)
> > > have been addressed. Had missed out tested-by/reviewed-by in the
> > > patches.
> > > Fixed that error in this series.
> > > Review comments from Ville (11th July 2017) addressed.
> > > Review comments from Paauwe, Bob (29th July 2017) addressed.
> >
> > Do we have a solid set of igt testcases for this (including against
> > stuff like rotation, checking all the planes/crtc combos, scaling and
> > placement)? Plus making sure that it all works with atomic updates too?
> >
> > Thanks, Daniel
> 

We have floated the basic NV12 with linear/X/Y/Yf support IGT here:
https://patchwork.freedesktop.org/patch/169036/
https://patchwork.freedesktop.org/patch/169037/

We haven’t added rotation in this basic test yet. This needs to be enhanced.
We will work on the same and submit the patches.

Regards
Vidya

> In IOTG, we have an IGT test case that does do scaling/rotating/placement
> on the various combinations. But I'm seeing a lot of problems both with
> scaling and Yf tiling.
> 
> Our IOTG branch has some differences in the NV12 support so I have spent
> some time comparing this series with our tree and trying to make our tree
> match this series.
> 
> At this point I'm still trying to determine if the problems are in the test
> program or the driver.
> 
> Vidya, do you have Yf tiling working using the current igt test_nv12
> program?
> 
> Bob
> >

Thank you so much for the inputs. The basic Yf tiling worked with
the test program posted in the link mentioned above (drm nightly branch).
Please let us know if we could use your IOTG based IGT tool to test the feature.
If so, please provide us the steps (link) to get the same. Thank you.

Regards
Vidya

> > >
> > > Chandra Konduru (6):
> > >   drm/i915: Set scaler mode for NV12
> > >   drm/i915: Update format_is_yuv() to include NV12
> > >   drm/i915: Upscale scaler max scale for NV12
> > >   drm/i915: Add NV12 as supported format for primary plane
> > >   drm/i915: Add NV12 as supported format for sprite plane
> > >   drm/i915: Add NV12 support to intel_framebuffer_init
> > >
> > > Ville Syrjälä (2):
> > >   drm/i915: Implement .get_format_info() hook for CCS
> > >   SKL+ display engine can scan out certain kinds of compressed surfaces
> > > produced by the render engine. This involved telling the display
> > > engine the location of the color control surfae (CCS) which
> > > describes which parts of the main surface are compressed and which
> > > are not. The location of CCS is provided by userspace as just
> > > another plane with its own offset.
> > >
> > >  drivers/gpu/drm/i915/i915_reg.h  |  24 +++
> > >  drivers/gpu/drm/i915/intel_atomic.c  |   8 +-
> > >  drivers/gpu/drm/i915/intel_display.c | 345
> ---
> > >  drivers/gpu/drm/i915/intel_drv.h |   3 +-
> > >  drivers/gpu/drm/i915/intel_pm.c  |  29 ++-
> > >  drivers/gpu/drm/i915/intel_sprite.c  |  38 +++-
> > >  include/uapi/drm/drm_fourcc.h|  20 ++
> > >  7 files changed, 428 insertions(+), 39 deletions(-)
> > >
> > > --
> > > 1.9.1
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> 
> 
> 
> --
> --
> Bob Paauwe
> bob.j.paa...@intel.com
> IOTG / PED Software Organization
> Intel Corp.  Folsom, CA
> (916) 356-6193

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

Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing command stream based OA reports and ctx id info.

[Kamble, Sagar A]

From: sourab gupta [mailto:sourabgu...@gmail.com]
Sent: Wednesday, August 2, 2017 8:17 AM
To: Landwerlin, Lionel G 
Cc: Kamble, Sagar A ; 
intel-gfx@lists.freedesktop.org; Sourab Gupta 
Subject: Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing 
command stream based OA reports and ctx id info.



On Wed, Aug 2, 2017 at 2:28 AM, Lionel Landwerlin 
mailto:lionel.g.landwer...@intel.com>> wrote:
On 01/08/17 19:05, sourab gupta wrote:


On Tue, Aug 1, 2017 at 2:59 PM, Kamble, Sagar A 
mailto:sagar.a.kam...@intel.com>> wrote:


-Original Message-
From: Landwerlin, Lionel G
Sent: Monday, July 31, 2017 9:16 PM
To: Kamble, Sagar A 
mailto:sagar.a.kam...@intel.com>>; 
intel-gfx@lists.freedesktop.org
Cc: Sourab Gupta mailto:sourab.gu...@intel.com>>
Subject: Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing 
command stream based OA reports and ctx id info.

On 31/07/17 08:59, Sagar Arun Kamble wrote:
> From: Sourab Gupta mailto:sourab.gu...@intel.com>>
>
> This patch introduces a framework to capture OA counter reports associated
> with Render command stream. We can then associate the reports captured
> through this mechanism with their corresponding context id's. This can be
> further extended to associate any other metadata information with the
> corresponding samples (since the association with Render command stream
> gives us the ability to capture these information while inserting the
> corresponding capture commands into the command stream).
>
> The OA reports generated in this way are associated with a corresponding
> workload, and thus can be used the delimit the workload (i.e. sample the
> counters at the workload boundaries), within an ongoing stream of periodic
> counter snapshots.
>
> There may be usecases wherein we need more than periodic OA capture mode
> which is supported currently. This mode is primarily used for two usecases:
>  - Ability to capture system wide metrics, alongwith the ability to map
>the reports back to individual contexts (particularly for HSW).
>  - Ability to inject tags for work, into the reports. This provides
>visibility into the multiple stages of work within single context.
>
> The userspace will be able to distinguish between the periodic and CS based
> OA reports by the virtue of source_info sample field.
>
> The command MI_REPORT_PERF_COUNT can be used to capture snapshots of OA
> counters, and is inserted at BB boundaries.
> The data thus captured will be stored in a separate buffer, which will
> be different from the buffer used otherwise for periodic OA capture mode.
> The metadata information pertaining to snapshot is maintained in a list,
> which also has offsets into the gem buffer object per captured snapshot.
> In order to track whether the gpu has completed processing the node,
> a field pertaining to corresponding gem request is added, which is tracked
> for completion of the command.
>
> Both periodic and CS based reports are associated with a single stream
> (corresponding to render engine), and it is expected to have the samples
> in the sequential order according to their timestamps. Now, since these
> reports are collected in separate buffers, these are merge sorted at the
> time of forwarding to userspace during the read call.
>
> v2: Aligning with the non-perf interface (custom drm ioctl based). Also,
> few related patches are squashed together for better readability
>
> v3: Updated perf sample capture emit hook name. Reserving space upfront
> in the ring for emitting sample capture commands and using
> req->fence.seqno for tracking samples. Added SRCU protection for streams.
> Changed the stream last_request tracking to resv object. (Chris)
> Updated perf.sample_lock spin_lock usage to avoid softlockups. Moved
> stream to global per-engine structure. (Sagar)
> Update unpin and put in the free routines to i915_vma_unpin_and_release.
> Making use of perf stream cs_buffer vma resv instead of separate resv obj.
> Pruned perf stream vma resv during gem_idle. (Chris)
> Changed payload field ctx_id to u64 to keep all sample data aligned at 8
> bytes. (Lionel)
> stall/flush prior to sample capture is not added. Do we need to give this
> control to user to select whether to stall/flush at each sample?
>
> Signed-off-by: Sourab Gupta 
> mailto:sourab.gu...@intel.com>>
> Signed-off-by: Robert Bragg 
> mailto:rob...@sixbynine.org>>
> Signed-off-by: Sagar Arun Kamble 
> mailto:sagar.a.kam...@intel.com>>
> ---
>   drivers/gpu/drm/i915/i915_drv.h|  101 ++-
>   drivers/gpu/drm/i915/i915_gem.c|1 +
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |8 +
>   drivers/gpu/drm/i915/i915_perf.c   | 1185 
> ++--
>   drivers/gpu/drm/i915/intel_engine_cs.c |4 +
>   drivers/gpu/drm/i915/intel_ringbuffer.c|2 +
>   drivers/gpu/drm/i915/intel_ringbuffer.h|5 +
>   include/uapi/drm/i915_drm.h

Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing command stream based OA reports and ctx id info.

[Kamble, Sagar A]

Hi Chis,

Understood the need to handle request reordering. 
Are you suggesting following paths:
1. cs samples list for stream be read based on the order of submission from 
submit timestamps/OA capture timestamps.
2. put the commands to capture during eb_submit and patch the offset in vma 
where data is to be captured, populate cs sample list during 
__i915_gem_request_submit

For preemption, it would then simplify by just discarding the cs sample and 
relying on corresponding next  __i915_gem_request_submit.

Thanks
Sagar

-Original Message-
From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] 
Sent: Monday, July 31, 2017 3:42 PM
To: Kamble, Sagar A ; intel-gfx@lists.freedesktop.org
Cc: Sourab Gupta 
Subject: Re: [Intel-gfx] [PATCH 03/12] drm/i915: Framework for capturing 
command stream based OA reports and ctx id info.

Quoting Chris Wilson (2017-07-31 09:34:30)
> Quoting Sagar Arun Kamble (2017-07-31 08:59:36)
> > +/**
> > + * i915_perf_stream_emit_sample_capture - Insert the commands to 
> > +capture perf
> > + * metrics into the GPU command stream
> > + * @stream: An i915-perf stream opened for GPU metrics
> > + * @request: request in whose context the metrics are being collected.
> > + * @preallocate: allocate space in ring for related sample.
> > + */
> > +static void i915_perf_stream_emit_sample_capture(
> > +   struct i915_perf_stream *stream,
> > +   struct drm_i915_gem_request 
> > *request,
> > +   bool preallocate) {
> > +   struct reservation_object *resv = stream->cs_buffer.vma->resv;
> > +   struct i915_perf_cs_sample *sample;
> > +   unsigned long flags;
> > +   int ret;
> > +
> > +   sample = kzalloc(sizeof(*sample), GFP_KERNEL);
> > +   if (sample == NULL) {
> > +   DRM_ERROR("Perf sample alloc failed\n");
> > +   return;
> > +   }
> > +
> > +   sample->request = i915_gem_request_get(request);
> > +   sample->ctx_id = request->ctx->hw_id;
> > +
> > +   insert_perf_sample(stream, sample);
> > +
> > +   if (stream->sample_flags & SAMPLE_OA_REPORT) {
> > +   ret = i915_emit_oa_report_capture(request,
> > + preallocate,
> > + sample->offset);
> > +   if (ret)
> > +   goto err_unref;
> > +   }
> 
> This is incorrect as the requests may be reordered. You either need to 
> declare the linear ordering of requests through the sample buffer, or 
> we have to delay setting sample->offset until execution, and even then 
> we need to disable preemption when using SAMPLE_OA_REPORT.

Thinking about it, you do need to serialise based on stream->vma, or else where 
a stream->vma per capture context.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx