On Thu, 05 Feb 2026, "Kandpal, Suraj" <[email protected]> wrote:
>> Subject: [PATCH v2 01/10] drm/i915/cmtg: enable cmtg LNL onwards
>> 
>> Introduce a flag for cmtg. LNL onwards CMTG support will be added.
>> Set the flag as per DISPLAY_VER() check.
>> 
>
> Use Capitalized versions of acronyms unless unavoidable(if you mention it as 
> a part of a function)
> both in commit subject and message, this review stands for all patch in 
> series it should be *CMTG
>
> Also please Bspec references to registers, sequences on all patches this 
> makes life very easy to review
> This is also a review for all the patches in the series
>
>> Signed-off-by: Animesh Manna <[email protected]>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display_types.h | 4 ++++
>>  drivers/gpu/drm/i915/display/intel_dp.c            | 4 ++++
>>  2 files changed, 8 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>> b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index e6298279dc89..1081615a14fb 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1569,6 +1569,10 @@ struct intel_crtc {  #endif
>> 
>>      bool vblank_psr_notify;
>> +
>> +    struct {
>> +            bool enable;
>> +    } cmtg;
>>  };
>> 
>>  struct intel_plane_error {
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index e2fd01d1a1e4..ecf8ed0c0265 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -3445,6 +3445,7 @@ intel_dp_compute_config(struct intel_encoder
>> *encoder,
>>      struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>>      const struct drm_display_mode *fixed_mode;
>>      struct intel_connector *connector = intel_dp->attached_connector;
>> +    struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
>>      int ret = 0, link_bpp_x16;
>> 
>>      fixed_mode = intel_panel_fixed_mode(connector, adjusted_mode);
>> @@ -3549,6 +3550,9 @@ intel_dp_compute_config(struct intel_encoder
>> *encoder,
>>      intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
>>      intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp,
>> pipe_config, conn_state);
>> 
>> +    if (DISPLAY_VER(display) >= 15 && intel_dp_is_edp(intel_dp))
>> +            crtc->cmtg.enable = true;
>
> Should be >= 20 since LNL's version was 20.
> Also I don't see a point of having this as a variable in intel_crtc this can 
> be checked as a macro or a function

Yeah, compute config should not modify anything but the crtc state.

BR,
Jani.


> Maybe you have to use intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP) 
> instead on intel_dp_is_edp but it should be better option
> According to me.
>
> Regards,
> Suraj Kandpal
>
>> +
>>      return intel_dp_tunnel_atomic_compute_stream_bw(state, intel_dp,
>> connector,
>>                                                      pipe_config);
>>  }
>> --
>> 2.29.0
>

-- 
Jani Nikula, Intel

Reply via email to