[PATCH 0/7] drm/amd/display: Drop DRM private objects from amdgpu_dm

2020-07-30 Thread Nicholas Kazlauskas 
Based on the analysis of the bug from [1] the best course of action seems
to be swapping off of DRM private objects back to subclassing DRM atomic
state instead.

This patch series implements this change, but not yet the other changes
suggested in the threads from that bug - these will come later.

CCing dri-devel per Daniel's suggestion since this issue brought
some interesting misuse of private objects.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=207383

Nicholas Kazlauskas (7):
  drm/amd/display: Store tiling_flags and tmz_surface on dm_plane_state
  drm/amd/display: Reset plane when tiling flags change
  drm/amd/display: Avoid using unvalidated tiling_flags and tmz_surface
in prepare_planes
  drm/amd/display: Use validated tiling_flags and tmz_surface in
commit_tail
  drm/amd/display: Reset plane for anything that's not a FAST update
  drm/amd/display: Drop dm_determine_update_type_for_commit
  drm/amd/display: Replace DRM private objects with subclassed DRM
atomic state

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 967 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  13 +-
 2 files changed, 343 insertions(+), 637 deletions(-)

-- 
2.25.1

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

[PATCH 6/7] drm/amd/display: Drop dm_determine_update_type_for_commit

2020-07-30 Thread Nicholas Kazlauskas 
[Why]
This was added in the past to solve the issue of not knowing when
to stall for medium and full updates in DM.

Since DC is ultimately decides what requires bandwidth changes we
wanted to make use of it directly to determine this.

The problem is that we can't actually pass any of the stream or surface
updates into DC global validation, so we don't actually check if the new
configuration is valid - we just validate the old existing config
instead and stall for outstanding commits to finish.

There's also the problem of grabbing the DRM private object for
pageflips which can lead to page faults in the case where commits
execute out of order and free a DRM private object state that was
still required for commit tail.

[How]
Now that we reset the plane in DM with the same conditions DC checks
we can have planes go through DC validation and we know when we need
to check and stall based on whether the stream or planes changed.

We mark lock_and_validation_needed whenever we've done this, so just
go back to using that instead of dm_determine_update_type_for_commit.

Since we'll skip resetting the plane for a pageflip we will no longer
grab the DRM private object for pageflips as well, avoiding the
page fault issued caused by pageflipping under load with commits
executing out of order.

Cc: Harry Wentland 
Cc: Bhawanpreet Lakha 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 199 ++
 1 file changed, 17 insertions(+), 182 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2cbb29199e61..59829ec81694 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8542,161 +8542,6 @@ static int dm_update_plane_state(struct dc *dc,
return ret;
 }
 
-static int
-dm_determine_update_type_for_commit(struct amdgpu_display_manager *dm,
-   struct drm_atomic_state *state,
-   enum surface_update_type *out_type)
-{
-   struct dc *dc = dm->dc;
-   struct dm_atomic_state *dm_state = NULL, *old_dm_state = NULL;
-   int i, j, num_plane, ret = 0;
-   struct drm_plane_state *old_plane_state, *new_plane_state;
-   struct dm_plane_state *new_dm_plane_state, *old_dm_plane_state;
-   struct drm_crtc *new_plane_crtc;
-   struct drm_plane *plane;
-
-   struct drm_crtc *crtc;
-   struct drm_crtc_state *new_crtc_state, *old_crtc_state;
-   struct dm_crtc_state *new_dm_crtc_state, *old_dm_crtc_state;
-   struct dc_stream_status *status = NULL;
-   enum surface_update_type update_type = UPDATE_TYPE_FAST;
-   struct surface_info_bundle {
-   struct dc_surface_update surface_updates[MAX_SURFACES];
-   struct dc_plane_info plane_infos[MAX_SURFACES];
-   struct dc_scaling_info scaling_infos[MAX_SURFACES];
-   struct dc_flip_addrs flip_addrs[MAX_SURFACES];
-   struct dc_stream_update stream_update;
-   } *bundle;
-
-   bundle = kzalloc(sizeof(*bundle), GFP_KERNEL);
-
-   if (!bundle) {
-   DRM_ERROR("Failed to allocate update bundle\n");
-   /* Set type to FULL to avoid crashing in DC*/
-   update_type = UPDATE_TYPE_FULL;
-   goto cleanup;
-   }
-
-   for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
-
-   memset(bundle, 0, sizeof(struct surface_info_bundle));
-
-   new_dm_crtc_state = to_dm_crtc_state(new_crtc_state);
-   old_dm_crtc_state = to_dm_crtc_state(old_crtc_state);
-   num_plane = 0;
-
-   if (new_dm_crtc_state->stream != old_dm_crtc_state->stream) {
-   update_type = UPDATE_TYPE_FULL;
-   goto cleanup;
-   }
-
-   if (!new_dm_crtc_state->stream)
-   continue;
-
-   for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
new_plane_state, j) {
-   struct dc_plane_info *plane_info = 
>plane_infos[num_plane];
-   struct dc_flip_addrs *flip_addr = 
>flip_addrs[num_plane];
-   struct dc_scaling_info *scaling_info = 
>scaling_infos[num_plane];
-
-   new_plane_crtc = new_plane_state->crtc;
-   new_dm_plane_state = to_dm_plane_state(new_plane_state);
-   old_dm_plane_state = to_dm_plane_state(old_plane_state);
-
-   if (plane->type == DRM_PLANE_TYPE_CURSOR)
-   continue;
-
-   if (new_dm_plane_state->dc_state != 
old_dm_plane_state->dc_state) {
-   update_type = UPDATE_TYPE_FULL;
-   goto cleanup;
-

[PATCH 5/7] drm/amd/display: Reset plane for anything that's not a FAST update

2020-07-30 Thread Nicholas Kazlauskas 
[Why]
MEDIUM or FULL updates can require global validation or affect
bandwidth. By treating these all simply as surface updates we aren't
actually passing this through DC global validation.

[How]
There's currently no way to pass surface updates through DC global
validation, nor do I think it's a good idea to change the interface
to accept these.

DC global validation itself is currently stateless, and we can move
our update type checking to be stateless as well by duplicating DC
surface checks in DM based on DRM properties.

We wanted to rely on DC automatically determining this since DC knows
best, but DM is ultimately what fills in everything into DC plane
state so it does need to know as well.

There are basically only three paths that we exercise in DM today:

1) Cursor (async update)
2) Pageflip (fast update)
3) Full pipe programming (medium/full updates)

Which means that anything that's more than a pageflip really needs to
go down path #3.

So this change duplicates all the surface update checks based on DRM
state instead inside of should_reset_plane().

Next step is dropping dm_determine_update_type_for_commit and we no
longer require the old DC state at all for global validation.

Optimization can come later so we don't reset DC planes at all for
MEDIUM udpates and avoid validation, but we might require some extra
checks in DM to achieve this.

Cc: Bhawanpreet Lakha 
Cc: Hersen Wu 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 25 +++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0d5f45742bb5..2cbb29199e61 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8336,6 +8336,31 @@ static bool should_reset_plane(struct drm_atomic_state 
*state,
if (old_other_state->crtc != new_other_state->crtc)
return true;
 
+   /* Src/dst size and scaling updates. */
+   if (old_other_state->src_w != new_other_state->src_w ||
+   old_other_state->src_h != new_other_state->src_h ||
+   old_other_state->crtc_w != new_other_state->crtc_w ||
+   old_other_state->crtc_h != new_other_state->crtc_h)
+   return true;
+
+   /* Rotation / mirroring updates. */
+   if (old_other_state->rotation != new_other_state->rotation)
+   return true;
+
+   /* Blending updates. */
+   if (old_other_state->pixel_blend_mode !=
+   new_other_state->pixel_blend_mode)
+   return true;
+
+   /* Alpha updates. */
+   if (old_other_state->alpha != new_other_state->alpha)
+   return true;
+
+   /* Colorspace changes. */
+   if (old_other_state->color_range != 
new_other_state->color_range ||
+   old_other_state->color_encoding != 
new_other_state->color_encoding)
+   return true;
+
/* Framebuffer checks fall at the end. */
if (!old_other_state->fb || !new_other_state->fb)
continue;
-- 
2.25.1

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

[PATCH 7/7] drm/amd/display: Replace DRM private objects with subclassed DRM atomic state

2020-07-30 Thread Nicholas Kazlauskas 
[Why]
DM atomic check was structured in a way that we required old DC state
in order to dynamically add and remove planes and streams from the
context to build the DC state context for validation.

DRM private objects were used to carry over the last DC state and
were added to the context on nearly every commit - regardless of fast
or full so we could check whether or not the new state could affect
bandwidth.

The problem with this model is that DRM private objects do not
implicitly stall out other commits.

So if you have two commits touching separate DRM objects they could
run concurrently and potentially execute out of order - leading to a
use-after-free.

If we want this to be safe we have two options:
1. Stall out concurrent commits since they touch the same private object
2. Refactor DM to not require old DC state and drop private object usage

[How]
This implements approach #2 since it still allows for judder free
updates in multi-display scenarios.

I'll list the big changes in order at a high level:

1. Subclass DRM atomic state instead of using DRM private objects.

DC relied on the old state to determine which changes cause bandwidth
updates but now we have DM perform similar checks based on DRM state
instead - dropping the requirement for old state to exist at all.

This means that we can now build a new DC context from scratch whenever
we have something that DM thinks could affect bandwidth.

Whenever we need to rebuild bandwidth we now add all CRTCs and planes
to the DRM state in order to get the absolute set of DC streams and
DC planes.

This introduces a stall on other commits, but this stall already
exists because of the lock_and_validation logic and it's necessary
since updates may move around pipes and require full reprogramming.

2. Drop workarounds to add planes to maintain z-order early in atomic
check. This is no longer needed because of the changes for (1).

This also involves fixing up should_plane_reset checks since we can just
avoid resetting streams and planes when they haven't actually changed.

3. Rework dm_update_crtc_state and dm_update_plane_state to be single
pass instead of two pass.

This is necessary since we no longer have the dc_state to add and
remove planes to the context in and we want to defer creation to the
end of commit_check.

It also makes the logic a lot simpler to follow since as an added bonus.

Cc: Bhawanpreet Lakha 
Cc: Harry Wentland 
Cc: Leo Li 
Cc: Daniel Vetter 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 720 +++---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  11 +-
 2 files changed, 280 insertions(+), 451 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 59829ec81694..97a7dfc620e8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1839,7 +1839,6 @@ static int dm_resume(void *handle)
struct drm_plane *plane;
struct drm_plane_state *new_plane_state;
struct dm_plane_state *dm_new_plane_state;
-   struct dm_atomic_state *dm_state = 
to_dm_atomic_state(dm->atomic_obj.state);
enum dc_connection_type new_connection_type = dc_connection_none;
struct dc_state *dc_state;
int i, r, j;
@@ -1879,11 +1878,6 @@ static int dm_resume(void *handle)
 
return 0;
}
-   /* Recreate dc_state - DC invalidates it when setting power state to 
S3. */
-   dc_release_state(dm_state->context);
-   dm_state->context = dc_create_state(dm->dc);
-   /* TODO: Remove dc_state->dccg, use dc->dccg directly. */
-   dc_resource_state_construct(dm->dc, dm_state->context);
 
/* Before powering on DC we need to re-initialize DMUB. */
r = dm_dmub_hw_init(adev);
@@ -2019,11 +2013,51 @@ const struct amdgpu_ip_block_version dm_ip_block =
  * *WIP*
  */
 
+struct drm_atomic_state *dm_atomic_state_alloc(struct drm_device *dev)
+{
+   struct dm_atomic_state *dm_state;
+
+   dm_state = kzalloc(sizeof(*dm_state), GFP_KERNEL);
+
+   if (!dm_state)
+   return NULL;
+
+   if (drm_atomic_state_init(dev, _state->base) < 0) {
+   kfree(dm_state);
+   return NULL;
+   }
+
+   return _state->base;
+}
+
+void dm_atomic_state_free(struct drm_atomic_state *state)
+{
+   struct dm_atomic_state *dm_state = to_dm_atomic_state(state);
+
+   if (dm_state->context) {
+   dc_release_state(dm_state->context);
+   dm_state->context = NULL;
+   }
+
+   drm_atomic_state_default_release(state);
+   kfree(state);
+}
+
+void dm_atomic_state_clear(struct drm_atomic_state *state)
+{
+   struct dm_atomic_state *dm_state = to_dm_atomic_state(state);
+
+   drm_atomic_state_default_clear(_state->base);
+}
+
 static const struct drm_mode_config_func

[PATCH 2/7] drm/amd/display: Reset plane when tiling flags change

2020-07-30 Thread Nicholas Kazlauskas 
[Why]
Enabling or disable DCC or switching between tiled and linear formats
can require bandwidth updates.

They're currently skipping all DC validation by being treated as purely
surface updates.

[How]
Treat tiling_flag changes (which encode DCC state) as a condition for
resetting the plane.

Cc: Bhawanpreet Lakha 
Cc: Rodrigo Siqueira 
Cc: Hersen Wu 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 7cc5ab90ce13..bf1881bd492c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8332,6 +8332,8 @@ static bool should_reset_plane(struct drm_atomic_state 
*state,
 * TODO: Come up with a more elegant solution for this.
 */
for_each_oldnew_plane_in_state(state, other, old_other_state, 
new_other_state, i) {
+   struct dm_plane_state *old_dm_plane_state, *new_dm_plane_state;
+
if (other->type == DRM_PLANE_TYPE_CURSOR)
continue;
 
@@ -8342,9 +8344,20 @@ static bool should_reset_plane(struct drm_atomic_state 
*state,
if (old_other_state->crtc != new_other_state->crtc)
return true;
 
-   /* TODO: Remove this once we can handle fast format changes. */
-   if (old_other_state->fb && new_other_state->fb &&
-   old_other_state->fb->format != new_other_state->fb->format)
+   /* Framebuffer checks fall at the end. */
+   if (!old_other_state->fb || !new_other_state->fb)
+   continue;
+
+   /* Pixel format changes can require bandwidth updates. */
+   if (old_other_state->fb->format != new_other_state->fb->format)
+   return true;
+
+   old_dm_plane_state = to_dm_plane_state(old_other_state);
+   new_dm_plane_state = to_dm_plane_state(new_other_state);
+
+   /* Tiling and DCC changes also require bandwidth updates. */
+   if (old_dm_plane_state->tiling_flags !=
+   new_dm_plane_state->tiling_flags)
return true;
}
 
-- 
2.25.1

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

[PATCH 3/7] drm/amd/display: Avoid using unvalidated tiling_flags and tmz_surface in prepare_planes

2020-07-30 Thread Nicholas Kazlauskas 
[Why]
We're racing with userspace as the flags could potentially change
from when we acquired and validated them in commit_check.

[How]
We unfortunately can't drop this function in its entirety from
prepare_planes since we don't know the afb->address at commit_check
time yet.

So instead of querying new tiling_flags and tmz_surface use the ones
from the plane_state directly.

While we're at it, also update the force_disable_dcc option based
on the state from atomic check.

Cc: Bhawanpreet Lakha 
Cc: Rodrigo Siqueira 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 36 ++-
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index bf1881bd492c..f78c09c9585e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5794,14 +5794,8 @@ static int dm_plane_helper_prepare_fb(struct drm_plane 
*plane,
struct list_head list;
struct ttm_validate_buffer tv;
struct ww_acquire_ctx ticket;
-   uint64_t tiling_flags;
uint32_t domain;
int r;
-   bool tmz_surface = false;
-   bool force_disable_dcc = false;
-
-   dm_plane_state_old = to_dm_plane_state(plane->state);
-   dm_plane_state_new = to_dm_plane_state(new_state);
 
if (!new_state->fb) {
DRM_DEBUG_DRIVER("No FB bound\n");
@@ -5845,27 +5839,35 @@ static int dm_plane_helper_prepare_fb(struct drm_plane 
*plane,
return r;
}
 
-   amdgpu_bo_get_tiling_flags(rbo, _flags);
-
-   tmz_surface = amdgpu_bo_encrypted(rbo);
-
ttm_eu_backoff_reservation(, );
 
afb->address = amdgpu_bo_gpu_offset(rbo);
 
amdgpu_bo_ref(rbo);
 
+   /**
+* We don't do surface updates on planes that have been newly created,
+* but we also don't have the afb->address during atomic check.
+*
+* Fill in buffer attributes depending on the address here, but only on
+* newly created planes since they're not being used by DC yet and this
+* won't modify global state.
+*/
+   dm_plane_state_old = to_dm_plane_state(plane->state);
+   dm_plane_state_new = to_dm_plane_state(new_state);
+
if (dm_plane_state_new->dc_state &&
-   dm_plane_state_old->dc_state != 
dm_plane_state_new->dc_state) {
-   struct dc_plane_state *plane_state = 
dm_plane_state_new->dc_state;
+   dm_plane_state_old->dc_state != dm_plane_state_new->dc_state) {
+   struct dc_plane_state *plane_state =
+   dm_plane_state_new->dc_state;
+   bool force_disable_dcc = !plane_state->dcc.enable;
 
-   force_disable_dcc = adev->asic_type == CHIP_RAVEN && 
adev->in_suspend;
fill_plane_buffer_attributes(
adev, afb, plane_state->format, plane_state->rotation,
-   tiling_flags, _state->tiling_info,
-   _state->plane_size, _state->dcc,
-   _state->address, tmz_surface,
-   force_disable_dcc);
+   dm_plane_state_new->tiling_flags,
+   _state->tiling_info, _state->plane_size,
+   _state->dcc, _state->address,
+   dm_plane_state_new->tmz_surface, force_disable_dcc);
}
 
return 0;
-- 
2.25.1

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

[PATCH 4/7] drm/amd/display: Use validated tiling_flags and tmz_surface in commit_tail

2020-07-30 Thread Nicholas Kazlauskas 
[Why]
So we're not racing with userspace or deadlocking DM.

[How]
These flags are now stored on dm_plane_state itself and acquried and
validated during commit_check, so just use those instead.

Cc: Daniel Vetter 
Cc: Bhawanpreet Lakha 
Cc: Rodrigo Siqueira 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f78c09c9585e..0d5f45742bb5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7094,8 +7094,6 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
long r;
unsigned long flags;
struct amdgpu_bo *abo;
-   uint64_t tiling_flags;
-   bool tmz_surface = false;
uint32_t target_vblank, last_flip_vblank;
bool vrr_active = amdgpu_dm_vrr_active(acrtc_state);
bool pflip_present = false;
@@ -7179,20 +7177,12 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
if (unlikely(r <= 0))
DRM_ERROR("Waiting for fences timed out!");
 
-   /*
-* We cannot reserve buffers here, which means the normal flag
-* access functions don't work. Paper over this with READ_ONCE,
-* but maybe the flags are invariant enough that not even that
-* would be needed.
-*/
-   tiling_flags = READ_ONCE(abo->tiling_flags);
-   tmz_surface = READ_ONCE(abo->flags) & 
AMDGPU_GEM_CREATE_ENCRYPTED;
-
fill_dc_plane_info_and_addr(
-   dm->adev, new_plane_state, tiling_flags,
+   dm->adev, new_plane_state,
+   dm_new_plane_state->tiling_flags,
>plane_infos[planes_count],
-   >flip_addrs[planes_count].address, tmz_surface,
-   false);
+   >flip_addrs[planes_count].address,
+   dm_new_plane_state->tmz_surface, false);
 
DRM_DEBUG_DRIVER("plane: id=%d dcc_en=%d\n",
 new_plane_state->plane->index,
-- 
2.25.1

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

[PATCH 1/7] drm/amd/display: Store tiling_flags and tmz_surface on dm_plane_state

2020-07-30 Thread Nicholas Kazlauskas 
[Why]
Store these in advance so we can reuse them later in commit_tail without
having to reserve the fbo again.

These will also be used for checking for tiling changes when deciding
to reset the plane or not.

[How]
This change should mostly be a refactor. Only commit check is affected
for now and I'll drop the get_fb_info calls in prepare_planes and
commit_tail after.

This runs a prepass loop once we think that all planes have been added
to the context and replaces the get_fb_info calls with accessing the
dm_plane_state instead.

Cc: Bhawanpreet Lakha 
Cc: Rodrigo Siqueira 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 60 +++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +
 2 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 8d64f5fde7e2..7cc5ab90ce13 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3700,8 +3700,17 @@ static int fill_dc_scaling_info(const struct 
drm_plane_state *state,
 static int get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb,
   uint64_t *tiling_flags, bool *tmz_surface)
 {
-   struct amdgpu_bo *rbo = gem_to_amdgpu_bo(amdgpu_fb->base.obj[0]);
-   int r = amdgpu_bo_reserve(rbo, false);
+   struct amdgpu_bo *rbo;
+   int r;
+
+   if (!amdgpu_fb) {
+   *tiling_flags = 0;
+   *tmz_surface = false;
+   return 0;
+   }
+
+   rbo = gem_to_amdgpu_bo(amdgpu_fb->base.obj[0]);
+   r = amdgpu_bo_reserve(rbo, false);
 
if (unlikely(r)) {
/* Don't show error message when returning -ERESTARTSYS */
@@ -4124,13 +4133,10 @@ static int fill_dc_plane_attributes(struct 
amdgpu_device *adev,
struct drm_crtc_state *crtc_state)
 {
struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(crtc_state);
-   const struct amdgpu_framebuffer *amdgpu_fb =
-   to_amdgpu_framebuffer(plane_state->fb);
+   struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
struct dc_scaling_info scaling_info;
struct dc_plane_info plane_info;
-   uint64_t tiling_flags;
int ret;
-   bool tmz_surface = false;
bool force_disable_dcc = false;
 
ret = fill_dc_scaling_info(plane_state, _info);
@@ -4142,15 +4148,12 @@ static int fill_dc_plane_attributes(struct 
amdgpu_device *adev,
dc_plane_state->clip_rect = scaling_info.clip_rect;
dc_plane_state->scaling_quality = scaling_info.scaling_quality;
 
-   ret = get_fb_info(amdgpu_fb, _flags, _surface);
-   if (ret)
-   return ret;
-
force_disable_dcc = adev->asic_type == CHIP_RAVEN && adev->in_suspend;
-   ret = fill_dc_plane_info_and_addr(adev, plane_state, tiling_flags,
+   ret = fill_dc_plane_info_and_addr(adev, plane_state,
+ dm_plane_state->tiling_flags,
  _info,
  _plane_state->address,
- tmz_surface,
+ dm_plane_state->tmz_surface,
  force_disable_dcc);
if (ret)
return ret;
@@ -5753,6 +5756,10 @@ dm_drm_plane_duplicate_state(struct drm_plane *plane)
dc_plane_state_retain(dm_plane_state->dc_state);
}
 
+   /* Framebuffer hasn't been updated yet, so retain old flags. */
+   dm_plane_state->tiling_flags = old_dm_plane_state->tiling_flags;
+   dm_plane_state->tmz_surface = old_dm_plane_state->tmz_surface;
+
return _plane_state->base;
 }
 
@@ -8557,13 +8564,9 @@ dm_determine_update_type_for_commit(struct 
amdgpu_display_manager *dm,
continue;
 
for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
new_plane_state, j) {
-   const struct amdgpu_framebuffer *amdgpu_fb =
-   to_amdgpu_framebuffer(new_plane_state->fb);
struct dc_plane_info *plane_info = 
>plane_infos[num_plane];
struct dc_flip_addrs *flip_addr = 
>flip_addrs[num_plane];
struct dc_scaling_info *scaling_info = 
>scaling_infos[num_plane];
-   uint64_t tiling_flags;
-   bool tmz_surface = false;
 
new_plane_crtc = new_plane_state->crtc;
new_dm_plane_state = to_dm_plane_state(new_plane_state);
@@ -8610,16 +8613,12 @@ dm_determine_update_type_for_commit(struct 
amdgpu_display_manager *dm,
 
bundle->surface_updates[num_pl

[PATCH 0/2] drm/amd/display: Add HDR output metadata support for amdgpu

2019-05-28 Thread Nicholas Kazlauskas 
This patch series enables HDR output metadata support in amdgpu using the
DRM HDR interface merged in drm-misc-next. Enabled for DCE and DCN ASICs
over DP and HDMI.

It's limited to static HDR metadata support for now since that's all the
DRM interface supports. It requires a modeset for entering and exiting HDR
but the metadata can be changed without one.

Cc: Harry Wentland 

Nicholas Kazlauskas (2):
  drm/amd/display: Expose HDR output metadata for supported connectors
  drm/amd/display: Only force modesets when toggling HDR

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 155 +-
 1 file changed, 151 insertions(+), 4 deletions(-)

-- 
2.17.1

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

[PATCH 2/2] drm/amd/display: Only force modesets when toggling HDR

2019-05-28 Thread Nicholas Kazlauskas 
[Why]
We can issue HDR static metadata as part of stream updates for
non-modesets as long as we force a modeset when entering or exiting HDR.

This avoids unnecessary blanking for simple metadata updates.

[How]
When changing scaling and abm for the stream also check if HDR has
changed and send the stream update. This will only happen in non-modeset
cases.

Cc: Harry Wentland 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 34 +++
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index eb31acca7ed6..443b13ec268d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3978,9 +3978,16 @@ amdgpu_dm_connector_atomic_check(struct drm_connector 
*conn,
 * DC considers the stream backends changed if the
 * static metadata changes. Forcing the modeset also
 * gives a simple way for userspace to switch from
-* 8bpc to 10bpc when setting the metadata.
+* 8bpc to 10bpc when setting the metadata to enter
+* or exit HDR.
+*
+* Changing the static metadata after it's been
+* set is permissible, however. So only force a
+* modeset if we're entering or exiting HDR.
 */
-   new_crtc_state->mode_changed = true;
+   new_crtc_state->mode_changed =
+   !old_con_state->hdr_output_metadata ||
+   !new_con_state->hdr_output_metadata;
}
 
return 0;
@@ -5881,7 +5888,9 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
struct amdgpu_crtc *acrtc = 
to_amdgpu_crtc(dm_new_con_state->base.crtc);
struct dc_surface_update dummy_updates[MAX_SURFACES];
struct dc_stream_update stream_update;
+   struct dc_info_packet hdr_packet;
struct dc_stream_status *status = NULL;
+   bool abm_changed, hdr_changed, scaling_changed;
 
memset(_updates, 0, sizeof(dummy_updates));
memset(_update, 0, sizeof(stream_update));
@@ -5898,11 +5907,19 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
 
-   if (!is_scaling_state_different(dm_new_con_state, 
dm_old_con_state) &&
-   (dm_new_crtc_state->abm_level == 
dm_old_crtc_state->abm_level))
+   scaling_changed = is_scaling_state_different(dm_new_con_state,
+dm_old_con_state);
+
+   abm_changed = dm_new_crtc_state->abm_level !=
+ dm_old_crtc_state->abm_level;
+
+   hdr_changed =
+   is_hdr_metadata_different(old_con_state, new_con_state);
+
+   if (!scaling_changed && !abm_changed && !hdr_changed)
continue;
 
-   if (is_scaling_state_different(dm_new_con_state, 
dm_old_con_state)) {
+   if (scaling_changed) {

update_stream_scaling_settings(_new_con_state->base.crtc->mode,
dm_new_con_state, (struct 
dc_stream_state *)dm_new_crtc_state->stream);
 
@@ -5910,12 +5927,17 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
stream_update.dst = dm_new_crtc_state->stream->dst;
}
 
-   if (dm_new_crtc_state->abm_level != 
dm_old_crtc_state->abm_level) {
+   if (abm_changed) {
dm_new_crtc_state->stream->abm_level = 
dm_new_crtc_state->abm_level;
 
stream_update.abm_level = _new_crtc_state->abm_level;
}
 
+   if (hdr_changed) {
+   fill_hdr_info_packet(new_con_state, _packet);
+   stream_update.hdr_static_metadata = _packet;
+   }
+
status = dc_stream_get_status(dm_new_crtc_state->stream);
WARN_ON(!status);
WARN_ON(!status->plane_count);
-- 
2.17.1

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

[PATCH 1/2] drm/amd/display: Expose HDR output metadata for supported connectors

2019-05-28 Thread Nicholas Kazlauskas 
[Why]
For userspace to send static HDR metadata to the display we need to
attach the property on the connector and send it to DC.

[How]
The property is attached to HDMI and DP connectors. Since the metadata
isn't actually available when creating the connector this isn't a
property we can dynamically support based on the extension block
being available or not.

When the HDR metadata is changed a modeset will be forced for now.
We need to switch from 8bpc to 10bpc in most cases anyway, and we want
to fully exit HDR mode when userspace gives us a NULL metadata, so this
isn't completely unnecessary.

The requirement can later be reduced to just entering and exiting HDR
or switching max bpc.

Cc: Harry Wentland 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 125 ++
 1 file changed, 125 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 995f9df66142..eb31acca7ed6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3871,6 +3871,121 @@ enum drm_mode_status 
amdgpu_dm_connector_mode_valid(struct drm_connector *connec
return result;
 }
 
+static int fill_hdr_info_packet(const struct drm_connector_state *state,
+   struct dc_info_packet *out)
+{
+   struct hdmi_drm_infoframe frame;
+   unsigned char buf[30]; /* 26 + 4 */
+   ssize_t len;
+   int ret, i;
+
+   memset(out, 0, sizeof(*out));
+
+   if (!state->hdr_output_metadata)
+   return 0;
+
+   ret = drm_hdmi_infoframe_set_hdr_metadata(, state);
+   if (ret)
+   return ret;
+
+   len = hdmi_drm_infoframe_pack_only(, buf, sizeof(buf));
+   if (len < 0)
+   return (int)len;
+
+   /* Static metadata is a fixed 26 bytes + 4 byte header. */
+   if (len != 30)
+   return -EINVAL;
+
+   /* Prepare the infopacket for DC. */
+   switch (state->connector->connector_type) {
+   case DRM_MODE_CONNECTOR_HDMIA:
+   out->hb0 = 0x87; /* type */
+   out->hb1 = 0x01; /* version */
+   out->hb2 = 0x1A; /* length */
+   out->sb[0] = buf[3]; /* checksum */
+   i = 1;
+   break;
+
+   case DRM_MODE_CONNECTOR_DisplayPort:
+   case DRM_MODE_CONNECTOR_eDP:
+   out->hb0 = 0x00; /* sdp id, zero */
+   out->hb1 = 0x87; /* type */
+   out->hb2 = 0x1D; /* payload len - 1 */
+   out->hb3 = (0x13 << 2); /* sdp version */
+   out->sb[0] = 0x01; /* version */
+   out->sb[1] = 0x1A; /* length */
+   i = 2;
+   break;
+
+   default:
+   return -EINVAL;
+   }
+
+   memcpy(>sb[i], [4], 26);
+   out->valid = true;
+
+   print_hex_dump(KERN_DEBUG, "HDR SB:", DUMP_PREFIX_NONE, 16, 1, out->sb,
+  sizeof(out->sb), false);
+
+   return 0;
+}
+
+static bool
+is_hdr_metadata_different(const struct drm_connector_state *old_state,
+ const struct drm_connector_state *new_state)
+{
+   struct drm_property_blob *old_blob = old_state->hdr_output_metadata;
+   struct drm_property_blob *new_blob = new_state->hdr_output_metadata;
+
+   if (old_blob != new_blob) {
+   if (old_blob && new_blob &&
+   old_blob->length == new_blob->length)
+   return memcmp(old_blob->data, new_blob->data,
+ old_blob->length);
+
+   return true;
+   }
+
+   return false;
+}
+
+static int
+amdgpu_dm_connector_atomic_check(struct drm_connector *conn,
+struct drm_connector_state *new_con_state)
+{
+   struct drm_atomic_state *state = new_con_state->state;
+   struct drm_connector_state *old_con_state =
+   drm_atomic_get_old_connector_state(state, conn);
+   struct drm_crtc *crtc = new_con_state->crtc;
+   struct drm_crtc_state *new_crtc_state;
+   int ret;
+
+   if (!crtc)
+   return 0;
+
+   if (is_hdr_metadata_different(old_con_state, new_con_state)) {
+   struct dc_info_packet hdr_infopacket;
+
+   ret = fill_hdr_info_packet(new_con_state, _infopacket);
+   if (ret)
+   return ret;
+
+   new_crtc_state = drm_atomic_get_crtc_state(state, crtc);
+   if (IS_ERR(new_crtc_state))
+   return PTR_ERR(new_crtc_state);
+
+   /*
+* DC considers the stream backends changed if the
+* static metadata changes. Forcing the modeset also
+* gives a simple way for userspace to switch fr

[PATCH v2] drm: Block fb changes for async plane updates

2019-01-07 Thread Nicholas Kazlauskas 
The prepare_fb call always happens on new_plane_state.

The drm_atomic_helper_cleanup_planes checks to see if
plane state pointer has changed when deciding to call cleanup_fb on
either the new_plane_state or the old_plane_state.

For a non-async atomic commit the state pointer is swapped, so this
helper calls prepare_fb on the new_plane_state and cleanup_fb on the
old_plane_state. This makes sense, since we want to prepare the
framebuffer we are going to use and cleanup the the framebuffer we are
no longer using.

For the async atomic update helpers this differs. The async atomic
update helpers perform in-place updates on the existing state. They call
drm_atomic_helper_cleanup_planes but the state pointer is not swapped.
This means that prepare_fb is called on the new_plane_state and
cleanup_fb is called on the new_plane_state (not the old).

In the case where old_plane_state->fb == new_plane_state->fb then
there should be no behavioral difference between an async update
and a non-async commit. But there are issues that arise when
old_plane_state->fb != new_plane_state->fb.

The first is that the new_plane_state->fb is immediately cleaned up
after it has been prepared, so we're using a fb that we shouldn't
be.

The second occurs during a sequence of async atomic updates and
non-async regular atomic commits. Suppose there are two framebuffers
being interleaved in a double-buffering scenario, fb1 and fb2:

- Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1
- Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2
- Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2

We call cleanup_fb on fb2 twice in this example scenario, and any
further use will result in use-after-free.

The simple fix to this problem is to block framebuffer changes
in the drm_atomic_helper_async_check function for now.

Cc: Daniel Vetter 
Cc: Harry Wentland 
Cc: Andrey Grodzovsky 
Cc:  # v4.14+
Fixes: fef9df8b5945 ("drm/atomic: initial support for asynchronous plane 
update")
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/drm_atomic_helper.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 54e2ae614dcc..f4290f6b0c38 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1602,6 +1602,15 @@ int drm_atomic_helper_async_check(struct drm_device *dev,
old_plane_state->crtc != new_plane_state->crtc)
return -EINVAL;
 
+   /*
+* FIXME: Since prepare_fb and cleanup_fb are always called on
+* the new_plane_state for async updates we need to block framebuffer
+* changes. This prevents use of a fb that's been cleaned up and
+* double cleanups from occuring.
+*/
+   if (old_plane_state->fb != new_plane_state->fb)
+   return -EINVAL;
+
funcs = plane->helper_private;
if (!funcs->atomic_async_update)
return -EINVAL;
-- 
2.17.1

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

[PATCH] drm: Block fb changes for async plane updates

2018-12-21 Thread Nicholas Kazlauskas 
The behavior of drm_atomic_helper_cleanup_planes differs depending on
whether the commit was an asynchronous update or not.

For a typical (non-async) atomic commit prepare_fb is called on the
new_plane_state and cleanup_fb is called on the old_plane_state.

However, async commits are performed in place and don't swap the state
for the plane. The call to prepare_fb happens on the new_plane_state
and the call to cleanup_fb is also called on the new_plane_state in
this case (since the state hasn't swapped).

This behavior can lead to use-after-free or unpin of an active fb.

Consider the following sequence of events for interleaving fbs:

- Async update, fb1 prepare, fb1 cleanup_fb
- Async update, fb2 prepare, fb2 cleanup_fb
- Non-async update, fb1 prepare, fb2 cleanup_fb
- Async update, fb2 cleanup_fb -> double cleanup, use-after-free

This situation has been observed in practice for a double buffered
cursor when closing an X client. The non-async update occurs because
the new_plane_state->crtc != old_plane_state->crtc which forces the
non-async path to occur.

The simplest fix for this is to block fb updates in
drm_atomic_helper_async_check. This guarantees that the framebuffer
will have previously been prepared and any subsequent async updates
will always call prepare and cleanup_fb like the non-async atomic
commit path would.

Cc: Michel Dänzer 
Cc: Daniel Vetter 
Cc: Andrey Grodzovsky 
Cc: Harry Wentland 
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/drm_atomic_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 54e2ae614dcc..d2f80bf14f86 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1599,7 +1599,8 @@ int drm_atomic_helper_async_check(struct drm_device *dev,
return -EINVAL;
 
if (!new_plane_state->crtc ||
-   old_plane_state->crtc != new_plane_state->crtc)
+   old_plane_state->crtc != new_plane_state->crtc ||
+   old_plane_state->fb != new_plane_state->fb)
return -EINVAL;
 
funcs = plane->helper_private;
-- 
2.17.1

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

[PATCH v7 5/5] drm/amdgpu: Set FreeSync state using drm VRR properties

2018-11-08 Thread Nicholas Kazlauskas 
Support for AMDGPU specific FreeSync properties and ioctls are dropped
from amdgpu_dm in favor of supporting drm variable refresh rate
properties.

The notify_freesync and set_freesync_property functions are dropped
from amdgpu_display_funcs.

The drm vrr_capable property is now attached to any DP/HDMI connector.
Its value is updated accordingly to the connector's FreeSync capabiltiy.

The freesync_enable logic and ioctl control has has been dropped in
favor of utilizing the vrr_enabled on the drm CRTC. This allows for more
fine grained atomic control over which CRTCs should support variable
refresh rate.

To handle state changes for vrr_enabled it was easiest to drop the
forced modeset on freesync_enabled change. This patch now performs the
required stream updates when planes are flipped.

This is done for a few reasons:

(1) VRR stream updates can be done in the fast update path

(2) amdgpu_dm_atomic_check would need to be hacked apart to check
desired variable refresh state and capability before the CRTC
disable pass.

(3) Performing VRR stream updates on-flip is needed for enabling BTR
support.

VRR packets and timing adjustments are now tracked and compared to
previous values sent to the hardware.

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Harry Wentland 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   7 -
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 3 files changed, 138 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index b9e9e8b02fb7..0cbe867ec375 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -295,13 +295,6 @@ struct amdgpu_display_funcs {
  uint16_t connector_object_id,
  struct amdgpu_hpd *hpd,
  struct amdgpu_router *router);
-   /* it is used to enter or exit into free sync mode */
-   int (*notify_freesync)(struct drm_device *dev, void *data,
-  struct drm_file *filp);
-   /* it is used to allow enablement of freesync mode */
-   int (*set_freesync_property)(struct drm_connector *connector,
-struct drm_property *property,
-uint64_t val);
 
 
 };
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b0df6dc9a775..53eb3d16f75c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1809,72 +1809,6 @@ static void dm_bandwidth_update(struct amdgpu_device 
*adev)
/* TODO: implement later */
 }
 
-static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
-   struct drm_file *filp)
-{
-   struct drm_atomic_state *state;
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_crtc *crtc;
-   struct drm_connector *connector;
-   struct drm_connector_state *old_con_state, *new_con_state;
-   int ret = 0;
-   uint8_t i;
-   bool enable = false;
-
-   drm_modeset_acquire_init(, 0);
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   state->acquire_ctx = 
-
-retry:
-   drm_for_each_crtc(crtc, dev) {
-   ret = drm_atomic_add_affected_connectors(state, crtc);
-   if (ret)
-   goto fail;
-
-   /* TODO rework amdgpu_dm_commit_planes so we don't need this */
-   ret = drm_atomic_add_affected_planes(state, crtc);
-   if (ret)
-   goto fail;
-   }
-
-   for_each_oldnew_connector_in_state(state, connector, old_con_state, 
new_con_state, i) {
-   struct dm_connector_state *dm_new_con_state = 
to_dm_connector_state(new_con_state);
-   struct drm_crtc_state *new_crtc_state;
-   struct amdgpu_crtc *acrtc = 
to_amdgpu_crtc(dm_new_con_state->base.crtc);
-   struct dm_crtc_state *dm_new_crtc_state;
-
-   if (!acrtc) {
-   ASSERT(0);
-   continue;
-   }
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);
-   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
-   dm_new_crtc_state->freesync_enabled = enable;
-   }
-
-   ret = drm_atomic_commit(state);
-
-fail:
-   if (ret == -EDEADLK) {
-   drm_atomic_state_clear(state);
-   drm_modeset_backoff();
-   goto retry;
-   }
-
-   drm_atomic_state_put(state);
-
-out:
-   drm_modeset_drop_locks();
-   drm_modeset_acquire_fini();
-   return ret;
-}
 
 static

[PATCH v7 2/5] drm: Add vrr_enabled property to drm CRTC

2018-11-08 Thread 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.

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Harry Wentland 
Cc: Manasi Navare 
---
 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 268a182ae189..6f8ddfcfaba5 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
-- 
2.19.1

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

[PATCH v7 4/5] drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal

2018-11-08 Thread Nicholas Kazlauskas 
When variable refresh rate is active the hardware counter can return
a position >= vtotal. This results in a vpos being returned from
amdgpu_display_get_crtc_scanoutpos that's a positive value. The
positive value indicates to the caller that the display is
currently in scanout when the display is actually still in vblank.

This is because the vfront porch duration is unknown with variable
refresh active and will end when either a page flip occurs or the
timeout specified by the driver/display is reached.

The behavior of the amdgpu_display_get_crtc_scanoutpos remains the
same when the position is below vtotal. When the position is above
vtotal the function will return a value that is effectively -vbl_end,
the size of the vback porch.

The only caller affected by this change is the DRM helper for
calculating vblank timestamps. This change corrects behavior for
calculating the page flip timestamp from being the previous timestamp
to the calculation to the next timestamp when position >= vtotal.

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Harry Wentland 
Cc: Michel Dänzer 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 6748cd7fc129..cb331319f225 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -850,7 +850,12 @@ int amdgpu_display_get_crtc_scanoutpos(struct drm_device 
*dev,
/* Inside "upper part" of vblank area? Apply corrective offset if so: */
if (in_vbl && (*vpos >= vbl_start)) {
vtotal = mode->crtc_vtotal;
-   *vpos = *vpos - vtotal;
+
+   /* With variable refresh rate displays the vpos can exceed
+* the vtotal value. Clamp to 0 to return -vbl_end instead
+* of guessing the remaining number of lines until scanout.
+*/
+   *vpos = (*vpos < vtotal) ? (*vpos - vtotal) : 0;
}
 
/* Correct for shifted end of vbl at vbl_end. */
-- 
2.19.1

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

[PATCH v7 3/5] drm: Document variable refresh properties

2018-11-08 Thread Nicholas Kazlauskas 
These include the drm_connector 'vrr_capable' and the drm_crtc
'vrr_enabled' properties.

Signed-off-by: Nicholas Kazlauskas 
Cc: Harry Wentland 
Cc: Manasi Navare 
Cc: Pekka Paalanen 
Cc: Ville Syrjälä 
Cc: Michel Dänzer 
---
 Documentation/gpu/drm-kms.rst   |  7 
 drivers/gpu/drm/drm_connector.c | 68 +
 2 files changed, 75 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 4b1501b4835b..8da2a178cf85 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -575,6 +575,13 @@ Explicit Fencing Properties
 .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
:doc: explicit fencing properties
 
+
+Variable Refresh Properties
+---
+
+.. kernel-doc:: drivers/gpu/drm/drm_connector.c
+   :doc: Variable refresh properties
+
 Existing KMS Properties
 ---
 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 49290060ab7b..0e4287461997 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1255,6 +1255,74 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * DOC: Variable refresh properties
+ *
+ * Variable refresh rate capable displays can dynamically adjust their
+ * refresh rate by extending the duration of their vertical front porch
+ * until page flip or timeout occurs. This can reduce or remove stuttering
+ * and latency in scenarios where the page flip does not align with the
+ * vblank interval.
+ *
+ * An example scenario would be an application flipping at a constant rate
+ * of 48Hz on a 60Hz display. The page flip will frequently miss the vblank
+ * interval and the same contents will be displayed twice. This can be
+ * observed as stuttering for content with motion.
+ *
+ * If variable refresh rate was active on a display that supported a
+ * variable refresh range from 35Hz to 60Hz no stuttering would be observable
+ * for the example scenario. The minimum supported variable refresh rate of
+ * 35Hz is below the page flip frequency and the vertical front porch can
+ * be extended until the page flip occurs. The vblank interval will be
+ * directly aligned to the page flip rate.
+ *
+ * Not all userspace content is suitable for use with variable refresh rate.
+ * Large and frequent changes in vertical front porch duration may worsen
+ * perceived stuttering for input sensitive applications.
+ *
+ * Panel brightness will also vary with vertical front porch duration. Some
+ * panels may have noticeable differences in brightness between the minimum
+ * vertical front porch duration and the maximum vertical front porch duration.
+ * Large and frequent changes in vertical front porch duration may produce
+ * observable flickering for such panels.
+ *
+ * Userspace control for variable refresh rate is supported via properties
+ * on the _connector and _crtc objects.
+ *
+ * "vrr_capable":
+ * Optional _connector boolean property that drivers should attach
+ * with drm_connector_attach_vrr_capable_property() on connectors that
+ * could support variable refresh rates. Drivers should update the
+ * property value by calling drm_connector_set_vrr_capable_property().
+ *
+ * Absence of the property should indicate absence of support.
+ *
+ * "vrr_enabled":
+ * Default _crtc boolean property that notifies the driver that the
+ * content on the CRTC is suitable for variable refresh rate presentation.
+ * The driver will take this property as a hint to enable variable
+ * refresh rate support if the receiver supports it, ie. if the
+ * "vrr_capable" property is true on the _connector object. The
+ * vertical front porch duration will be extended until page-flip or
+ * timeout when enabled.
+ *
+ * The minimum vertical front porch duration is defined as the vertical
+ * front porch duration for the current mode.
+ *
+ * The maximum vertical front porch duration is greater than or equal to
+ * the minimum vertical front porch duration. The duration is derived
+ * from the minimum supported variable refresh rate for the connector.
+ *
+ * The driver may place further restrictions within these minimum
+ * and maximum bounds.
+ *
+ * The semantics for the vertical blank timestamp differ when
+ * variable refresh rate is active. The vertical blank timestamp
+ * is defined to be an estimate using the current mode's fixed
+ * refresh rate timings. The semantics for the page-flip event
+ * timestamp remain the same.
+ */
+
 /**
  * drm_connector_attach_vrr_capable_property - creates the
  * vrr_capable property
-- 
2.19.1

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

[PATCH v7 0/5] A DRM API for adaptive sync and variable refresh rate support

2018-11-08 Thread Nicholas Kazlauskas 
 monitor setup. They also work on KDE in a 
single monitor setup with the compositor disabled.

The patches require that suitable applications flip via the Present extension 
to xf86-video-amdgpu for the entire Screen. Some compositors may interfere with 
this if they are unable to unredirect the window.

Full implementation details for these changes can be reviewed in their 
respective mailing lists and the xf86-video-amdgpu GitLab.

=== Previous discussions ===

These patches are based upon feedback from previous threads on the subject. 
These are linked below for reference:

https://lists.freedesktop.org/archives/amd-gfx/2018-April/021047.html
https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html
https://lists.freedesktop.org/archives/dri-devel/2018-September/189404.htm
https://lists.freedesktop.org/archives/dri-devel/2018-September/190910.html
https://lists.freedesktop.org/archives/dri-devel/2018-October/192211.html
https://lists.freedesktop.org/archives/dri-devel/2018-October/192874.html

Nicholas Kazlauskas (5):
  drm: Add vrr_capable property to the drm connector
  drm: Add vrr_enabled property to drm CRTC
  drm: Document variable refresh properties
  drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal
  drm/amdgpu: Set FreeSync state using drm VRR properties

 Documentation/gpu/drm-kms.rst |   7 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |   7 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   7 -
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 drivers/gpu/drm/drm_atomic_uapi.c |   4 +
 drivers/gpu/drm/drm_connector.c   | 117 
 drivers/gpu/drm/drm_crtc.c|   2 +
 drivers/gpu/drm/drm_mode_config.c |   6 +
 include/drm/drm_connector.h   |  15 ++
 include/drm/drm_crtc.h|   9 +
 include/drm/drm_mode_config.h |   5 +
 12 files changed, 309 insertions(+), 132 deletions(-)

-- 
2.19.1

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

[PATCH v7 1/5] drm: Add vrr_capable property to the drm connector

2018-11-08 Thread Nicholas Kazlauskas 
Modern display hardware is capable of supporting variable refresh rates.
This patch introduces the "vrr_capable" property on the connector to
allow userspace to query support for variable refresh rates.

Atomic drivers should attach this property to connectors that are
capable of driving variable refresh rates using
drm_connector_attach_vrr_capable_property().

The value should be updated based on driver and hardware capability
by using drm_connector_set_vrr_capable_property().

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Manasi Navare 
Reviewed-by: Harry Wentland 
---
 drivers/gpu/drm/drm_connector.c | 49 +
 include/drm/drm_connector.h | 15 ++
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4943cef178be..49290060ab7b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1255,6 +1255,37 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * drm_connector_attach_vrr_capable_property - creates the
+ * vrr_capable property
+ * @connector: connector to create the vrr_capable property on.
+ *
+ * This is used by atomic drivers to add support for querying
+ * variable refresh rate capability for a connector.
+ *
+ * Returns:
+ * Zero on success, negative errono on failure.
+ */
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->vrr_capable_property) {
+   prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE,
+   "vrr_capable");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->vrr_capable_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_vrr_capable_property);
+
 /**
  * drm_connector_attach_scaling_mode_property - attach atomic scaling mode 
property
  * @connector: connector to attach scaling mode property on.
@@ -1583,6 +1614,24 @@ void drm_connector_set_link_status_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_set_link_status_property);
 
+/**
+ * drm_connector_set_vrr_capable_property - sets the variable refresh rate
+ * capable property for a connector
+ * @connector: drm connector
+ * @capable: True if the connector is variable refresh rate capable
+ *
+ * Should be used by atomic drivers to update the indicated support for
+ * variable refresh rate over a connector.
+ */
+void drm_connector_set_vrr_capable_property(
+   struct drm_connector *connector, bool capable)
+{
+   drm_object_property_set_value(>base,
+ connector->vrr_capable_property,
+ capable);
+}
+EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
+
 /**
  * drm_connector_init_panel_orientation_property -
  * initialize the connecters panel_orientation property
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 9ccad6b062f2..4e6befff153b 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -959,6 +959,17 @@ struct drm_connector {
 */
struct drm_property *scaling_mode_property;
 
+   /**
+* @vrr_capable_property: Optional property to help userspace
+* query hardware support for variable refresh rate on a connector.
+* connector. Drivers can add the property to a connector by
+* calling drm_connector_attach_vrr_capable_property().
+*
+* This should be updated only by calling
+* drm_connector_set_vrr_capable_property().
+*/
+   struct drm_property *vrr_capable_property;
+
/**
 * @content_protection_property: DRM ENUM property for content
 * protection. See drm_connector_attach_content_protection_property().
@@ -1250,6 +1261,8 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev);
 int drm_connector_attach_content_type_property(struct drm_connector *dev);
 int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
   u32 scaling_mode_mask);
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector);
 int drm_connector_attach_content_protection_property(
struct drm_connector *connector);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
@@ -1266,6 +1279,8 @@ int drm_connector_update_edid_property(struct 
drm_connector *connector,
   const struct edid *edid);
 void drm_connector_set_link_status_property(struct drm_connector *connector,

[PATCH v6 1/5] drm: Add vrr_capable property to the drm connector

2018-11-06 Thread Nicholas Kazlauskas 
Modern display hardware is capable of supporting variable refresh rates.
This patch introduces the "vrr_capable" property on the connector to
allow userspace to query support for variable refresh rates.

Atomic drivers should attach this property to connectors that are
capable of driving variable refresh rates using
drm_connector_attach_vrr_capable_property().

The value should be updated based on driver and hardware capabiltiy
by using drm_connector_set_vrr_capable_property().

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Manasi Navare 
Reviewed-by: Harry Wentland 
---
 drivers/gpu/drm/drm_connector.c | 49 +
 include/drm/drm_connector.h | 15 ++
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4943cef178be..49290060ab7b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1255,6 +1255,37 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * drm_connector_attach_vrr_capable_property - creates the
+ * vrr_capable property
+ * @connector: connector to create the vrr_capable property on.
+ *
+ * This is used by atomic drivers to add support for querying
+ * variable refresh rate capability for a connector.
+ *
+ * Returns:
+ * Zero on success, negative errono on failure.
+ */
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->vrr_capable_property) {
+   prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE,
+   "vrr_capable");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->vrr_capable_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_vrr_capable_property);
+
 /**
  * drm_connector_attach_scaling_mode_property - attach atomic scaling mode 
property
  * @connector: connector to attach scaling mode property on.
@@ -1583,6 +1614,24 @@ void drm_connector_set_link_status_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_set_link_status_property);
 
+/**
+ * drm_connector_set_vrr_capable_property - sets the variable refresh rate
+ * capable property for a connector
+ * @connector: drm connector
+ * @capable: True if the connector is variable refresh rate capable
+ *
+ * Should be used by atomic drivers to update the indicated support for
+ * variable refresh rate over a connector.
+ */
+void drm_connector_set_vrr_capable_property(
+   struct drm_connector *connector, bool capable)
+{
+   drm_object_property_set_value(>base,
+ connector->vrr_capable_property,
+ capable);
+}
+EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
+
 /**
  * drm_connector_init_panel_orientation_property -
  * initialize the connecters panel_orientation property
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 9ccad6b062f2..4e6befff153b 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -959,6 +959,17 @@ struct drm_connector {
 */
struct drm_property *scaling_mode_property;
 
+   /**
+* @vrr_capable_property: Optional property to help userspace
+* query hardware support for variable refresh rate on a connector.
+* connector. Drivers can add the property to a connector by
+* calling drm_connector_attach_vrr_capable_property().
+*
+* This should be updated only by calling
+* drm_connector_set_vrr_capable_property().
+*/
+   struct drm_property *vrr_capable_property;
+
/**
 * @content_protection_property: DRM ENUM property for content
 * protection. See drm_connector_attach_content_protection_property().
@@ -1250,6 +1261,8 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev);
 int drm_connector_attach_content_type_property(struct drm_connector *dev);
 int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
   u32 scaling_mode_mask);
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector);
 int drm_connector_attach_content_protection_property(
struct drm_connector *connector);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
@@ -1266,6 +1279,8 @@ int drm_connector_update_edid_property(struct 
drm_connector *connector,
   const struct edid *edid);
 void drm_connector_set_link_status_property(struct drm_connector *connector,

[PATCH v6 5/5] drm/amdgpu: Set FreeSync state using drm VRR properties

2018-11-06 Thread Nicholas Kazlauskas 
Support for AMDGPU specific FreeSync properties and ioctls are dropped
from amdgpu_dm in favor of supporting drm variable refresh rate
properties.

The notify_freesync and set_freesync_property functions are dropped
from amdgpu_display_funcs.

The drm vrr_capable property is now attached to any DP/HDMI connector.
Its value is updated accordingly to the connector's FreeSync capabiltiy.

The freesync_enable logic and ioctl control has has been dropped in
favor of utilizing the vrr_enabled on the drm CRTC. This allows for more
fine grained atomic control over which CRTCs should support variable
refresh rate.

To handle state changes for vrr_enabled it was easiest to drop the
forced modeset on freesync_enabled change. This patch now performs the
required stream updates when planes are flipped.

This is done for a few reasons:

(1) VRR stream updates can be done in the fast update path

(2) amdgpu_dm_atomic_check would need to be hacked apart to check
desired variable refresh state and capability before the CRTC
disable pass.

(3) Performing VRR stream updates on-flip is needed for enabling BTR
support.

VRR packets and timing adjustments are now tracked and compared to
previous values sent to the hardware.

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Harry Wentland 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   7 -
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 3 files changed, 138 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index b9e9e8b02fb7..0cbe867ec375 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -295,13 +295,6 @@ struct amdgpu_display_funcs {
  uint16_t connector_object_id,
  struct amdgpu_hpd *hpd,
  struct amdgpu_router *router);
-   /* it is used to enter or exit into free sync mode */
-   int (*notify_freesync)(struct drm_device *dev, void *data,
-  struct drm_file *filp);
-   /* it is used to allow enablement of freesync mode */
-   int (*set_freesync_property)(struct drm_connector *connector,
-struct drm_property *property,
-uint64_t val);
 
 
 };
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b0df6dc9a775..53eb3d16f75c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1809,72 +1809,6 @@ static void dm_bandwidth_update(struct amdgpu_device 
*adev)
/* TODO: implement later */
 }
 
-static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
-   struct drm_file *filp)
-{
-   struct drm_atomic_state *state;
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_crtc *crtc;
-   struct drm_connector *connector;
-   struct drm_connector_state *old_con_state, *new_con_state;
-   int ret = 0;
-   uint8_t i;
-   bool enable = false;
-
-   drm_modeset_acquire_init(, 0);
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   state->acquire_ctx = 
-
-retry:
-   drm_for_each_crtc(crtc, dev) {
-   ret = drm_atomic_add_affected_connectors(state, crtc);
-   if (ret)
-   goto fail;
-
-   /* TODO rework amdgpu_dm_commit_planes so we don't need this */
-   ret = drm_atomic_add_affected_planes(state, crtc);
-   if (ret)
-   goto fail;
-   }
-
-   for_each_oldnew_connector_in_state(state, connector, old_con_state, 
new_con_state, i) {
-   struct dm_connector_state *dm_new_con_state = 
to_dm_connector_state(new_con_state);
-   struct drm_crtc_state *new_crtc_state;
-   struct amdgpu_crtc *acrtc = 
to_amdgpu_crtc(dm_new_con_state->base.crtc);
-   struct dm_crtc_state *dm_new_crtc_state;
-
-   if (!acrtc) {
-   ASSERT(0);
-   continue;
-   }
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);
-   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
-   dm_new_crtc_state->freesync_enabled = enable;
-   }
-
-   ret = drm_atomic_commit(state);
-
-fail:
-   if (ret == -EDEADLK) {
-   drm_atomic_state_clear(state);
-   drm_modeset_backoff();
-   goto retry;
-   }
-
-   drm_atomic_state_put(state);
-
-out:
-   drm_modeset_drop_locks();
-   drm_modeset_acquire_fini();
-   return ret;
-}
 
 static

[PATCH v6 4/5] drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal

2018-11-06 Thread Nicholas Kazlauskas 
When variable refresh rate is active the hardware counter can return
a position >= vtotal. This results in a vpos being returned from
amdgpu_display_get_crtc_scanoutpos that's a positive value. The
positive value indicates to the caller that the display is
currently in scanout when the display is actually still in vblank.

This is because the vfront porch duration is unknown with variable
refresh active and will end when either a page flip occurs or the
timeout specified by the driver/display is reached.

The behavior of the amdgpu_display_get_crtc_scanoutpos remains the
same when the position is below vtotal. When the position is above
vtotal the function will return a value that is effectively -vbl_end,
the size of the vback porch.

The only caller affected by this change is the DRM helper for
calculating vblank timestamps. This change corrects behavior for
calculating the page flip timestap from being the previous timestamp
to the calculation to the next timestamp when position >= vtotal.

Signed-off-by: Nicholas Kazlauskas 
Cc: Michel Dänzer 
Cc: Harry Wentland 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 6748cd7fc129..cb331319f225 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -850,7 +850,12 @@ int amdgpu_display_get_crtc_scanoutpos(struct drm_device 
*dev,
/* Inside "upper part" of vblank area? Apply corrective offset if so: */
if (in_vbl && (*vpos >= vbl_start)) {
vtotal = mode->crtc_vtotal;
-   *vpos = *vpos - vtotal;
+
+   /* With variable refresh rate displays the vpos can exceed
+* the vtotal value. Clamp to 0 to return -vbl_end instead
+* of guessing the remaining number of lines until scanout.
+*/
+   *vpos = (*vpos < vtotal) ? (*vpos - vtotal) : 0;
}
 
/* Correct for shifted end of vbl at vbl_end. */
-- 
2.19.1

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

[PATCH v6 3/5] drm: Document variable refresh properties

2018-11-06 Thread Nicholas Kazlauskas 
These include the drm_connector 'vrr_capable' and the drm_crtc
'vrr_enabled' properties.

Signed-off-by: Nicholas Kazlauskas 
Cc: Harry Wentland 
Cc: Manasi Navare 
Cc: Pekka Paalanen 
Cc: Ville Syrjälä 
Cc: Michel Dänzer 
---
 Documentation/gpu/drm-kms.rst   |  7 
 drivers/gpu/drm/drm_connector.c | 61 +
 2 files changed, 68 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 4b1501b4835b..8da2a178cf85 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -575,6 +575,13 @@ Explicit Fencing Properties
 .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
:doc: explicit fencing properties
 
+
+Variable Refresh Properties
+---
+
+.. kernel-doc:: drivers/gpu/drm/drm_connector.c
+   :doc: Variable refresh properties
+
 Existing KMS Properties
 ---
 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 49290060ab7b..a6adf5450db3 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1255,6 +1255,67 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * DOC: Variable refresh properties
+ *
+ * Variable refresh rate capable displays can dynamically adjust their
+ * refresh rate by extending the duration of their vertical porch until
+ * page flip or timeout occurs. This can reduce or remove stuttering
+ * and latency in scenarios where the page flip does not align with the
+ * vblank interval.
+ *
+ * An example scenario would be an application flipping at a constant rate
+ * of 48Hz on a 60Hz display. The page flip will frequently miss the vblank
+ * interval and the same contents will be displayed twice. This can be
+ * observed as stuttering for content with motion.
+ *
+ * If variable refresh rate was active on a display that supported a
+ * variable refresh range from 35Hz to 60Hz no stuttering would be observable
+ * for the example scenario. The minimum supported variable refresh rate of
+ * 35Hz is below the page flip frequency and the vertical front porch can
+ * be extended until the page flip occurs. The vblank interval will be
+ * directly aligned to the page flip rate.
+ *
+ * Userspace control for variable refresh rate is supported via properties
+ * on the _connector and _crtc objects.
+ *
+ * "vrr_capable":
+ * Optional _connector boolean property that drivers should attach
+ * with drm_connector_attach_vrr_capable_property() on connectors that
+ * could support variable refresh rates. Drivers should update the
+ * property value by calling drm_connector_set_vrr_capable_property().
+ *
+ * Absence of the property should indicate absence of support.
+ *
+ * "vrr_enabled":
+ * Default _crtc boolean property that notifies the driver that the
+ * content on the CRTC is suitable for variable refresh rate presentation.
+ * The driver will take this property as a hint to enable variable
+ * refresh rate support if the receiver supports it, ie. if the
+ * "vrr_capable" property is true on the _connector object. The
+ * veritcal front porch duration will be extended until page-flip or
+ * timeout when enabled.
+ *
+ * The minimum vertical front porch duration is defined as the vertical
+ * front porch duration for the current mode.
+ *
+ * The maximum vertical front porch duration is greater than or equal to
+ * the minimum vertical front porch duration. The duration is derived
+ * from the minimum supported variable refresh rate for the connector.
+ *
+ * The driver may place further restrictions within these minimum
+ * and maximum bounds.
+ *
+ * Some displays may exhibit noticeable differences in brightness when
+ * varying vertical front porch duration.
+ *
+ * The semantics for the vertical blank timestamp differ when
+ * variable refresh rate is active. The vertical blank timestamp
+ * is defined to be an estimate using the current mode's fixed
+ * refresh rate timings. The semantics for the page-flip event
+ * timestamp remain the same.
+ */
+
 /**
  * drm_connector_attach_vrr_capable_property - creates the
  * vrr_capable property
-- 
2.19.1

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

[PATCH v6 2/5] drm: Add vrr_enabled property to drm CRTC

2018-11-06 Thread 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.

Signed-off-by: Nicholas Kazlauskas 
Reviewed-by: Harry Wentland 
Cc: Manasi Navare 
---
 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 268a182ae189..6f8ddfcfaba5 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
-- 
2.19.1

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

[PATCH v6 0/5] A DRM API for adaptive sync and variable refresh rate support

2018-11-06 Thread Nicholas Kazlauskas 
via the Present extension 
to xf86-video-amdgpu for the entire Screen. Some compositors may interfere with 
this if they are unable to unredirect the window.

Full implementation details for these changes can be reviewed in their 
respective mailing lists and the xf86-video-amdgpu GitLab.

=== Previous discussions ===

These patches are based upon feedback from previous threads on the subject. 
These are linked below for reference:

https://lists.freedesktop.org/archives/amd-gfx/2018-April/021047.html
https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html
https://lists.freedesktop.org/archives/dri-devel/2018-September/189404.htm
https://lists.freedesktop.org/archives/dri-devel/2018-September/190910.html
https://lists.freedesktop.org/archives/dri-devel/2018-October/192211.html
https://lists.freedesktop.org/archives/dri-devel/2018-October/192874.html

Nicholas Kazlauskas (5):
  drm: Add vrr_capable property to the drm connector
  drm: Add vrr_enabled property to drm CRTC
  drm: Document variable refresh properties
  drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal
  drm/amdgpu: Set FreeSync state using drm VRR properties

 Documentation/gpu/drm-kms.rst |   7 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |   7 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   7 -
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 drivers/gpu/drm/drm_atomic_uapi.c |   4 +
 drivers/gpu/drm/drm_connector.c   | 110 
 drivers/gpu/drm/drm_crtc.c|   2 +
 drivers/gpu/drm/drm_mode_config.c |   6 +
 include/drm/drm_connector.h   |  15 ++
 include/drm/drm_crtc.h|   9 +
 include/drm/drm_mode_config.h |   5 +
 12 files changed, 302 insertions(+), 132 deletions(-)

-- 
2.19.1

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

[PATCH v5 4/4] drm/amdgpu: Set FreeSync state using drm VRR properties

2018-10-12 Thread Nicholas Kazlauskas 
Support for AMDGPU specific FreeSync properties and ioctls are dropped
from amdgpu_dm in favor of supporting drm variable refresh rate
properties.

The notify_freesync and set_freesync_property functions are dropped
from amdgpu_display_funcs.

The drm vrr_capable property is now attached to any DP/HDMI connector.
Its value is updated accordingly to the connector's FreeSync capabiltiy.

The freesync_enable logic and ioctl control has has been dropped in
favor of utilizing the vrr_enabled on the drm CRTC. This allows for more
fine grained atomic control over which CRTCs should support variable
refresh rate.

To handle state changes for vrr_enabled it was easiest to drop the
forced modeset on freesync_enabled change. This patch now performs the
required stream updates when planes are flipped.

This is done for a few reasons:

(1) VRR stream updates can be done in the fast update path

(2) amdgpu_dm_atomic_check would need to be hacked apart to check
desired variable refresh state and capability before the CRTC
disable pass.

(3) Performing VRR stream updates on-flip is needed for enabling BTR
support.

VRR packets and timing adjustments are now tracked and compared to
previous values sent to the hardware.

Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   7 -
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 3 files changed, 138 insertions(+), 131 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index b9e9e8b02fb7..0cbe867ec375 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -295,13 +295,6 @@ struct amdgpu_display_funcs {
  uint16_t connector_object_id,
  struct amdgpu_hpd *hpd,
  struct amdgpu_router *router);
-   /* it is used to enter or exit into free sync mode */
-   int (*notify_freesync)(struct drm_device *dev, void *data,
-  struct drm_file *filp);
-   /* it is used to allow enablement of freesync mode */
-   int (*set_freesync_property)(struct drm_connector *connector,
-struct drm_property *property,
-uint64_t val);
 
 
 };
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index e224f23e2215..f6af388cc32d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1802,72 +1802,6 @@ static void dm_bandwidth_update(struct amdgpu_device 
*adev)
/* TODO: implement later */
 }
 
-static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
-   struct drm_file *filp)
-{
-   struct drm_atomic_state *state;
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_crtc *crtc;
-   struct drm_connector *connector;
-   struct drm_connector_state *old_con_state, *new_con_state;
-   int ret = 0;
-   uint8_t i;
-   bool enable = false;
-
-   drm_modeset_acquire_init(, 0);
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   state->acquire_ctx = 
-
-retry:
-   drm_for_each_crtc(crtc, dev) {
-   ret = drm_atomic_add_affected_connectors(state, crtc);
-   if (ret)
-   goto fail;
-
-   /* TODO rework amdgpu_dm_commit_planes so we don't need this */
-   ret = drm_atomic_add_affected_planes(state, crtc);
-   if (ret)
-   goto fail;
-   }
-
-   for_each_oldnew_connector_in_state(state, connector, old_con_state, 
new_con_state, i) {
-   struct dm_connector_state *dm_new_con_state = 
to_dm_connector_state(new_con_state);
-   struct drm_crtc_state *new_crtc_state;
-   struct amdgpu_crtc *acrtc = 
to_amdgpu_crtc(dm_new_con_state->base.crtc);
-   struct dm_crtc_state *dm_new_crtc_state;
-
-   if (!acrtc) {
-   ASSERT(0);
-   continue;
-   }
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);
-   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
-   dm_new_crtc_state->freesync_enabled = enable;
-   }
-
-   ret = drm_atomic_commit(state);
-
-fail:
-   if (ret == -EDEADLK) {
-   drm_atomic_state_clear(state);
-   drm_modeset_backoff();
-   goto retry;
-   }
-
-   drm_atomic_state_put(state);
-
-out:
-   drm_modeset_drop_locks();
-   drm_modeset_acquire_fini();
-   return ret;
-}
 
 static const struct amdgpu_display_funcs dm_d

[PATCH v5 3/4] drm: Document variable refresh properties

2018-10-12 Thread Nicholas Kazlauskas 
These include the drm_connector 'vrr_capable' and the drm_crtc
'vrr_enabled' properties.

Signed-off-by: Nicholas Kazlauskas 
---
 Documentation/gpu/drm-kms.rst   |  7 +++
 drivers/gpu/drm/drm_connector.c | 22 ++
 2 files changed, 29 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 4b1501b4835b..8da2a178cf85 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -575,6 +575,13 @@ Explicit Fencing Properties
 .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
:doc: explicit fencing properties
 
+
+Variable Refresh Properties
+---
+
+.. kernel-doc:: drivers/gpu/drm/drm_connector.c
+   :doc: Variable refresh properties
+
 Existing KMS Properties
 ---
 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index f0deeb7298d0..2a12853ca917 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * DOC: Variable refresh properties
+ *
+ * Variable refresh rate control is supported via properties on the
+ * _connector and _crtc objects.
+ *
+ * "vrr_capable":
+ * Optional _connector boolean property that drivers should attach
+ * with drm_connector_attach_vrr_capable_property() on connectors that
+ * could support variable refresh rates. Drivers should update the
+ * property value by calling drm_connector_set_vrr_capable_property().
+ *
+ * Absence of the property should indicate absence of support.
+ *
+ * "vrr_enabled":
+ * Default _crtc boolean property that notifies the driver that the
+ * variable refresh rate adjustment should be enabled for the CRTC.
+ *
+ * Support for variable refresh rate will depend on the "vrr_capable"
+ * property exposed on the _connector object.
+ */
+
 /**
  * drm_connector_attach_vrr_capable_property - creates the
  * vrr_capable property
-- 
2.19.1

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

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

2018-10-12 Thread 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.

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
-- 
2.19.1

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

[PATCH v5 1/4] drm: Add vrr_capable property to the drm connector

2018-10-12 Thread Nicholas Kazlauskas 
Modern display hardware is capable of supporting variable refresh rates.
This patch introduces the "vrr_capable" property on the connector to
allow userspace to query support for variable refresh rates.

Atomic drivers should attach this property to connectors that are
capable of driving variable refresh rates using
drm_connector_attach_vrr_capable_property().

The value should be updated based on driver and hardware capabiltiy
by using drm_connector_set_vrr_capable_property().

Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/drm_connector.c | 49 +
 include/drm/drm_connector.h | 15 ++
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 1e40e5decbe9..f0deeb7298d0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1254,6 +1254,37 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * drm_connector_attach_vrr_capable_property - creates the
+ * vrr_capable property
+ * @connector: connector to create the vrr_capable property on.
+ *
+ * This is used by atomic drivers to add support for querying
+ * variable refresh rate capability for a connector.
+ *
+ * Returns:
+ * Zero on success, negative errono on failure.
+ */
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->vrr_capable_property) {
+   prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE,
+   "vrr_capable");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->vrr_capable_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_vrr_capable_property);
+
 /**
  * drm_connector_attach_scaling_mode_property - attach atomic scaling mode 
property
  * @connector: connector to attach scaling mode property on.
@@ -1582,6 +1613,24 @@ void drm_connector_set_link_status_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_set_link_status_property);
 
+/**
+ * drm_connector_set_vrr_capable_property - sets the variable refresh rate
+ * capable property for a connector
+ * @connector: drm connector
+ * @capable: True if the connector is variable refresh rate capable
+ *
+ * Should be used by atomic drivers to update the indicated support for
+ * variable refresh rate over a connector.
+ */
+void drm_connector_set_vrr_capable_property(
+   struct drm_connector *connector, bool capable)
+{
+   drm_object_property_set_value(>base,
+ connector->vrr_capable_property,
+ capable);
+}
+EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
+
 /**
  * drm_connector_init_panel_orientation_property -
  * initialize the connecters panel_orientation property
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 91a877fa00cb..b2263005234a 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -910,6 +910,17 @@ struct drm_connector {
 */
struct drm_property *scaling_mode_property;
 
+   /**
+* @vrr_capable_property: Optional property to help userspace
+* query hardware support for variable refresh rate on a connector.
+* connector. Drivers can add the property to a connector by
+* calling drm_connector_attach_vrr_capable_property().
+*
+* This should be updated only by calling
+* drm_connector_set_vrr_capable_property().
+*/
+   struct drm_property *vrr_capable_property;
+
/**
 * @content_protection_property: DRM ENUM property for content
 * protection. See drm_connector_attach_content_protection_property().
@@ -1183,6 +1194,8 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev);
 int drm_connector_attach_content_type_property(struct drm_connector *dev);
 int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
   u32 scaling_mode_mask);
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector);
 int drm_connector_attach_content_protection_property(
struct drm_connector *connector);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
@@ -1199,6 +1212,8 @@ int drm_connector_update_edid_property(struct 
drm_connector *connector,
   const struct edid *edid);
 void drm_connector_set_link_status_property(struct drm_connector *connector,
uint64_t link_status);
+void drm_connector_set_vrr_ca

[PATCH v5 0/4] A DRM API for adaptive sync and variable refresh rate support

2018-10-12 Thread Nicholas Kazlauskas 
 in their 
respective mailing lists and the xf86-video-amdgpu GitLab.

=== Previous discussions ===

These patches are based upon feedback from previous threads on the subject. 
These are linked below for reference:

https://lists.freedesktop.org/archives/amd-gfx/2018-April/021047.html
https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html
https://lists.freedesktop.org/archives/dri-devel/2018-September/189404.htm
https://lists.freedesktop.org/archives/dri-devel/2018-September/190910.html
https://lists.freedesktop.org/archives/dri-devel/2018-October/192211.html
https://lists.freedesktop.org/archives/dri-devel/2018-October/192874.html

Nicholas Kazlauskas (4):
  drm: Add vrr_capable property to the drm connector
  drm: Add vrr_enabled property to drm CRTC
  drm: Document variable refresh properties
  drm/amdgpu: Set FreeSync state using drm VRR properties

 Documentation/gpu/drm-kms.rst |   7 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   7 -
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 drivers/gpu/drm/drm_atomic_uapi.c |   4 +
 drivers/gpu/drm/drm_connector.c   |  71 +
 drivers/gpu/drm/drm_crtc.c|   2 +
 drivers/gpu/drm/drm_mode_config.c |   6 +
 include/drm/drm_connector.h   |  15 ++
 include/drm/drm_crtc.h|   9 +
 include/drm/drm_mode_config.h |   5 +
 11 files changed, 257 insertions(+), 131 deletions(-)

-- 
2.19.1

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

[PATCH v4 4/4] drm/amd/display: Set FreeSync state using drm VRR properties

2018-10-11 Thread Nicholas Kazlauskas 
Support for AMDGPU specific FreeSync properties and ioctls are dropped
from amdgpu_dm in favor of supporting drm variable refresh rate
properties.

The drm vrr_capable property is now attached to any DP/HDMI connector.
Its value is updated accordingly to the connector's FreeSync capabiltiy.

The freesync_enable logic and ioctl control has has been dropped in
favor of utilizing the vrr_enabled on the drm CRTC. This allows for more
fine grained atomic control over which CRTCs should support variable
refresh rate.

To handle state changes for vrr_enabled it was easiest to drop the
forced modeset on freesync_enabled change. This patch now performs the
required stream updates when planes are flipped.

This is done for a few reasons:

(1) VRR stream updates can be done in the fast update path

(2) amdgpu_dm_atomic_check would need to be hacked apart to check
desired variable refresh state and capability before the CRTC
disable pass.

(3) Performing VRR stream updates on-flip is needed for enabling BTR
support.

VRR packets and timing adjustments are now tracked and compared to
previous values sent to the hardware.

Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 2 files changed, 138 insertions(+), 124 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 6a2342d72742..d5de4b91e144 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1802,72 +1802,6 @@ static void dm_bandwidth_update(struct amdgpu_device 
*adev)
/* TODO: implement later */
 }
 
-static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
-   struct drm_file *filp)
-{
-   struct drm_atomic_state *state;
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_crtc *crtc;
-   struct drm_connector *connector;
-   struct drm_connector_state *old_con_state, *new_con_state;
-   int ret = 0;
-   uint8_t i;
-   bool enable = false;
-
-   drm_modeset_acquire_init(, 0);
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   state->acquire_ctx = 
-
-retry:
-   drm_for_each_crtc(crtc, dev) {
-   ret = drm_atomic_add_affected_connectors(state, crtc);
-   if (ret)
-   goto fail;
-
-   /* TODO rework amdgpu_dm_commit_planes so we don't need this */
-   ret = drm_atomic_add_affected_planes(state, crtc);
-   if (ret)
-   goto fail;
-   }
-
-   for_each_oldnew_connector_in_state(state, connector, old_con_state, 
new_con_state, i) {
-   struct dm_connector_state *dm_new_con_state = 
to_dm_connector_state(new_con_state);
-   struct drm_crtc_state *new_crtc_state;
-   struct amdgpu_crtc *acrtc = 
to_amdgpu_crtc(dm_new_con_state->base.crtc);
-   struct dm_crtc_state *dm_new_crtc_state;
-
-   if (!acrtc) {
-   ASSERT(0);
-   continue;
-   }
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);
-   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
-   dm_new_crtc_state->freesync_enabled = enable;
-   }
-
-   ret = drm_atomic_commit(state);
-
-fail:
-   if (ret == -EDEADLK) {
-   drm_atomic_state_clear(state);
-   drm_modeset_backoff();
-   goto retry;
-   }
-
-   drm_atomic_state_put(state);
-
-out:
-   drm_modeset_drop_locks();
-   drm_modeset_acquire_fini();
-   return ret;
-}
 
 static const struct amdgpu_display_funcs dm_display_funcs = {
.bandwidth_update = dm_bandwidth_update, /* called unconditionally */
@@ -1881,7 +1815,6 @@ static const struct amdgpu_display_funcs dm_display_funcs 
= {
dm_crtc_get_scanoutpos,/* called unconditionally */
.add_encoder = NULL, /* VBIOS parsing. DAL does it. */
.add_connector = NULL, /* VBIOS parsing. DAL does it. */
-   .notify_freesync = amdgpu_notify_freesync,
 
 };
 
@@ -2834,7 +2767,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
 
state->adjust = cur->adjust;
state->vrr_infopacket = cur->vrr_infopacket;
-   state->freesync_enabled = cur->freesync_enabled;
+   state->vrr_supported = cur->vrr_supported;
+   state->freesync_config = cur->freesync_config;
 
/* TODO Duplicate dc_stream after objects are stream object is 
flattened */
 
@@ -3053,8 +2987,6 @@ amdgpu_dm_connector_atomic_duplicate_state(struct 
drm_connector *connector)
__drm_atomic_helper_connector_duplicate_state(connector, 
_state->base);

[PATCH v4 3/4] drm: Document variable refresh properties

2018-10-11 Thread Nicholas Kazlauskas 
These include the drm_connector 'vrr_capable' and the drm_crtc
'vrr_enabled' properties.

Signed-off-by: Nicholas Kazlauskas 
---
 Documentation/gpu/drm-kms.rst   |  7 +++
 drivers/gpu/drm/drm_connector.c | 22 ++
 2 files changed, 29 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 4b1501b4835b..8da2a178cf85 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -575,6 +575,13 @@ Explicit Fencing Properties
 .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
:doc: explicit fencing properties
 
+
+Variable Refresh Properties
+---
+
+.. kernel-doc:: drivers/gpu/drm/drm_connector.c
+   :doc: Variable refresh properties
+
 Existing KMS Properties
 ---
 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index f0deeb7298d0..2a12853ca917 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * DOC: Variable refresh properties
+ *
+ * Variable refresh rate control is supported via properties on the
+ * _connector and _crtc objects.
+ *
+ * "vrr_capable":
+ * Optional _connector boolean property that drivers should attach
+ * with drm_connector_attach_vrr_capable_property() on connectors that
+ * could support variable refresh rates. Drivers should update the
+ * property value by calling drm_connector_set_vrr_capable_property().
+ *
+ * Absence of the property should indicate absence of support.
+ *
+ * "vrr_enabled":
+ * Default _crtc boolean property that notifies the driver that the
+ * variable refresh rate adjustment should be enabled for the CRTC.
+ *
+ * Support for variable refresh rate will depend on the "vrr_capable"
+ * property exposed on the _connector object.
+ */
+
 /**
  * drm_connector_attach_vrr_capable_property - creates the
  * vrr_capable property
-- 
2.19.1

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

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

2018-10-11 Thread 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.

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
-- 
2.19.1

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

[PATCH v4 1/4] drm: Add vrr_capable property to the drm connector

2018-10-11 Thread Nicholas Kazlauskas 
Modern display hardware is capable of supporting variable refresh rates.
This patch introduces the "vrr_capable" property on the connector to
allow userspace to query support for variable refresh rates.

Atomic drivers should attach this property to connectors that are
capable of driving variable refresh rates using
drm_connector_attach_vrr_capable_property().

The value should be updated based on driver and hardware capabiltiy
by using drm_connector_set_vrr_capable_property().

Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/drm_connector.c | 49 +
 include/drm/drm_connector.h | 15 ++
 2 files changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 1e40e5decbe9..f0deeb7298d0 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1254,6 +1254,37 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * drm_connector_attach_vrr_capable_property - creates the
+ * vrr_capable property
+ * @connector: connector to create the vrr_capable property on.
+ *
+ * This is used by atomic drivers to add support for querying
+ * variable refresh rate capability for a connector.
+ *
+ * Returns:
+ * Zero on success, negative errono on failure.
+ */
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->vrr_capable_property) {
+   prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE,
+   "vrr_capable");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->vrr_capable_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_vrr_capable_property);
+
 /**
  * drm_connector_attach_scaling_mode_property - attach atomic scaling mode 
property
  * @connector: connector to attach scaling mode property on.
@@ -1582,6 +1613,24 @@ void drm_connector_set_link_status_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_set_link_status_property);
 
+/**
+ * drm_connector_set_vrr_capable_property - sets the variable refresh rate
+ * capable property for a connector
+ * @connector: drm connector
+ * @capable: True if the connector is variable refresh rate capable
+ *
+ * Should be used by atomic drivers to update the indicated support for
+ * variable refresh rate over a connector.
+ */
+void drm_connector_set_vrr_capable_property(
+   struct drm_connector *connector, bool capable)
+{
+   drm_object_property_set_value(>base,
+ connector->vrr_capable_property,
+ capable);
+}
+EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
+
 /**
  * drm_connector_init_panel_orientation_property -
  * initialize the connecters panel_orientation property
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 91a877fa00cb..b2263005234a 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -910,6 +910,17 @@ struct drm_connector {
 */
struct drm_property *scaling_mode_property;
 
+   /**
+* @vrr_capable_property: Optional property to help userspace
+* query hardware support for variable refresh rate on a connector.
+* connector. Drivers can add the property to a connector by
+* calling drm_connector_attach_vrr_capable_property().
+*
+* This should be updated only by calling
+* drm_connector_set_vrr_capable_property().
+*/
+   struct drm_property *vrr_capable_property;
+
/**
 * @content_protection_property: DRM ENUM property for content
 * protection. See drm_connector_attach_content_protection_property().
@@ -1183,6 +1194,8 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev);
 int drm_connector_attach_content_type_property(struct drm_connector *dev);
 int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
   u32 scaling_mode_mask);
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector);
 int drm_connector_attach_content_protection_property(
struct drm_connector *connector);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
@@ -1199,6 +1212,8 @@ int drm_connector_update_edid_property(struct 
drm_connector *connector,
   const struct edid *edid);
 void drm_connector_set_link_status_property(struct drm_connector *connector,
uint64_t link_status);
+void drm_connector_set_vrr_ca

[PATCH v4 0/4] A DRM API for adaptive sync and variable refresh rate support

2018-10-11 Thread Nicholas Kazlauskas 
scussions ===

These patches are based upon feedback from previous threads on the subject. 
These are linked below for reference:

https://lists.freedesktop.org/archives/amd-gfx/2018-April/021047.html
https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html
https://lists.freedesktop.org/archives/dri-devel/2018-September/189404.htm
https://lists.freedesktop.org/archives/dri-devel/2018-September/190910.html
https://lists.freedesktop.org/archives/dri-devel/2018-October/192211.html

Nicholas Kazlauskas (4):
  drm: Add vrr_capable property to the drm connector
  drm: Add vrr_enabled property to drm CRTC
  drm: Document variable refresh properties
  drm/amd/display: Set FreeSync state using drm VRR properties

 Documentation/gpu/drm-kms.rst |   7 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 drivers/gpu/drm/drm_atomic_uapi.c |   4 +
 drivers/gpu/drm/drm_connector.c   |  71 +
 drivers/gpu/drm/drm_crtc.c|   2 +
 drivers/gpu/drm/drm_mode_config.c |   6 +
 include/drm/drm_connector.h   |  15 ++
 include/drm/drm_crtc.h|   9 +
 include/drm/drm_mode_config.h |   5 +
 10 files changed, 257 insertions(+), 124 deletions(-)

-- 
2.19.1

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

[PATCH v3 4/4] drm/amd/display: Set FreeSync state using drm VRR properties

2018-10-05 Thread Nicholas Kazlauskas 
Support for AMDGPU specific FreeSync properties and ioctls are dropped
from amdgpu_dm in favor of supporting drm variable refresh rate
properties.

The drm vrr_capable property is now attached to any DP/HDMI connector.
Its value is updated accordingly to the connector's FreeSync capabiltiy.

The freesync_enable logic and ioctl control has has been dropped in
favor of utilizing the vrr_enabled on the drm CRTC. This allows for more
fine grained atomic control over which CRTCs should support variable
refresh rate.

To handle state changes for vrr_enabled it was easiest to drop the
forced modeset on freesync_enabled change. This patch now performs the
required strema updates when planes are flipped.

This is done for a few reasons:

(1) VRR stream updates can be done in the fast update path

(2) amdgpu_dm_atomic_check would have had to been hacked apart to check
desired variable refresh state and capability before the CRTC
disable pass.

(3) Performing VRR stream updates on-flip is needed for enabling BTR
support.

VRR packets and timing adjustments are now tracked and compared to
previous values sent to the hardware.

Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 2 files changed, 138 insertions(+), 124 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 6a2342d72742..0a608e31ad95 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1802,72 +1802,6 @@ static void dm_bandwidth_update(struct amdgpu_device 
*adev)
/* TODO: implement later */
 }
 
-static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
-   struct drm_file *filp)
-{
-   struct drm_atomic_state *state;
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_crtc *crtc;
-   struct drm_connector *connector;
-   struct drm_connector_state *old_con_state, *new_con_state;
-   int ret = 0;
-   uint8_t i;
-   bool enable = false;
-
-   drm_modeset_acquire_init(, 0);
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   state->acquire_ctx = 
-
-retry:
-   drm_for_each_crtc(crtc, dev) {
-   ret = drm_atomic_add_affected_connectors(state, crtc);
-   if (ret)
-   goto fail;
-
-   /* TODO rework amdgpu_dm_commit_planes so we don't need this */
-   ret = drm_atomic_add_affected_planes(state, crtc);
-   if (ret)
-   goto fail;
-   }
-
-   for_each_oldnew_connector_in_state(state, connector, old_con_state, 
new_con_state, i) {
-   struct dm_connector_state *dm_new_con_state = 
to_dm_connector_state(new_con_state);
-   struct drm_crtc_state *new_crtc_state;
-   struct amdgpu_crtc *acrtc = 
to_amdgpu_crtc(dm_new_con_state->base.crtc);
-   struct dm_crtc_state *dm_new_crtc_state;
-
-   if (!acrtc) {
-   ASSERT(0);
-   continue;
-   }
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);
-   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
-   dm_new_crtc_state->freesync_enabled = enable;
-   }
-
-   ret = drm_atomic_commit(state);
-
-fail:
-   if (ret == -EDEADLK) {
-   drm_atomic_state_clear(state);
-   drm_modeset_backoff();
-   goto retry;
-   }
-
-   drm_atomic_state_put(state);
-
-out:
-   drm_modeset_drop_locks();
-   drm_modeset_acquire_fini();
-   return ret;
-}
 
 static const struct amdgpu_display_funcs dm_display_funcs = {
.bandwidth_update = dm_bandwidth_update, /* called unconditionally */
@@ -1881,7 +1815,6 @@ static const struct amdgpu_display_funcs dm_display_funcs 
= {
dm_crtc_get_scanoutpos,/* called unconditionally */
.add_encoder = NULL, /* VBIOS parsing. DAL does it. */
.add_connector = NULL, /* VBIOS parsing. DAL does it. */
-   .notify_freesync = amdgpu_notify_freesync,
 
 };
 
@@ -2834,7 +2767,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
 
state->adjust = cur->adjust;
state->vrr_infopacket = cur->vrr_infopacket;
-   state->freesync_enabled = cur->freesync_enabled;
+   state->vrr_supported = cur->vrr_supported;
+   state->freesync_config = cur->freesync_config;
 
/* TODO Duplicate dc_stream after objects are stream object is 
flattened */
 
@@ -3053,8 +2987,6 @@ amdgpu_dm_connector_atomic_duplicate_state(struct 
drm_connector *connector)
__drm_atomic_helper_connector_duplicate_state(connector, 
_state->

[PATCH v3 3/4] drm: Document variable refresh properties

2018-10-05 Thread Nicholas Kazlauskas 
These include the drm_connector 'vrr_capable' and the drm_crtc
'vrr_enabled' properties.

Signed-off-by: Nicholas Kazlauskas 
---
 Documentation/gpu/drm-kms.rst   |  7 +++
 drivers/gpu/drm/drm_connector.c | 22 ++
 2 files changed, 29 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 4b1501b4835b..8da2a178cf85 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -575,6 +575,13 @@ Explicit Fencing Properties
 .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c
:doc: explicit fencing properties
 
+
+Variable Refresh Properties
+---
+
+.. kernel-doc:: drivers/gpu/drm/drm_connector.c
+   :doc: Variable refresh properties
+
 Existing KMS Properties
 ---
 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 3283703b9822..3b786f3c47ef 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * DOC: Variable refresh properties
+ *
+ * Variable refresh rate control is supported via properties on the
+ * _connector and _crtc objects.
+ *
+ * "vrr_capable":
+ * Optional _connector boolean property that drivers should attach
+ * with drm_connector_attach_vrr_capable_property() on connectors that
+ * could support variable refresh rates. Drivers should update the
+ * property value by calling drm_connector_set_vrr_capable_property().
+ *
+ * Absence of the property should indicate absence of support.
+ *
+ * "vrr_enabled":
+ * Default _crtc boolean property that notifies the driver that the
+ * variable refresh rate adjustment should be enabled for the CRTC.
+ *
+ * Support for variable refresh rate will depend on the "vrr_capable"
+ * property exposed on the _connector object.
+ */
+
 /**
  * drm_connector_attach_vrr_capable_property - creates the
  * vrr_capable property
-- 
2.19.0

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

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

2018-10-05 Thread 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.

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
-- 
2.19.0

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

[PATCH v3 1/4] drm: Add vrr_capable property to the drm connector

2018-10-05 Thread Nicholas Kazlauskas 
Modern display hardware is capable of supporting variable refresh rates.
This patch introduces the "vrr_capable" property on the connector to
allow userspace to query support for variable refresh rates.

Atomic drivers should attach this property to connectors that are
capable of driving variable refresh rates using
drm_connector_attach_vrr_capable_property().

The value should be updated based on driver and hardware capabiltiy
by using drm_connector_set_vrr_capable_property().

Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/drm_connector.c | 51 +
 include/drm/drm_connector.h | 15 ++
 2 files changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 1e40e5decbe9..3283703b9822 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1254,6 +1254,39 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * drm_connector_attach_vrr_capable_property - creates the
+ * vrr_capable property
+ * @connector: connector to create the vrr_capable property on.
+ *
+ * This is used by atomic drivers to add support for querying
+ * variable refresh rate capability for a connector.
+ *
+ * The value from _connector_state.vrr_capable will reads
+ *
+ * Returns:
+ * Zero on success, negative errono on failure.
+ */
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->vrr_capable_property) {
+   prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE,
+   "vrr_capable");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->vrr_capable_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_vrr_capable_property);
+
 /**
  * drm_connector_attach_scaling_mode_property - attach atomic scaling mode 
property
  * @connector: connector to attach scaling mode property on.
@@ -1582,6 +1615,24 @@ void drm_connector_set_link_status_property(struct 
drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_set_link_status_property);
 
+/**
+ * drm_connector_set_vrr_capable_property - sets the variable refresh rate
+ * capable property for a connector
+ * @connector: drm connector
+ * @capable: True if the connector is variable refresh rate capable
+ *
+ * Should be used by atomic drivers to update the indicated support for
+ * variable refresh rate over a connector.
+ */
+void drm_connector_set_vrr_capable_property(
+   struct drm_connector *connector, bool capable)
+{
+   return drm_object_property_set_value(>base,
+connector->vrr_capable_property,
+capable);
+}
+EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
+
 /**
  * drm_connector_init_panel_orientation_property -
  * initialize the connecters panel_orientation property
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 91a877fa00cb..b2263005234a 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -910,6 +910,17 @@ struct drm_connector {
 */
struct drm_property *scaling_mode_property;
 
+   /**
+* @vrr_capable_property: Optional property to help userspace
+* query hardware support for variable refresh rate on a connector.
+* connector. Drivers can add the property to a connector by
+* calling drm_connector_attach_vrr_capable_property().
+*
+* This should be updated only by calling
+* drm_connector_set_vrr_capable_property().
+*/
+   struct drm_property *vrr_capable_property;
+
/**
 * @content_protection_property: DRM ENUM property for content
 * protection. See drm_connector_attach_content_protection_property().
@@ -1183,6 +1194,8 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev);
 int drm_connector_attach_content_type_property(struct drm_connector *dev);
 int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
   u32 scaling_mode_mask);
+int drm_connector_attach_vrr_capable_property(
+   struct drm_connector *connector);
 int drm_connector_attach_content_protection_property(
struct drm_connector *connector);
 int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
@@ -1199,6 +1212,8 @@ int drm_connector_update_edid_property(struct 
drm_connector *connector,
   const struct edid *edid);
 void drm_connector_set_link_status_property(struct drm_connector *connector,

[PATCH v3 0/4] A DRM API for adaptive sync and variable refresh rate support

2018-10-05 Thread Nicholas Kazlauskas 
.freedesktop.org/archives/amd-gfx/2018-April/021047.html
https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html
https://lists.freedesktop.org/archives/dri-devel/2018-September/189404.html
https://lists.freedesktop.org/archives/dri-devel/2018-September/190910.html

Nicholas Kazlauskas (4):
  drm: Add vrr_capable property to the drm connector
  drm: Add vrr_enabled property to drm CRTC
  drm: Document variable refresh properties
  drm/amd/display: Set FreeSync state using drm VRR properties

 Documentation/gpu/drm-kms.rst |   7 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   7 +-
 drivers/gpu/drm/drm_atomic_uapi.c |   4 +
 drivers/gpu/drm/drm_connector.c   |  73 +
 drivers/gpu/drm/drm_crtc.c|   2 +
 drivers/gpu/drm/drm_mode_config.c |   6 +
 include/drm/drm_connector.h   |  15 ++
 include/drm/drm_crtc.h|   9 +
 include/drm/drm_mode_config.h |   5 +
 10 files changed, 259 insertions(+), 124 deletions(-)

-- 
2.19.0

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

[PATCH v2 3/3] drm/amd/display: Set FreeSync state using DRM VRR properties

2018-09-24 Thread Nicholas Kazlauskas 
The AMDGPU specific FreeSync properties and IOCTLs are dropped
from amdgpu_dm in favor of the DRM variable refresh properties.

The AMDGPU connector properties freesync_capable and freesync_enabled
are mostly direct mappings to the DRM variable_refresh_capable and
variable_refresh_enabled.

The AMDGPU CRTC freesync_enabled property requires special care to
handle correctly. The DRM variable_refresh property is a content hint
to the driver which is independent from actual hardware variable
refresh capability and user configuration.

To handle changes with this property it was easier to drop the forced
modeset on variable refresh change. This patch now performs the required
stream updates when planes are attached *or* flipped.

This is done for a few reasons:

(1) VRR stream updates can be done in the fast update path

(2) amdgpu_dm_atomic_check would have had to been hacked apart to check
desired variable refresh state and capability before the CRTC
disable pass.

(3) Performing VRR stream updates on-flip is needed for enabling BTR
support.

FreeSync state on the stream is now tracked and compared to the previous
packet sent to the hardware.

It's worth noting that the current implementation treats
variable_refresh_enabled as a strict requirement for sending
the VRR enable *or* disable packet.

Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 232 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   6 +-
 2 files changed, 121 insertions(+), 117 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 6c849e055038..0fddc2e66f49 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1683,72 +1683,6 @@ static void dm_bandwidth_update(struct amdgpu_device 
*adev)
/* TODO: implement later */
 }
 
-static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
-   struct drm_file *filp)
-{
-   struct drm_atomic_state *state;
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_crtc *crtc;
-   struct drm_connector *connector;
-   struct drm_connector_state *old_con_state, *new_con_state;
-   int ret = 0;
-   uint8_t i;
-   bool enable = false;
-
-   drm_modeset_acquire_init(, 0);
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   state->acquire_ctx = 
-
-retry:
-   drm_for_each_crtc(crtc, dev) {
-   ret = drm_atomic_add_affected_connectors(state, crtc);
-   if (ret)
-   goto fail;
-
-   /* TODO rework amdgpu_dm_commit_planes so we don't need this */
-   ret = drm_atomic_add_affected_planes(state, crtc);
-   if (ret)
-   goto fail;
-   }
-
-   for_each_oldnew_connector_in_state(state, connector, old_con_state, 
new_con_state, i) {
-   struct dm_connector_state *dm_new_con_state = 
to_dm_connector_state(new_con_state);
-   struct drm_crtc_state *new_crtc_state;
-   struct amdgpu_crtc *acrtc = 
to_amdgpu_crtc(dm_new_con_state->base.crtc);
-   struct dm_crtc_state *dm_new_crtc_state;
-
-   if (!acrtc) {
-   ASSERT(0);
-   continue;
-   }
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);
-   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
-   dm_new_crtc_state->freesync_enabled = enable;
-   }
-
-   ret = drm_atomic_commit(state);
-
-fail:
-   if (ret == -EDEADLK) {
-   drm_atomic_state_clear(state);
-   drm_modeset_backoff();
-   goto retry;
-   }
-
-   drm_atomic_state_put(state);
-
-out:
-   drm_modeset_drop_locks();
-   drm_modeset_acquire_fini();
-   return ret;
-}
 
 static const struct amdgpu_display_funcs dm_display_funcs = {
.bandwidth_update = dm_bandwidth_update, /* called unconditionally */
@@ -1762,7 +1696,6 @@ static const struct amdgpu_display_funcs dm_display_funcs 
= {
dm_crtc_get_scanoutpos,/* called unconditionally */
.add_encoder = NULL, /* VBIOS parsing. DAL does it. */
.add_connector = NULL, /* VBIOS parsing. DAL does it. */
-   .notify_freesync = amdgpu_notify_freesync,
 
 };
 
@@ -2645,7 +2578,7 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
 
update_stream_signal(stream);
 
-   if (dm_state && dm_state->freesync_capable)
+   if (dm_state && dm_state->base.variable_refresh_capable)
stream->ignore_msa_timing_param = true;
 finish:
if (sink && sink->sink_signal == SIGNAL_TYPE_VIRTUAL)
@@ -2713,9 +2646,10

[PATCH v2 2/3] drm: Add variable refresh property to DRM CRTC

2018-09-24 Thread Nicholas Kazlauskas 
Variable refresh rate algorithms have typically been enabled only
when the display is covered by a single source of content.

This patch introduces a new default CRTC property that helps
hint to the driver when the CRTC composition is suitable for variable
refresh rate algorithms. Userspace can set this property dynamically
as the composition changes.

Whether the variable refresh rate algorithms are active will still
depend on the CRTC being suitable and the connector being capable
and enabled by the user for variable refresh rate support.

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

Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/drm_atomic_helper.c |  1 +
 drivers/gpu/drm/drm_atomic_uapi.c   |  6 ++
 drivers/gpu/drm/drm_crtc.c  |  2 ++
 drivers/gpu/drm/drm_mode_config.c   |  6 ++
 include/drm/drm_crtc.h  | 13 +
 include/drm/drm_mode_config.h   |  8 
 6 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 3cf1aa132778..b768d397a811 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3473,6 +3473,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct 
drm_crtc *crtc,
state->planes_changed = false;
state->connectors_changed = false;
state->color_mgmt_changed = false;
+   state->variable_refresh_changed = false;
state->zpos_changed = false;
state->commit = NULL;
state->event = NULL;
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index 0bb27a24a55c..32a4cf8a01c3 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -433,6 +433,10 @@ 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_variable_refresh) {
+   if (state->variable_refresh != val)
+   state->variable_refresh_changed = true;
+   state->variable_refresh = val;
} else if (property == config->degamma_lut_property) {
ret = drm_atomic_replace_property_blob_from_id(dev,
>degamma_lut,
@@ -491,6 +495,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_variable_refresh)
+   *val = state->variable_refresh;
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..ca33d6fb90ac 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_variable_refresh, 0);
}
 
return 0;
diff --git a/drivers/gpu/drm/drm_mode_config.c 
b/drivers/gpu/drm/drm_mode_config.c
index ee80788f2c40..1287c161d65d 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,
+   "VARIABLE_REFRESH");
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.prop_variable_refresh = 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..32b77f18ce6d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -168,6 +168,12 @@ struct drm_crtc_state {
 * drivers to steer the atomic commit control flow.
 */
bool color_mgmt_changed : 1;
+   /**
+* @variable_refresh_changed: Variable refresh support has changed
+* on the CRTC. Used by the atomic helpers and drivers to steer the
+*

[PATCH v2 1/3] drm: Add variable refresh rate properties to connector

2018-09-24 Thread Nicholas Kazlauskas 
Modern display hardware is capable of supporting variable refresh rates
and adaptive sync technologies. The properties for querying and
controlling these features should be exposed on the DRM connector.

This patch introduces two new properties for variable refresh rate
support:

- variable_refresh_capable
- variable_refresh_enabled

These are optional properties that can be added to a DRM connector
dynamically by using drm_connector_attach_variable_refresh_properties.

DRM drivers should set variable_refresh_capable as applicable for
their hardware. The property variable_refresh_enabled is a userspace
controlled option.

Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/drm_atomic_uapi.c |  6 ++
 drivers/gpu/drm/drm_connector.c   | 35 +++
 include/drm/drm_connector.h   | 27 
 3 files changed, 68 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c 
b/drivers/gpu/drm/drm_atomic_uapi.c
index d5b7f315098c..0bb27a24a55c 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -723,6 +723,8 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
state->content_type = val;
} else if (property == connector->scaling_mode_property) {
state->scaling_mode = val;
+   } else if (property == connector->variable_refresh_enabled_property) {
+   state->variable_refresh_enabled = val;
} else if (property == connector->content_protection_property) {
if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
DRM_DEBUG_KMS("only drivers can set CP Enabled\n");
@@ -797,6 +799,10 @@ drm_atomic_connector_get_property(struct drm_connector 
*connector,
*val = state->content_type;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
+   } else if (property == connector->variable_refresh_capable_property) {
+   *val = state->variable_refresh_capable;
+   } else if (property == connector->variable_refresh_enabled_property) {
+   *val = state->variable_refresh_enabled;
} else if (property == connector->content_protection_property) {
*val = state->content_protection;
} else if (property == config->writeback_fb_id_property) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 1e40e5decbe9..fc1732639bd3 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1254,6 +1254,41 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * drm_connector_attach_variable_refresh_properties - creates and attaches
+ * properties for connectors that support adaptive refresh
+ * @connector: connector to create adaptive refresh properties on
+ */
+int drm_connector_attach_variable_refresh_properties(
+   struct drm_connector *connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->variable_refresh_capable_property) {
+   prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE,
+   "variable_refresh_capable");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->variable_refresh_capable_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   if (!connector->variable_refresh_enabled_property) {
+   prop = drm_property_create_bool(dev, 0,
+   "variable_refresh_enabled");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->variable_refresh_enabled_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_variable_refresh_properties);
+
 /**
  * drm_connector_attach_scaling_mode_property - attach atomic scaling mode 
property
  * @connector: connector to attach scaling mode property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 91a877fa00cb..e2c26842bd50 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -449,6 +449,18 @@ struct drm_connector_state {
 */
unsigned int content_protection;
 
+   /**
+* @variable_refresh_enabled: Connector property used to check
+* if variable refresh is supported on the device.
+*/
+   bool variable_refresh_capable;
+
+   /**
+* @variable_refresh_enabled: Connector property used to check
+* if variable refresh is enabled.
+*/
+   bool variable_refresh_enabled;
+
/**
 * @writeback_job: Writeback job for writeback connec

[PATCH v2 0/3] A DRM API for adaptive sync and variable refresh rate support

2018-09-24 Thread Nicholas Kazlauskas 
These patches are part of a proposed new interface for supporting variable 
refresh rate via DRM properties.

=== Changes from v1 ===

For drm:

* The variable_refresh_capable property is now flagged as 
DRM_MODE_PROP_IMMUTABLE

For drm/gpu/amd/display:

* Patches no longer pull in IOCTL/FreeSync refactoring code
* FreeSync enable/disable behavior has been modified to reflect changes in 
userspace behavior from xf86-video-amdgpu and mesa

=== Adaptive sync and variable refresh rate ===

Adaptive sync is part of the DisplayPort spec and allows for graphics adapters 
to drive displays with varying frame timings.

Variable refresh rate (VRR) is essentially the same, but defined for HDMI.

=== Use cases for variable refresh rate ===

Variable frame (flip) timings don't align well with fixed refresh rate 
displays. This results in stuttering, tearing and/or input lag. By adjusting 
the display refresh rate dynamically these issues can be reduced or eliminated.

However, not all content is suitable for dynamic refresh adaptation. Content 
that is flipped infrequently or at random intervals tends to fair poorly. 
Multiple clients trying to flip under the same screen can similarly interfere 
with prediction.

Userland needs a way to let the driver know when the content on the screen is 
suitable for variable refresh rate and if the user wishes to have the feature 
enabled.

=== DRM API to support variable refresh rates ===

This patch introduces a new API via atomic properties on the DRM connector and 
CRTC.

The connector has two new optional properties:

* bool variable_refresh_capable - set by the driver if the hardware is capable 
of supporting variable refresh tech

* bool variable_refresh_enabled - set by the user to enable variable refresh 
adjustment over the connector

The CRTC has one additional default property:

* bool variable_refresh - a content hint to the driver specifying that the CRTC 
contents are suitable for variable refresh adjustment

== Overview for DRM driver developers ===

Driver developers can attach the optional connector properties via 
drm_connector_attach_variable_refresh_properties on connectors that support 
variable refresh (typically DP or HDMI).

The variable_refresh_capable property should be managed as the output on the 
connector changes. The property is read only from userspace.

The variable_refresh_enabled property is intended to be a property controlled 
by userland as a global on/off switch for variable refresh technology. It 
should be checked before enabling variable refresh rate.

=== Overview for Userland developers ==

The variable_refresh property on the CRTC should be set to true when the CRTCs 
are suitable for variable refresh rate. In practice this is probably an 
application like a game - a single window that covers the whole CRTC surface 
and is the only client issuing flips.

To demonstrate the suitability of the API for variable refresh and dynamic 
adaptation there are additional patches using this API that implement adaptive 
variable refresh across kernel and userland projects:

* DRM (dri-devel)
* amdgpu DRM kernel driver (amd-gfx)
* xf86-video-amdgpu (amd-gfx)
* mesa (mesa-dev)

These patches enable adaptive variable refresh on X for AMD hardware provided 
that the user sets the variable_refresh_enabled property to true on supported 
connectors (ie. using xrandr --set-prop).

The patches have been tested as working on upstream userland with the GNOME 
desktop environment under a single monitor setup. They also work on KDE in 
single monitor setup if the compositor is disabled.

The patches require that the application window can issue screen flips via the 
Present extension to xf86-video-amdgpu. Due to Present extension limitations 
some desktop environments and multi-monitor setups are currently not compatible.

Full implementation details for these changes can be reviewed in their 
respective mailing lists.

=== Previous discussions ===

These patches are based upon feedback from patches and feedback from two 
previous threads on the subject which are linked below for reference:

https://lists.freedesktop.org/archives/amd-gfx/2018-April/021047.html
https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html
https://lists.freedesktop.org/archives/dri-devel/2018-September/189404.html

Nicholas Kazlauskas

Nicholas Kazlauskas (3):
  drm: Add variable refresh rate properties to connector
  drm: Add variable refresh property to DRM CRTC
  drm/amd/display: Set FreeSync state using DRM VRR properties

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 232 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   6 +-
 drivers/gpu/drm/drm_atomic_helper.c   |   1 +
 drivers/gpu/drm/drm_atomic_uapi.c |  12 +
 drivers/gpu/drm/drm_connector.c   |  35 +++
 drivers/gpu/drm/drm_crtc.c|   2 +
 drivers/gpu/drm/drm_mode_config.c |   6 +
 include/drm/drm_connector.h

[PATCH 9/9] drm/amdgpu: Drop unneeded freesync properties from amdpgu

2018-09-11 Thread Nicholas Kazlauskas 
With the introduction of new properties in DRM these amdgpu driver
specific ones are no longer necessary.

Change-Id: Idc88f2e3e036aacc8fe726b15db03d900e509e7c
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 12 
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h|  4 
 2 files changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 7d6a36bca9dd..4f2928e91f9c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -626,18 +626,6 @@ int amdgpu_display_modeset_create_props(struct 
amdgpu_device *adev)
 "dither",
 amdgpu_dither_enum_list, sz);
 
-   if (amdgpu_device_has_dc_support(adev)) {
-   adev->mode_info.freesync_property =
-   drm_property_create_bool(adev->ddev, 0, "freesync");
-   if (!adev->mode_info.freesync_property)
-   return -ENOMEM;
-   adev->mode_info.freesync_capable_property =
-   drm_property_create_bool(adev->ddev,
-0,
-"freesync_capable");
-   if (!adev->mode_info.freesync_capable_property)
-   return -ENOMEM;
-   }
 
return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index f91a9bdcd63c..b9e9e8b02fb7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -339,10 +339,6 @@ struct amdgpu_mode_info {
struct drm_property *audio_property;
/* FMT dithering */
struct drm_property *dither_property;
-   /* it is used to allow enablement of freesync mode */
-   struct drm_property *freesync_property;
-   /* it is used to know about display capability of freesync mode */
-   struct drm_property *freesync_capable_property;
/* hardcoded DFP edid from BIOS */
struct edid *bios_hardcoded_edid;
int bios_hardcoded_edid_size;
-- 
2.17.1

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

[PATCH 8/9] drm/amd/display: Drop FreeSync ioctl notification support

2018-09-11 Thread Nicholas Kazlauskas 
This is no longer needed with the addition of the DRM properties.

The base driver correctly checks that notify_freesync is non-null before
calling so there shouldn't be any null pointer dereferences as a result
of this.

Change-Id: If0833b201c81303ca4062393e873faf3ef7c143b
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 71 ---
 1 file changed, 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index d28bab0f4657..c9de8f555c6a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1581,76 +1581,6 @@ static void dm_bandwidth_update(struct amdgpu_device 
*adev)
/* TODO: implement later */
 }
 
-static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
-   struct drm_file *filp)
-{
-   struct drm_amdgpu_freesync *args = data;
-   struct drm_atomic_state *state;
-   struct drm_modeset_acquire_ctx ctx;
-   struct drm_crtc *crtc;
-   struct drm_connector *connector;
-   struct drm_connector_state *old_con_state, *new_con_state;
-   int ret = 0;
-   uint8_t i;
-   bool enable = false;
-
-   if (args->op == AMDGPU_FREESYNC_FULLSCREEN_ENTER)
-   enable = true;
-
-   drm_modeset_acquire_init(, 0);
-
-   state = drm_atomic_state_alloc(dev);
-   if (!state) {
-   ret = -ENOMEM;
-   goto out;
-   }
-   state->acquire_ctx = 
-
-retry:
-   drm_for_each_crtc(crtc, dev) {
-   ret = drm_atomic_add_affected_connectors(state, crtc);
-   if (ret)
-   goto fail;
-
-   /* TODO rework amdgpu_dm_commit_planes so we don't need this */
-   ret = drm_atomic_add_affected_planes(state, crtc);
-   if (ret)
-   goto fail;
-   }
-
-   for_each_oldnew_connector_in_state(state, connector, old_con_state, 
new_con_state, i) {
-   struct dm_connector_state *dm_new_con_state = 
to_dm_connector_state(new_con_state);
-   struct drm_crtc_state *new_crtc_state;
-   struct amdgpu_crtc *acrtc = 
to_amdgpu_crtc(dm_new_con_state->base.crtc);
-   struct dm_crtc_state *dm_new_crtc_state;
-
-   if (!acrtc) {
-   ASSERT(0);
-   continue;
-   }
-
-   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);
-   dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
-   dm_new_crtc_state->base.variable_refresh = enable;
-   }
-
-   ret = drm_atomic_commit(state);
-
-fail:
-   if (ret == -EDEADLK) {
-   drm_atomic_state_clear(state);
-   drm_modeset_backoff();
-   goto retry;
-   }
-
-   drm_atomic_state_put(state);
-
-out:
-   drm_modeset_drop_locks();
-   drm_modeset_acquire_fini();
-   return ret;
-}
 
 static const struct amdgpu_display_funcs dm_display_funcs = {
.bandwidth_update = dm_bandwidth_update, /* called unconditionally */
@@ -1664,7 +1594,6 @@ static const struct amdgpu_display_funcs dm_display_funcs 
= {
dm_crtc_get_scanoutpos,/* called unconditionally */
.add_encoder = NULL, /* VBIOS parsing. DAL does it. */
.add_connector = NULL, /* VBIOS parsing. DAL does it. */
-   .notify_freesync = amdgpu_notify_freesync,
 
 };
 
-- 
2.17.1

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

[PATCH 7/9] drm/amd/display: Replace FreeSync props with DRM VRR props

2018-09-11 Thread Nicholas Kazlauskas 
DRM has built-in support for variable refresh properties on the
connector and CRTC. Make use of these instead of the amdpgu specific
freesync properties.

The connector properties freesync and freesync_capable are replaced with
variable_refresh_enabled and variable_refresh_capable.

The CRTC property freesync_enable is replaced with the variable_refresh
property.

The old FreeSync properties are no longer accessible from userspace and
the DRM properties should be instead for enabling/disabling variable
refresh support.

Change-Id: I7c8117c09282a938c87292402af39d22e4eb823b
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 --
 2 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 56598ed53123..d28bab0f4657 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1632,7 +1632,7 @@ static int amdgpu_notify_freesync(struct drm_device *dev, 
void *data,
new_crtc_state = drm_atomic_get_new_crtc_state(state, 
>base);
dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
 
-   dm_new_crtc_state->freesync_enabled = enable;
+   dm_new_crtc_state->base.variable_refresh = enable;
}
 
ret = drm_atomic_commit(state);
@@ -2541,7 +2541,7 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
 
update_stream_signal(stream);
 
-   if (dm_state && dm_state->freesync_capable)
+   if (dm_state && dm_state->base.variable_refresh_capable)
stream->ignore_msa_timing_param = true;
 finish:
if (sink && sink->sink_signal == SIGNAL_TYPE_VIRTUAL)
@@ -2611,7 +2611,6 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
 
state->adjust = cur->adjust;
state->vrr_infopacket = cur->vrr_infopacket;
-   state->freesync_enabled = cur->freesync_enabled;
 
/* TODO Duplicate dc_stream after objects are stream object is 
flattened */
 
@@ -2722,12 +2721,6 @@ int amdgpu_dm_connector_atomic_set_property(struct 
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
dm_new_state->underscan_enable = val;
ret = 0;
-   } else if (property == adev->mode_info.freesync_property) {
-   dm_new_state->freesync_enable = val;
-   ret = 0;
-   } else if (property == adev->mode_info.freesync_capable_property) {
-   dm_new_state->freesync_capable = val;
-   ret = 0;
}
 
return ret;
@@ -2770,12 +2763,6 @@ int amdgpu_dm_connector_atomic_get_property(struct 
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
*val = dm_state->underscan_enable;
ret = 0;
-   } else if (property == adev->mode_info.freesync_property) {
-   *val = dm_state->freesync_enable;
-   ret = 0;
-   } else if (property == adev->mode_info.freesync_capable_property) {
-   *val = dm_state->freesync_capable;
-   ret = 0;
}
return ret;
 }
@@ -2839,9 +2826,6 @@ amdgpu_dm_connector_atomic_duplicate_state(struct 
drm_connector *connector)
 
__drm_atomic_helper_connector_duplicate_state(connector, 
_state->base);
 
-   new_state->freesync_capable = state->freesync_capable;
-   new_state->freesync_enable = state->freesync_enable;
-
return _state->base;
 }
 
@@ -3602,10 +3586,8 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
 
if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
-   drm_object_attach_property(>base.base,
-   adev->mode_info.freesync_property, 0);
-   drm_object_attach_property(>base.base,
-   adev->mode_info.freesync_capable_property, 0);
+   drm_connector_attach_variable_refresh_properties(
+   >base);
}
 }
 
@@ -4123,7 +4105,8 @@ static bool commit_planes_to_stream(
stream_update->dst = dc_stream->dst;
stream_update->out_transfer_func = dc_stream->out_transfer_func;
 
-   if (dm_new_crtc_state->freesync_enabled != 
dm_old_crtc_state->freesync_enabled) {
+   if (dm_new_crtc_state->base.variable_refresh !=
+   dm_old_crtc_state->base.variable_refresh) {
stream_update->vrr_infopacket = _stream->vrr_infopacket;
stream_update->adjust = _stream->adjust;
}
@@ -4695,9 +4678,9 @@ void set_freesync_on_

[PATCH 6/9] drm/amdgpu: add freesync ioctl

2018-09-11 Thread Nicholas Kazlauskas 
From: Harry Wentland 

Add the ioctl to enable/disable freesync.

Signed-off-by: Harry Wentland 
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  3 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c  | 15 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c  |  3 ++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  4 
 include/uapi/drm/amdgpu_drm.h| 16 
 5 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 447c4c7a36d6..95af917007f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1193,6 +1193,9 @@ int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, 
void *data,
 int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp);
 
+int amdgpu_display_freesync_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp);
+
 /* VRAM scratch page for HDP bug, default vram page */
 struct amdgpu_vram_scratch {
struct amdgpu_bo*robj;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 642b47c5f4b8..7d6a36bca9dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -894,3 +894,18 @@ int amdgpu_display_crtc_idx_to_irq_type(struct 
amdgpu_device *adev, int crtc)
return AMDGPU_CRTC_IRQ_NONE;
}
 }
+
+int amdgpu_display_freesync_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp)
+{
+   int ret = -EPERM;
+   struct amdgpu_device *adev = dev->dev_private;
+
+   if (adev->mode_info.funcs->notify_freesync)
+   ret = adev->mode_info.funcs->notify_freesync(dev,data,filp);
+   else
+   DRM_DEBUG("amdgpu no notify_freesync ioctl\n");
+
+   return ret;
+}
+
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index bd98cc5fb97b..5b26e0447221 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1099,7 +1099,8 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_METADATA, amdgpu_gem_metadata_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_VA, amdgpu_gem_va_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
-   DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW)
+   DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(AMDGPU_FREESYNC, amdgpu_display_freesync_ioctl, 
DRM_MASTER)
 };
 const int amdgpu_max_kms_ioctl = ARRAY_SIZE(amdgpu_ioctls_kms);
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 8be3028850b6..56598ed53123 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1584,6 +1584,7 @@ static void dm_bandwidth_update(struct amdgpu_device 
*adev)
 static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
struct drm_file *filp)
 {
+   struct drm_amdgpu_freesync *args = data;
struct drm_atomic_state *state;
struct drm_modeset_acquire_ctx ctx;
struct drm_crtc *crtc;
@@ -1593,6 +1594,9 @@ static int amdgpu_notify_freesync(struct drm_device *dev, 
void *data,
uint8_t i;
bool enable = false;
 
+   if (args->op == AMDGPU_FREESYNC_FULLSCREEN_ENTER)
+   enable = true;
+
drm_modeset_acquire_init(, 0);
 
state = drm_atomic_state_alloc(dev);
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 1ceec56de015..9eeba55b 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -54,6 +54,8 @@ extern "C" {
 #define DRM_AMDGPU_VM  0x13
 #define DRM_AMDGPU_FENCE_TO_HANDLE 0x14
 #define DRM_AMDGPU_SCHED   0x15
+/* not upstream */
+#define DRM_AMDGPU_FREESYNC0x5d
 
 #define DRM_IOCTL_AMDGPU_GEM_CREATEDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_CREATE, union drm_amdgpu_gem_create)
 #define DRM_IOCTL_AMDGPU_GEM_MMAP  DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_MMAP, union drm_amdgpu_gem_mmap)
@@ -71,6 +73,7 @@ extern "C" {
 #define DRM_IOCTL_AMDGPU_VMDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_VM, union drm_amdgpu_vm)
 #define DRM_IOCTL_AMDGPU_FENCE_TO_HANDLE DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_FENCE_TO_HANDLE, union drm_amdgpu_fence_to_handle)
 #define DRM_IOCTL_AMDGPU_SCHED DRM_IOW(DRM_COMMAND_BASE + 
DRM_AMDGPU_SCHED, union drm_amdgpu_sched)

[PATCH 5/9] drm/amdgpu/display: add freesync drm properties

2018-09-11 Thread Nicholas Kazlauskas 
From: Harry Wentland 

Add connector properties for controlling freesync.

Signed-off-by: Harry Wentland 
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   | 13 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  4 
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 +++
 3 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 6748cd7fc129..642b47c5f4b8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -626,6 +626,19 @@ int amdgpu_display_modeset_create_props(struct 
amdgpu_device *adev)
 "dither",
 amdgpu_dither_enum_list, sz);
 
+   if (amdgpu_device_has_dc_support(adev)) {
+   adev->mode_info.freesync_property =
+   drm_property_create_bool(adev->ddev, 0, "freesync");
+   if (!adev->mode_info.freesync_property)
+   return -ENOMEM;
+   adev->mode_info.freesync_capable_property =
+   drm_property_create_bool(adev->ddev,
+0,
+"freesync_capable");
+   if (!adev->mode_info.freesync_capable_property)
+   return -ENOMEM;
+   }
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index b9e9e8b02fb7..f91a9bdcd63c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -339,6 +339,10 @@ struct amdgpu_mode_info {
struct drm_property *audio_property;
/* FMT dithering */
struct drm_property *dither_property;
+   /* it is used to allow enablement of freesync mode */
+   struct drm_property *freesync_property;
+   /* it is used to know about display capability of freesync mode */
+   struct drm_property *freesync_capable_property;
/* hardcoded DFP edid from BIOS */
struct edid *bios_hardcoded_edid;
int bios_hardcoded_edid_size;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index d599fbfa895b..8be3028850b6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2718,6 +2718,12 @@ int amdgpu_dm_connector_atomic_set_property(struct 
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
dm_new_state->underscan_enable = val;
ret = 0;
+   } else if (property == adev->mode_info.freesync_property) {
+   dm_new_state->freesync_enable = val;
+   ret = 0;
+   } else if (property == adev->mode_info.freesync_capable_property) {
+   dm_new_state->freesync_capable = val;
+   ret = 0;
}
 
return ret;
@@ -2760,6 +2766,12 @@ int amdgpu_dm_connector_atomic_get_property(struct 
drm_connector *connector,
} else if (property == adev->mode_info.underscan_property) {
*val = dm_state->underscan_enable;
ret = 0;
+   } else if (property == adev->mode_info.freesync_property) {
+   *val = dm_state->freesync_enable;
+   ret = 0;
+   } else if (property == adev->mode_info.freesync_capable_property) {
+   *val = dm_state->freesync_capable;
+   ret = 0;
}
return ret;
 }
@@ -3584,6 +3596,13 @@ void amdgpu_dm_connector_init_helper(struct 
amdgpu_display_manager *dm,
adev->mode_info.underscan_vborder_property,
0);
 
+   if (connector_type == DRM_MODE_CONNECTOR_HDMIA ||
+   connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
+   drm_object_attach_property(>base.base,
+   adev->mode_info.freesync_property, 0);
+   drm_object_attach_property(>base.base,
+   adev->mode_info.freesync_capable_property, 0);
+   }
 }
 
 static int amdgpu_dm_i2c_xfer(struct i2c_adapter *i2c_adap,
-- 
2.17.1

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

[PATCH 3/9] drm/amdgpu: fill in amdgpu_dm_remove_sink_from_freesync_module

2018-09-11 Thread Nicholas Kazlauskas 
From: Harry Wentland 

Add code to tear down freesync modules when disabled.

Signed-off-by: Harry Wentland 
Signed-off-by: Alex Deucher 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 40 ++-
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index af6adffba788..9dad505d132f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5230,19 +5230,37 @@ void amdgpu_dm_add_sink_to_freesync_module(struct 
drm_connector *connector,
dm_con_state->freesync_capable = true;
}
}
-
-   /*
-* TODO figure out how to notify user-mode or DRM of freesync caps
-* once we figure out how to deal with freesync in an upstreamable
-* fashion
-*/
-
 }
 
 void amdgpu_dm_remove_sink_from_freesync_module(struct drm_connector 
*connector)
 {
-   /*
-* TODO fill in once we figure out how to deal with freesync in
-* an upstreamable fashion
-*/
+   struct amdgpu_dm_connector *amdgpu_dm_connector =
+   to_amdgpu_dm_connector(connector);
+   struct dm_connector_state *dm_con_state;
+   struct drm_device *dev = connector->dev;
+   struct amdgpu_device *adev = dev->dev_private;
+
+   if (!amdgpu_dm_connector->dc_sink || !adev->dm.freesync_module) {
+   DRM_ERROR("dc_sink NULL or no free_sync module.\n");
+   return;
+   }
+
+   if (!connector->state) {
+   DRM_ERROR("%s - Connector has no state", __func__);
+   return;
+   }
+
+   dm_con_state = to_dm_connector_state(connector->state);
+
+   amdgpu_dm_connector->min_vfreq = 0;
+   amdgpu_dm_connector->max_vfreq = 0;
+   amdgpu_dm_connector->pixel_clock_mhz = 0;
+
+   memset(_dm_connector->caps, 0, 
sizeof(amdgpu_dm_connector->caps));
+
+   dm_con_state->freesync_capable = false;
+
+   dm_con_state->user_enable.enable_for_gaming = false;
+   dm_con_state->user_enable.enable_for_static = false;
+   dm_con_state->user_enable.enable_for_video = false;
 }
-- 
2.17.1

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

[PATCH 2/9] drm: Add variable refresh property to DRM CRTC

2018-09-11 Thread Nicholas Kazlauskas 
Variable refresh rate algorithms have typically been enabled only
when the display is covered by a single source of content.

This patch introduces a new default CRTC property that helps
hint to the driver when the CRTC composition is suitable for variable
refresh rate algorithms. Userspace can set this property dynamically
as the composition changes.

Whether the variable refresh rate algorithms are active will still
depend on the CRTC being suitable and the connector being capable
and enabled by the user for variable refresh rate support.

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

Change-Id: I5a5044f48fc68fcdbcfaa5141e83b44747d7116b
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/drm_atomic.c|  6 ++
 drivers/gpu/drm/drm_atomic_helper.c |  1 +
 drivers/gpu/drm/drm_crtc.c  |  2 ++
 drivers/gpu/drm/drm_mode_config.c   |  6 ++
 include/drm/drm_crtc.h  | 13 +
 include/drm/drm_mode_config.h   |  8 
 6 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 2f89ab0fac87..46c50c1f267b 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -553,6 +553,10 @@ 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_variable_refresh) {
+   if (state->variable_refresh != val)
+   state->variable_refresh_changed = true;
+   state->variable_refresh = val;
} else if (property == config->degamma_lut_property) {
ret = drm_atomic_replace_property_blob_from_id(dev,
>degamma_lut,
@@ -627,6 +631,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_variable_refresh)
+   *val = state->variable_refresh;
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_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 2c23a48482da..dd01ab041eff 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3472,6 +3472,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct 
drm_crtc *crtc,
state->planes_changed = false;
state->connectors_changed = false;
state->color_mgmt_changed = false;
+   state->variable_refresh_changed = false;
state->zpos_changed = false;
state->commit = NULL;
state->event = NULL;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index bae43938c8f6..f18b60ce7599 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -337,6 +337,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_variable_refresh, 0);
}
 
return 0;
diff --git a/drivers/gpu/drm/drm_mode_config.c 
b/drivers/gpu/drm/drm_mode_config.c
index 21e353bd3948..5847ac1e88c1 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -311,6 +311,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,
+   "VARIABLE_REFRESH");
+   if (!prop)
+   return -ENOMEM;
+   dev->mode_config.prop_variable_refresh = 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..32b77f18ce6d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -168,6 +168,12 @@ struct drm_crtc_state {
 * drivers to steer the atomic commit control flow.
 */
bool color_mgmt_changed : 1;
+   /**
+* @variable_refresh_changed: Variable refresh support has changed
+* on the CRTC. Used by the atomic helper

[PATCH 1/9] drm: Add variable refresh rate properties to DRM connector

2018-09-11 Thread Nicholas Kazlauskas 
Modern monitor hardware is capable of supporting variable refresh rates
and adaptive sync technologies. The properties for querying and
controlling these features should be exposed on the DRM connector.

This patch introduces two new properties for variable refresh rate
support:

- variable_refresh_capable
- variable_refresh_enabled

These are optional properties that can be added to a DRM connector
dynamically by using drm_connector_attach_variable_refresh_properties.

DRM drivers should set variable_refresh_capable as applicable for
their hardware. The property variable_refresh_enabled as a userspace
controlled option.

Change-Id: I5f60f8b57534e1d3dacda4c64c6c9106b42f4439
Signed-off-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/drm_atomic.c|  9 +
 drivers/gpu/drm/drm_connector.c | 35 +
 include/drm/drm_connector.h | 27 +
 3 files changed, 71 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index d0478abc01bd..2f89ab0fac87 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1403,6 +1403,11 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
state->content_type = val;
} else if (property == connector->scaling_mode_property) {
state->scaling_mode = val;
+   } else if (property == connector->variable_refresh_capable_property) {
+   DRM_DEBUG_KMS("only drivers can set 
variable_refresh_capable\n");
+   return -EINVAL;
+   } else if (property == connector->variable_refresh_enabled_property) {
+   state->variable_refresh_enabled = val;
} else if (property == connector->content_protection_property) {
if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
DRM_DEBUG_KMS("only drivers can set CP Enabled\n");
@@ -1508,6 +1513,10 @@ drm_atomic_connector_get_property(struct drm_connector 
*connector,
*val = state->content_type;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
+   } else if (property == connector->variable_refresh_capable_property) {
+   *val = state->variable_refresh_capable;
+   } else if (property == connector->variable_refresh_enabled_property) {
+   *val = state->variable_refresh_enabled;
} else if (property == connector->content_protection_property) {
*val = state->content_protection;
} else if (property == config->writeback_fb_id_property) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 6011d769d50b..37fb33fa77b9 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1250,6 +1250,41 @@ int drm_mode_create_scaling_mode_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
 
+/**
+ * drm_connector_attach_variable_refresh_properties - creates and attaches
+ * properties for connectors that support adaptive refresh
+ * @connector: connector to create adaptive refresh properties on
+ */
+int drm_connector_attach_variable_refresh_properties(
+   struct drm_connector *connector)
+{
+   struct drm_device *dev = connector->dev;
+   struct drm_property *prop;
+
+   if (!connector->variable_refresh_capable_property) {
+   prop = drm_property_create_bool(dev, 0,
+   "variable_refresh_capable");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->variable_refresh_capable_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   if (!connector->variable_refresh_enabled_property) {
+   prop = drm_property_create_bool(dev, 0,
+   "variable_refresh_enabled");
+   if (!prop)
+   return -ENOMEM;
+
+   connector->variable_refresh_enabled_property = prop;
+   drm_object_attach_property(>base, prop, 0);
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_variable_refresh_properties);
+
 /**
  * drm_connector_attach_scaling_mode_property - attach atomic scaling mode 
property
  * @connector: connector to attach scaling mode property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 97ea41dc678f..105a127e9191 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -448,6 +448,18 @@ struct drm_connector_state {
 */
unsigned int content_protection;
 
+   /**
+* @variable_refresh_enabled: Connector property used to check
+* if variable refresh is supported on the device.
+*/
+   bool variable_refresh_capable;
+
+   /**

[PATCH 0/9] A DRM API for adaptive sync and variable refresh rate support

2018-09-11 Thread Nicholas Kazlauskas 
=== Adaptive sync and variable refresh rate ===

Adaptive sync is part of the DisplayPort spec and allows for graphics adapters 
to drive displays with varying frame timings.

Variable refresh rate (VRR) is essentially the same, but defined for HDMI.

=== Use cases for variable refresh rate ===

Variable frame (flip) timings don't align well with fixed refresh rate 
displays. This results in stuttering, tearing and/or input lag. By adjusting 
the display refresh rate dynamically these issues can be reduced or eliminated.

However, not all content is suitable for dynamic refresh adaptation. Content 
that is flipped infrequently or at random intervals tends to fair poorly. 
Multiple clients trying to flip under the same screen can similarly interfere 
with prediction.

Userland needs a way to let the driver know when the content on the screen is 
suitable for variable refresh rate and if the user wishes to have the feature 
enabled.

=== DRM API to support variable refresh rates ===

This patch introduces a new API via atomic properties on the DRM connector and 
CRTC.

The connector has two new optional properties:

* bool variable_refresh_capable - set by the driver if the hardware is capable 
of supporting variable refresh tech

* bool variable_refresh_enabled - set by the user to enable variable refresh 
adjustment over the connector

The CRTC has one additional default property:

* bool variable_refresh - a content hint to the driver specifying that the CRTC 
contents are suitable for variable refresh adjustment

== Overview for DRM driver developers ===

Driver developers can attach the optional connector properties via 
drm_connector_attach_variable_refresh_properties on connectors that support 
variable refresh (typically DP or HDMI).

The variable_refresh_capable property should be managed as the output on the 
connector changes. The property is read only from userspace.

The variable_refresh_enabled property is intended to be a property controlled 
by userland as a global on/off switch for variable refresh technology. It 
should be checked before enabling variable refresh rate.

=== Overview for Userland developers ==

The variable_refresh property on the CRTC should be set to true when the CRTCs 
are suitable for variable refresh rate. In practice this is probably an 
application like a game - a single window that covers the whole CRTC surface 
and is the only client issuing flips.

To demonstrate the suitability of the API for variable refresh and dynamic 
adaptation there are additional patches using this API that implement adaptive 
variable refresh across kernel and userland projects:

- DRM (dri-devel)
- amdgpu DRM kernel driver (amd-gfx)
- xf86-video-amdgpu (amd-gfx)
- mesa (mesa-dev)

These patches enable adaptive variable refresh on X for AMD hardware provided 
that the user sets the variable_refresh_enabled property to true on supported 
connectors (ie. using xrandr --set-prop).

They have been tested on upstream userland under GNOME/KDE desktop environments 
under single and multi-monitor setups for a number of GL applications. Most 
games and benchmarks should work as expected provided that the compositor 
correctly unredirects the application's surface. KDE seems to have the best 
support for this with an explicit option to disable tearing support.

Full implementation details for these changes can be reviewed in their 
respective mailing lists.

=== Previous discussions ===

These patches are based upon feedback from patches and feedback from two 
previous threads on the subject which are linked below for reference:

https://lists.freedesktop.org/archives/amd-gfx/2018-April/021047.html
https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html

Nicholas Kazlauskas

Anthony Koo (1):
  drm/amd/display: Refactor FreeSync module

Harry Wentland (3):
  drm/amdgpu: fill in amdgpu_dm_remove_sink_from_freesync_module
  drm/amdgpu/display: add freesync drm properties
  drm/amdgpu: add freesync ioctl

Nicholas Kazlauskas (5):
  drm: Add variable refresh rate properties to DRM connector
  drm: Add variable refresh property to DRM CRTC
  drm/amd/display: Replace FreeSync props with DRM VRR props
  drm/amd/display: Drop FreeSync ioctl notification support
  drm/amdgpu: Drop unneeded freesync properties from amdpgu

 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |   16 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |3 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  240 +--
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   17 +-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |   10 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c  |   60 +-
 .../gpu/drm/amd/display/dc/core/dc_link_dp.c  |3 +
 .../gpu/drm/amd/display/dc/core/dc_resource.c |  110 +-
 drivers/gpu/drm/amd/display/dc/dc_hw_types.h  |6 -
 drivers/gpu/drm/amd/display/dc/dc_stream.h|   29 +-
 drivers/gpu/drm/amd/display/dc/dc_types.h |   22