RE: [PATCH v8 1/7] drm/i915: Define and compute Transcoder CMRR registers

2024-05-13 Thread Jani Nikula
On Mon, 13 May 2024, "Murthy, Arun R"  wrote:
>> -Original Message-
>> From: Intel-gfx  On Behalf Of Mitul
>> Golani
>> Sent: Thursday, May 9, 2024 1:28 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Shankar, Uma ; Nikula, Jani
>> 
>> Subject: [PATCH v8 1/7] drm/i915: Define and compute Transcoder CMRR
>> registers
>>
>> Add register definitions for Transcoder Fixed Average Vtotal mode/CMRR
>> function, with the necessary bitfields.
>> Compute these registers when CMRR is enabled, extending Adaptive refresh
>> rate capabilities.
>>
>> --v2:
>> - Use intel_de_read64_2x32 in intel_vrr_get_config. [Jani]
>> - Fix indent and order based on register offset. [Jani]
>>
>> --v3:
>> - Removing RFC tag.
>>
>> --v4:
>> - Update place holder for CMRR register definition. (Jani)
>>
>> Signed-off-by: Mitul Golani 
>> ---
>>  drivers/gpu/drm/i915/display/intel_display.c  | 23 ++-
>>  .../drm/i915/display/intel_display_types.h|  6 +
>>  drivers/gpu/drm/i915/display/intel_vrr.c  | 22 ++
>>  drivers/gpu/drm/i915/i915_reg.h   | 10 
> Please create a new header file to add the CMRR related register definitions.

Or just intel_vrr_regs.h?

BR,
Jani.

>
> Thanks and Regards,
> Arun R Murthy
> 
>
>>  4 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>> b/drivers/gpu/drm/i915/display/intel_display.c
>> index ef986b508431..258a78447fba 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -1001,6 +1001,13 @@ static bool vrr_params_changed(const struct
>> intel_crtc_state *old_crtc_state,
>>   old_crtc_state->vrr.pipeline_full != new_crtc_state-
>> >vrr.pipeline_full;
>>  }
>>
>> +static bool cmrr_params_changed(const struct intel_crtc_state 
>> *old_crtc_state,
>> + const struct intel_crtc_state *new_crtc_state) 
>> {
>> + return old_crtc_state->cmrr.cmrr_m != new_crtc_state->cmrr.cmrr_m ||
>> + old_crtc_state->cmrr.cmrr_n != new_crtc_state->cmrr.cmrr_n; }
>> +
>>  static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
>>const struct intel_crtc_state *new_crtc_state)  { @@ -
>> 5051,6 +5058,16 @@ intel_pipe_config_compare(const struct intel_crtc_state
>> *current_config,
>>   } \
>>  } while (0)
>>
>> +#define PIPE_CONF_CHECK_LLI(name) do { \
>> + if (current_config->name != pipe_config->name) { \
>> + pipe_config_mismatch(, fastset, crtc, __stringify(name), \
>> +  "(expected %lli, found %lli)", \
>> +  current_config->name, \
>> +  pipe_config->name); \
>> + ret = false; \
>> + } \
>> +} while (0)
>> +
>>  #define PIPE_CONF_CHECK_BOOL(name) do { \
>>   if (current_config->name != pipe_config->name) { \
>>   BUILD_BUG_ON_MSG(!__same_type(current_config->name,
>> bool), \ @@ -5415,10 +5432,13 @@ intel_pipe_config_compare(const struct
>> intel_crtc_state *current_config,
>>   PIPE_CONF_CHECK_I(vrr.guardband);
>>   PIPE_CONF_CHECK_I(vrr.vsync_start);
>>   PIPE_CONF_CHECK_I(vrr.vsync_end);
>> + PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
>> + PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
>>   }
>>
>>  #undef PIPE_CONF_CHECK_X
>>  #undef PIPE_CONF_CHECK_I
>> +#undef PIPE_CONF_CHECK_LLI
>>  #undef PIPE_CONF_CHECK_BOOL
>>  #undef PIPE_CONF_CHECK_P
>>  #undef PIPE_CONF_CHECK_FLAGS
>> @@ -6807,7 +6827,8 @@ static void intel_pre_update_crtc(struct
>> intel_atomic_state *state,
>>   intel_crtc_needs_fastset(new_crtc_state))
>>   icl_set_pipe_chicken(new_crtc_state);
>>
>> - if (vrr_params_changed(old_crtc_state, new_crtc_state))
>> + if (vrr_params_changed(old_crtc_state, new_crtc_state) ||
>> + cmrr_params_changed(old_crtc_state, new_crtc_state))
>>   intel_vrr_set_transcoder_timings(new_crtc_state);
>>   }
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>> b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index a06a154d587b..475fb5252dd4 100644

Re: [PATCH v8 1/7] drm/i915: Define and compute Transcoder CMRR registers

2024-05-13 Thread Jani Nikula
On Thu, 09 May 2024, Mitul Golani  wrote:
> Add register definitions for Transcoder Fixed Average
> Vtotal mode/CMRR function, with the necessary bitfields.
> Compute these registers when CMRR is enabled, extending
> Adaptive refresh rate capabilities.
>
> --v2:
> - Use intel_de_read64_2x32 in intel_vrr_get_config. [Jani]
> - Fix indent and order based on register offset. [Jani]

How does this match with...

>  
> +#define  _TRANS_CMRR_M_LO_A  0x604F0
> +#define  _TRANS_CMRR_M_HI_A  0x604F4
> +#define  _TRANS_CMRR_N_LO_A  0x604F8
> +#define  _TRANS_CMRR_N_HI_A  0x604FC
> +#define  TRANS_CMRR_M_LO(trans)  _MMIO_TRANS2(dev_priv, trans, 
> _TRANS_CMRR_M_LO_A)
> +#define  TRANS_CMRR_M_HI(trans)  _MMIO_TRANS2(dev_priv, trans, 
> _TRANS_CMRR_M_HI_A)
> +#define  TRANS_CMRR_N_LO(trans)  _MMIO_TRANS2(dev_priv, trans, 
> _TRANS_CMRR_N_LO_A)
> +#define  TRANS_CMRR_N_HI(trans)  _MMIO_TRANS2(dev_priv, trans, 
> _TRANS_CMRR_N_HI_A)
> +

...this?

Please read the comment at the top of i915_reg.h

BR,
Jani.


>  /* VGA port control */
>  #define ADPA _MMIO(0x61100)
>  #define PCH_ADPA_MMIO(0xe1100)

-- 
Jani Nikula, Intel


RE: [PATCH v8 1/7] drm/i915: Define and compute Transcoder CMRR registers

2024-05-13 Thread Murthy, Arun R


> -Original Message-
> From: Intel-gfx  On Behalf Of Mitul
> Golani
> Sent: Thursday, May 9, 2024 1:28 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Shankar, Uma ; Nikula, Jani
> 
> Subject: [PATCH v8 1/7] drm/i915: Define and compute Transcoder CMRR
> registers
> 
> Add register definitions for Transcoder Fixed Average Vtotal mode/CMRR
> function, with the necessary bitfields.
> Compute these registers when CMRR is enabled, extending Adaptive refresh
> rate capabilities.
> 
> --v2:
> - Use intel_de_read64_2x32 in intel_vrr_get_config. [Jani]
> - Fix indent and order based on register offset. [Jani]
> 
> --v3:
> - Removing RFC tag.
> 
> --v4:
> - Update place holder for CMRR register definition. (Jani)
> 
> Signed-off-by: Mitul Golani 
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  | 23 ++-
>  .../drm/i915/display/intel_display_types.h|  6 +
>  drivers/gpu/drm/i915/display/intel_vrr.c  | 22 ++
>  drivers/gpu/drm/i915/i915_reg.h   | 10 
Please create a new header file to add the CMRR related register definitions.

Thanks and Regards,
Arun R Murthy


>  4 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index ef986b508431..258a78447fba 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1001,6 +1001,13 @@ static bool vrr_params_changed(const struct
> intel_crtc_state *old_crtc_state,
>   old_crtc_state->vrr.pipeline_full != new_crtc_state-
> >vrr.pipeline_full;
>  }
> 
> +static bool cmrr_params_changed(const struct intel_crtc_state 
> *old_crtc_state,
> + const struct intel_crtc_state *new_crtc_state) {
> + return old_crtc_state->cmrr.cmrr_m != new_crtc_state->cmrr.cmrr_m ||
> + old_crtc_state->cmrr.cmrr_n != new_crtc_state->cmrr.cmrr_n; }
> +
>  static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
>const struct intel_crtc_state *new_crtc_state)  { @@ -
> 5051,6 +5058,16 @@ intel_pipe_config_compare(const struct intel_crtc_state
> *current_config,
>   } \
>  } while (0)
> 
> +#define PIPE_CONF_CHECK_LLI(name) do { \
> + if (current_config->name != pipe_config->name) { \
> + pipe_config_mismatch(, fastset, crtc, __stringify(name), \
> +  "(expected %lli, found %lli)", \
> +  current_config->name, \
> +  pipe_config->name); \
> + ret = false; \
> + } \
> +} while (0)
> +
>  #define PIPE_CONF_CHECK_BOOL(name) do { \
>   if (current_config->name != pipe_config->name) { \
>   BUILD_BUG_ON_MSG(!__same_type(current_config->name,
> bool), \ @@ -5415,10 +5432,13 @@ intel_pipe_config_compare(const struct
> intel_crtc_state *current_config,
>   PIPE_CONF_CHECK_I(vrr.guardband);
>   PIPE_CONF_CHECK_I(vrr.vsync_start);
>   PIPE_CONF_CHECK_I(vrr.vsync_end);
> + PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
> + PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
>   }
> 
>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
> +#undef PIPE_CONF_CHECK_LLI
>  #undef PIPE_CONF_CHECK_BOOL
>  #undef PIPE_CONF_CHECK_P
>  #undef PIPE_CONF_CHECK_FLAGS
> @@ -6807,7 +6827,8 @@ static void intel_pre_update_crtc(struct
> intel_atomic_state *state,
>   intel_crtc_needs_fastset(new_crtc_state))
>   icl_set_pipe_chicken(new_crtc_state);
> 
> - if (vrr_params_changed(old_crtc_state, new_crtc_state))
> + if (vrr_params_changed(old_crtc_state, new_crtc_state) ||
> + cmrr_params_changed(old_crtc_state, new_crtc_state))
>   intel_vrr_set_transcoder_timings(new_crtc_state);
>   }
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index a06a154d587b..475fb5252dd4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1396,6 +1396,12 @@ struct intel_crtc_state {
>   u32 vsync_end, vsync_start;
>   } vrr;
> 
> + /* Content Match Refresh Rate state */
> + struct {
> + bool enable;
> + u64 cmrr_n, cmrr_m;
> + } cmrr;
> +
>   /* Stream Splitter for eDP MSO */
>   struct {
>  

[PATCH v8 1/7] drm/i915: Define and compute Transcoder CMRR registers

2024-05-09 Thread Mitul Golani
Add register definitions for Transcoder Fixed Average
Vtotal mode/CMRR function, with the necessary bitfields.
Compute these registers when CMRR is enabled, extending
Adaptive refresh rate capabilities.

--v2:
- Use intel_de_read64_2x32 in intel_vrr_get_config. [Jani]
- Fix indent and order based on register offset. [Jani]

--v3:
- Removing RFC tag.

--v4:
- Update place holder for CMRR register definition. (Jani)

Signed-off-by: Mitul Golani 
---
 drivers/gpu/drm/i915/display/intel_display.c  | 23 ++-
 .../drm/i915/display/intel_display_types.h|  6 +
 drivers/gpu/drm/i915/display/intel_vrr.c  | 22 ++
 drivers/gpu/drm/i915/i915_reg.h   | 10 
 4 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index ef986b508431..258a78447fba 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1001,6 +1001,13 @@ static bool vrr_params_changed(const struct 
intel_crtc_state *old_crtc_state,
old_crtc_state->vrr.pipeline_full != 
new_crtc_state->vrr.pipeline_full;
 }
 
+static bool cmrr_params_changed(const struct intel_crtc_state *old_crtc_state,
+   const struct intel_crtc_state *new_crtc_state)
+{
+   return old_crtc_state->cmrr.cmrr_m != new_crtc_state->cmrr.cmrr_m ||
+   old_crtc_state->cmrr.cmrr_n != new_crtc_state->cmrr.cmrr_n;
+}
+
 static bool vrr_enabling(const struct intel_crtc_state *old_crtc_state,
 const struct intel_crtc_state *new_crtc_state)
 {
@@ -5051,6 +5058,16 @@ intel_pipe_config_compare(const struct intel_crtc_state 
*current_config,
} \
 } while (0)
 
+#define PIPE_CONF_CHECK_LLI(name) do { \
+   if (current_config->name != pipe_config->name) { \
+   pipe_config_mismatch(, fastset, crtc, __stringify(name), \
+"(expected %lli, found %lli)", \
+current_config->name, \
+pipe_config->name); \
+   ret = false; \
+   } \
+} while (0)
+
 #define PIPE_CONF_CHECK_BOOL(name) do { \
if (current_config->name != pipe_config->name) { \
BUILD_BUG_ON_MSG(!__same_type(current_config->name, bool), \
@@ -5415,10 +5432,13 @@ intel_pipe_config_compare(const struct intel_crtc_state 
*current_config,
PIPE_CONF_CHECK_I(vrr.guardband);
PIPE_CONF_CHECK_I(vrr.vsync_start);
PIPE_CONF_CHECK_I(vrr.vsync_end);
+   PIPE_CONF_CHECK_LLI(cmrr.cmrr_m);
+   PIPE_CONF_CHECK_LLI(cmrr.cmrr_n);
}
 
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
+#undef PIPE_CONF_CHECK_LLI
 #undef PIPE_CONF_CHECK_BOOL
 #undef PIPE_CONF_CHECK_P
 #undef PIPE_CONF_CHECK_FLAGS
@@ -6807,7 +6827,8 @@ static void intel_pre_update_crtc(struct 
intel_atomic_state *state,
intel_crtc_needs_fastset(new_crtc_state))
icl_set_pipe_chicken(new_crtc_state);
 
-   if (vrr_params_changed(old_crtc_state, new_crtc_state))
+   if (vrr_params_changed(old_crtc_state, new_crtc_state) ||
+   cmrr_params_changed(old_crtc_state, new_crtc_state))
intel_vrr_set_transcoder_timings(new_crtc_state);
}
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index a06a154d587b..475fb5252dd4 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1396,6 +1396,12 @@ struct intel_crtc_state {
u32 vsync_end, vsync_start;
} vrr;
 
+   /* Content Match Refresh Rate state */
+   struct {
+   bool enable;
+   u64 cmrr_n, cmrr_m;
+   } cmrr;
+
/* Stream Splitter for eDP MSO */
struct {
bool enable;
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c 
b/drivers/gpu/drm/i915/display/intel_vrr.c
index 894ee97b3e1b..831554ea46b2 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -217,6 +217,19 @@ void intel_vrr_set_transcoder_timings(const struct 
intel_crtc_state *crtc_state)
return;
}
 
+   if (crtc_state->cmrr.enable) {
+   intel_de_write(dev_priv, TRANS_VRR_CTL(cpu_transcoder),
+  VRR_CTL_CMRR_ENABLE | trans_vrr_ctl(crtc_state));
+   intel_de_write(dev_priv, TRANS_CMRR_M_HI(cpu_transcoder),
+  upper_32_bits(crtc_state->cmrr.cmrr_m));
+   intel_de_write(dev_priv, TRANS_CMRR_M_LO(cpu_transcoder),
+  lower_32_bits(crtc_state->cmrr.cmrr_m));
+   intel_de_write(dev_priv, TRANS_CMRR_N_HI(cpu_transcoder),