Re: [PATCH 17/26] drm/amd/display: implement PERF_TRACE on Linux

2018-10-13 Thread Christian König

Am 12.10.2018 um 18:56 schrieb Michel Dänzer:

On 2018-10-11 12:09 a.m., Bhawanpreet Lakha wrote:

From: David Francis 

[Why]
A quick-and-dirty way of getting performance data for the amdgpu
driver would make performance improvements easier

[How]
The PERF_TRACE functionality is a tic-toc style debug method.
Put PERF_TRACE calls on either side of the code you want to test.
PERF_TRACE requires access to struct dc_context.  PERF_TRACE()
will pick up the CTX macro, and PERF_TRACE_CTX(struct dc_context)
allows you to pass the context explicitly.

The last 20 results can be read through the debugfs entry
amdgpu_perf_trace.  Each result contains the time in ns and
number of GPU read/writes since the result before it.

In my experimentation, each PERF_TRACE() call uses at most 700ns

Should this use the Linux tracing infrastructure?


Yeah, agree that looks like reimplementing the tracing infrastructure to 
me as well.


Christian.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v5 2/4] drm: Add vrr_enabled property to drm CRTC

2018-10-13 Thread Christian König

Am 12.10.2018 um 18:44 schrieb Nicholas Kazlauskas:

This patch introduces the 'vrr_enabled' CRTC property to allow
dynamic control over variable refresh rate support for a CRTC.

This property should be treated like a content hint to the driver -
if the hardware or driver is not capable of driving variable refresh
timings then this is not considered an error.

Capability for variable refresh rate support should be determined
by querying the vrr_capable drm connector property.

It is worth noting that while the property is intended for atomic use
it isn't filtered from legacy userspace queries. This allows for Xorg
userspace drivers to implement support.


I'm honestly still not convinced that a per CRTC property is actually 
the right approach.


What we should rather do instead is to implement a target timestamp for 
the page flip.


Regards,
Christian.



Signed-off-by: Nicholas Kazlauskas 
---
  drivers/gpu/drm/drm_atomic_uapi.c | 4 
  drivers/gpu/drm/drm_crtc.c| 2 ++
  drivers/gpu/drm/drm_mode_config.c | 6 ++
  include/drm/drm_crtc.h| 9 +
  include/drm/drm_mode_config.h | 5 +
  5 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index d5b7f315098c..eec396a57b88 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -433,6 +433,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc 
*crtc,
ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
drm_property_blob_put(mode);
return ret;
+   } else if (property == config->prop_vrr_enabled) {
+   state->vrr_enabled = val;
} else if (property == config->degamma_lut_property) {
ret = drm_atomic_replace_property_blob_from_id(dev,
>degamma_lut,
@@ -491,6 +493,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = state->active;
else if (property == config->prop_mode_id)
*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+   else if (property == config->prop_vrr_enabled)
+   *val = state->vrr_enabled;
else if (property == config->degamma_lut_property)
*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
else if (property == config->ctm_property)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 5f488aa80bcd..e4eb2c897ff4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -340,6 +340,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, 
struct drm_crtc *crtc,
drm_object_attach_property(>base, config->prop_mode_id, 
0);
drm_object_attach_property(>base,
   config->prop_out_fence_ptr, 0);
+   drm_object_attach_property(>base,
+  config->prop_vrr_enabled, 0);
}
  
  	return 0;

diff --git a/drivers/gpu/drm/drm_mode_config.c 
b/drivers/gpu/drm/drm_mode_config.c
index ee80788f2c40..5670c67f28d4 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -310,6 +310,12 @@ static int drm_mode_create_standard_properties(struct 
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_mode_id = prop;
  
+	prop = drm_property_create_bool(dev, 0,

+   "VRR_ENABLED");
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.prop_vrr_enabled = prop;
+
prop = drm_property_create(dev,
DRM_MODE_PROP_BLOB,
"DEGAMMA_LUT", 0);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b21437bc95bf..39c3900aab3c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -290,6 +290,15 @@ struct drm_crtc_state {
 */
u32 pageflip_flags;
  
+	/**

+* @vrr_enabled:
+*
+* Indicates if variable refresh rate should be enabled for the CRTC.
+* Support for the requested vrr state will depend on driver and
+* hardware capabiltiy - lacking support is not treated as failure.
+*/
+   bool vrr_enabled;
+
/**
 * @event:
 *
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 928e4172a0bb..49f2fcfdb5fc 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -639,6 +639,11 @@ struct drm_mode_config {
 * connectors must be of and active must be set to disabled, too.
 */
struct drm_property *prop_mode_id;
+   /**
+* @prop_vrr_enabled: Default atomic CRTC property to indicate
+* whether variable refresh rate should be enabled on the CRTC.
+*/
+   struct drm_property *prop_vrr_enabled;
  
  	/**

 * @dvi_i_subconnector_property: Optional DVI-I property to