Re: [PATCH] drm/amd/display: Add below the range support for FreeSync

2019-01-03 Thread Wentland, Harry
On 2018-12-05 8:40 a.m., Nicholas Kazlauskas wrote:
> [Why]
> When application flip-rate is below the minimum vrr refresh rate.
> 
> Variable refresh rate monitors extend the front porch duration until
> flip or timeout occurs. For cases where the application flip-rate is
> 
> When the flip-rate is below the minimum supported variable refresh rate
> range for the monitor the front porch wait will timeout and be
> frequently misaligned resulting in stuttering and/or flickering.
> 
> The FreeSync module can still maintain a smooth and flicker free
> image when the monitor has a refresh rate range such that the maximum
> refresh > 2 * minimum refresh by utilizing low framerate compensation,
> "below the range".
> 
> [How]
> Hook up the pre-flip and post-flip handlers from the FreeSync module.
> These adjust the minimum/maximum vrr range to duplicate frames
> when appropriate by tracking flip timestamps.
> 
> Cc: Leo Li 
> Cc: Harry Wentland 
> Signed-off-by: Nicholas Kazlauskas 

Reviewed-by: Harry Wentland 

Harry

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 ++-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +-
>  2 files changed, 62 insertions(+), 19 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 23d61570df17..e2de064426fc 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -328,12 +328,29 @@ static void dm_crtc_high_irq(void *interrupt_params)
>   struct common_irq_params *irq_params = interrupt_params;
>   struct amdgpu_device *adev = irq_params->adev;
>   struct amdgpu_crtc *acrtc;
> + struct dm_crtc_state *acrtc_state;
>  
>   acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - 
> IRQ_TYPE_VBLANK);
>  
>   if (acrtc) {
>   drm_crtc_handle_vblank(>base);
>   amdgpu_dm_crtc_handle_crc_irq(>base);
> +
> + acrtc_state = to_dm_crtc_state(acrtc->base.state);
> +
> + if (acrtc_state->stream &&
> + acrtc_state->vrr_params.supported &&
> + acrtc_state->freesync_config.state == 
> VRR_STATE_ACTIVE_VARIABLE) {
> + mod_freesync_handle_v_update(
> + adev->dm.freesync_module,
> + acrtc_state->stream,
> + _state->vrr_params);
> +
> + dc_stream_adjust_vmin_vmax(
> + adev->dm.dc,
> + acrtc_state->stream,
> + _state->vrr_params.adjust);
> + }
>   }
>  }
>  
> @@ -3001,7 +3018,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
>   dc_stream_retain(state->stream);
>   }
>  
> - state->adjust = cur->adjust;
> + state->vrr_params = cur->vrr_params;
>   state->vrr_infopacket = cur->vrr_infopacket;
>   state->abm_level = cur->abm_level;
>   state->vrr_supported = cur->vrr_supported;
> @@ -4396,9 +4413,11 @@ struct dc_stream_status *dc_state_get_stream_status(
>  static void update_freesync_state_on_stream(
>   struct amdgpu_display_manager *dm,
>   struct dm_crtc_state *new_crtc_state,
> - struct dc_stream_state *new_stream)
> + struct dc_stream_state *new_stream,
> + struct dc_plane_state *surface,
> + u32 flip_timestamp_in_us)
>  {
> - struct mod_vrr_params vrr = {0};
> + struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
>   struct dc_info_packet vrr_infopacket = {0};
>   struct mod_freesync_config config = new_crtc_state->freesync_config;
>  
> @@ -4425,43 +,52 @@ static void update_freesync_state_on_stream(
>  
>   mod_freesync_build_vrr_params(dm->freesync_module,
> new_stream,
> -   , );
> +   , _params);
> +
> + if (surface) {
> + mod_freesync_handle_preflip(
> + dm->freesync_module,
> + surface,
> + new_stream,
> + flip_timestamp_in_us,
> + _params);
> + }
>  
>   mod_freesync_build_vrr_infopacket(
>   dm->freesync_module,
>   new_stream,
> - ,
> + _params,
>   PACKET_TYPE_VRR,
>   TRANSFER_FUNC_UNKNOWN,
>   _infopacket);
>  
>   new_crtc_state->freesync_timing_changed =
> - (memcmp(_crtc_state->adjust,
> - ,
> - sizeof(vrr.adjust)) != 0);
> + (memcmp(_crtc_state->vrr_params.adjust,
> + _params.adjust,
> + sizeof(vrr_params.adjust)) != 0);
>  
>   new_crtc_state->freesync_vrr_info_changed =
>   (memcmp(_crtc_state->vrr_infopacket,
>   

Re: [PATCH] drm/amd/display: Add below the range support for FreeSync

2018-12-05 Thread Nicholas Kazlauskas
On 2018-12-05 9:30 a.m., Li, Sun peng (Leo) wrote:
> 
> 
> On 2018-12-05 8:40 a.m., Nicholas Kazlauskas wrote:
>> [Why]
>> When application flip-rate is below the minimum vrr refresh rate.
>>
>> Variable refresh rate monitors extend the front porch duration until
>> flip or timeout occurs. For cases where the application flip-rate is
> 
> Did something get cut off here? With that fixed,
> Acked-by: Leo Li 

Those top two paragraphs were supposed to be cut, I think I formatted 
the wrong patch for that.

The code itself is the correct version, however. Thanks.

Nicholas Kazlauskas

> 
>>
>> When the flip-rate is below the minimum supported variable refresh rate
>> range for the monitor the front porch wait will timeout and be
>> frequently misaligned resulting in stuttering and/or flickering.
>>
>> The FreeSync module can still maintain a smooth and flicker free
>> image when the monitor has a refresh rate range such that the maximum
>> refresh > 2 * minimum refresh by utilizing low framerate compensation,
>> "below the range".
>>
>> [How]
>> Hook up the pre-flip and post-flip handlers from the FreeSync module.
>> These adjust the minimum/maximum vrr range to duplicate frames
>> when appropriate by tracking flip timestamps.
>>
>> Cc: Leo Li 
>> Cc: Harry Wentland 
>> Signed-off-by: Nicholas Kazlauskas 
>> ---
>>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 ++-
>>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +-
>>2 files changed, 62 insertions(+), 19 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 23d61570df17..e2de064426fc 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -328,12 +328,29 @@ static void dm_crtc_high_irq(void *interrupt_params)
>>  struct common_irq_params *irq_params = interrupt_params;
>>  struct amdgpu_device *adev = irq_params->adev;
>>  struct amdgpu_crtc *acrtc;
>> +struct dm_crtc_state *acrtc_state;
>>
>>  acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - 
>> IRQ_TYPE_VBLANK);
>>
>>  if (acrtc) {
>>  drm_crtc_handle_vblank(>base);
>>  amdgpu_dm_crtc_handle_crc_irq(>base);
>> +
>> +acrtc_state = to_dm_crtc_state(acrtc->base.state);
>> +
>> +if (acrtc_state->stream &&
>> +acrtc_state->vrr_params.supported &&
>> +acrtc_state->freesync_config.state == 
>> VRR_STATE_ACTIVE_VARIABLE) {
>> +mod_freesync_handle_v_update(
>> +adev->dm.freesync_module,
>> +acrtc_state->stream,
>> +_state->vrr_params);
>> +
>> +dc_stream_adjust_vmin_vmax(
>> +adev->dm.dc,
>> +acrtc_state->stream,
>> +_state->vrr_params.adjust);
>> +}
>>  }
>>}
>>
>> @@ -3001,7 +3018,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
>>  dc_stream_retain(state->stream);
>>  }
>>
>> -state->adjust = cur->adjust;
>> +state->vrr_params = cur->vrr_params;
>>  state->vrr_infopacket = cur->vrr_infopacket;
>>  state->abm_level = cur->abm_level;
>>  state->vrr_supported = cur->vrr_supported;
>> @@ -4396,9 +4413,11 @@ struct dc_stream_status *dc_state_get_stream_status(
>>static void update_freesync_state_on_stream(
>>  struct amdgpu_display_manager *dm,
>>  struct dm_crtc_state *new_crtc_state,
>> -struct dc_stream_state *new_stream)
>> +struct dc_stream_state *new_stream,
>> +struct dc_plane_state *surface,
>> +u32 flip_timestamp_in_us)
>>{
>> -struct mod_vrr_params vrr = {0};
>> +struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
>>  struct dc_info_packet vrr_infopacket = {0};
>>  struct mod_freesync_config config = new_crtc_state->freesync_config;
>>
>> @@ -4425,43 +,52 @@ static void update_freesync_state_on_stream(
>>
>>  mod_freesync_build_vrr_params(dm->freesync_module,
>>new_stream,
>> -  , );
>> +  , _params);
>> +
>> +if (surface) {
>> +mod_freesync_handle_preflip(
>> +dm->freesync_module,
>> +surface,
>> +new_stream,
>> +flip_timestamp_in_us,
>> +_params);
>> +}
>>
>>  mod_freesync_build_vrr_infopacket(
>>  dm->freesync_module,
>>  new_stream,
>> -,
>> +_params,
>>  PACKET_TYPE_VRR,
>>  TRANSFER_FUNC_UNKNOWN,
>>  _infopacket);
>>
>>  new_crtc_state->freesync_timing_changed =
>> -(memcmp(_crtc_state->adjust,
>> -  

[PATCH] drm/amd/display: Add below the range support for FreeSync

2018-12-05 Thread Nicholas Kazlauskas
[Why]
When application flip-rate is below the minimum vrr refresh rate.

Variable refresh rate monitors extend the front porch duration until
flip or timeout occurs. For cases where the application flip-rate is

When the flip-rate is below the minimum supported variable refresh rate
range for the monitor the front porch wait will timeout and be
frequently misaligned resulting in stuttering and/or flickering.

The FreeSync module can still maintain a smooth and flicker free
image when the monitor has a refresh rate range such that the maximum
refresh > 2 * minimum refresh by utilizing low framerate compensation,
"below the range".

[How]
Hook up the pre-flip and post-flip handlers from the FreeSync module.
These adjust the minimum/maximum vrr range to duplicate frames
when appropriate by tracking flip timestamps.

Cc: Leo Li 
Cc: Harry Wentland 
Signed-off-by: Nicholas Kazlauskas 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 79 ++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +-
 2 files changed, 62 insertions(+), 19 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 23d61570df17..e2de064426fc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -328,12 +328,29 @@ static void dm_crtc_high_irq(void *interrupt_params)
struct common_irq_params *irq_params = interrupt_params;
struct amdgpu_device *adev = irq_params->adev;
struct amdgpu_crtc *acrtc;
+   struct dm_crtc_state *acrtc_state;
 
acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - 
IRQ_TYPE_VBLANK);
 
if (acrtc) {
drm_crtc_handle_vblank(>base);
amdgpu_dm_crtc_handle_crc_irq(>base);
+
+   acrtc_state = to_dm_crtc_state(acrtc->base.state);
+
+   if (acrtc_state->stream &&
+   acrtc_state->vrr_params.supported &&
+   acrtc_state->freesync_config.state == 
VRR_STATE_ACTIVE_VARIABLE) {
+   mod_freesync_handle_v_update(
+   adev->dm.freesync_module,
+   acrtc_state->stream,
+   _state->vrr_params);
+
+   dc_stream_adjust_vmin_vmax(
+   adev->dm.dc,
+   acrtc_state->stream,
+   _state->vrr_params.adjust);
+   }
}
 }
 
@@ -3001,7 +3018,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
dc_stream_retain(state->stream);
}
 
-   state->adjust = cur->adjust;
+   state->vrr_params = cur->vrr_params;
state->vrr_infopacket = cur->vrr_infopacket;
state->abm_level = cur->abm_level;
state->vrr_supported = cur->vrr_supported;
@@ -4396,9 +4413,11 @@ struct dc_stream_status *dc_state_get_stream_status(
 static void update_freesync_state_on_stream(
struct amdgpu_display_manager *dm,
struct dm_crtc_state *new_crtc_state,
-   struct dc_stream_state *new_stream)
+   struct dc_stream_state *new_stream,
+   struct dc_plane_state *surface,
+   u32 flip_timestamp_in_us)
 {
-   struct mod_vrr_params vrr = {0};
+   struct mod_vrr_params vrr_params = new_crtc_state->vrr_params;
struct dc_info_packet vrr_infopacket = {0};
struct mod_freesync_config config = new_crtc_state->freesync_config;
 
@@ -4425,43 +,52 @@ static void update_freesync_state_on_stream(
 
mod_freesync_build_vrr_params(dm->freesync_module,
  new_stream,
- , );
+ , _params);
+
+   if (surface) {
+   mod_freesync_handle_preflip(
+   dm->freesync_module,
+   surface,
+   new_stream,
+   flip_timestamp_in_us,
+   _params);
+   }
 
mod_freesync_build_vrr_infopacket(
dm->freesync_module,
new_stream,
-   ,
+   _params,
PACKET_TYPE_VRR,
TRANSFER_FUNC_UNKNOWN,
_infopacket);
 
new_crtc_state->freesync_timing_changed =
-   (memcmp(_crtc_state->adjust,
-   ,
-   sizeof(vrr.adjust)) != 0);
+   (memcmp(_crtc_state->vrr_params.adjust,
+   _params.adjust,
+   sizeof(vrr_params.adjust)) != 0);
 
new_crtc_state->freesync_vrr_info_changed =
(memcmp(_crtc_state->vrr_infopacket,
_infopacket,
sizeof(vrr_infopacket)) != 0);
 
-   new_crtc_state->adjust = vrr.adjust;
+   new_crtc_state->vrr_params = vrr_params;
new_crtc_state->vrr_infopacket =