On Wed, 02 Apr 2025, Arun R Murthy <[email protected]> wrote:
> Support for UHBR 13.5 has been removed for mtl by the
> commit caf3d748f646 ("drm/i915/dp: Remove support for UHBR13.5")
> Removing UHBR13.5 on all display14- platforms due to the same
> retimer constraint.

If you're removing UHBR 13.5 support from DG2, why does the subject say
display 14 (DG2 is display 13, but it's not the only display 13), and
the commit message say display14- (which is not a convention we ever use
anywhere)?

Commit message for caf3d748f646 does not mention anything about
retimers, so what is "the same retimer constraint"? It's not explained
anywhere. I genuinely don't even know this.

I'm not nitpicking this just for the sake of nitpicking or to be
annoying. I want the commit messages to accurately reflect what the
changes do and why.

Maybe you've seen the changelogs I write for pull requests? See [1]. Now
imagine writing that changelog yourself, for hundreds of commits. What
if you couldn't even rely on the commit messages to desribe what they
do, and you had to look at the actual changes. For every single commit.

And obviously when you get, say, a bisected regression report on a
commit, and you observe the commit message doesn't match what the commit
does, then you're left wondering what is correct and how does that
correlate with the bug report.


BR,
Jani.


[1] https://lore.kernel.org/r/[email protected]



>
> v2: Reframed the commit msg (Jani)
> v4: Reframed the commit msg & update the max rate supported (Jani)
>
> Signed-off-by: Arun R Murthy <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 
> f21f9b441fc2a4e644c69410e6ec6b3d37907478..92bca701a989b03e2ad4b3d9e7d0a9ef12567e5a
>  100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -481,7 +481,7 @@ bool intel_dp_has_joiner(struct intel_dp *intel_dp)
>  
>  static int dg2_max_source_rate(struct intel_dp *intel_dp)
>  {
> -     return intel_dp_is_edp(intel_dp) ? 810000 : 1350000;
> +     return intel_dp_is_edp(intel_dp) ? 810000 : 1000000;
>  }
>  
>  static int icl_max_source_rate(struct intel_dp *intel_dp)
> @@ -550,7 +550,7 @@ intel_dp_set_source_rates(struct intel_dp *intel_dp)
>       };
>       static const int icl_rates[] = {
>               162000, 216000, 270000, 324000, 432000, 540000, 648000, 810000,
> -             1000000, 1350000,
> +             1000000,
>       };
>       static const int bxt_rates[] = {
>               162000, 216000, 243000, 270000, 324000, 432000, 540000

-- 
Jani Nikula, Intel

Reply via email to