All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 7/9] drm/i915/display: add phy, vbt and ddi indexes
Date: Tue, 31 Dec 2019 12:33:10 +0200	[thread overview]
Message-ID: <87imlwlr89.fsf@intel.com> (raw)
In-Reply-To: <20191223195850.25997-8-lucas.demarchi@intel.com>

On Mon, 23 Dec 2019, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> Identify 3 possible cases in which the index numbers can be different
> from the "port" and add them to the description-based ddi initialization
> table.  This can be used in place of additional functions mapping from
> on to the other.  Right now we already cover part of this by creating kind of
> virtual phy numbering, but that comes with downsides:
>
> a) there's not really a "phy numbering" in the spec, this is purely a
> software thing; hardware uses whatever they want thinking mapping from
> one to the other arbitrarily is easy in software.
>
> b) currently the mapping occurs on "leaf" functions, making the decision
> based on the platform.
>
> With this new table the approach will be: the port as defined by the
> enum port is purely a driver convention and won't be used anymore to
> define the register offset or register bits. For that we have the other
> 3 indexes, identified as being possibly different from the current usage
> of register bits: ddi, vbt and phy. The phy type is also added here,
> meant to replace the checks for combo vs tc (although the helper
> functions can remain so we may differentiate between, e.g. Dekel and MG
> phys).

I'm not sure how the vbt_idx is supposed to be used (because it's
actually not used anywhere here). I would like to reduce the amount of
VBT info spread around in the driver, so I'd really need to know more.

> While at it, also give names to the ports so they can be easily
> identified.
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  | 54 +++++++++----------
>  drivers/gpu/drm/i915/display/intel_display.h  |  7 +++
>  .../drm/i915/display/intel_display_types.h    |  5 ++
>  3 files changed, 39 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index ad85cf75c815..219f180fa395 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -16277,14 +16277,14 @@ static bool ddi_is_port_present(struct drm_i915_private *i915,
>  static const struct intel_output tgl_output = {
>  	.dsi_init = icl_dsi_init,
>  	.ddi_ports = {
> -		{ .port = PORT_A },
> -		{ .port = PORT_B },
> -		{ .port = PORT_D },
> -		{ .port = PORT_E },
> -		{ .port = PORT_F },
> -		{ .port = PORT_G },
> -		{ .port = PORT_H },
> -		{ .port = PORT_I },
> +		{ .name = "DDI A",   .port = PORT_A, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0, },
> +		{ .name = "DDI B",   .port = PORT_B, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1, },
> +		{ .name = "DDI TC1", .port = PORT_D, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x3, .phy_idx = 0x0, .vbt_idx = 0x2, },
> +		{ .name = "DDI TC2", .port = PORT_E, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x4, .phy_idx = 0x1, .vbt_idx = 0x3, },
> +		{ .name = "DDI TC3", .port = PORT_F, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x5, .phy_idx = 0x2, .vbt_idx = 0x4, },
> +		{ .name = "DDI TC4", .port = PORT_G, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x6, .phy_idx = 0x3, .vbt_idx = 0x5, },
> +		{ .name = "DDI TC5", .port = PORT_H, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x7, .phy_idx = 0x4, .vbt_idx = 0x6, },
> +		{ .name = "DDI TC6", .port = PORT_I, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x8, .phy_idx = 0x5, .vbt_idx = 0x7, },
>  		{ .port = PORT_NONE }

Makes you wonder if this sort of info should be linked from the
intel_device_info instead of adding new arrays. Not sure.

BR,
Jani.


>  	}
>  };
> @@ -16293,12 +16293,12 @@ static const struct intel_output icl_output = {
>  	.dsi_init = icl_dsi_init,
>  	.is_port_present = icl_is_port_present,
>  	.ddi_ports = {
> -		{ .port = PORT_A },
> -		{ .port = PORT_B },
> -		{ .port = PORT_C },
> -		{ .port = PORT_D },
> -		{ .port = PORT_E },
> -		{ .port = PORT_F },
> +		{ .name = "DDI A",   .port = PORT_A, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0, },
> +		{ .name = "DDI B",   .port = PORT_B, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1, },
> +		{ .name = "DDI TC1", .port = PORT_C, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x2, .phy_idx = 0x0, .vbt_idx = 0x2, },
> +		{ .name = "DDI TC2", .port = PORT_D, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x3, .phy_idx = 0x1, .vbt_idx = 0x3, },
> +		{ .name = "DDI TC3", .port = PORT_E, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x4, .phy_idx = 0x2, .vbt_idx = 0x4, },
> +		{ .name = "DDI TC4", .port = PORT_F, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x5, .phy_idx = 0x3, .vbt_idx = 0x5, },
>  		{ .port = PORT_NONE }
>  	}
>  };
> @@ -16306,10 +16306,10 @@ static const struct intel_output icl_output = {
>  static const struct intel_output ehl_output = {
>  	.dsi_init = icl_dsi_init,
>  	.ddi_ports = {
> -		{ .port = PORT_A },
> -		{ .port = PORT_B },
> -		{ .port = PORT_C },
> -		{ .port = PORT_D },
> +		{ .name = "DDI A",   .port = PORT_A, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0, },
> +		{ .name = "DDI B",   .port = PORT_B, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1, },
> +		{ .name = "DDI C",   .port = PORT_C, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x2, .phy_idx = 0x2, .vbt_idx = 0x2, },
> +		{ .name = "DDI D",   .port = PORT_D, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x3, .phy_idx = 0x0, .vbt_idx = 0x3, },
>  		{ .port = PORT_NONE }
>  	}
>  };
> @@ -16317,9 +16317,9 @@ static const struct intel_output ehl_output = {
>  static const struct intel_output gen9lp_output = {
>  	.dsi_init = vlv_dsi_init,
>  	.ddi_ports = {
> -		{ .port = PORT_A },
> -		{ .port = PORT_B },
> -		{ .port = PORT_C },
> +		{ .name = "DDI A", .port = PORT_A, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0 },
> +		{ .name = "DDI B", .port = PORT_B, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1 },
> +		{ .name = "DDI C", .port = PORT_C, .ddi_idx = 0x2, .phy_idx = 0x2, .vbt_idx = 0x2 },
>  		{ .port = PORT_NONE }
>  	},
>  };
> @@ -16327,12 +16327,12 @@ static const struct intel_output gen9lp_output = {
>  static const struct intel_output ddi_output = {
>  	.is_port_present = ddi_is_port_present,
>  	.ddi_ports = {
> -		{ .port = PORT_A },
> -		{ .port = PORT_B },
> -		{ .port = PORT_C },
> -		{ .port = PORT_D },
> -		{ .port = PORT_E },
> -		{ .port = PORT_F },
> +		{ .name = "DDI A", .port = PORT_A, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0 },
> +		{ .name = "DDI B", .port = PORT_B, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1 },
> +		{ .name = "DDI C", .port = PORT_C, .ddi_idx = 0x2, .phy_idx = 0x2, .vbt_idx = 0x2 },
> +		{ .name = "DDI D", .port = PORT_D, .ddi_idx = 0x3, .phy_idx = 0x3, .vbt_idx = 0x3 },
> +		{ .name = "DDI E", .port = PORT_E, .ddi_idx = 0x4, .phy_idx = 0x4, .vbt_idx = 0x4 },
> +		{ .name = "DDI F", .port = PORT_F, .ddi_idx = 0x5, .phy_idx = 0x5, .vbt_idx = 0x5 },
>  		{ .port = PORT_NONE }
>  	}
>  };
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index 05d68bd393dc..db19e5eee248 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -303,6 +303,13 @@ enum phy {
>  	I915_MAX_PHYS
>  };
>  
> +enum phy_type {
> +	PHY_TYPE_NONE = 0,
> +
> +	PHY_TYPE_COMBO,
> +	PHY_TYPE_TC,
> +};
> +
>  #define phy_name(a) ((a) + 'A')
>  
>  enum phy_fia {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 4d2f4ee35812..23a885895803 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1377,7 +1377,12 @@ struct intel_dp_mst_encoder {
>  };
>  
>  struct intel_ddi_port_info {
> +	const char *name;
>  	enum port port;
> +	s8 phy_type;
> +	u8 ddi_idx;
> +	u8 phy_idx;
> +	u8 vbt_idx;
>  };
>  
>  static inline enum dpio_channel

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2019-12-31 10:32 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-23 19:58 [Intel-gfx] [PATCH 0/9] RFC: display/ddi: keep register indexes in a table Lucas De Marchi
2019-12-23 19:58 ` [Intel-gfx] [PATCH 1/9] drm/i915/display: nuke skl workaround for pre-production hw Lucas De Marchi
2019-12-23 19:58 ` [Intel-gfx] [PATCH 2/9] drm/i915/display: remove alias to dig_port Lucas De Marchi
2019-12-23 21:05   ` Matt Roper
2019-12-23 19:58 ` [Intel-gfx] [PATCH 3/9] drm/i915/display: prefer the more common dig_port name Lucas De Marchi
2019-12-23 21:16   ` Matt Roper
2019-12-23 21:25     ` Lucas De Marchi
2019-12-23 19:58 ` [Intel-gfx] [PATCH 4/9] drm/i915/display: start description-based ddi initialization Lucas De Marchi
2019-12-31  9:58   ` Jani Nikula
2020-01-02  7:19     ` Lucas De Marchi
2020-06-22 23:50       ` Lucas De Marchi
2020-07-03 13:22         ` Jani Nikula
2019-12-23 19:58 ` [Intel-gfx] [PATCH 5/9] drm/i915/display: move icl to description-based ddi init Lucas De Marchi
2019-12-24  0:00   ` Matt Roper
2019-12-31 10:14   ` Jani Nikula
2020-01-02  7:32     ` Lucas De Marchi
2019-12-23 19:58 ` [Intel-gfx] [PATCH 6/9] drm/i915/display: description-based initialization for remaining ddi platforms Lucas De Marchi
2019-12-31 10:25   ` Jani Nikula
2019-12-31 10:26     ` Jani Nikula
2020-01-02  7:23     ` Lucas De Marchi
2019-12-23 19:58 ` [Intel-gfx] [PATCH 7/9] drm/i915/display: add phy, vbt and ddi indexes Lucas De Marchi
2019-12-24  0:10   ` Matt Roper
2019-12-24  0:17     ` Lucas De Marchi
2019-12-31 10:33   ` Jani Nikula [this message]
2020-01-02  7:50     ` Lucas De Marchi
2019-12-23 19:58 ` [Intel-gfx] [PATCH 8/9] drm/i915/display: refer to vbt info as vbt_port_info Lucas De Marchi
2019-12-31 10:39   ` Jani Nikula
2019-12-23 19:58 ` [Intel-gfx] [PATCH 9/9] drm/i915/display: use port_info on intel_ddi_init Lucas De Marchi
2019-12-24  0:16   ` Matt Roper
2019-12-31 11:20     ` Jani Nikula
2020-06-23  1:11       ` Lucas De Marchi
2019-12-31 11:22   ` Jani Nikula
2019-12-23 20:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for RFC: display/ddi: keep register indexes in a table Patchwork
2019-12-23 20:57 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

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=87imlwlr89.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@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.