Re: [PATCH] drm/amd/display: re-indent dpp401_dscl_program_isharp()

2024-04-28 Thread Aurabindo Pillai

Patch has been merged to amd-staging-drm-next.

On 4/28/24 12:09 PM, Aurabindo Pillai wrote:

Thanks for the fix!

Reviewed-by: Aurabindo Pillai 

On 4/28/24 8:42 AM, Dan Carpenter wrote:

Smatch complains because some lines are indented more than they should
be.  I went a bit crazy re-indenting this.  ;)

The comments were not useful except as a marker of things which are left
to implement so I deleted most of them except for the TODO.

I introduced a "data" pointer so that I could replace
"scl_data->dscl_prog_data." with just "data->" and shorten the lines a
bit.  It's more readable without the line breaks.

I also tried to align it so you can see what is changing on each line.

Signed-off-by: Dan Carpenter 
---
  .../display/dc/dpp/dcn401/dcn401_dpp_dscl.c   | 93 ++-
  1 file changed, 30 insertions(+), 63 deletions(-)

diff --git 
a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c 
b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c

index c20376083441..696ccf96b847 100644
--- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
+++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
@@ -779,75 +779,42 @@ static void dpp401_dscl_program_isharp(struct 
dpp *dpp_base,

  const struct scaler_data *scl_data)
  {
  struct dcn401_dpp *dpp = TO_DCN401_DPP(dpp_base);
+    const struct dscl_prog_data *data;
  if (memcmp(>scl_data, scl_data, sizeof(*scl_data)) == 0)
  return;
  PERF_TRACE();
  dpp->scl_data = *scl_data;
-    // ISHARP_EN
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_EN, scl_data->dscl_prog_data.isharp_en);
-    // ISHARP_NOISEDET_EN
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_NOISEDET_EN, 
scl_data->dscl_prog_data.isharp_noise_det.enable);

-    // ISHARP_NOISEDET_MODE
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);

-    // ISHARP_NOISEDET_UTHRE
-    REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-    ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);

-    // ISHARP_NOISEDET_DTHRE
-    REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-    ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);

-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);

-    // ISHARP_NOISEDET_UTHRE
-    REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-    ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);

-    // ISHARP_NOISEDET_DTHRE
-    REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-    ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);

-    // ISHARP_NOISEDET_PWL_START_IN
-    REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-    ISHARP_NOISEDET_PWL_START_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_start_in);

-    // ISHARP_NOISEDET_PWL_END_IN
-    REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-    ISHARP_NOISEDET_PWL_END_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_end_in);

-    // ISHARP_NOISEDET_PWL_SLOPE
-    REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-    ISHARP_NOISEDET_PWL_SLOPE, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_slope);

-    // ISHARP_LBA_MODE
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_LBA_MODE, 
scl_data->dscl_prog_data.isharp_lba.mode);

-    // TODO: ISHARP_LBA: IN_SEG, BASE_SEG, SLOPE_SEG
-    // ISHARP_FMT_MODE
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_FMT_MODE, 
scl_data->dscl_prog_data.isharp_fmt.mode);

-    // ISHARP_FMT_NORM
-    REG_SET(ISHARP_MODE, 0,
-    ISHARP_FMT_NORM, 
scl_data->dscl_prog_data.isharp_fmt.norm);

-    // ISHARP_DELTA_LUT
-    dpp401_dscl_set_isharp_filter(dpp, 
scl_data->dscl_prog_data.isharp_delta);

-    // ISHARP_NLDELTA_SCLIP_EN_P
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_EN_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.enable_p);

-    // ISHARP_NLDELTA_SCLIP_PIVOT_P
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_PIVOT_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.pivot_p);

-    // ISHARP_NLDELTA_SCLIP_SLOPE_P
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_SLOPE_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.slope_p);

-    // ISHARP_NLDELTA_SCLIP_EN_N
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_EN_N, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.enable_n);

-    // ISHARP_NLDELTA_SCLIP_PIVOT_N
-    REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-    ISHARP_NLDELTA_SCLIP_PIVOT_N, 
scl_data->dscl_prog_data.isharp_nldelta_

Re: [PATCH] drm/amd/display: re-indent dpp401_dscl_program_isharp()

2024-04-28 Thread Aurabindo Pillai

Thanks for the fix!

Reviewed-by: Aurabindo Pillai 

On 4/28/24 8:42 AM, Dan Carpenter wrote:

Smatch complains because some lines are indented more than they should
be.  I went a bit crazy re-indenting this.  ;)

The comments were not useful except as a marker of things which are left
to implement so I deleted most of them except for the TODO.

I introduced a "data" pointer so that I could replace
"scl_data->dscl_prog_data." with just "data->" and shorten the lines a
bit.  It's more readable without the line breaks.

I also tried to align it so you can see what is changing on each line.

Signed-off-by: Dan Carpenter 
---
  .../display/dc/dpp/dcn401/dcn401_dpp_dscl.c   | 93 ++-
  1 file changed, 30 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c 
b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
index c20376083441..696ccf96b847 100644
--- a/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
+++ b/drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_dscl.c
@@ -779,75 +779,42 @@ static void dpp401_dscl_program_isharp(struct dpp 
*dpp_base,
const struct scaler_data *scl_data)
  {
struct dcn401_dpp *dpp = TO_DCN401_DPP(dpp_base);
+   const struct dscl_prog_data *data;
  
  	if (memcmp(>scl_data, scl_data, sizeof(*scl_data)) == 0)

return;
  
  	PERF_TRACE();

dpp->scl_data = *scl_data;
-   // ISHARP_EN
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_EN, scl_data->dscl_prog_data.isharp_en);
-   // ISHARP_NOISEDET_EN
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_NOISEDET_EN, 
scl_data->dscl_prog_data.isharp_noise_det.enable);
-   // ISHARP_NOISEDET_MODE
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);
-   // ISHARP_NOISEDET_UTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);
-   // ISHARP_NOISEDET_DTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_NOISEDET_MODE, 
scl_data->dscl_prog_data.isharp_noise_det.mode);
-   // ISHARP_NOISEDET_UTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_UTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.uthreshold);
-   // ISHARP_NOISEDET_DTHRE
-   REG_SET(ISHARP_NOISEDET_THRESHOLD, 0,
-   ISHARP_NOISEDET_DTHRE, 
scl_data->dscl_prog_data.isharp_noise_det.dthreshold);
-   // ISHARP_NOISEDET_PWL_START_IN
-   REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-   ISHARP_NOISEDET_PWL_START_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_start_in);
-   // ISHARP_NOISEDET_PWL_END_IN
-   REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-   ISHARP_NOISEDET_PWL_END_IN, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_end_in);
-   // ISHARP_NOISEDET_PWL_SLOPE
-   REG_SET(ISHARP_NOISE_GAIN_PWL, 0,
-   ISHARP_NOISEDET_PWL_SLOPE, 
scl_data->dscl_prog_data.isharp_noise_det.pwl_slope);
-   // ISHARP_LBA_MODE
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_LBA_MODE, 
scl_data->dscl_prog_data.isharp_lba.mode);
-   // TODO: ISHARP_LBA: IN_SEG, BASE_SEG, SLOPE_SEG
-   // ISHARP_FMT_MODE
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_FMT_MODE, 
scl_data->dscl_prog_data.isharp_fmt.mode);
-   // ISHARP_FMT_NORM
-   REG_SET(ISHARP_MODE, 0,
-   ISHARP_FMT_NORM, 
scl_data->dscl_prog_data.isharp_fmt.norm);
-   // ISHARP_DELTA_LUT
-   dpp401_dscl_set_isharp_filter(dpp, 
scl_data->dscl_prog_data.isharp_delta);
-   // ISHARP_NLDELTA_SCLIP_EN_P
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_EN_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.enable_p);
-   // ISHARP_NLDELTA_SCLIP_PIVOT_P
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_PIVOT_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.pivot_p);
-   // ISHARP_NLDELTA_SCLIP_SLOPE_P
-   REG_SET(ISHARP_NLDELTA_SOFT_CLIP, 0,
-   ISHARP_NLDELTA_SCLIP_SLOPE_P, 
scl_data->dscl_prog_data.isharp_nldelta_sclip.slope_p);

Re: [PATCH v2] drm/amd/display: add a debugfs interface for the DMUB trace mask

2023-11-13 Thread Aurabindo Pillai
   }
+
+   status = dmub_srv_send_gpint_command(srv, cmd, res, 30);
+
+   if (status == DMUB_STATUS_TIMEOUT)
+   return -ETIMEDOUT;
+   else if (status != DMUB_STATUS_OK)
+   return -EIO;
+
+   usleep_range(100, 1000);
+
+   mask <<= 16;
+   shift += 16;
+   }
+
+   return 0;
+}
+
+static int dmub_trace_mask_show(void *data, u64 *val)
+{
+   enum dmub_gpint_command cmd = DMUB_GPINT__GET_TRACE_BUFFER_MASK_WORD0;
+   struct amdgpu_device *adev = data;
+   struct dmub_srv *srv = adev->dm.dc->ctx->dmub_srv->dmub;
+   enum dmub_status status;
+   u8 shift = 0;
+   u64 raw = 0;
+   u64 res = 0;
+   int i = 0;
+
+   if (!srv->fw_version)
+   return -EINVAL;
+
+   while (i < 4) {
+   status = dmub_srv_send_gpint_command(srv, cmd, 0, 30);
+
+   if (status == DMUB_STATUS_OK) {
+   status = dmub_srv_get_gpint_response(srv, (u32 *) );
+
+   if (status == DMUB_STATUS_TIMEOUT)
+   return -ETIMEDOUT;
+   else if (status != DMUB_STATUS_OK)
+   return -EIO;
+   } else if (status == DMUB_STATUS_TIMEOUT) {
+   return -ETIMEDOUT;
+   } else {
+   return -EIO;
+   }
+
+   usleep_range(100, 1000);
+
+   cmd++;
+   res |= (raw << shift);
+   shift += 16;
+   i++;
+   }
+
+   *val = res;
+
+   return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(dmub_trace_mask_fops, dmub_trace_mask_show,
+dmub_trace_mask_set, "0x%llx\n");
+
  /*
   * Set dmcub trace event IRQ enable or disable.
   * Usage to enable dmcub trace event IRQ: echo 1 > 
/sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
@@ -3884,6 +3978,9 @@ void dtn_debugfs_init(struct amdgpu_device *adev)
debugfs_create_file_unsafe("amdgpu_dm_force_timing_sync", 0644, root,
   adev, _timing_sync_ops);
  
+	debugfs_create_file_unsafe("amdgpu_dm_dmub_trace_mask", 0644, root,

+  adev, _trace_mask_fops);
+
debugfs_create_file_unsafe("amdgpu_dm_dmcub_trace_event_en", 0644, root,
   adev, _trace_event_state_fops);
  
diff --git a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h

index ed4379c04715..aa6e6923afed 100644
--- a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
+++ b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
@@ -818,18 +818,54 @@ enum dmub_gpint_command {
 * RETURN: Lower 32-bit mask.
 */
DMUB_GPINT__UPDATE_TRACE_BUFFER_MASK = 101,
+
/**
-* DESC: Updates the trace buffer lower 32-bit mask.
+* DESC: Updates the trace buffer mask bit0~bit15.
 * ARGS: The new mask
 * RETURN: Lower 32-bit mask.
 */
DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD0 = 102,
+
/**
-* DESC: Updates the trace buffer mask bi0~bit15.
+* DESC: Updates the trace buffer mask bit16~bit31.
 * ARGS: The new mask
 * RETURN: Lower 32-bit mask.
 */
DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD1 = 103,
+
+   /**
+* DESC: Updates the trace buffer mask bit32~bit47.
+* ARGS: The new mask
+* RETURN: Lower 32-bit mask.
+*/
+   DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD2 = 114,
+
+   /**
+* DESC: Updates the trace buffer mask bit48~bit63.
+* ARGS: The new mask
+* RETURN: Lower 32-bit mask.
+*/
+   DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD3 = 115,
+
+   /**
+* DESC: Read the trace buffer mask bi0~bit15.
+*/
+   DMUB_GPINT__GET_TRACE_BUFFER_MASK_WORD0 = 116,
+
+   /**
+* DESC: Read the trace buffer mask bit16~bit31.
+*/
+   DMUB_GPINT__GET_TRACE_BUFFER_MASK_WORD1 = 117,
+
+   /**
+* DESC: Read the trace buffer mask bi32~bit47.
+*/
+   DMUB_GPINT__GET_TRACE_BUFFER_MASK_WORD2 = 118,
+
+   /**
+* DESC: Updates the trace buffer mask bit32~bit63.
+    */
+   DMUB_GPINT__GET_TRACE_BUFFER_MASK_WORD3 = 119,
  };
  
  /**


Reviewed-by: Aurabindo Pillai 


Re: [PATCH] drm/amd/display: add a debugfs interface for the DMUB trace mask

2023-11-10 Thread Aurabindo Pillai




On 2023-11-10 12:18, Hamza Mahfooz wrote:

For features that are implemented primarily in DMUB (e.g. PSR), it is
useful to be able to trace them at a DMUB level from the kernel,
especially when debugging issues. So, introduce a debugfs interface that
is able to read and set the DMUB trace mask dynamically at runtime and
document how to use it.

Cc: Alex Deucher 
Cc: Mario Limonciello 
Signed-off-by: Hamza Mahfooz 
---
  Documentation/gpu/amdgpu/display/dc-debug.rst | 41 +
  .../gpu/amdgpu/display/trace-groups-table.csv | 29 ++
  .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 91 +++
  .../gpu/drm/amd/display/dmub/inc/dmub_cmd.h   | 40 +++-
  4 files changed, 199 insertions(+), 2 deletions(-)
  create mode 100644 Documentation/gpu/amdgpu/display/trace-groups-table.csv

diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst 
b/Documentation/gpu/amdgpu/display/dc-debug.rst
index 40c55a618918..817631b1dbf3 100644
--- a/Documentation/gpu/amdgpu/display/dc-debug.rst
+++ b/Documentation/gpu/amdgpu/display/dc-debug.rst
@@ -75,3 +75,44 @@ change in real-time by using something like::
  
  When reporting a bug related to DC, consider attaching this log before and

  after you reproduce the bug.
+
+DMUB Firmware Debug
+===
+
+Sometimes, dmesg logs aren't enough. This is especially true if a feature is
+implemented primarily in DMUB firmware. In such cases, all we see in dmesg when
+an issue arises is some generic timeout error. So, to get more relevant
+information, we can trace DMUB commands by enabling the relevant bits in
+`amdgpu_dm_dmub_trace_mask`.
+
+Currently, we support the tracing of the following groups:
+
+Trace Groups
+
+
+.. csv-table::
+   :header-rows: 1
+   :widths: 1, 1
+   :file: ./trace-groups-table.csv
+
+**Note: Not all ASICs support all of the listed trace groups**
+
+So, to enable just PSR tracing you can use the following command::
+
+  # echo 0x8020 > /sys/kernel/debug/dri/0/amdgpu_dm_dmub_trace_mask
+
+Then, you need to enable logging trace events to the buffer, which you can do
+using the following::
+
+  # echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+
+Lastly, after you are able to reproduce the issue you are trying to debug,
+you can disable tracing and read the trace log by using the following::
+
+  # echo 0 > /sys/kernel/debug/dri/0/amdgpu_dm_dmcub_trace_event_en
+  # cat /sys/kernel/debug/dri/0/amdgpu_dm_dmub_tracebuffer
+
+So, when reporting bugs related to features such as PSR and ABM, consider
+enabling the relevant bits in the mask before reproducing the issue and
+attach the log that you obtain from the trace buffer in any bug reports that 
you
+create.
diff --git a/Documentation/gpu/amdgpu/display/trace-groups-table.csv 
b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
new file mode 100644
index ..3f6a50d1d883
--- /dev/null
+++ b/Documentation/gpu/amdgpu/display/trace-groups-table.csv
@@ -0,0 +1,29 @@
+Name, Mask Value
+INFO, 0x1
+IRQ SVC, 0x2
+VBIOS, 0x4
+REGISTER, 0x8
+PHY DBG, 0x10
+PSR, 0x20
+AUX, 0x40
+SMU, 0x80
+MALL, 0x100
+ABM, 0x200
+ALPM, 0x400
+TIMER, 0x800
+HW LOCK MGR, 0x1000
+INBOX1, 0x2000
+PHY SEQ, 0x4000
+PSR STATE, 0x8000
+ZSTATE, 0x1
+TRANSMITTER CTL, 0x2
+PANEL CNTL, 0x4
+FAMS, 0x8
+DPIA, 0x10
+SUBVP, 0x20
+INBOX0, 0x40
+SDP, 0x400
+REPLAY, 0x800
+REPLAY RESIDENCY, 0x2000
+CURSOR INFO, 0x8000
+IPS, 0x1
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 13a177d34376..06a73f283e9d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -2971,6 +2971,94 @@ static int allow_edp_hotplug_detection_set(void *data, 
u64 val)
return 0;
  }
  
+static int dmub_trace_mask_set(void *data, u64 val)

+{
+   struct amdgpu_device *adev = data;
+   struct dmub_srv *srv = adev->dm.dc->ctx->dmub_srv->dmub;
+   enum dmub_gpint_command cmd;
+   enum dmub_status status;
+   u64 mask = 0x;
+   u8 shift = 0;
+   u32 res;
+   int i;
+
+   if (!srv->fw_version)
+   return -EINVAL;
+
+   for (i = 0;  i < 4; i++) {
+   res = (val & mask) >> shift;
+
+   switch (i) {
+   case 0:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD0;
+   break;
+   case 1:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD1;
+   break;
+   case 2:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD2;
+   break;
+   case 3:
+   cmd = DMUB_GPINT__SET_TRACE_BUFFER_MASK_WORD3;
+   break;
+   }
+
+   status = dmub_srv_send_gpint_command(srv, cmd, res, 30);
+
+   if 

Re: [PATCH] drm/amd/display: remove duplicated argument

2023-11-08 Thread Aurabindo Pillai




On 2023-10-29 05:39, José Pekkarinen wrote:

Spotted by coccicheck, there is a redundant check for
v->SourcePixelFormat[k] != dm_444_16. This patch will
remove it. The corresponding output follows.

drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c:5130:86-122: duplicated 
argument to && or ||

Signed-off-by: José Pekkarinen 
---
  drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
index ad741a723c0e..3686f1e7de3a 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
@@ -5128,7 +5128,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
ViewportExceedsSurface = true;
  
  		if (v->SourcePixelFormat[k] != dm_444_64 && v->SourcePixelFormat[k] != dm_444_32 && v->SourcePixelFormat[k] != dm_444_16

-   && v->SourcePixelFormat[k] != dm_444_16 && 
v->SourcePixelFormat[k] != dm_444_8 && v->SourcePixelFormat[k] != dm_rgbe) {
+   && v->SourcePixelFormat[k] != dm_444_8 && 
v->SourcePixelFormat[k] != dm_rgbe) {
if (v->ViewportWidthChroma[k] > v->SurfaceWidthC[k] || 
v->ViewportHeightChroma[k] > v->SurfaceHeightC[k]) {
ViewportExceedsSurface = true;
}


Hi José,

Sorry, I've just queued it. Should be merged to amd-staging-drm-next soon.


Re: [PATCH] drm/amd/display: remove redundant check

2023-10-31 Thread Aurabindo Pillai




On 2023-10-31 08:32, José Pekkarinen wrote:

On 2023-10-31 14:20, Greg KH wrote:

On Tue, Oct 31, 2023 at 01:42:17PM +0200, José Pekkarinen wrote:

On 2023-10-31 07:48, Greg KH wrote:
> On Mon, Oct 30, 2023 at 07:17:48PM +0200, José Pekkarinen wrote:
> > This patch addresses the following warning spotted by
> > using coccinelle where the case checked does the same
> > than the else case.
> >
> > 
drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c:4664:8-10:

> > WARNING: possible condition with no effect (if == else)
> >
> > Fixes: 974ce181 ("drm/amd/display: Add check for PState change in
> > DCN32")
> >
> > Cc: sta...@vger.kernel.org
>
> Why is this relevant for stable?

    Hi,

    I was asked to send it for stable because this code
looks different in amd-staging-drm-next, see here.

https://gitlab.freedesktop.org/agd5f/linux/-/blob/amd-staging-drm-next/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c#L4661


I don't know what I am looking at, sorry.


    Feel free to let me know if this is wrong, or if I
need to review some other guidelines I may have missed.


Please see the list of rules for stable patches:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

Does "remove code that does nothing" fit here?


     Yep, it is a trivial fix which compilers should be able
to optimize, so no real benefit to the user. Sorry Greg!

     José.


Hi Greg,

Sorry, I asked José to send it to stable because I realized the patch 
listed in the fixes tag wasnt quite needed on stable. Patch from José 
effectively removes it.


I hope that clears the context. But there is no impact whether you apply 
José's patch or not, so we're good either way.


Re: [PATCH] drm/amd/display: remove redundant check

2023-10-30 Thread Aurabindo Pillai




On 2023-10-30 13:17, José Pekkarinen wrote:

This patch addresses the following warning spotted by
using coccinelle where the case checked does the same
than the else case.

drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c:4664:8-10: 
WARNING: possible condition with no effect (if == else)

Fixes: 974ce181 ("drm/amd/display: Add check for PState change in DCN32")

Cc: sta...@vger.kernel.org
Signed-off-by: José Pekkarinen 
---
  .../drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c   | 4 
  1 file changed, 4 deletions(-)

diff --git 
a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
index ecea008f19d3..d940dfa5ae43 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
@@ -4661,10 +4661,6 @@ void dml32_CalculateMinAndMaxPrefetchMode(
} else if (AllowForPStateChangeOrStutterInVBlankFinal == 
dm_prefetch_support_uclk_fclk_and_stutter) {
*MinPrefetchMode = 0;
*MaxPrefetchMode = 0;
-   } else if (AllowForPStateChangeOrStutterInVBlankFinal ==
-   dm_prefetch_support_uclk_fclk_and_stutter_if_possible) {
-   *MinPrefetchMode = 0;
-   *MaxPrefetchMode = 3;
} else {
*MinPrefetchMode = 0;
*MaxPrefetchMode = 3;


Reviewed-by: Aurabindo Pillai 


Re: [PATCH] drm/amd/display: remove redundant check

2023-10-30 Thread Aurabindo Pillai




On 2023-10-30 12:26, José Pekkarinen wrote:

On 2023-10-30 15:52, Aurabindo Pillai wrote:

On 10/29/2023 8:44 AM, José Pekkarinen wrote:

This patch addresses the following warning spotted by
using coccinelle where the case checked does the same
than the else case.

drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c:4664:8-10: 
WARNING: possible condition with no effect (if == else)

Signed-off-by: José Pekkarinen 
---
  .../drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c   | 4 
  1 file changed, 4 deletions(-)

diff --git 
a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c

index ecea008f19d3..d940dfa5ae43 100644
--- 
a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
+++ 
b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c

@@ -4661,10 +4661,6 @@ void dml32_CalculateMinAndMaxPrefetchMode(
  } else if (AllowForPStateChangeOrStutterInVBlankFinal == 
dm_prefetch_support_uclk_fclk_and_stutter) {

  *MinPrefetchMode = 0;
  *MaxPrefetchMode = 0;
-    } else if (AllowForPStateChangeOrStutterInVBlankFinal ==
-    dm_prefetch_support_uclk_fclk_and_stutter_if_possible) {
-    *MinPrefetchMode = 0;
-    *MaxPrefetchMode = 3;
  } else {
  *MinPrefetchMode = 0;
  *MaxPrefetchMode = 3;


What tree did you use to generate the patch? On amd-staging-drm-next,
MaxPrefetchMode is 0 for the second last branch, which is the correct
one, so this patch isnt needed.


     I'm using the stable tree, sorry, if it is out of
date just ignore it then.

     Thanks!

     José.

Actually, you're right - the patch's limited context mislead me, sorry.
But please add the following tag and sent it to sta...@vger.kernel.org 
instead:


Fixes: 974ce181 ("drm/amd/display: Add check for PState change in DCN32")


Re: [PATCH] drm/amd/display: remove duplicated argument

2023-10-30 Thread Aurabindo Pillai




On 10/29/2023 5:39 AM, José Pekkarinen wrote:

Spotted by coccicheck, there is a redundant check for
v->SourcePixelFormat[k] != dm_444_16. This patch will
remove it. The corresponding output follows.

drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c:5130:86-122: duplicated 
argument to && or ||

Signed-off-by: José Pekkarinen 
---
  drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
index ad741a723c0e..3686f1e7de3a 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn30/display_mode_vba_30.c
@@ -5128,7 +5128,7 @@ void dml30_ModeSupportAndSystemConfigurationFull(struct 
display_mode_lib *mode_l
ViewportExceedsSurface = true;
  
  		if (v->SourcePixelFormat[k] != dm_444_64 && v->SourcePixelFormat[k] != dm_444_32 && v->SourcePixelFormat[k] != dm_444_16

-   && v->SourcePixelFormat[k] != dm_444_16 && 
v->SourcePixelFormat[k] != dm_444_8 && v->SourcePixelFormat[k] != dm_rgbe) {
+   && v->SourcePixelFormat[k] != dm_444_8 && 
v->SourcePixelFormat[k] != dm_rgbe) {
if (v->ViewportWidthChroma[k] > v->SurfaceWidthC[k] || 
v->ViewportHeightChroma[k] > v->SurfaceHeightC[k]) {
    ViewportExceedsSurface = true;
}


Thanks for catching.

Reviewed-by: Aurabindo Pillai 


Re: [PATCH] drm/amd/display: remove redundant check

2023-10-30 Thread Aurabindo Pillai




On 10/29/2023 8:44 AM, José Pekkarinen wrote:

This patch addresses the following warning spotted by
using coccinelle where the case checked does the same
than the else case.

drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c:4664:8-10: 
WARNING: possible condition with no effect (if == else)

Signed-off-by: José Pekkarinen 
---
  .../drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c   | 4 
  1 file changed, 4 deletions(-)

diff --git 
a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c 
b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
index ecea008f19d3..d940dfa5ae43 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
+++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_util_32.c
@@ -4661,10 +4661,6 @@ void dml32_CalculateMinAndMaxPrefetchMode(
} else if (AllowForPStateChangeOrStutterInVBlankFinal == 
dm_prefetch_support_uclk_fclk_and_stutter) {
*MinPrefetchMode = 0;
*MaxPrefetchMode = 0;
-   } else if (AllowForPStateChangeOrStutterInVBlankFinal ==
-   dm_prefetch_support_uclk_fclk_and_stutter_if_possible) {
-   *MinPrefetchMode = 0;
-   *MaxPrefetchMode = 3;
} else {
*MinPrefetchMode = 0;
*MaxPrefetchMode = 3;


What tree did you use to generate the patch? On amd-staging-drm-next, 
MaxPrefetchMode is 0 for the second last branch, which is the correct 
one, so this patch isnt needed.


Re: [PATCH] drm/amd/display: prevent potential division by zero errors

2023-09-05 Thread Aurabindo Pillai




On 2023-09-05 14:53, Hamza Mahfooz wrote:

There are two places in apply_below_the_range() where it's possible for
a divide by zero error to occur. So, to fix this make sure the divisor
is non-zero before attempting the computation in both cases.

Cc: sta...@vger.kernel.org
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2637
Fixes: a463b263032f ("drm/amd/display: Fix frames_to_insert math")
Fixes: ded6119e825a ("drm/amd/display: Reinstate LFC optimization")
Signed-off-by: Hamza Mahfooz 
---
  drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c 
b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
index dbd60811f95d..ef3a67409021 100644
--- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
+++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
@@ -338,7 +338,9 @@ static void apply_below_the_range(struct core_freesync 
*core_freesync,
 *  - Delta for CEIL: delta_from_mid_point_in_us_1
 *  - Delta for FLOOR: delta_from_mid_point_in_us_2
 */
-   if ((last_render_time_in_us / mid_point_frames_ceil) < 
in_out_vrr->min_duration_in_us) {
+   if (mid_point_frames_ceil &&
+   (last_render_time_in_us / mid_point_frames_ceil) <
+   in_out_vrr->min_duration_in_us) {
/* Check for out of range.
 * If using CEIL produces a value that is out of range,
 * then we are forced to use FLOOR.
@@ -385,8 +387,9 @@ static void apply_below_the_range(struct core_freesync 
*core_freesync,
/* Either we've calculated the number of frames to insert,
 * or we need to insert min duration frames
 */
-   if (last_render_time_in_us / frames_to_insert <
-   in_out_vrr->min_duration_in_us){
+   if (frames_to_insert &&
+   (last_render_time_in_us / frames_to_insert) <
+   in_out_vrr->min_duration_in_us){
frames_to_insert -= (frames_to_insert > 1) ?
        1 : 0;
}



Reviewed-by: Aurabindo Pillai 

--

Thanks & Regards,
Jay


Re: [PATCH] Revert "drm/amd/display: Program OTG vtotal min/max selectors unconditionally for DCN1+"

2023-07-12 Thread Aurabindo Pillai




On 7/12/2023 12:24 PM, Melissa Wen wrote:

On 07/12, Pillai, Aurabindo wrote:

[Public]

Hi Guilherme,

Sorry there was one more patch which I missed to attach. Please add this 3rd 
patch and retry.

Reverting that patch would cause high power consumption on Navi2x GPU also 
cause hangs on certain multi monitor configurations. With these 3 patches, 
you're getting the same effect as reverting the aforementioned patches, but it 
makes the reverted sequence available only for Steam deck hardware.



Hi Jay,

Thanks for looking at this issue.

You mention power consumption and multi-monitor configuration issues
that can affect a driver if we revert this OTG change, and both sounds
quite relevant to me. Can they not affect DCN301 too? Is there something
that needs further work so the DCN301 can benefit from this improvement
as well?

Also, let us know if we can contribute in any way.



Hi Melissa,

Unfortunately, DCN301 does not support Firmware Assisted Memory Clock 
Switching, which is the feature that gets blocked on Navi2x if we revert 
the patch in question.  This is the feature that enables lower power 
consumption on some multi monitor configurations and high refresh rate 
single monitor configurations.


Navi2x is configured to use FAMS in the driver, but without this change, 
firmware wont be able to actually enable the feature in DMCUB, which 
puts the driver in a unexpected state. On DCN301, this unexpected state 
will not occur, because there is no FAMS support in driver nor firmware.


--
Jay





Re: [PATCH] Revert "drm/amd/display: Program OTG vtotal min/max selectors unconditionally for DCN1+"

2023-07-12 Thread Aurabindo Pillai




On 7/12/2023 11:50 AM, Guilherme G. Piccoli wrote:

On 12/07/2023 11:47, Pillai, Aurabindo wrote:

Hi Guilherme,

Sorry there was one more patch which I missed to attach. Please add this
3^rd  patch and retry.

Reverting that patch would cause high power consumption on Navi2x GPU
also cause hangs on certain multi monitor configurations. With these 3
patches, you're getting the same effect as reverting the aforementioned
patches, but it makes the reverted sequence available only for Steam
deck hardware.



Thanks a lot for your detailed explanation, and the 3rd patch! Indeed,
amdgpu works fine on Deck with that - great =)

Feel free to add my:
Tested-by: Guilherme G. Piccoli  #Steam Deck

Oh, a fixes tag would also makes sense, I guess.
BTW, if possible to submit the 3 patches in a proper series to get it
merged on 6.5-rc cycle (the sooner the better), I'd really appreciate!



Thanks for confirmation! I'll add the fixes tag so that it gets picked up.


Re: [PATCH] Revert "drm/amd/display: Program OTG vtotal min/max selectors unconditionally for DCN1+"

2023-07-11 Thread Aurabindo Pillai


On 7/2/23 12:44, Guilherme G. Piccoli wrote:
> This reverts commit 06c3a652a787efc960af7c8816036d25c4227c6c.
> 
> After this commit, the Steam Deck cannot boot with graphics anymore;
> the following message is observed on dmesg:
> 
> "[drm] ERROR [CRTC:67:crtc-0] flip_done timed out"
> 
> No other error is observed, it just stays like that. After bisecting
> amd-staging-drm-next, we narrowed it down to this commit. Seems it
> makes sense to revert it to have the tree bootable until a proper
> solution is worked.
> 
> Cc: Aurabindo Pillai 
> Cc: André Almeida 
> Cc: Melissa Wen 
> Cc: Rodrigo Siqueira 
> Signed-off-by: Guilherme G. Piccoli 
> 
> ---
> 
> Hi Alex / Aurabindo, we couldn't boot the Deck with in HEAD
> (amd-staging-drm-next), git bisect led to this commit. Since its
> description already mentions a potential proper solution, related
> to the DMCUB (and some complex state tracking), I thought it was
> more effective to revert it to allow booting the tree in Deck (and
> maybe other HW - I just tested the Deck BTW).
> Lemme know your thoughts.
> 
> Special thanks to André and Melissa for helping the debug / bisect!
> We're open to test alternative patches, feel free to ping.
> Cheers,
> 
> Guilherme
> 
> 
>  drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c | 15 ---
>  drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c | 10 --
>  2 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c 
> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c
> index 0e8f4f36c87c..27419cd98264 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_optc.c
> @@ -945,10 +945,19 @@ void optc1_set_drr(
>   OTG_FORCE_LOCK_ON_EVENT, 0,
>   OTG_SET_V_TOTAL_MIN_MASK_EN, 0,
>   OTG_SET_V_TOTAL_MIN_MASK, 0);
> - }
>  
> - // Setup manual flow control for EOF via TRIG_A
> - optc->funcs->setup_manual_trigger(optc);
> + // Setup manual flow control for EOF via TRIG_A
> + optc->funcs->setup_manual_trigger(optc);
> +
> + } else {
> + REG_UPDATE_4(OTG_V_TOTAL_CONTROL,
> + OTG_SET_V_TOTAL_MIN_MASK, 0,
> + OTG_V_TOTAL_MIN_SEL, 0,
> + OTG_V_TOTAL_MAX_SEL, 0,
> + OTG_FORCE_LOCK_ON_EVENT, 0);
> +
> + optc->funcs->set_vtotal_min_max(optc, 0, 0);
> + }
>  }
>  
>  void optc1_set_vtotal_min_max(struct timing_generator *optc, int vtotal_min, 
> int vtotal_max)
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c 
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c
> index 58bdbd859bf9..d6f095b4555d 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_optc.c
> @@ -462,16 +462,6 @@ void optc2_setup_manual_trigger(struct timing_generator 
> *optc)
>  {
>   struct optc *optc1 = DCN10TG_FROM_TG(optc);
>  
> - /* Set the min/max selectors unconditionally so that
> -  * DMCUB fw may change OTG timings when necessary
> -  * TODO: Remove the w/a after fixing the issue in DMCUB firmware
> -  */
> - REG_UPDATE_4(OTG_V_TOTAL_CONTROL,
> -  OTG_V_TOTAL_MIN_SEL, 1,
> -  OTG_V_TOTAL_MAX_SEL, 1,
> -  OTG_FORCE_LOCK_ON_EVENT, 0,
> -  OTG_SET_V_TOTAL_MIN_MASK, (1 << 1)); /* TRIGA 
> */
> -
>   REG_SET_8(OTG_TRIGA_CNTL, 0,
>   OTG_TRIGA_SOURCE_SELECT, 21,
>   OTG_TRIGA_SOURCE_PIPE_SELECT, optc->inst,


Hi,

Sorry for the delayed response, this patch went unnoticed. This revert would 
break asics. Could you try the attached patch without reverting this one ?From 9ac8b837900ac242fcc9e948c8de7aca51a5be7e Mon Sep 17 00:00:00 2001
From: Aurabindo Pillai 
Date: Tue, 11 Jul 2023 14:16:27 -0400
Subject: [PATCH 2/2] drm/amd/display: add DCN301 specific logic for OTG
 programming

[Why]
DCN301 does not have FAMS hence the workaround needed on other DCN3x
variants related to OTG min/max selector programming is not applicable for it.
Hence isolate it and have it use the old sequence without workaround.

Signed-off-by: Aurabindo Pillai 
---
 .../gpu/drm/amd/display/dc/dcn301/Makefile|   3 +-
 .../drm/amd/display/dc/dcn301/dcn301_optc.c   | 185 ++
 .../drm/amd/display/dc/dcn301/dcn301_optc.h   |  36 
 3 files changed, 223 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/amd

Re: [PATCH] [v2] drm/amd/display: fix is_timing_changed() prototype

2023-04-18 Thread Aurabindo Pillai




On 4/17/2023 6:07 PM, Arnd Bergmann wrote:

From: Arnd Bergmann 

Three functions in the amdgpu display driver cause -Wmissing-prototype
warnings:

drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.c:1858:6: error: no 
previous prototype for 'is_timing_changed' [-Werror=missing-prototypes]

is_timing_changed() is actually meant to be a global symbol, but needs
a proper name and prototype.

Fixes: 17ce8a6907f7 ("drm/amd/display: Add dsc pre-validation in atomic check")
Signed-off-by: Arnd Bergmann 
---
  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 5 ++---
  drivers/gpu/drm/amd/display/dc/core/dc_resource.c   | 6 +++---
  drivers/gpu/drm/amd/display/dc/dc.h | 3 +++
  3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 994ba426ca66..442511061178 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -45,8 +45,7 @@
  #endif
  
  #include "dc/dcn20/dcn20_resource.h"

-bool is_timing_changed(struct dc_stream_state *cur_stream,
-  struct dc_stream_state *new_stream);
+
  #define PEAK_FACTOR_X1000 1006
  
  static ssize_t dm_dp_aux_transfer(struct drm_dp_aux *aux,

@@ -1417,7 +1416,7 @@ int pre_validate_dsc(struct drm_atomic_state *state,
struct dc_stream_state *stream = dm_state->context->streams[i];
  
  		if (local_dc_state->streams[i] &&

-   is_timing_changed(stream, local_dc_state->streams[i])) {
+   dc_is_timing_changed(stream, local_dc_state->streams[i])) {
DRM_INFO_ONCE("crtc[%d] needs mode_changed\n", i);
} else {
int ind = find_crtc_index_in_state_by_stream(state, 
stream);
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
index 85d54bfb595c..344533623cb9 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
@@ -1855,7 +1855,7 @@ bool dc_add_all_planes_for_stream(
return add_all_planes_for_stream(dc, stream, , 1, context);
  }
  
-bool is_timing_changed(struct dc_stream_state *cur_stream,

+bool dc_is_timing_changed(struct dc_stream_state *cur_stream,
   struct dc_stream_state *new_stream)
  {
if (cur_stream == NULL)
@@ -1880,7 +1880,7 @@ static bool are_stream_backends_same(
if (stream_a == NULL || stream_b == NULL)
return false;
  
-	if (is_timing_changed(stream_a, stream_b))

+   if (dc_is_timing_changed(stream_a, stream_b))
return false;
  
  	if (stream_a->signal != stream_b->signal)

@@ -3505,7 +3505,7 @@ bool pipe_need_reprogram(
if (pipe_ctx_old->stream_res.stream_enc != 
pipe_ctx->stream_res.stream_enc)
return true;
  
-	if (is_timing_changed(pipe_ctx_old->stream, pipe_ctx->stream))

+   if (dc_is_timing_changed(pipe_ctx_old->stream, pipe_ctx->stream))
return true;
  
  	if (pipe_ctx_old->stream->dpms_off != pipe_ctx->stream->dpms_off)

diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index 23ee63b98dcd..e7ab6cb3769b 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -2225,4 +2225,7 @@ void dc_process_dmub_dpia_hpd_int_enable(const struct dc 
*dc,
  /* Disable acc mode Interfaces */
  void dc_disable_accelerated_mode(struct dc *dc);
  
+bool dc_is_timing_changed(struct dc_stream_state *cur_stream,

+  struct dc_stream_state *new_stream);
+
  #endif /* DC_INTERFACE_H_ */




Reviewed-by: Aurabindo Pillai 


Re: [PATCH] drm/amd/display: fix read errors pertaining to dp_lttpr_status_show()

2023-02-03 Thread Aurabindo Pillai


On 2/3/23 10:43, Hamza Mahfooz wrote:

Currently, it is likely that we will read the relevant LTTPR caps after
link training has completed (which can cause garbage data to be read),
however according to the DP 2.0 spec that should be done before link
training has commenced. So, instead of reading the registers on demand,
use the values provided to us by DC.

Signed-off-by: Hamza Mahfooz
---
  .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 72 ++-
  1 file changed, 22 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index e783082a4eef..cbc241596c1f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -36,6 +36,7 @@
  #include "dsc.h"
  #include "link_hwss.h"
  #include "dc/dc_dmub_srv.h"
+#include "link/protocols/link_dp_capability.h"
  
  #ifdef CONFIG_DRM_AMD_SECURE_DISPLAY

  #include "amdgpu_dm_psr.h"
@@ -418,67 +419,38 @@ static ssize_t dp_phy_settings_read(struct file *f, char 
__user *buf,
return result;
  }
  
-static int dp_lttpr_status_show(struct seq_file *m, void *d)

+static int dp_lttpr_status_show(struct seq_file *m, void *unused)
  {
-   char *data;
-   struct amdgpu_dm_connector *connector = file_inode(m->file)->i_private;
-   struct dc_link *link = connector->dc_link;
-   uint32_t read_size = 1;
-   uint8_t repeater_count = 0;
+   struct drm_connector *connector = m->private;
+   struct amdgpu_dm_connector *aconnector =
+   to_amdgpu_dm_connector(connector);
+   struct dc_lttpr_caps caps = aconnector->dc_link->dpcd_caps.lttpr_caps;
  
-	data = kzalloc(read_size, GFP_KERNEL);

-   if (!data)
-   return 0;
+   if (connector->status != connector_status_connected)
+   return -ENODEV;
  
-	dm_helpers_dp_read_dpcd(link->ctx, link, 0xF0002, data, read_size);

+   seq_printf(m, "phy repeater count: %u (raw: 0x%x)\n",
+  dp_parse_lttpr_repeater_count(caps.phy_repeater_cnt),
+  caps.phy_repeater_cnt);
  
-	switch ((uint8_t)*data) {

-   case 0x80:
-   repeater_count = 1;
-   break;
-   case 0x40:
-   repeater_count = 2;
-   break;
-   case 0x20:
-   repeater_count = 3;
-   break;
-   case 0x10:
-   repeater_count = 4;
-   break;
-   case 0x8:
-   repeater_count = 5;
-   break;
-   case 0x4:
-   repeater_count = 6;
-   break;
-   case 0x2:
-   repeater_count = 7;
+   seq_puts(m, "phy repeater mode: ");
+
+   switch (caps.mode) {
+   case DP_PHY_REPEATER_MODE_TRANSPARENT:
+   seq_puts(m, "transparent");
break;
-   case 0x1:
-   repeater_count = 8;
+   case DP_PHY_REPEATER_MODE_NON_TRANSPARENT:
+   seq_puts(m, "non-transparent");
break;
-   case 0x0:
-   repeater_count = 0;
+   case 0x00:
+   seq_puts(m, "non lttpr");
break;
default:
-   repeater_count = (uint8_t)*data;
+   seq_printf(m, "read error (raw: 0x%x)", caps.mode);
break;
}
  
-	seq_printf(m, "phy repeater count: %d\n", repeater_count);

-
-   dm_helpers_dp_read_dpcd(link->ctx, link, 0xF0003, data, read_size);
-
-   if ((uint8_t)*data == 0x55)
-   seq_printf(m, "phy repeater mode: transparent\n");
-   else if ((uint8_t)*data == 0xAA)
-   seq_printf(m, "phy repeater mode: non-transparent\n");
-   else if ((uint8_t)*data == 0x00)
-   seq_printf(m, "phy repeater mode: non lttpr\n");
-   else
-   seq_printf(m, "phy repeater mode: read error\n");
-
-   kfree(data);
+   seq_puts(m, "\n");
return 0;
  }
  



Reviewed-by: Aurabindo Pillai 


Re: [PATCH] drm/amd/display: fix i386 frame size warning

2022-08-18 Thread Aurabindo Pillai
refetchPixelLinesTime[k] / 
4),
(2 * ExtraLatencyCycles + 
PixelDCFCLKCyclesRequiredInPrefetch[k]) / (MaximumTvmPlus2Tr0PlusTsw - 
MinimumTvmPlus2Tr0));
}
}
-   DCFCLKState[i][j] = dml_min(DCFCLKPerState[i], 1.05 * (1 + 
mode_lib->vba.PercentMarginOverMinimumRequiredDCFCLK / 100)
+   v->DCFCLKState[i][j] = dml_min(v->DCFCLKPerState[i], 1.05 
* (1 + mode_lib->vba.PercentMarginOverMinimumRequiredDCFCLK / 100)
* 
dml_max(DCFCLKRequiredForAverageBandwidth, DCFCLKRequiredForPeakBandwidth));
}
}



Reviewed-by: Aurabindo Pillai 


Re: [PATCH] drm/amd/display: change to_dal_irq_source_dnc32() storage class specifier to static

2022-06-27 Thread Aurabindo Pillai




On 2022-06-26 10:46, Tom Rix wrote:

sparse reports
drivers/gpu/drm/amd/amdgpu/../display/dc/irq/dcn32/irq_service_dcn32.c:39:20: 
warning: symbol 'to_dal_irq_source_dcn32' was not declared. Should it be static?

to_dal_irq_source_dnc32() is only referenced in irq_service_dnc32.c, so change 
its
storage class specifier to static.

Fixes: 0efd4374f6b4 ("drm/amd/display: add dcn32 IRQ changes")
Signed-off-by: Tom Rix 
---
  drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c 
b/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
index 3a213ca2f077..b1012fa1977b 100644
--- a/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
+++ b/drivers/gpu/drm/amd/display/dc/irq/dcn32/irq_service_dcn32.c
@@ -36,7 +36,7 @@
  
  #define DCN_BASE__INST0_SEG2   0x34C0
  
-enum dc_irq_source to_dal_irq_source_dcn32(

+static enum dc_irq_source to_dal_irq_source_dcn32(
struct irq_service *irq_service,
uint32_t src_id,
uint32_t ext_id)


Reviewed-by: Aurabindo Pillai 


Re: [PATCH] drm/amd/display: Remove unused globals FORCE_RATE and FORCE_LANE_COUNT

2022-06-27 Thread Aurabindo Pillai




On 2022-06-26 10:20, Tom Rix wrote:

sparse reports
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:3885:6: warning: 
symbol 'FORCE_RATE' was not declared. Should it be static?
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:3886:10: warning: 
symbol 'FORCE_LANE_COUNT' was not declared. Should it be static?

Neither of thse variables is used in dc_link_dp.c.  Reviewing the commit listed 
in
the fixes tag shows neither was used in the original patch.  So remove them.

Fixes: 265280b99822 ("drm/amd/display: add CLKMGR changes for DCN32/321")
Signed-off-by: Tom Rix 
---
  drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index be1dcb0a2a06..f3421f2bd52e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -3882,9 +3882,6 @@ static bool decide_mst_link_settings(const struct dc_link 
*link, struct dc_link_
return true;
  }
  
-bool FORCE_RATE = false;

-uint32_t FORCE_LANE_COUNT = 0;
-
  void decide_link_settings(struct dc_stream_state *stream,
struct dc_link_settings *link_setting)
  {



Reviewed-by: Aurabindo Pillai 


Re: [PATCH] drm/amd/display: remove unused function dc_link_perform_link_training

2021-05-10 Thread Aurabindo Pillai




On 2021-05-10 5:33 p.m., Rodrigo Siqueira wrote:

LGTM,

Jay, any comment?


None, LGTM, and this applied already.


Reviewed-by: Rodrigo Siqueira 

On 05/08, Rouven Czerwinski wrote:

This function is not used anywhere, remove it. It was added in
40dd6bd376a4 ("drm/amd/display: Linux Set/Read link rate and lane count
through debugfs") and moved in fe798de53a7a ("drm/amd/display: Move link
functions from dc to dc_link"), but a user is missing.

Signed-off-by: Rouven Czerwinski 
---
  drivers/gpu/drm/amd/display/dc/core/dc_link.c | 13 -
  drivers/gpu/drm/amd/display/dc/dc_link.h  |  3 ---
  2 files changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 3fb0cebd6938..55c5cf2264b3 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -3553,19 +3553,6 @@ void dc_link_set_drive_settings(struct dc *dc,
dc_link_dp_set_drive_settings(dc->links[i], lt_settings);
  }
  
-void dc_link_perform_link_training(struct dc *dc,

-  struct dc_link_settings *link_setting,
-  bool skip_video_pattern)
-{
-   int i;
-
-   for (i = 0; i < dc->link_count; i++)
-   dc_link_dp_perform_link_training(
-   dc->links[i],
-   link_setting,
-   skip_video_pattern);
-}
-
  void dc_link_set_preferred_link_settings(struct dc *dc,
 struct dc_link_settings *link_setting,
 struct dc_link *link)
diff --git a/drivers/gpu/drm/amd/display/dc/dc_link.h 
b/drivers/gpu/drm/amd/display/dc/dc_link.h
index fc5622ffec3d..45c927cd27ab 100644
--- a/drivers/gpu/drm/amd/display/dc/dc_link.h
+++ b/drivers/gpu/drm/amd/display/dc/dc_link.h
@@ -363,9 +363,6 @@ bool dc_link_is_hdcp22(struct dc_link *link, enum 
signal_type signal);
  void dc_link_set_drive_settings(struct dc *dc,
struct link_training_settings *lt_settings,
const struct dc_link *link);
-void dc_link_perform_link_training(struct dc *dc,
-  struct dc_link_settings *link_setting,
-  bool skip_video_pattern);
  void dc_link_set_preferred_link_settings(struct dc *dc,
 struct dc_link_settings *link_setting,
 struct dc_link *link);
--
2.31.1

___
amd-gfx mailing list
amd-...@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CRodrigo.Siqueira%40amd.com%7C9724972184d64ad6e7e008d913010665%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637561717696066502%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000sdata=vUFEeBJwjTDnI9l8MGDiW8%2FoX7LINZi%2FfD4A004QfLs%3Dreserved=0




--
Regards,
Aurabindo Pillai


Re: [PATCH 2/2] drm/amdgpu/display: buffer INTERRUPT_LOW_IRQ_CONTEXT interrupt work

2021-02-26 Thread Aurabindo Pillai
_highpri_wq, _data_add->work))
+    DRM_DEBUG("__func__: a work_struct is allocated and
queued, "
+ "src %d\n", irq_source);
+    else
+    DRM_ERROR("__func__: a new work_struct cannot be
queued, "
+  "something is wrong, src %d\n", irq_source);


A better error message that points to the failure would be nice, 
something along the lines of  "Failed to queue work for handling 
interrupt request from display"


With the comments addressed, this series is:

Reviewed-by: Aurabindo Pillai 


   }
     }





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



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


[PATCH v6 2/3] drm/amd/display: Add freesync video modes based on preferred modes

2021-02-12 Thread Aurabindo Pillai
[Why]
While possible for userspace to create and add custom mode based off the
optimized mode for the connected display which differs only in front porch
timing, this patch set adds a list of common video modes in advance.

The list of common video refresh rates is small, well known and the optimized
mode has specific requirements to be able to enable HW frame doubling and
tripling so it makes most sense to create the modes that video players will need
in advance. The optimized mode matches the preferred mode resolution but has the
highest refresh rate available to enable the largest front porch extension.

[How]
Find the optimized mode and store it on the connector so we can check it
later during our optimized modeset.

Prepopulate the mode list with a list of common video mades based on the
optimized mode (but with a longer front porch) if the panel doesn't support a
variant of the mode natively.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
Reviewed-by: Shashank Sharma 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 159 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
 2 files changed, 161 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 626a8cc92d65..c472905c7d72 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5187,6 +5187,59 @@ static void dm_enable_per_frame_crtc_master_sync(struct 
dc_state *context)
set_master_stream(context->streams, context->stream_count);
 }
 
+static struct drm_display_mode *
+get_highest_refresh_rate_mode(struct amdgpu_dm_connector *aconnector,
+ bool use_probed_modes)
+{
+   struct drm_display_mode *m, *m_pref = NULL;
+   u16 current_refresh, highest_refresh;
+   struct list_head *list_head = use_probed_modes ?
+   
>base.probed_modes :
+   >base.modes;
+
+   if (aconnector->freesync_vid_base.clock != 0)
+   return >freesync_vid_base;
+
+   /* Find the preferred mode */
+   list_for_each_entry (m, list_head, head) {
+   if (m->type & DRM_MODE_TYPE_PREFERRED) {
+   m_pref = m;
+   break;
+   }
+   }
+
+   if (!m_pref) {
+   /* Probably an EDID with no preferred mode. Fallback to first 
entry */
+   m_pref = list_first_entry_or_null(
+   >base.modes, struct drm_display_mode, head);
+   if (!m_pref) {
+   DRM_DEBUG_DRIVER("No preferred mode found in EDID\n");
+   return NULL;
+   }
+   }
+
+   highest_refresh = drm_mode_vrefresh(m_pref);
+
+   /*
+* Find the mode with highest refresh rate with same resolution.
+* For some monitors, preferred mode is not the mode with highest
+* supported refresh rate.
+*/
+   list_for_each_entry (m, list_head, head) {
+   current_refresh  = drm_mode_vrefresh(m);
+
+   if (m->hdisplay == m_pref->hdisplay &&
+   m->vdisplay == m_pref->vdisplay &&
+   highest_refresh < current_refresh) {
+   highest_refresh = current_refresh;
+   m_pref = m;
+   }
+   }
+
+   aconnector->freesync_vid_base = *m_pref;
+   return m_pref;
+}
+
 static struct dc_stream_state *
 create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
   const struct drm_display_mode *drm_mode,
@@ -7000,6 +7053,111 @@ static void amdgpu_dm_connector_ddc_get_modes(struct 
drm_connector *connector,
}
 }
 
+static bool is_duplicate_mode(struct amdgpu_dm_connector *aconnector,
+ struct drm_display_mode *mode)
+{
+   struct drm_display_mode *m;
+
+   list_for_each_entry (m, >base.probed_modes, head) {
+   if (drm_mode_equal(m, mode))
+   return true;
+   }
+
+   return false;
+}
+
+static uint add_fs_modes(struct amdgpu_dm_connector *aconnector)
+{
+   const struct drm_display_mode *m;
+   struct drm_display_mode *new_mode;
+   uint i;
+   uint32_t new_modes_count = 0;
+
+   /* Standard FPS values
+*
+* 23.976   - TV/NTSC
+* 24   - Cinema
+* 25   - TV/PAL
+* 29.97- TV/NTSC
+* 30   - TV/NTSC
+* 48   - Cinema HFR
+* 50   - TV/PAL
+* 60   - Commonly used
+* 48,72,96 - Multiples of 24
+*/
+   const uint32_t common_rates[] = { 23976, 24000, 25000, 29970, 3,
+48000, 5, 6, 72000, 96000 };
+
+  

[PATCH v6 1/3] drm/amd/display: Add module parameter for freesync video mode

2021-02-12 Thread Aurabindo Pillai
[Why]
This option shall be opt-in by default since it is a temporary solution
until long term solution is agreed upon which may require userspace interface
changes. This feature give the user a seamless experience when freesync aware
programs (media players for instance) switches to a compatible freesync mode
when playing videos. Enabling this feature also have the potential side effect
of causing higher power consumption due to running a mode with lower resolution
and base clock frequency with the highest base clock supported on the monitor as
per its advertised modes. There has been precedent of manufacturing modes in the
kernel. In AMDGPU, the existing usage are for common modes and scaling modes.
Other driver have a similar approach as well.

[How]
Adds a module parameter to enable freesync video mode modeset
optimization. Enabling this mode allows the driver to skip a full modeset when a
freesync compatible mode is requested by the userspace. This parameter will also
add some additional modes that are within the connected monitor's VRR range
corresponding to common video modes, which media players can use for a seamless
experience while making use of freesync.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e3d4d2dcb3a0..e242b7607dca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -179,6 +179,7 @@ extern int amdgpu_gpu_recovery;
 extern int amdgpu_emu_mode;
 extern uint amdgpu_smu_memory_pool_size;
 extern uint amdgpu_dc_feature_mask;
+extern uint amdgpu_freesync_vid_mode;
 extern uint amdgpu_dc_debug_mask;
 extern uint amdgpu_dm_abm_level;
 extern struct amdgpu_mgpu_info mgpu_info;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index b7ee587484b2..aefbe14c30fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -162,6 +162,7 @@ int amdgpu_mes;
 int amdgpu_noretry = -1;
 int amdgpu_force_asic_type = -1;
 int amdgpu_tmz;
+uint amdgpu_freesync_vid_mode;
 int amdgpu_reset_method = -1; /* auto */
 int amdgpu_num_kcq = -1;
 
@@ -790,6 +791,17 @@ module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 
0444);
 MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto, 0 = off (default), 1 = 
on)");
 module_param_named(tmz, amdgpu_tmz, int, 0444);
 
+/**
+ * DOC: freesync_video (uint)
+ * Enabled the optimization to adjust front porch timing to achieve seamless 
mode change experience
+ * when setting a freesync supported mode for which full modeset is not needed.
+ * The default value: 0 (off).
+ */
+MODULE_PARM_DESC(
+   freesync_video,
+   "Enable freesync modesetting optimization feature (0 = off (default), 1 
= on)");
+module_param_named(freesync_video, amdgpu_freesync_vid_mode, uint, 0444);
+
 /**
  * DOC: reset_method (int)
  * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 
= mode2, 4 = baco, 5 = pci)
-- 
2.30.0

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


[PATCH v6 3/3] drm/amd/display: Skip modeset for front porch change

2021-02-12 Thread Aurabindo Pillai
[Why]
A seamless transition between modes can be performed if the new incoming
mode has the same timing parameters as the optimized mode on a display with a
variable vtotal min/max.

Smooth video playback usecases can be enabled with this seamless transition by
switching to a new mode which has a refresh rate matching the video.

[How]
Skip full modeset if userspace requested a compatible freesync mode which only
differs in the front porch timing from the current mode.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 220 ++
 1 file changed, 180 insertions(+), 40 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 c472905c7d72..628fec855e14 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -212,6 +212,9 @@ static bool amdgpu_dm_psr_disable_all(struct 
amdgpu_display_manager *dm);
 static const struct drm_format_info *
 amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd);
 
+static bool
+is_timing_unchanged_for_freesync(struct drm_crtc_state *old_crtc_state,
+struct drm_crtc_state *new_crtc_state);
 /*
  * dm_vblank_get_counter
  *
@@ -335,6 +338,17 @@ static inline bool amdgpu_dm_vrr_active(struct 
dm_crtc_state *dm_state)
   dm_state->freesync_config.state == VRR_STATE_ACTIVE_FIXED;
 }
 
+static inline bool is_dc_timing_adjust_needed(struct dm_crtc_state *old_state,
+ struct dm_crtc_state *new_state)
+{
+   if (new_state->freesync_config.state ==  VRR_STATE_ACTIVE_FIXED)
+   return true;
+   else if (amdgpu_dm_vrr_active(old_state) != 
amdgpu_dm_vrr_active(new_state))
+   return true;
+   else
+   return false;
+}
+
 /**
  * dm_pflip_high_irq() - Handle pageflip interrupt
  * @interrupt_params: ignored
@@ -5008,19 +5022,16 @@ static void 
fill_stream_properties_from_drm_display_mode(
timing_out->hdmi_vic = hv_frame.vic;
}
 
-   timing_out->h_addressable = mode_in->crtc_hdisplay;
-   timing_out->h_total = mode_in->crtc_htotal;
-   timing_out->h_sync_width =
-   mode_in->crtc_hsync_end - mode_in->crtc_hsync_start;
-   timing_out->h_front_porch =
-   mode_in->crtc_hsync_start - mode_in->crtc_hdisplay;
-   timing_out->v_total = mode_in->crtc_vtotal;
-   timing_out->v_addressable = mode_in->crtc_vdisplay;
-   timing_out->v_front_porch =
-   mode_in->crtc_vsync_start - mode_in->crtc_vdisplay;
-   timing_out->v_sync_width =
-   mode_in->crtc_vsync_end - mode_in->crtc_vsync_start;
-   timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+   timing_out->h_addressable = mode_in->hdisplay;
+   timing_out->h_total = mode_in->htotal;
+   timing_out->h_sync_width = mode_in->hsync_end - mode_in->hsync_start;
+   timing_out->h_front_porch = mode_in->hsync_start - mode_in->hdisplay;
+   timing_out->v_total = mode_in->vtotal;
+   timing_out->v_addressable = mode_in->vdisplay;
+   timing_out->v_front_porch = mode_in->vsync_start - mode_in->vdisplay;
+   timing_out->v_sync_width = mode_in->vsync_end - mode_in->vsync_start;
+   timing_out->pix_clk_100hz = mode_in->clock * 10;
+
timing_out->aspect_ratio = get_aspect_ratio(mode_in);
 
stream->output_color_space = get_output_color_space(timing_out);
@@ -5240,6 +5251,33 @@ get_highest_refresh_rate_mode(struct amdgpu_dm_connector 
*aconnector,
return m_pref;
 }
 
+static bool is_freesync_video_mode(struct drm_display_mode *mode,
+  struct amdgpu_dm_connector *aconnector)
+{
+   struct drm_display_mode *high_mode;
+   int timing_diff;
+
+   high_mode = get_highest_refresh_rate_mode(aconnector, false);
+   if (!high_mode || !mode)
+   return false;
+
+   timing_diff = high_mode->vtotal - mode->vtotal;
+
+   if (high_mode->clock == 0 || high_mode->clock != mode->clock ||
+   high_mode->hdisplay != mode->hdisplay ||
+   high_mode->vdisplay != mode->vdisplay ||
+   high_mode->hsync_start != mode->hsync_start ||
+   high_mode->hsync_end != mode->hsync_end ||
+   high_mode->htotal != mode->htotal ||
+   high_mode->hskew != mode->hskew ||
+   high_mode->vscan != mode->vscan ||
+   high_mode->vsync_start - mode->vsync_start != timing_diff ||
+   high_mode->vsync_end - mode->vsync_end != timing_diff)
+   return false;
+   else
+   return true;
+}
+
 static struct dc_stream_state

[PATCH v6 0/3] Freesync video mode optimization

2021-02-12 Thread Aurabindo Pillai
Changes in V6
=

1) Skip modeset for front porch change
  * Minor optimizations

2) Add freesync video modes based on preferred modes:
  * Remove edid parsing and use the already parsed valuescached in dm
for the monitor's freesync range

3) Add module parameter for freesync video mode
  * Add info about potential higher power consumption when using this
feature in the commit message as a rationale for keeping this option
disabled by default.

Changes in V5
=

* More info in commit messages on the rationale of changes being added
to the kernel.
* Minor fixes

Changes in V4
=

1) Add module parameter for freesync video mode

* Change module parameter name to freesync_video

2) Add freesync video modes based on preferred modes:

* Cosmetic fixes
* Added comments about all modes being added by the driver.

3) Skip modeset for front porch change

* Added more conditions for checking freesync video mode

Changes in V3
=

1) Add freesync video modes based on preferred modes:

* Cache base freesync video mode during the first iteration to avoid
  iterating over modelist again later.
* Add mode for 60 fps videos

2) Skip modeset for front porch change

* Fixes for bug exposed by caching of modes.

Changes in V2
=

1) Add freesync video modes based on preferred modes:

* Remove check for connector type before adding freesync compatible
  modes as VRR support is being checked, and there is no reason to block
  freesync video support on eDP.
* use drm_mode_equal() instead of creating same functionality.
* Additional null pointer deference check
* Removed unnecessary variables.
* Cosmetic fixes.

2) Skip modeset for front porch change

* Remove _FSV string being appended to freesync video modes so as to not
  define new policies or break existing application that might use the
  mode name to figure out mode resolution.
* Remove unnecessary variables
* Cosmetic fixes.


This patchset enables freesync video mode usecase where the userspace
can request a freesync compatible video mode such that switching to this
mode does not trigger blanking.

This feature is guarded by a module parameter which is disabled by
default (due to slightly higher power consumption). Enabling this
paramters adds additional modes to the driver modelist, and also
enables the optimization to skip modeset when using one of these modes.

---
---

Aurabindo Pillai (3):
  drm/amd/display: Add module parameter for freesync video mode
  drm/amd/display: Add freesync video modes based on preferred modes
  drm/amd/display: Skip modeset for front porch change

 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  12 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 369 --
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
 4 files changed, 349 insertions(+), 35 deletions(-)

-- 
2.30.0

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


Re: [PATCH 3/3] drm/amd/display: Skip modeset for front porch change

2021-02-12 Thread Aurabindo Pillai



On 2021-02-08 10:06 a.m., Kazlauskas, Nicholas wrote:

On 2021-01-24 11:00 p.m., Aurabindo Pillai wrote:



On 2021-01-21 2:05 p.m., Kazlauskas, Nicholas wrote:

On 2021-01-19 10:50 a.m., Aurabindo Pillai wrote:

[Why]
A seamless transition between modes can be performed if the new 
incoming
mode has the same timing parameters as the optimized mode on a 
display with a

variable vtotal min/max.

Smooth video playback usecases can be enabled with this seamless 
transition by

switching to a new mode which has a refresh rate matching the video.

[How]
Skip full modeset if userspace requested a compatible freesync mode 
which only

differs in the front porch timing from the current mode.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 233 
+++---

  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 +
  2 files changed, 198 insertions(+), 36 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 aaef2fb528fd..d66494cdd8c8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -213,6 +213,9 @@ static bool amdgpu_dm_psr_disable_all(struct 
amdgpu_display_manager *dm);

  static const struct drm_format_info *
  amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd);
+static bool
+is_timing_unchanged_for_freesync(struct drm_crtc_state 
*old_crtc_state,

+ struct drm_crtc_state *new_crtc_state);
  /*
   * dm_vblank_get_counter
   *
@@ -4940,7 +4943,8 @@ static void 
fill_stream_properties_from_drm_display_mode(

  const struct drm_connector *connector,
  const struct drm_connector_state *connector_state,
  const struct dc_stream_state *old_stream,
-    int requested_bpc)
+    int requested_bpc,
+    bool is_in_modeset)
  {
  struct dc_crtc_timing *timing_out = >timing;
  const struct drm_display_info *info = >display_info;
@@ -4995,19 +4999,28 @@ static void 
fill_stream_properties_from_drm_display_mode(

  timing_out->hdmi_vic = hv_frame.vic;
  }
-    timing_out->h_addressable = mode_in->crtc_hdisplay;
-    timing_out->h_total = mode_in->crtc_htotal;
-    timing_out->h_sync_width =
-    mode_in->crtc_hsync_end - mode_in->crtc_hsync_start;
-    timing_out->h_front_porch =
-    mode_in->crtc_hsync_start - mode_in->crtc_hdisplay;
-    timing_out->v_total = mode_in->crtc_vtotal;
-    timing_out->v_addressable = mode_in->crtc_vdisplay;
-    timing_out->v_front_porch =
-    mode_in->crtc_vsync_start - mode_in->crtc_vdisplay;
-    timing_out->v_sync_width =
-    mode_in->crtc_vsync_end - mode_in->crtc_vsync_start;
-    timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+    if (is_in_modeset) {
+    timing_out->h_addressable = mode_in->hdisplay;
+    timing_out->h_total = mode_in->htotal;
+    timing_out->h_sync_width = mode_in->hsync_end - 
mode_in->hsync_start;
+    timing_out->h_front_porch = mode_in->hsync_start - 
mode_in->hdisplay;

+    timing_out->v_total = mode_in->vtotal;
+    timing_out->v_addressable = mode_in->vdisplay;
+    timing_out->v_front_porch = mode_in->vsync_start - 
mode_in->vdisplay;
+    timing_out->v_sync_width = mode_in->vsync_end - 
mode_in->vsync_start;

+    timing_out->pix_clk_100hz = mode_in->clock * 10;
+    } else {
+    timing_out->h_addressable = mode_in->crtc_hdisplay;
+    timing_out->h_total = mode_in->crtc_htotal;
+    timing_out->h_sync_width = mode_in->crtc_hsync_end - 
mode_in->crtc_hsync_start;
+    timing_out->h_front_porch = mode_in->crtc_hsync_start - 
mode_in->crtc_hdisplay;

+    timing_out->v_total = mode_in->crtc_vtotal;
+    timing_out->v_addressable = mode_in->crtc_vdisplay;
+    timing_out->v_front_porch = mode_in->crtc_vsync_start - 
mode_in->crtc_vdisplay;
+    timing_out->v_sync_width = mode_in->crtc_vsync_end - 
mode_in->crtc_vsync_start;

+    timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+    }
+


Not sure if I commented on this last time but I don't really 
understand what this is_in_modeset logic is supposed to be doing here.


This is so because create_stream_for_link() that ends up calling this 
function has two callers, one which is for stream validation in which 
the created stream is immediately discarded. The other is during 
modeset. Depending on these two cases, we want to copy the right 
timing parameters. With this method, major refactor wasn't necessary 
with the upper layers.


I don't understand why the timing parameters would change between what 
we validated and what we're planning on applying to the hardware. I 
think we should be validating the same thing

Re: [PATCH 3/3] drm/amd/display: Skip modeset for front porch change

2021-01-24 Thread Aurabindo Pillai



On 2021-01-21 2:05 p.m., Kazlauskas, Nicholas wrote:

On 2021-01-19 10:50 a.m., Aurabindo Pillai wrote:

[Why]
A seamless transition between modes can be performed if the new incoming
mode has the same timing parameters as the optimized mode on a display 
with a

variable vtotal min/max.

Smooth video playback usecases can be enabled with this seamless 
transition by

switching to a new mode which has a refresh rate matching the video.

[How]
Skip full modeset if userspace requested a compatible freesync mode 
which only

differs in the front porch timing from the current mode.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 233 +++---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 +
  2 files changed, 198 insertions(+), 36 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 aaef2fb528fd..d66494cdd8c8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -213,6 +213,9 @@ static bool amdgpu_dm_psr_disable_all(struct 
amdgpu_display_manager *dm);

  static const struct drm_format_info *
  amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd);
+static bool
+is_timing_unchanged_for_freesync(struct drm_crtc_state *old_crtc_state,
+ struct drm_crtc_state *new_crtc_state);
  /*
   * dm_vblank_get_counter
   *
@@ -4940,7 +4943,8 @@ static void 
fill_stream_properties_from_drm_display_mode(

  const struct drm_connector *connector,
  const struct drm_connector_state *connector_state,
  const struct dc_stream_state *old_stream,
-    int requested_bpc)
+    int requested_bpc,
+    bool is_in_modeset)
  {
  struct dc_crtc_timing *timing_out = >timing;
  const struct drm_display_info *info = >display_info;
@@ -4995,19 +4999,28 @@ static void 
fill_stream_properties_from_drm_display_mode(

  timing_out->hdmi_vic = hv_frame.vic;
  }
-    timing_out->h_addressable = mode_in->crtc_hdisplay;
-    timing_out->h_total = mode_in->crtc_htotal;
-    timing_out->h_sync_width =
-    mode_in->crtc_hsync_end - mode_in->crtc_hsync_start;
-    timing_out->h_front_porch =
-    mode_in->crtc_hsync_start - mode_in->crtc_hdisplay;
-    timing_out->v_total = mode_in->crtc_vtotal;
-    timing_out->v_addressable = mode_in->crtc_vdisplay;
-    timing_out->v_front_porch =
-    mode_in->crtc_vsync_start - mode_in->crtc_vdisplay;
-    timing_out->v_sync_width =
-    mode_in->crtc_vsync_end - mode_in->crtc_vsync_start;
-    timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+    if (is_in_modeset) {
+    timing_out->h_addressable = mode_in->hdisplay;
+    timing_out->h_total = mode_in->htotal;
+    timing_out->h_sync_width = mode_in->hsync_end - 
mode_in->hsync_start;
+    timing_out->h_front_porch = mode_in->hsync_start - 
mode_in->hdisplay;

+    timing_out->v_total = mode_in->vtotal;
+    timing_out->v_addressable = mode_in->vdisplay;
+    timing_out->v_front_porch = mode_in->vsync_start - 
mode_in->vdisplay;
+    timing_out->v_sync_width = mode_in->vsync_end - 
mode_in->vsync_start;

+    timing_out->pix_clk_100hz = mode_in->clock * 10;
+    } else {
+    timing_out->h_addressable = mode_in->crtc_hdisplay;
+    timing_out->h_total = mode_in->crtc_htotal;
+    timing_out->h_sync_width = mode_in->crtc_hsync_end - 
mode_in->crtc_hsync_start;
+    timing_out->h_front_porch = mode_in->crtc_hsync_start - 
mode_in->crtc_hdisplay;

+    timing_out->v_total = mode_in->crtc_vtotal;
+    timing_out->v_addressable = mode_in->crtc_vdisplay;
+    timing_out->v_front_porch = mode_in->crtc_vsync_start - 
mode_in->crtc_vdisplay;
+    timing_out->v_sync_width = mode_in->crtc_vsync_end - 
mode_in->crtc_vsync_start;

+    timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+    }
+


Not sure if I commented on this last time but I don't really understand 
what this is_in_modeset logic is supposed to be doing here.


This is so because create_stream_for_link() that ends up calling this 
function has two callers, one which is for stream validation in which 
the created stream is immediately discarded. The other is during 
modeset. Depending on these two cases, we want to copy the right timing 
parameters. With this method, major refactor wasn't necessary with the 
upper layers.




We should be modifying crtc_vsync_* for the generated modes, no? Not 
just the vsync_* parameters.


This is already handled with:

if (!dm_state)
drm_mode_set_crtcinfo(, 0);

if (dm_state && is_fs_vid_mode)
drm_mode_set_crtcinfo(_mode, 0);

[PATCH 2/3] drm/amd/display: Add freesync video modes based on preferred modes

2021-01-19 Thread Aurabindo Pillai
[Why]
While possible for userspace to create and add custom mode based off the
optimized mode for the connected display which differs only in front porch
timing, this patch set adds a list of common video modes in advance.

The list of common video refresh rates is small, well known and the optimized
mode has specific requirements to be able to enable HW frame doubling and
tripling so it makes most sense to create the modes that video players will need
in advance. The optimized mode matches the preferred mode resolution but has the
highest refresh rate available to enable the largest front porch extension.

[How]
Find the optimized mode and store it on the connector so we can check it
later during our optimized modeset.

Prepopulate the mode list with a list of common video mades based on the
optimized mode (but with a longer front porch) if the panel doesn't support a
variant of the mode natively.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
Reviewed-by: Shashank Sharma 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 170 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
 2 files changed, 172 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 245bd1284e5f..aaef2fb528fd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5174,6 +5174,59 @@ static void dm_enable_per_frame_crtc_master_sync(struct 
dc_state *context)
set_master_stream(context->streams, context->stream_count);
 }
 
+static struct drm_display_mode *
+get_highest_refresh_rate_mode(struct amdgpu_dm_connector *aconnector,
+ bool use_probed_modes)
+{
+   struct drm_display_mode *m, *m_pref = NULL;
+   u16 current_refresh, highest_refresh;
+   struct list_head *list_head = use_probed_modes ?
+   
>base.probed_modes :
+   >base.modes;
+
+   if (aconnector->freesync_vid_base.clock != 0)
+   return >freesync_vid_base;
+
+   /* Find the preferred mode */
+   list_for_each_entry (m, list_head, head) {
+   if (m->type & DRM_MODE_TYPE_PREFERRED) {
+   m_pref = m;
+   break;
+   }
+   }
+
+   if (!m_pref) {
+   /* Probably an EDID with no preferred mode. Fallback to first 
entry */
+   m_pref = list_first_entry_or_null(
+   >base.modes, struct drm_display_mode, head);
+   if (!m_pref) {
+   DRM_DEBUG_DRIVER("No preferred mode found in EDID\n");
+   return NULL;
+   }
+   }
+
+   highest_refresh = drm_mode_vrefresh(m_pref);
+
+   /*
+* Find the mode with highest refresh rate with same resolution.
+* For some monitors, preferred mode is not the mode with highest
+* supported refresh rate.
+*/
+   list_for_each_entry (m, list_head, head) {
+   current_refresh  = drm_mode_vrefresh(m);
+
+   if (m->hdisplay == m_pref->hdisplay &&
+   m->vdisplay == m_pref->vdisplay &&
+   highest_refresh < current_refresh) {
+   highest_refresh = current_refresh;
+   m_pref = m;
+   }
+   }
+
+   aconnector->freesync_vid_base = *m_pref;
+   return m_pref;
+}
+
 static struct dc_stream_state *
 create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
   const struct drm_display_mode *drm_mode,
@@ -6999,6 +7052,122 @@ static void amdgpu_dm_connector_ddc_get_modes(struct 
drm_connector *connector,
}
 }
 
+static bool is_duplicate_mode(struct amdgpu_dm_connector *aconnector,
+ struct drm_display_mode *mode)
+{
+   struct drm_display_mode *m;
+
+   list_for_each_entry (m, >base.probed_modes, head) {
+   if (drm_mode_equal(m, mode))
+   return true;
+   }
+
+   return false;
+}
+
+static uint add_fs_modes(struct amdgpu_dm_connector *aconnector,
+struct detailed_data_monitor_range *range)
+{
+   const struct drm_display_mode *m;
+   struct drm_display_mode *new_mode;
+   uint i;
+   uint32_t new_modes_count = 0;
+
+   /* Standard FPS values
+*
+* 23.976   - TV/NTSC
+* 24   - Cinema
+* 25   - TV/PAL
+* 29.97- TV/NTSC
+* 30   - TV/NTSC
+* 48   - Cinema HFR
+* 50   - TV/PAL
+* 60   - Commonly used
+* 48,72,96 - Multiples of 24
+*/
+   const uint32_t common_rates[] = { 23976, 24000, 25000, 29970, 3,
+   

[PATCH 3/3] drm/amd/display: Skip modeset for front porch change

2021-01-19 Thread Aurabindo Pillai
[Why]
A seamless transition between modes can be performed if the new incoming
mode has the same timing parameters as the optimized mode on a display with a
variable vtotal min/max.

Smooth video playback usecases can be enabled with this seamless transition by
switching to a new mode which has a refresh rate matching the video.

[How]
Skip full modeset if userspace requested a compatible freesync mode which only
differs in the front porch timing from the current mode.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 233 +++---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 +
 2 files changed, 198 insertions(+), 36 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 aaef2fb528fd..d66494cdd8c8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -213,6 +213,9 @@ static bool amdgpu_dm_psr_disable_all(struct 
amdgpu_display_manager *dm);
 static const struct drm_format_info *
 amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd);
 
+static bool
+is_timing_unchanged_for_freesync(struct drm_crtc_state *old_crtc_state,
+struct drm_crtc_state *new_crtc_state);
 /*
  * dm_vblank_get_counter
  *
@@ -4940,7 +4943,8 @@ static void fill_stream_properties_from_drm_display_mode(
const struct drm_connector *connector,
const struct drm_connector_state *connector_state,
const struct dc_stream_state *old_stream,
-   int requested_bpc)
+   int requested_bpc,
+   bool is_in_modeset)
 {
struct dc_crtc_timing *timing_out = >timing;
const struct drm_display_info *info = >display_info;
@@ -4995,19 +4999,28 @@ static void 
fill_stream_properties_from_drm_display_mode(
timing_out->hdmi_vic = hv_frame.vic;
}
 
-   timing_out->h_addressable = mode_in->crtc_hdisplay;
-   timing_out->h_total = mode_in->crtc_htotal;
-   timing_out->h_sync_width =
-   mode_in->crtc_hsync_end - mode_in->crtc_hsync_start;
-   timing_out->h_front_porch =
-   mode_in->crtc_hsync_start - mode_in->crtc_hdisplay;
-   timing_out->v_total = mode_in->crtc_vtotal;
-   timing_out->v_addressable = mode_in->crtc_vdisplay;
-   timing_out->v_front_porch =
-   mode_in->crtc_vsync_start - mode_in->crtc_vdisplay;
-   timing_out->v_sync_width =
-   mode_in->crtc_vsync_end - mode_in->crtc_vsync_start;
-   timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+   if (is_in_modeset) {
+   timing_out->h_addressable = mode_in->hdisplay;
+   timing_out->h_total = mode_in->htotal;
+   timing_out->h_sync_width = mode_in->hsync_end - 
mode_in->hsync_start;
+   timing_out->h_front_porch = mode_in->hsync_start - 
mode_in->hdisplay;
+   timing_out->v_total = mode_in->vtotal;
+   timing_out->v_addressable = mode_in->vdisplay;
+   timing_out->v_front_porch = mode_in->vsync_start - 
mode_in->vdisplay;
+   timing_out->v_sync_width = mode_in->vsync_end - 
mode_in->vsync_start;
+   timing_out->pix_clk_100hz = mode_in->clock * 10;
+   } else {
+   timing_out->h_addressable = mode_in->crtc_hdisplay;
+   timing_out->h_total = mode_in->crtc_htotal;
+   timing_out->h_sync_width = mode_in->crtc_hsync_end - 
mode_in->crtc_hsync_start;
+   timing_out->h_front_porch = mode_in->crtc_hsync_start - 
mode_in->crtc_hdisplay;
+   timing_out->v_total = mode_in->crtc_vtotal;
+   timing_out->v_addressable = mode_in->crtc_vdisplay;
+   timing_out->v_front_porch = mode_in->crtc_vsync_start - 
mode_in->crtc_vdisplay;
+   timing_out->v_sync_width = mode_in->crtc_vsync_end - 
mode_in->crtc_vsync_start;
+   timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+   }
+
timing_out->aspect_ratio = get_aspect_ratio(mode_in);
 
stream->output_color_space = get_output_color_space(timing_out);
@@ -5227,6 +5240,33 @@ get_highest_refresh_rate_mode(struct amdgpu_dm_connector 
*aconnector,
return m_pref;
 }
 
+static bool is_freesync_video_mode(struct drm_display_mode *mode,
+  struct amdgpu_dm_connector *aconnector)
+{
+   struct drm_display_mode *high_mode;
+   int timing_diff;
+
+   high_mode = get_highest_refresh_rate_mode(aconnector, false);
+   if (!high_mode || !mode)
+   return false;
+
+   timing_diff = high_mode->vtotal - mode->vtotal;
+
+   if (hi

[PATCH 1/3] drm/amd/display: Add module parameter for freesync video mode

2021-01-19 Thread Aurabindo Pillai
[Why]
This option shall be opt-in by default since it is a temporary solution
until long term solution is agreed upon which may require userspace interface
changes. There has been precedent of manufacturing modes in the kernel. In
AMDGPU, the existing usage are for common modes and scaling modes. Other driver
have a similar approach as well.

[How]
Adds a module parameter to enable freesync video mode modeset
optimization. Enabling this mode allows the driver to skip a full modeset when a
freesync compatible mode is requested by the userspace. This parameter will also
add some additional modes that are within the connected monitor's VRR range
corresponding to common video modes, which media players can use for a seamless
experience while making use of freesync.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 100a431f0792..770e42fcaa62 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -177,6 +177,7 @@ extern int amdgpu_gpu_recovery;
 extern int amdgpu_emu_mode;
 extern uint amdgpu_smu_memory_pool_size;
 extern uint amdgpu_dc_feature_mask;
+extern uint amdgpu_freesync_vid_mode;
 extern uint amdgpu_dc_debug_mask;
 extern uint amdgpu_dm_abm_level;
 extern struct amdgpu_mgpu_info mgpu_info;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index b48d7a3c2a11..5c6dc8362e6d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -158,6 +158,7 @@ int amdgpu_mes;
 int amdgpu_noretry = -1;
 int amdgpu_force_asic_type = -1;
 int amdgpu_tmz;
+uint amdgpu_freesync_vid_mode;
 int amdgpu_reset_method = -1; /* auto */
 int amdgpu_num_kcq = -1;
 
@@ -786,6 +787,17 @@ module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 
0444);
 MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto, 0 = off (default), 1 = 
on)");
 module_param_named(tmz, amdgpu_tmz, int, 0444);
 
+/**
+ * DOC: freesync_video (uint)
+ * Enabled the optimization to adjust front porch timing to achieve seamless 
mode change experience
+ * when setting a freesync supported mode for which full modeset is not needed.
+ * The default value: 0 (off).
+ */
+MODULE_PARM_DESC(
+   freesync_video,
+   "Enable freesync modesetting optimization feature (0 = off (default), 1 
= on)");
+module_param_named(freesync_video, amdgpu_freesync_vid_mode, uint, 0444);
+
 /**
  * DOC: reset_method (int)
  * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 
= mode2, 4 = baco)
-- 
2.30.0

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


[PATCH 0/3] Experimental freesync video mode optimization

2021-01-19 Thread Aurabindo Pillai
Changes in V5
=

* More info in commit messages on the rationale of changes being added
to the kernel.
* Minor fixes

Changes in V4
=

1) Add module parameter for freesync video mode

* Change module parameter name to freesync_video

2) Add freesync video modes based on preferred modes:

* Cosmetic fixes
* Added comments about all modes being added by the driver.

3) Skip modeset for front porch change

* Added more conditions for checking freesync video mode

Changes in V3
=

1) Add freesync video modes based on preferred modes:

* Cache base freesync video mode during the first iteration to avoid
  iterating over modelist again later.
* Add mode for 60 fps videos

2) Skip modeset for front porch change

* Fixes for bug exposed by caching of modes.

Changes in V2
=

1) Add freesync video modes based on preferred modes:

* Remove check for connector type before adding freesync compatible
  modes as VRR support is being checked, and there is no reason to block
  freesync video support on eDP.
* use drm_mode_equal() instead of creating same functionality.
* Additional null pointer deference check
* Removed unnecessary variables.
* Cosmetic fixes.

2) Skip modeset for front porch change

* Remove _FSV string being appended to freesync video modes so as to not
  define new policies or break existing application that might use the
  mode name to figure out mode resolution.
* Remove unnecessary variables
* Cosmetic fixes.

--

This patchset enables freesync video mode usecase where the userspace
can request a freesync compatible video mode such that switching to this
mode does not trigger blanking.

This feature is guarded by a module parameter which is disabled by
default. Enabling this paramters adds additional modes to the driver
modelist, and also enables the optimization to skip modeset when using
one of these modes.

--

Aurabindo Pillai (3):
  drm/amd/display: Add module parameter for freesync video mode
  drm/amd/display: Add freesync video modes based on preferred modes
  drm/amd/display: Skip modeset for front porch change

 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  12 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 401 --
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   3 +
 4 files changed, 382 insertions(+), 35 deletions(-)

-- 
2.30.0

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


Re: [PATCH v3 1/3] drm/amd/display: Add module parameter for freesync video mode

2021-01-18 Thread Aurabindo Pillai
On Thu, 2021-01-14 at 11:14 +0200, Pekka Paalanen wrote:
> 
> Hi,
> 
> please document somewhere that ends up in git history (commit
> message,
> code comments, description of the parameter would be the best but
> maybe
> there isn't enough space?) what Christian König explained in
> 
>  
> https://lists.freedesktop.org/archives/dri-devel/2020-December/291254.html
> 
> that this is a stop-gap feature intended to be removed as soon as
> possible (when a better solution comes up, which could be years).
> 
> So far I have not seen a single mention of this intention in your
> patch
> submissions, and I think it is very important to make known.

Hi,

Thanks for the headsup, I shall add the relevant info in the next
verison.

> 
> I also did not see an explanation of why this instead of
> manufacturing
> these video modes in userspace (an idea mentioned by Christian in the
> referenced email). I think that too should be part of a commit
> message.

This is an opt-in feature, which shall be superseded by a better
solution. We also add a set of common modes for scaling similarly.
Userspace can still add whatever mode they want. So I dont see a reason
why this cant be in the kernel.

--

Regards,
Aurabindo Pillai

> 
> 
> Thanks,
> pq


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


Re: [PATCH v3 3/3] drm/amd/display: Skip modeset for front porch change

2021-01-17 Thread Aurabindo Pillai



--

Thanks & Regards,
Aurabindo Pillai

On Wed, Jan 6, 2021 at 15:02, "Kazlauskas, Nicholas" 
 wrote:

On 2021-01-04 4:08 p.m., Aurabindo Pillai wrote:

[Why]
Inorder to enable freesync video mode, driver adds extra
modes based on preferred modes for common freesync frame rates.
When commiting these mode changes, a full modeset is not needed.
If the change in only in the front porch timing value, skip full
modeset and continue using the same stream.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 219 
+++---

  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 +
  2 files changed, 188 insertions(+), 32 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 aaef2fb528fd..315756207f0f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -213,6 +213,9 @@ static bool amdgpu_dm_psr_disable_all(struct 
amdgpu_display_manager *dm);

  static const struct drm_format_info *
  amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd);
  +static bool
+is_timing_unchanged_for_freesync(struct drm_crtc_state 
*old_crtc_state,

+struct drm_crtc_state *new_crtc_state);
  /*
   * dm_vblank_get_counter
   *
@@ -4940,7 +4943,8 @@ static void 
fill_stream_properties_from_drm_display_mode(

const struct drm_connector *connector,
const struct drm_connector_state *connector_state,
const struct dc_stream_state *old_stream,
-   int requested_bpc)
+   int requested_bpc,
+   bool is_in_modeset)
  {
struct dc_crtc_timing *timing_out = >timing;
const struct drm_display_info *info = >display_info;
@@ -4995,19 +4999,28 @@ static void 
fill_stream_properties_from_drm_display_mode(

timing_out->hdmi_vic = hv_frame.vic;
}
  -timing_out->h_addressable = mode_in->crtc_hdisplay;
-   timing_out->h_total = mode_in->crtc_htotal;
-   timing_out->h_sync_width =
-   mode_in->crtc_hsync_end - mode_in->crtc_hsync_start;
-   timing_out->h_front_porch =
-   mode_in->crtc_hsync_start - mode_in->crtc_hdisplay;
-   timing_out->v_total = mode_in->crtc_vtotal;
-   timing_out->v_addressable = mode_in->crtc_vdisplay;
-   timing_out->v_front_porch =
-   mode_in->crtc_vsync_start - mode_in->crtc_vdisplay;
-   timing_out->v_sync_width =
-   mode_in->crtc_vsync_end - mode_in->crtc_vsync_start;
-   timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+   if (is_in_modeset) {
+   timing_out->h_addressable = mode_in->hdisplay;
+   timing_out->h_total = mode_in->htotal;
+		timing_out->h_sync_width = mode_in->hsync_end - 
mode_in->hsync_start;
+		timing_out->h_front_porch = mode_in->hsync_start - 
mode_in->hdisplay;

+   timing_out->v_total = mode_in->vtotal;
+   timing_out->v_addressable = mode_in->vdisplay;
+		timing_out->v_front_porch = mode_in->vsync_start - 
mode_in->vdisplay;
+		timing_out->v_sync_width = mode_in->vsync_end - 
mode_in->vsync_start;

+   timing_out->pix_clk_100hz = mode_in->clock * 10;
+   } else {
+   timing_out->h_addressable = mode_in->crtc_hdisplay;
+   timing_out->h_total = mode_in->crtc_htotal;
+		timing_out->h_sync_width = mode_in->crtc_hsync_end - 
mode_in->crtc_hsync_start;
+		timing_out->h_front_porch = mode_in->crtc_hsync_start - 
mode_in->crtc_hdisplay;

+   timing_out->v_total = mode_in->crtc_vtotal;
+   timing_out->v_addressable = mode_in->crtc_vdisplay;
+		timing_out->v_front_porch = mode_in->crtc_vsync_start - 
mode_in->crtc_vdisplay;
+		timing_out->v_sync_width = mode_in->crtc_vsync_end - 
mode_in->crtc_vsync_start;

+   timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+   }
+
timing_out->aspect_ratio = get_aspect_ratio(mode_in);
    	stream->output_color_space = 
get_output_color_space(timing_out);
@@ -5227,6 +5240,33 @@ get_highest_refresh_rate_mode(struct 
amdgpu_dm_connector *aconnector,

return m_pref;
  }
  +static bool is_freesync_video_mode(struct drm_display_mode *mode,
+  struct amdgpu_dm_connector *aconnector)
+{
+   struct drm_display_mode *high_mode;
+   int timing_diff;
+
+   high_mode = get_highest_refresh_rate_mode(aconnector, false);
+   if (!high_mode || !mode)
+   return false;
+
+   timing_diff = high_mode->vtotal - mode->vtotal;
+
+   if (high_mode->clock == 0 || high_mode->clock != mode->clock ||
+   high_mode-&

[PATCH v3 2/3] drm/amd/display: Add freesync video modes based on preferred modes

2021-01-04 Thread Aurabindo Pillai
[Why]
If experimental freesync video mode module parameter is enabled, add
few extra display modes into the driver's mode list corresponding to common
video frame rates. When userspace sets these modes, no modeset will be
performed (if current mode was one of freesync modes or the base freesync mode
based off which timings have been generated for the rest of the freesync modes)
since these modes only differ from the base mode with front porch timing.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
Reviewed-by: Shashank Sharma 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 170 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
 2 files changed, 172 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 245bd1284e5f..aaef2fb528fd 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5174,6 +5174,59 @@ static void dm_enable_per_frame_crtc_master_sync(struct 
dc_state *context)
set_master_stream(context->streams, context->stream_count);
 }
 
+static struct drm_display_mode *
+get_highest_refresh_rate_mode(struct amdgpu_dm_connector *aconnector,
+ bool use_probed_modes)
+{
+   struct drm_display_mode *m, *m_pref = NULL;
+   u16 current_refresh, highest_refresh;
+   struct list_head *list_head = use_probed_modes ?
+   
>base.probed_modes :
+   >base.modes;
+
+   if (aconnector->freesync_vid_base.clock != 0)
+   return >freesync_vid_base;
+
+   /* Find the preferred mode */
+   list_for_each_entry (m, list_head, head) {
+   if (m->type & DRM_MODE_TYPE_PREFERRED) {
+   m_pref = m;
+   break;
+   }
+   }
+
+   if (!m_pref) {
+   /* Probably an EDID with no preferred mode. Fallback to first 
entry */
+   m_pref = list_first_entry_or_null(
+   >base.modes, struct drm_display_mode, head);
+   if (!m_pref) {
+   DRM_DEBUG_DRIVER("No preferred mode found in EDID\n");
+   return NULL;
+   }
+   }
+
+   highest_refresh = drm_mode_vrefresh(m_pref);
+
+   /*
+* Find the mode with highest refresh rate with same resolution.
+* For some monitors, preferred mode is not the mode with highest
+* supported refresh rate.
+*/
+   list_for_each_entry (m, list_head, head) {
+   current_refresh  = drm_mode_vrefresh(m);
+
+   if (m->hdisplay == m_pref->hdisplay &&
+   m->vdisplay == m_pref->vdisplay &&
+   highest_refresh < current_refresh) {
+   highest_refresh = current_refresh;
+   m_pref = m;
+   }
+   }
+
+   aconnector->freesync_vid_base = *m_pref;
+   return m_pref;
+}
+
 static struct dc_stream_state *
 create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
   const struct drm_display_mode *drm_mode,
@@ -6999,6 +7052,122 @@ static void amdgpu_dm_connector_ddc_get_modes(struct 
drm_connector *connector,
}
 }
 
+static bool is_duplicate_mode(struct amdgpu_dm_connector *aconnector,
+ struct drm_display_mode *mode)
+{
+   struct drm_display_mode *m;
+
+   list_for_each_entry (m, >base.probed_modes, head) {
+   if (drm_mode_equal(m, mode))
+   return true;
+   }
+
+   return false;
+}
+
+static uint add_fs_modes(struct amdgpu_dm_connector *aconnector,
+struct detailed_data_monitor_range *range)
+{
+   const struct drm_display_mode *m;
+   struct drm_display_mode *new_mode;
+   uint i;
+   uint32_t new_modes_count = 0;
+
+   /* Standard FPS values
+*
+* 23.976   - TV/NTSC
+* 24   - Cinema
+* 25   - TV/PAL
+* 29.97- TV/NTSC
+* 30   - TV/NTSC
+* 48   - Cinema HFR
+* 50   - TV/PAL
+* 60   - Commonly used
+* 48,72,96 - Multiples of 24
+*/
+   const uint32_t common_rates[] = { 23976, 24000, 25000, 29970, 3,
+48000, 5, 6, 72000, 96000 };
+
+   /*
+* Find mode with highest refresh rate with the same resolution
+* as the preferred mode. Some monitors report a preferred mode
+* with lower resolution than the highest refresh rate supported.
+*/
+
+   m = get_highest_refresh_rate_mode(aconnector, true);
+   if (!m)
+   return 0;
+
+   for (i = 0; i < ARRAY_SIZE(common_rates); i++) {
+ 

[PATCH v3 3/3] drm/amd/display: Skip modeset for front porch change

2021-01-04 Thread Aurabindo Pillai
[Why]
Inorder to enable freesync video mode, driver adds extra
modes based on preferred modes for common freesync frame rates.
When commiting these mode changes, a full modeset is not needed.
If the change in only in the front porch timing value, skip full
modeset and continue using the same stream.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 219 +++---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 +
 2 files changed, 188 insertions(+), 32 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 aaef2fb528fd..315756207f0f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -213,6 +213,9 @@ static bool amdgpu_dm_psr_disable_all(struct 
amdgpu_display_manager *dm);
 static const struct drm_format_info *
 amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd);
 
+static bool
+is_timing_unchanged_for_freesync(struct drm_crtc_state *old_crtc_state,
+struct drm_crtc_state *new_crtc_state);
 /*
  * dm_vblank_get_counter
  *
@@ -4940,7 +4943,8 @@ static void fill_stream_properties_from_drm_display_mode(
const struct drm_connector *connector,
const struct drm_connector_state *connector_state,
const struct dc_stream_state *old_stream,
-   int requested_bpc)
+   int requested_bpc,
+   bool is_in_modeset)
 {
struct dc_crtc_timing *timing_out = >timing;
const struct drm_display_info *info = >display_info;
@@ -4995,19 +4999,28 @@ static void 
fill_stream_properties_from_drm_display_mode(
timing_out->hdmi_vic = hv_frame.vic;
}
 
-   timing_out->h_addressable = mode_in->crtc_hdisplay;
-   timing_out->h_total = mode_in->crtc_htotal;
-   timing_out->h_sync_width =
-   mode_in->crtc_hsync_end - mode_in->crtc_hsync_start;
-   timing_out->h_front_porch =
-   mode_in->crtc_hsync_start - mode_in->crtc_hdisplay;
-   timing_out->v_total = mode_in->crtc_vtotal;
-   timing_out->v_addressable = mode_in->crtc_vdisplay;
-   timing_out->v_front_porch =
-   mode_in->crtc_vsync_start - mode_in->crtc_vdisplay;
-   timing_out->v_sync_width =
-   mode_in->crtc_vsync_end - mode_in->crtc_vsync_start;
-   timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+   if (is_in_modeset) {
+   timing_out->h_addressable = mode_in->hdisplay;
+   timing_out->h_total = mode_in->htotal;
+   timing_out->h_sync_width = mode_in->hsync_end - 
mode_in->hsync_start;
+   timing_out->h_front_porch = mode_in->hsync_start - 
mode_in->hdisplay;
+   timing_out->v_total = mode_in->vtotal;
+   timing_out->v_addressable = mode_in->vdisplay;
+   timing_out->v_front_porch = mode_in->vsync_start - 
mode_in->vdisplay;
+   timing_out->v_sync_width = mode_in->vsync_end - 
mode_in->vsync_start;
+   timing_out->pix_clk_100hz = mode_in->clock * 10;
+   } else {
+   timing_out->h_addressable = mode_in->crtc_hdisplay;
+   timing_out->h_total = mode_in->crtc_htotal;
+   timing_out->h_sync_width = mode_in->crtc_hsync_end - 
mode_in->crtc_hsync_start;
+   timing_out->h_front_porch = mode_in->crtc_hsync_start - 
mode_in->crtc_hdisplay;
+   timing_out->v_total = mode_in->crtc_vtotal;
+   timing_out->v_addressable = mode_in->crtc_vdisplay;
+   timing_out->v_front_porch = mode_in->crtc_vsync_start - 
mode_in->crtc_vdisplay;
+   timing_out->v_sync_width = mode_in->crtc_vsync_end - 
mode_in->crtc_vsync_start;
+   timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+   }
+
timing_out->aspect_ratio = get_aspect_ratio(mode_in);
 
stream->output_color_space = get_output_color_space(timing_out);
@@ -5227,6 +5240,33 @@ get_highest_refresh_rate_mode(struct amdgpu_dm_connector 
*aconnector,
return m_pref;
 }
 
+static bool is_freesync_video_mode(struct drm_display_mode *mode,
+  struct amdgpu_dm_connector *aconnector)
+{
+   struct drm_display_mode *high_mode;
+   int timing_diff;
+
+   high_mode = get_highest_refresh_rate_mode(aconnector, false);
+   if (!high_mode || !mode)
+   return false;
+
+   timing_diff = high_mode->vtotal - mode->vtotal;
+
+   if (high_mode->clock == 0 || high_mode->clock != mode->clock ||
+   high_mode->hdisplay != mode->hdisplay ||
+   high_mode->vdisplay != mode->vdisplay |

[PATCH v3 1/3] drm/amd/display: Add module parameter for freesync video mode

2021-01-04 Thread Aurabindo Pillai
[Why]
Adds a module parameter to enable experimental freesync video mode modeset
optimization. Enabling this mode allows the driver to skip a full modeset when
freesync compatible modes are requested by the userspace. This paramters also
adds some standard modes based on the connected monitor's VRR range.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 100a431f0792..12b13a90eddf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -177,6 +177,7 @@ extern int amdgpu_gpu_recovery;
 extern int amdgpu_emu_mode;
 extern uint amdgpu_smu_memory_pool_size;
 extern uint amdgpu_dc_feature_mask;
+extern uint amdgpu_exp_freesync_vid_mode;
 extern uint amdgpu_dc_debug_mask;
 extern uint amdgpu_dm_abm_level;
 extern struct amdgpu_mgpu_info mgpu_info;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index b48d7a3c2a11..2badbc8b2294 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -158,6 +158,7 @@ int amdgpu_mes;
 int amdgpu_noretry = -1;
 int amdgpu_force_asic_type = -1;
 int amdgpu_tmz;
+uint amdgpu_exp_freesync_vid_mode;
 int amdgpu_reset_method = -1; /* auto */
 int amdgpu_num_kcq = -1;
 
@@ -786,6 +787,17 @@ module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 
0444);
 MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto, 0 = off (default), 1 = 
on)");
 module_param_named(tmz, amdgpu_tmz, int, 0444);
 
+/**
+ * DOC: experimental_freesync_video (uint)
+ * Enabled the optimization to adjust front porch timing to achieve seamless 
mode change experience
+ * when setting a freesync supported mode for which full modeset is not needed.
+ * The default value: 0 (off).
+ */
+MODULE_PARM_DESC(
+   freesync_video,
+   "Enable freesync modesetting optimization feature (0 = off (default), 1 
= on)");
+module_param_named(freesync_video, amdgpu_exp_freesync_vid_mode, uint, 0444);
+
 /**
  * DOC: reset_method (int)
  * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 
= mode2, 4 = baco)
-- 
2.30.0

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


[PATCH v3 0/3] Experimental freesync video mode optimization

2021-01-04 Thread Aurabindo Pillai
Changes in V4
=

1) Add module parameter for freesync video mode

* Change module parameter name to freesync_video

2) Add freesync video modes based on preferred modes:

* Cosmetic fixes
* Added comments about all modes being added by the driver.

3) Skip modeset for front porch change

* Added more conditions for checking freesync video mode

Changes in V3
=

1) Add freesync video modes based on preferred modes:

* Cache base freesync video mode during the first iteration to avoid
  iterating over modelist again later.
* Add mode for 60 fps videos

2) Skip modeset for front porch change

* Fixes for bug exposed by caching of modes.

Changes in V2
=

1) Add freesync video modes based on preferred modes:

* Remove check for connector type before adding freesync compatible
  modes as VRR support is being checked, and there is no reason to block
  freesync video support on eDP.
* use drm_mode_equal() instead of creating same functionality.
* Additional null pointer deference check
* Removed unnecessary variables.
* Cosmetic fixes.

2) Skip modeset for front porch change

* Remove _FSV string being appended to freesync video modes so as to not
  define new policies or break existing application that might use the
  mode name to figure out mode resolution.
* Remove unnecessary variables
* Cosmetic fixes.

--

This patchset enables freesync video mode usecase where the userspace
can request a freesync compatible video mode such that switching to this
mode does not trigger blanking.

This feature is guarded by a module parameter which is disabled by
default. Enabling this paramters adds additional modes to the driver
modelist, and also enables the optimization to skip modeset when using
one of these modes.

--

Aurabindo Pillai (3):
  drm/amd/display: Add module parameter for freesync video mode
  drm/amd/display: Add freesync video modes based on preferred modes
  drm/amd/display: Skip modeset for front porch change

 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  12 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 389 --
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   3 +
 4 files changed, 373 insertions(+), 32 deletions(-)

-- 
2.30.0

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


Re: [PATCH v3 1/3] drm/amd/display: Add module parameter for freesync video mode

2020-12-17 Thread Aurabindo Pillai





On Thu, Dec 17, 2020 at 14:11, Alex Deucher  
wrote:

On Mon, Dec 14, 2020 at 5:21 PM Aurabindo Pillai
 wrote:


 [Why]
 Adds a module parameter to enable experimental freesync video mode 
modeset
 optimization. Enabling this mode allows the driver to skip a full 
modeset when
 freesync compatible modes are requested by the userspace. This 
paramters also

 adds some standard modes based on the connected monitor's VRR range.

 Signed-off-by: Aurabindo Pillai 
 Acked-by: Christian König 
 Reviewed-by: Shashank Sharma 
 ---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h |  1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 
  2 files changed, 13 insertions(+)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

 index eed5410947e9..e0942184efdd 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 @@ -177,6 +177,7 @@ extern int amdgpu_gpu_recovery;
  extern int amdgpu_emu_mode;
  extern uint amdgpu_smu_memory_pool_size;
  extern uint amdgpu_dc_feature_mask;
 +extern uint amdgpu_exp_freesync_vid_mode;
  extern uint amdgpu_dc_debug_mask;
  extern uint amdgpu_dm_abm_level;
  extern struct amdgpu_mgpu_info mgpu_info;
 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c

 index b2a1dd7581bf..ece51ecd53d1 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
 @@ -158,6 +158,7 @@ int amdgpu_mes;
  int amdgpu_noretry = -1;
  int amdgpu_force_asic_type = -1;
  int amdgpu_tmz;
 +uint amdgpu_exp_freesync_vid_mode;
  int amdgpu_reset_method = -1; /* auto */
  int amdgpu_num_kcq = -1;

 @@ -786,6 +787,17 @@ module_param_named(abmlevel, 
amdgpu_dm_abm_level, uint, 0444);
  MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto, 0 = off 
(default), 1 = on)");

  module_param_named(tmz, amdgpu_tmz, int, 0444);

 +/**
 + * DOC: experimental_freesync_video (uint)
 + * Enabled the optimization to adjust front porch timing to 
achieve seamless mode change experience
 + * when setting a freesync supported mode for which full modeset 
is not needed.

 + * The default value: 0 (off).
 + */
 +MODULE_PARM_DESC(
 +   experimental_freesync_video,
 +   "Enable freesync modesetting optimization feature (0 = off 
(default), 1 = on)");


Maybe just call this freesync_video so that we can change the default
if we decide to at some point.


Sure, will do.



Alex



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


Re: [PATCH v3 2/3] drm/amd/display: Add freesync video modes based on preferred modes

2020-12-15 Thread Aurabindo Pillai
On Tue, 2020-12-15 at 08:32 +0530, Shashank Sharma wrote:
> 
> On 15/12/20 3:50 am, Aurabindo Pillai wrote:
> > [Why]
> > If experimental freesync video mode module parameter is enabled,
> > add
> > few extra display modes into the driver's mode list corresponding
> > to common
> > video frame rates. When userspace sets these modes, no modeset will
> > be
> > performed (if current mode was one of freesync modes or the base
> > freesync mode
> > based off which timings have been generated for the rest of the
> > freesync modes)
> > since these modes only differ from the base mode with front porch
> > timing.
> > 
> > Signed-off-by: Aurabindo Pillai 
> > Acked-by: Christian König 
> > ---
> >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 167
> > ++
> >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
> >  2 files changed, 169 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 e7ee2467eadb..c1ffd33e9d83 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -5174,6 +5174,59 @@ static void
> > dm_enable_per_frame_crtc_master_sync(struct dc_state *context)
> > set_master_stream(context->streams, context->stream_count);
> >  }
> >  
> > +static struct drm_display_mode *
> > +get_highest_refresh_rate_mode(struct amdgpu_dm_connector
> > *aconnector,
> > + bool use_probed_modes)
> > +{
> > +   struct drm_display_mode *m, *m_pref = NULL;
> > +   u16 current_refresh, highest_refresh;
> > +   struct list_head *list_head = use_probed_modes ?
> > +   
> > >base.probed_modes :
> > +   
> > >base.modes;
> > +
> > +   if (aconnector->freesync_vid_base.clock != 0)
> > +   return >freesync_vid_base;
> > +
> > +   /* Find the preferred mode */
> > +   list_for_each_entry (m, list_head, head) {
> > +   if (m->type & DRM_MODE_TYPE_PREFERRED) {
> > +   m_pref = m;
> > +   break;
> > +   }
> > +   }
> > +
> > +   if (!m_pref) {
> > +   /* Probably an EDID with no preferred mode.
> > Fallback to first entry */
> > +   m_pref = list_first_entry_or_null(
> > +   >base.modes, struct
> > drm_display_mode, head);
> > +   if (!m_pref) {
> > +   DRM_DEBUG_DRIVER("No preferred mode found
> > in EDID\n");
> > +   return NULL;
> > +   }
> > +   }
> > +
> > +   highest_refresh = drm_mode_vrefresh(m_pref);
> > +
> > +   /*
> > +    * Find the mode with highest refresh rate with same
> > resolution.
> > +    * For some monitors, preferred mode is not the mode with
> > highest
> > +    * supported refresh rate.
> > +    */
> > +   list_for_each_entry (m, list_head, head) {
> > +   current_refresh  = drm_mode_vrefresh(m);
> > +
> > +   if (m->hdisplay == m_pref->hdisplay &&
> > +   m->vdisplay == m_pref->vdisplay &&
> > +   highest_refresh < current_refresh) {
> > +   highest_refresh = current_refresh;
> > +   m_pref = m;
> > +   }
> > +   }
> > +
> > +   aconnector->freesync_vid_base = *m_pref;
> > +   return m_pref;
> > +}
> > +
> >  static struct dc_stream_state *
> >  create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
> >    const struct drm_display_mode *drm_mode,
> > @@ -6999,6 +7052,119 @@ static void
> > amdgpu_dm_connector_ddc_get_modes(struct drm_connector *connector,
> > }
> >  }
> >  
> > +static bool is_duplicate_mode(struct amdgpu_dm_connector
> > *aconnector,
> > + struct drm_display_mode *mode)
> > +{
> > +   struct drm_display_mode *m;
> > +
> > +   list_for_each_entry (m, >base.probed_modes,
> > head) {
> > +   if (drm_mode_equal(m, mode))
> > +   return true;
> > +   }
> > +
> >

[PATCH v3 3/3] drm/amd/display: Skip modeset for front porch change

2020-12-14 Thread Aurabindo Pillai
[Why]
Inorder to enable freesync video mode, driver adds extra
modes based on preferred modes for common freesync frame rates.
When commiting these mode changes, a full modeset is not needed.
If the change in only in the front porch timing value, skip full
modeset and continue using the same stream.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 210 +++---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 +
 2 files changed, 179 insertions(+), 32 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 c1ffd33e9d83..4ba6be59dcd5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -213,6 +213,9 @@ static bool amdgpu_dm_psr_disable_all(struct 
amdgpu_display_manager *dm);
 static const struct drm_format_info *
 amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd);
 
+static bool
+is_timing_unchanged_for_freesync(struct drm_crtc_state *old_crtc_state,
+struct drm_crtc_state *new_crtc_state);
 /*
  * dm_vblank_get_counter
  *
@@ -4940,7 +4943,8 @@ static void fill_stream_properties_from_drm_display_mode(
const struct drm_connector *connector,
const struct drm_connector_state *connector_state,
const struct dc_stream_state *old_stream,
-   int requested_bpc)
+   int requested_bpc,
+   bool is_in_modeset)
 {
struct dc_crtc_timing *timing_out = >timing;
const struct drm_display_info *info = >display_info;
@@ -4995,19 +4999,28 @@ static void 
fill_stream_properties_from_drm_display_mode(
timing_out->hdmi_vic = hv_frame.vic;
}
 
-   timing_out->h_addressable = mode_in->crtc_hdisplay;
-   timing_out->h_total = mode_in->crtc_htotal;
-   timing_out->h_sync_width =
-   mode_in->crtc_hsync_end - mode_in->crtc_hsync_start;
-   timing_out->h_front_porch =
-   mode_in->crtc_hsync_start - mode_in->crtc_hdisplay;
-   timing_out->v_total = mode_in->crtc_vtotal;
-   timing_out->v_addressable = mode_in->crtc_vdisplay;
-   timing_out->v_front_porch =
-   mode_in->crtc_vsync_start - mode_in->crtc_vdisplay;
-   timing_out->v_sync_width =
-   mode_in->crtc_vsync_end - mode_in->crtc_vsync_start;
-   timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+   if (is_in_modeset) {
+   timing_out->h_addressable = mode_in->hdisplay;
+   timing_out->h_total = mode_in->htotal;
+   timing_out->h_sync_width = mode_in->hsync_end - 
mode_in->hsync_start;
+   timing_out->h_front_porch = mode_in->hsync_start - 
mode_in->hdisplay;
+   timing_out->v_total = mode_in->vtotal;
+   timing_out->v_addressable = mode_in->vdisplay;
+   timing_out->v_front_porch = mode_in->vsync_start - 
mode_in->vdisplay;
+   timing_out->v_sync_width = mode_in->vsync_end - 
mode_in->vsync_start;
+   timing_out->pix_clk_100hz = mode_in->clock * 10;
+   } else {
+   timing_out->h_addressable = mode_in->crtc_hdisplay;
+   timing_out->h_total = mode_in->crtc_htotal;
+   timing_out->h_sync_width = mode_in->crtc_hsync_end - 
mode_in->crtc_hsync_start;
+   timing_out->h_front_porch = mode_in->crtc_hsync_start - 
mode_in->crtc_hdisplay;
+   timing_out->v_total = mode_in->crtc_vtotal;
+   timing_out->v_addressable = mode_in->crtc_vdisplay;
+   timing_out->v_front_porch = mode_in->crtc_vsync_start - 
mode_in->crtc_vdisplay;
+   timing_out->v_sync_width = mode_in->crtc_vsync_end - 
mode_in->crtc_vsync_start;
+   timing_out->pix_clk_100hz = mode_in->crtc_clock * 10;
+   }
+
timing_out->aspect_ratio = get_aspect_ratio(mode_in);
 
stream->output_color_space = get_output_color_space(timing_out);
@@ -5227,6 +5240,24 @@ get_highest_refresh_rate_mode(struct amdgpu_dm_connector 
*aconnector,
return m_pref;
 }
 
+static bool is_freesync_video_mode(struct drm_display_mode *mode,
+  struct amdgpu_dm_connector *aconnector)
+{
+   struct drm_display_mode *high_mode;
+
+   high_mode = get_highest_refresh_rate_mode(aconnector, false);
+   if (!high_mode || !mode)
+   return false;
+
+   if (high_mode->clock == 0 ||
+   high_mode->hdisplay != mode->hdisplay ||
+   high_mode->vdisplay != mode->vdisplay ||
+   high_mode->clock != mode->clock)
+   return false;
+   else
+   return true;
+}
+

[PATCH v3 2/3] drm/amd/display: Add freesync video modes based on preferred modes

2020-12-14 Thread Aurabindo Pillai
[Why]
If experimental freesync video mode module parameter is enabled, add
few extra display modes into the driver's mode list corresponding to common
video frame rates. When userspace sets these modes, no modeset will be
performed (if current mode was one of freesync modes or the base freesync mode
based off which timings have been generated for the rest of the freesync modes)
since these modes only differ from the base mode with front porch timing.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 167 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   2 +
 2 files changed, 169 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 e7ee2467eadb..c1ffd33e9d83 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5174,6 +5174,59 @@ static void dm_enable_per_frame_crtc_master_sync(struct 
dc_state *context)
set_master_stream(context->streams, context->stream_count);
 }
 
+static struct drm_display_mode *
+get_highest_refresh_rate_mode(struct amdgpu_dm_connector *aconnector,
+ bool use_probed_modes)
+{
+   struct drm_display_mode *m, *m_pref = NULL;
+   u16 current_refresh, highest_refresh;
+   struct list_head *list_head = use_probed_modes ?
+   
>base.probed_modes :
+   >base.modes;
+
+   if (aconnector->freesync_vid_base.clock != 0)
+   return >freesync_vid_base;
+
+   /* Find the preferred mode */
+   list_for_each_entry (m, list_head, head) {
+   if (m->type & DRM_MODE_TYPE_PREFERRED) {
+   m_pref = m;
+   break;
+   }
+   }
+
+   if (!m_pref) {
+   /* Probably an EDID with no preferred mode. Fallback to first 
entry */
+   m_pref = list_first_entry_or_null(
+   >base.modes, struct drm_display_mode, head);
+   if (!m_pref) {
+   DRM_DEBUG_DRIVER("No preferred mode found in EDID\n");
+   return NULL;
+   }
+   }
+
+   highest_refresh = drm_mode_vrefresh(m_pref);
+
+   /*
+* Find the mode with highest refresh rate with same resolution.
+* For some monitors, preferred mode is not the mode with highest
+* supported refresh rate.
+*/
+   list_for_each_entry (m, list_head, head) {
+   current_refresh  = drm_mode_vrefresh(m);
+
+   if (m->hdisplay == m_pref->hdisplay &&
+   m->vdisplay == m_pref->vdisplay &&
+   highest_refresh < current_refresh) {
+   highest_refresh = current_refresh;
+   m_pref = m;
+   }
+   }
+
+   aconnector->freesync_vid_base = *m_pref;
+   return m_pref;
+}
+
 static struct dc_stream_state *
 create_stream_for_sink(struct amdgpu_dm_connector *aconnector,
   const struct drm_display_mode *drm_mode,
@@ -6999,6 +7052,119 @@ static void amdgpu_dm_connector_ddc_get_modes(struct 
drm_connector *connector,
}
 }
 
+static bool is_duplicate_mode(struct amdgpu_dm_connector *aconnector,
+ struct drm_display_mode *mode)
+{
+   struct drm_display_mode *m;
+
+   list_for_each_entry (m, >base.probed_modes, head) {
+   if (drm_mode_equal(m, mode))
+   return true;
+   }
+
+   return false;
+}
+
+static uint add_fs_modes(struct amdgpu_dm_connector *aconnector,
+struct detailed_data_monitor_range *range)
+{
+   const struct drm_display_mode *m;
+   struct drm_display_mode *new_mode;
+   uint i;
+   uint64_t target_vtotal, target_vtotal_diff;
+   uint32_t new_modes_count = 0;
+   uint64_t num, den;
+
+   /* Standard FPS values
+*
+* 23.976 - TV/NTSC
+* 24 - Cinema
+* 25 - TV/PAL
+* 29.97  - TV/NTSC
+* 30 - TV/NTSC
+* 48 - Cinema HFR
+* 50 - TV/PAL
+*/
+   const uint32_t common_rates[] = { 23976, 24000, 25000, 29970, 3,
+48000, 5, 6, 72000, 96000 };
+
+   /*
+* Find mode with highest refresh rate with the same resolution
+* as the preferred mode. Some monitors report a preferred mode
+* with lower resolution than the highest refresh rate supported.
+*/
+
+   m = get_highest_refresh_rate_mode(aconnector, true);
+   if (!m)
+   return 0;
+
+   for (i = 0; i < ARRAY_SIZE(common_rates); i++) {
+   if (d

[PATCH v3 1/3] drm/amd/display: Add module parameter for freesync video mode

2020-12-14 Thread Aurabindo Pillai
[Why]
Adds a module parameter to enable experimental freesync video mode modeset
optimization. Enabling this mode allows the driver to skip a full modeset when
freesync compatible modes are requested by the userspace. This paramters also
adds some standard modes based on the connected monitor's VRR range.

Signed-off-by: Aurabindo Pillai 
Acked-by: Christian König 
Reviewed-by: Shashank Sharma 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index eed5410947e9..e0942184efdd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -177,6 +177,7 @@ extern int amdgpu_gpu_recovery;
 extern int amdgpu_emu_mode;
 extern uint amdgpu_smu_memory_pool_size;
 extern uint amdgpu_dc_feature_mask;
+extern uint amdgpu_exp_freesync_vid_mode;
 extern uint amdgpu_dc_debug_mask;
 extern uint amdgpu_dm_abm_level;
 extern struct amdgpu_mgpu_info mgpu_info;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index b2a1dd7581bf..ece51ecd53d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -158,6 +158,7 @@ int amdgpu_mes;
 int amdgpu_noretry = -1;
 int amdgpu_force_asic_type = -1;
 int amdgpu_tmz;
+uint amdgpu_exp_freesync_vid_mode;
 int amdgpu_reset_method = -1; /* auto */
 int amdgpu_num_kcq = -1;
 
@@ -786,6 +787,17 @@ module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 
0444);
 MODULE_PARM_DESC(tmz, "Enable TMZ feature (-1 = auto, 0 = off (default), 1 = 
on)");
 module_param_named(tmz, amdgpu_tmz, int, 0444);
 
+/**
+ * DOC: experimental_freesync_video (uint)
+ * Enabled the optimization to adjust front porch timing to achieve seamless 
mode change experience
+ * when setting a freesync supported mode for which full modeset is not needed.
+ * The default value: 0 (off).
+ */
+MODULE_PARM_DESC(
+   experimental_freesync_video,
+   "Enable freesync modesetting optimization feature (0 = off (default), 1 
= on)");
+module_param_named(experimental_freesync_video, amdgpu_exp_freesync_vid_mode, 
uint, 0444);
+
 /**
  * DOC: reset_method (int)
  * GPU reset method (-1 = auto (default), 0 = legacy, 1 = mode0, 2 = mode1, 3 
= mode2, 4 = baco)
-- 
2.29.2

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


[PATCH v3 0/3] Experimental freesync video mode optimization

2020-12-14 Thread Aurabindo Pillai
Changes in V3
=

1) Add freesync video modes based on preferred modes:

* Cache base freesync video mode during the first iteration to avoid
  iterating over modelist again later.
* Add mode for 60 fps videos

2) Skip modeset for front porch change

* Fixes for bug exposed by caching of modes.

Changes in V2
=

1) Add freesync video modes based on preferred modes:

* Remove check for connector type before adding freesync compatible
  modes as VRR support is being checked, and there is no reason to block
  freesync video support on eDP.
* use drm_mode_equal() instead of creating same functionality.
* Additional null pointer deference check
* Removed unnecessary variables.
* Cosmetic fixes.

2) Skip modeset for front porch change

* Remove _FSV string being appended to freesync video modes so as to not
  define new policies or break existing application that might use the
  mode name to figure out mode resolution.
* Remove unnecessary variables
* Cosmetic fixes.

--

This patchset enables freesync video mode usecase where the userspace
can request a freesync compatible video mode such that switching to this
mode does not trigger blanking.

This feature is guarded by a module parameter which is disabled by
default. Enabling this paramters adds additional modes to the driver
modelist, and also enables the optimization to skip modeset when using
one of these modes.

--

Aurabindo Pillai (3):
  drm/amd/display: Add module parameter for freesync video mode
  drm/amd/display: Add freesync video modes based on preferred modes
  drm/amd/display: Skip modeset for front porch change

 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  12 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 377 --
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   3 +
 4 files changed, 361 insertions(+), 32 deletions(-)

-- 
2.29.2

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


Re: [PATCH] drm/amd/amdkfd: Fix large framesize for kfd_smi_ev_read()

2020-05-21 Thread Aurabindo Pillai
On 05/20, Felix Kuehling wrote:
> Am 2020-05-20 um 9:53 a.m. schrieb Aurabindo Pillai:
> > The buffer allocated is of 1024 bytes. Allocate this from
> > heap instead of stack.
> >
> > Also remove check for stack size since we're allocating from heap
> >
> > Signed-off-by: Aurabindo Pillai 
> > Tested-by: Amber Lin 
> 
> See one comment inline. With that fixed, the patch is
> 
> Reviewed-by: Felix Kuehling 
> 
> 
> > ---
> >  drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 26 +++--
> >  1 file changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c 
> > b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> > index f5fd18eacf0d..5aebe169f8c6 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> > @@ -77,9 +77,11 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char 
> > __user *user,
> > int ret;
> > size_t to_copy;
> > struct kfd_smi_client *client = filep->private_data;
> > -   unsigned char buf[MAX_KFIFO_SIZE];
> > +   unsigned char *buf;
> >  
> > -   BUILD_BUG_ON(MAX_KFIFO_SIZE > 1024);
> > +   buf = kzalloc(MAX_KFIFO_SIZE * sizeof(*buf), GFP_KERNEL);
> 
> kzalloc is not necessary here, you could use kmalloc. The part of that
> allocation that matters will be overwritten by kfifo_out.
> 
> Regards,
>   Felix
> 
>

Thank you Felix, Alex for the review. I shall make that change and submit it.


Thanks & Regards,
Aurabindo


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


[PATCH] drm/amd/amdkfd: Fix large framesize for kfd_smi_ev_read()

2020-05-20 Thread Aurabindo Pillai
The buffer allocated is of 1024 bytes. Allocate this from
heap instead of stack.

Also remove check for stack size since we're allocating from heap

Signed-off-by: Aurabindo Pillai 
Tested-by: Amber Lin 
---
 drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 26 +++--
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
index f5fd18eacf0d..5aebe169f8c6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
@@ -77,9 +77,11 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char 
__user *user,
int ret;
size_t to_copy;
struct kfd_smi_client *client = filep->private_data;
-   unsigned char buf[MAX_KFIFO_SIZE];
+   unsigned char *buf;
 
-   BUILD_BUG_ON(MAX_KFIFO_SIZE > 1024);
+   buf = kzalloc(MAX_KFIFO_SIZE * sizeof(*buf), GFP_KERNEL);
+   if (!buf)
+   return -ENOMEM;
 
/* kfifo_to_user can sleep so we can't use spinlock protection around
 * it. Instead, we kfifo out as spinlocked then copy them to the user.
@@ -88,19 +90,29 @@ static ssize_t kfd_smi_ev_read(struct file *filep, char 
__user *user,
to_copy = kfifo_len(>fifo);
if (!to_copy) {
spin_unlock(>lock);
-   return -EAGAIN;
+   ret = -EAGAIN;
+   goto ret_err;
}
to_copy = min3(size, sizeof(buf), to_copy);
ret = kfifo_out(>fifo, buf, to_copy);
spin_unlock(>lock);
-   if (ret <= 0)
-   return -EAGAIN;
+   if (ret <= 0) {
+   ret = -EAGAIN;
+   goto ret_err;
+   }
 
ret = copy_to_user(user, buf, to_copy);
-   if (ret)
-   return -EFAULT;
+   if (ret) {
+   ret = -EFAULT;
+   goto ret_err;
+   }
 
+   kfree(buf);
return to_copy;
+
+ret_err:
+   kfree(buf);
+   return ret;
 }
 
 static ssize_t kfd_smi_ev_write(struct file *filep, const char __user *user,
-- 
2.25.1

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


Re: [PATCH] drm: dpcd: Print more useful information during error

2020-04-16 Thread Aurabindo Pillai
When DPCD access errors occur, knowing the register and request
associated with the error helps debugging, so print the
details in the debug message.

Signed-off-by: Aurabindo Pillai 
---
 drivers/gpu/drm/drm_dp_helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index c6fbe6e6b..8aafc01f5 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -257,7 +257,9 @@ static int drm_dp_dpcd_access(struct drm_dp_aux
*aux, u8 request,
err = ret;
}

-   DRM_DEBUG_KMS("Too many retries, giving up. First error: %d\n", err);
+   DRM_DEBUG_KMS("dpcd: Too many retries, giving up. First error: %d,"
+ " address: 0x%x, request: 0x%x, size:%zu\n",
+ err, msg.address, msg.request, msg.size);
ret = err;

 unlock:
--
2.26.0

On Tue, Apr 14, 2020 at 7:04 AM Jani Nikula  wrote:
>
> On Thu, 09 Apr 2020, Aurabindo Pillai  wrote:
> > When DPCD access errors occur, knowing the register and request
> > associated with the error helps debugging, so print the
> > details in the debug message.
> >
> > Signed-off-by: Aurabindo Pillai 
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c 
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index a5364b519..545606aac 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -257,7 +257,9 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, 
> > u8 request,
> >   err = ret;
> >   }
> >
> > - DRM_DEBUG_KMS("Too many retries, giving up. First error: %d\n", err);
> > + DRM_DEBUG_KMS("dpcd: Too many retries, giving up. First error: %d\t"
> > +   "address: %x\trequest: %x\t size:%zu\n",
> > +   err, msg.address, msg.request, msg.size);
>
> Nitpicks, please don't add tabs, maybe use commas instead, and please
> add 0x in front of hex.
>
> BR,
> Jani.
>
>
> >   ret = err;
> >
> >  unlock:
>
> --
> Jani Nikula, Intel Open Source Graphics Center



-- 

Thanks and Regards,

Aurabindo J Pillai
https://aurabindo.in
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] drm: dpcd: Print more useful information during error

2020-04-10 Thread Aurabindo Pillai
When DPCD access errors occur, knowing the register and request
associated with the error helps debugging, so print the
details in the debug message.

Signed-off-by: Aurabindo Pillai 
---
 drivers/gpu/drm/drm_dp_helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index a5364b519..545606aac 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -257,7 +257,9 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 
request,
err = ret;
}
 
-   DRM_DEBUG_KMS("Too many retries, giving up. First error: %d\n", err);
+   DRM_DEBUG_KMS("dpcd: Too many retries, giving up. First error: %d\t"
+ "address: %x\trequest: %x\t size:%zu\n",
+ err, msg.address, msg.request, msg.size);
ret = err;
 
 unlock:
-- 
2.26.0

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


[PATCH v2 3/3] drm/amd/amdgpu: remove hardcoded module name in prints

2020-04-09 Thread Aurabindo Pillai
Let format prefixes take care of printing the module name
through pr_fmt and dev_fmt definitions.

Signed-off-by: Aurabindo Pillai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   | 4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c  | 2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c| 2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c| 2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c| 2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index fa8ac9d19..4afd4ef54 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -325,13 +325,13 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm)
ret = amdgpu_vm_validate_pt_bos(adev, vm, amdgpu_amdkfd_validate,
);
if (ret) {
-   pr_err("amdgpu: failed to validate PT BOs\n");
+   pr_err("failed to validate PT BOs\n");
return ret;
}
 
ret = amdgpu_amdkfd_validate(, pd);
if (ret) {
-   pr_err("amdgpu: failed to validate PD\n");
+   pr_err("failed to validate PD\n");
return ret;
}
 
@@ -340,7 +340,7 @@ static int vm_validate_pt_pd_bos(struct amdgpu_vm *vm)
if (vm->use_cpu_for_update) {
ret = amdgpu_bo_kmap(pd, NULL);
if (ret) {
-   pr_err("amdgpu: failed to kmap PD, ret=%d\n", ret);
+   pr_err("failed to kmap PD, ret=%d\n", ret);
return ret;
}
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b8975857d..0a8c4a266 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1092,7 +1092,7 @@ static void amdgpu_switcheroo_set_state(struct pci_dev 
*pdev, enum vga_switchero
return;
 
if (state == VGA_SWITCHEROO_ON) {
-   pr_info("amdgpu: switched on\n");
+   pr_info("switched on\n");
/* don't suspend or resume card normally */
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
@@ -1106,7 +1106,7 @@ static void amdgpu_switcheroo_set_state(struct pci_dev 
*pdev, enum vga_switchero
dev->switch_power_state = DRM_SWITCH_POWER_ON;
drm_kms_helper_poll_enable(dev);
} else {
-   pr_info("amdgpu: switched off\n");
+   pr_info("switched off\n");
drm_kms_helper_poll_disable(dev);
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
amdgpu_device_suspend(dev, true);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 5ed4227f3..0cc4c67f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -260,7 +260,7 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags);
if (nvec > 0) {
adev->irq.msi_enabled = true;
-   dev_dbg(adev->dev, "amdgpu: using MSI/MSI-X.\n");
+   dev_dbg(adev->dev, "using MSI/MSI-X.\n");
}
}
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index b20503935..c1a530dbe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -858,7 +858,7 @@ static int gmc_v6_0_sw_init(void *handle)
 
r = dma_set_mask_and_coherent(adev->dev, DMA_BIT_MASK(44));
if (r) {
-   dev_warn(adev->dev, "amdgpu: No suitable DMA available.\n");
+   dev_warn(adev->dev, "No suitable DMA available.\n");
return r;
}
adev->need_swiotlb = drm_need_swiotlb(44);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 9da9596a3..e8529e244 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -1019,7 +1019,7 @@ static int gmc_v7_0_sw_init(void *handle)
 
r = dma_set_mask_and_coherent(adev->dev, DMA_BIT_MASK(40));
if (r) {
-   pr_warn("amdgpu: No suitable DMA available\n");
+   pr_warn("No suitable DMA available\n");
return r;
}
adev->need_swiotlb = drm_need_swiotlb(40);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc

[PATCH] drm/amd/amdgpu: add prefix for pr_* prints

2020-04-09 Thread Aurabindo Pillai
amdgpu uses lots of pr_* calls for printing error messages.
With this prefix, errors shall be more obvious to the end
use regarding its origin, and may help debugging.

Prefix format:

[xxx.x] amdgpu: ...

Signed-off-by: Aurabindo Pillai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index da3bcff61..67d654a89 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -28,6 +28,12 @@
 #ifndef __AMDGPU_H__
 #define __AMDGPU_H__
 
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) "amdgpu: " fmt
+
 #include "amdgpu_ctx.h"
 
 #include 
-- 
2.26.0

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


[PATCH v2 0/3] drm/amd/amdgpu: Add print format prefix

2020-04-09 Thread Aurabindo Pillai
Changes in v2

* Add dev_fmt format prefix
* Removed hardcoded module names in pr_* and dev_* prints

Aurabindo Pillai (3):
  drm/amd/amdgpu: add prefix for pr_* prints
  drm/amd/amdgpu: add print prefix for dev_* variants
  drm/amd/amdgpu: remove hardcoded module name in prints

 drivers/gpu/drm/amd/amdgpu/amdgpu.h  | 12 
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c |  6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c|  2 +-
 7 files changed, 21 insertions(+), 9 deletions(-)

-- 
2.26.0

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


Re: [PATCH] drm/amd/amdgpu: add prefix for pr_* prints

2020-04-09 Thread Aurabindo Pillai
Hi Joe,

On Wed, Apr 8, 2020 at 11:37 AM Joe Perches  wrote:
>
> All the embedded uses of "amdgpu:" in logging
> messages should also be deleted.
>
> $ git grep -P '(?:dev_|pr_).*"amdgpu:' drivers/gpu/drm/amd/amdgpu/
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:   
> pr_err("amdgpu: failed to validate PT BOs\n");
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:   
> pr_err("amdgpu: failed to validate PD\n");
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:   
> pr_err("amdgpu: failed to kmap PD, ret=%d\n", ret);
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: pr_info("amdgpu: 
> switched on\n");
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: pr_info("amdgpu: 
> switched off\n");
> drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:
> dev_dbg(adev->dev, "amdgpu: using MSI/MSI-X.\n");
> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c:  dev_warn(adev->dev, "amdgpu: 
> No suitable DMA available.\n");
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:  pr_warn("amdgpu: No suitable 
> DMA available\n");
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:  pr_warn("amdgpu: No suitable 
> DMA available\n");
>
>
>

Thanks for the heads up, I shall submit another set with those changes
you suggested.

-- 

Thanks and Regards,

Aurabindo J Pillai
https://aurabindo.in
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 2/3] drm/amd/amdgpu: add print prefix for dev_* variants

2020-04-09 Thread Aurabindo Pillai
Define dev_fmt macro for informative print messages

Signed-off-by: Aurabindo Pillai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 67d654a89..7a52d37e6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -34,6 +34,12 @@
 
 #define pr_fmt(fmt) "amdgpu: " fmt
 
+#ifdef dev_fmt
+#undef dev_fmt
+#endif
+
+#define dev_fmt(fmt) "amdgpu: " fmt
+
 #include "amdgpu_ctx.h"
 
 #include 
-- 
2.26.0

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


[PATCH v2 1/3] drm/amd/amdgpu: add prefix for pr_* prints

2020-04-09 Thread Aurabindo Pillai
amdgpu uses lots of pr_* calls for printing error messages.
With this prefix, errors shall be more obvious to the end
use regarding its origin, and may help debugging.

Prefix format:

[xxx.x] amdgpu: ...

Signed-off-by: Aurabindo Pillai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index da3bcff61..67d654a89 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -28,6 +28,12 @@
 #ifndef __AMDGPU_H__
 #define __AMDGPU_H__
 
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) "amdgpu: " fmt
+
 #include "amdgpu_ctx.h"
 
 #include 
-- 
2.26.0

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


[PATCH] amdgpu_kms: Remove unnecessary condition check

2020-04-08 Thread Aurabindo Pillai
Execution will only reach here if the asserted condition is true.
Hence there is no need for the additional check.

Signed-off-by: Aurabindo Pillai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 60591dbc2..9fedfa531 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -179,12 +179,10 @@ int amdgpu_driver_load_kms(struct drm_device *dev, 
unsigned long flags)
/* Call ACPI methods: require modeset init
 * but failure is not fatal
 */
-   if (!r) {
-   acpi_status = amdgpu_acpi_init(adev);
-   if (acpi_status)
-   dev_dbg(>pdev->dev,
-   "Error during ACPI methods call\n");
-   }
+
+   acpi_status = amdgpu_acpi_init(adev);
+   if (acpi_status)
+   dev_dbg(>pdev->dev, "Error during ACPI methods call\n");
 
if (adev->runpm) {
dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
-- 
2.26.0

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