All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Khaled Almahallawy" <khaled.almahallawy@intel.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
	Imre Deak <imre.deak@intel.com>
Subject: Re: [PATCH] drm/i915/display: Extend i915_display_info with Type-C port details
Date: Fri, 26 Sep 2025 11:45:52 +0300	[thread overview]
Message-ID: <ca0a2b05c577d368cdf296685bda2dc9bcba9cfe@intel.com> (raw)
In-Reply-To: <aNZNqOni0az7GUqK@intel.com>

On Fri, 26 Sep 2025, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Sep 25, 2025 at 04:51:53PM -0700, Khaled Almahallawy wrote:
>> Expose key Type-C port data in i915_display_info to make it easier to
>> understand the port configuration and active mode, especially whether
>> the link is in DP-Alt or TBT-Alt, without having to scan kernel logs.
>> 
>> Tested in DP-Alt, TBT-Alt, SST, and MST.
>> 
>> Expected output:
>> 
>> [CONNECTOR:290:DP-2]: status: connected
>> 	TC Port: E/TC#2 mode: tbt-alt pin assignment: - max lanes: 4
>> 	physical dimensions: 600x340mm
>> ...
>> [CONNECTOR:263:DP-5]: status: connected
>> 	TC Port: G/TC#4 mode: dp-alt pin assignment: C max lanes: 4
>> 	physical dimensions: 610x350mm
>> 
>> Cc: Imre Deak <imre.deak@intel.com>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_display_debugfs.c |  4 ++++
>>  drivers/gpu/drm/i915/display/intel_tc.c              | 10 ++++++++++
>>  drivers/gpu/drm/i915/display/intel_tc.h              |  3 +++
>>  3 files changed, 17 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> index 10dddec3796f..6687fc18e1f5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> @@ -47,6 +47,7 @@
>>  #include "intel_psr_regs.h"
>>  #include "intel_vdsc.h"
>>  #include "intel_wm.h"
>> +#include "intel_tc.h"
>>  
>>  static struct intel_display *node_to_intel_display(struct drm_info_node *node)
>>  {
>> @@ -254,6 +255,9 @@ static void intel_connector_info(struct seq_file *m,
>>  	if (connector->status == connector_status_disconnected)
>>  		return;
>>  
>> +	if (intel_encoder_is_tc(intel_attached_encoder(intel_connector)))
>> +		intel_tc_info(m, intel_attached_dig_port(intel_connector));
>> +
>>  	seq_printf(m, "\tphysical dimensions: %dx%dmm\n",
>>  		   connector->display_info.width_mm,
>>  		   connector->display_info.height_mm);
>> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
>> index c4a5601c5107..4cd7ccac28b7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_tc.c
>> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
>> @@ -1703,6 +1703,16 @@ void intel_tc_port_sanitize_mode(struct intel_digital_port *dig_port,
>>  	mutex_unlock(&tc->lock);
>>  }
>>  
>> +void intel_tc_info(struct seq_file *m,  struct intel_digital_port *dig_port)
>
> Please use drm_printer instead of seq_file so this can be reused
> elsewhere.

Yeah.

I'm also really not a big fan of throwing everything into
i915_display_info. The general idea of debugfs is to have individual
files for individual things, not to have one massive thing that includes
everything, in a loop across crtcs and connectors.

I get that one file is a convenience, but at some point we have to say
too much is just too much. I think so far we've just added everything
indiscriminately.

I think it's a bit wild that we don't even have per crtc debugfs file
for intel_crtc_info, or per connector file for intel_connector_info, or
most of the individual parts in them.

Anyway, it's a bit difficult to claim this is the straw that breaks the
camel's back, but I think at some point this whole thing needs an
overhaul.


BR,
Jani.


>
>> +{
>> +	struct intel_tc_port *tc = to_tc_port(dig_port);
>> +
>> +	seq_printf(m, "\tTC Port: %s mode: %s pin assignment: %c max lanes: %d\n", tc->port_name,
>> +		   tc_port_mode_name(tc->mode),
>> +		   pin_assignment_name(tc->pin_assignment),
>> +		   tc->max_lane_count);
>> +}
>> +
>>  /*
>>   * The type-C ports are different because even when they are connected, they may
>>   * not be available/usable by the graphics driver: see the comment on
>> diff --git a/drivers/gpu/drm/i915/display/intel_tc.h b/drivers/gpu/drm/i915/display/intel_tc.h
>> index fff8b96e4972..3e983d1cc0b8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_tc.h
>> +++ b/drivers/gpu/drm/i915/display/intel_tc.h
>> @@ -7,6 +7,7 @@
>>  #define __INTEL_TC_H__
>>  
>>  #include <linux/types.h>
>> +#include <linux/seq_file.h>
>>  
>>  struct intel_crtc_state;
>>  struct intel_digital_port;
>> @@ -113,4 +114,6 @@ void intel_tc_port_cleanup(struct intel_digital_port *dig_port);
>>  
>>  bool intel_tc_cold_requires_aux_pw(struct intel_digital_port *dig_port);
>>  
>> +void intel_tc_info(struct seq_file *m,  struct intel_digital_port *dig_port);
>> +
>>  #endif /* __INTEL_TC_H__ */
>> -- 
>> 2.43.0

-- 
Jani Nikula, Intel

  reply	other threads:[~2025-09-26  8:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-25 23:51 [PATCH] drm/i915/display: Extend i915_display_info with Type-C port details Khaled Almahallawy
2025-09-26  0:56 ` ✓ CI.KUnit: success for " Patchwork
2025-09-26  1:16 ` ✓ i915.CI.BAT: " Patchwork
2025-09-26  1:32 ` ✓ Xe.CI.BAT: " Patchwork
2025-09-26  6:19 ` ✗ i915.CI.Full: failure " Patchwork
2025-09-26  8:20 ` ✗ Xe.CI.Full: " Patchwork
2025-09-26  8:24 ` [PATCH] " Ville Syrjälä
2025-09-26  8:45   ` Jani Nikula [this message]
2025-09-29 20:07     ` Almahallawy, Khaled
2025-09-29 20:16 ` Imre Deak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ca0a2b05c577d368cdf296685bda2dc9bcba9cfe@intel.com \
    --to=jani.nikula@intel.com \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=khaled.almahallawy@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.