From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
Lucas De Marchi <lucas.demarchi@intel.com>
Subject: Re: [PATCH 06/10] drm/i915: For HPD connected port use hpd_pin instead of port.
Date: Mon, 22 Jan 2018 18:40:56 +0200 [thread overview]
Message-ID: <20180122164056.GD5453@intel.com> (raw)
In-Reply-To: <20180120000524.4746-6-rodrigo.vivi@intel.com>
On Fri, Jan 19, 2018 at 04:05:20PM -0800, Rodrigo Vivi wrote:
> Let's try to simplify this mapping to hpd_pin -> bit
> instead using port.
> So for CNL with port F where we have this port using
> hdp_pin and bits of other ports we don't need to duplicated
> the mapping.
>
> But for now this is only a re-org with no functional change
> expected.
>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 144 ++++++++++++++++++------------------
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_lspcon.c | 3 +-
> 3 files changed, 72 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ae3b0b030177..4ff6fcf542e9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4487,173 +4487,170 @@ edp_detect(struct intel_dp *intel_dp)
> return status;
> }
>
> -static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +static bool ibx_digital_port_connected(struct intel_encoder *encoder)
> {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> u32 bit;
>
> - switch (port->base.port) {
> - case PORT_B:
> + switch (encoder->hpd_pin) {
> + case HPD_PORT_B:
> bit = SDE_PORTB_HOTPLUG;
> break;
> - case PORT_C:
> + case HPD_PORT_C:
> bit = SDE_PORTC_HOTPLUG;
> break;
> - case PORT_D:
> + case HPD_PORT_D:
> bit = SDE_PORTD_HOTPLUG;
> break;
> default:
> - MISSING_CASE(port->base.port);
> + MISSING_CASE(encoder->hpd_pin);
> return false;
> }
>
> return I915_READ(SDEISR) & bit;
> }
>
> -static bool cpt_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +static bool cpt_digital_port_connected(struct intel_encoder *encoder)
> {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> u32 bit;
>
> - switch (port->base.port) {
> - case PORT_B:
> + switch (encoder->hpd_pin) {
> + case HPD_PORT_B:
> bit = SDE_PORTB_HOTPLUG_CPT;
> break;
> - case PORT_C:
> + case HPD_PORT_C:
> bit = SDE_PORTC_HOTPLUG_CPT;
> break;
> - case PORT_D:
> + case HPD_PORT_D:
> bit = SDE_PORTD_HOTPLUG_CPT;
> break;
> default:
> - MISSING_CASE(port->base.port);
> + MISSING_CASE(encoder->hpd_pin);
> return false;
> }
>
> return I915_READ(SDEISR) & bit;
> }
>
> -static bool spt_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +static bool spt_digital_port_connected(struct intel_encoder *encoder)
> {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> u32 bit;
>
> - switch (port->base.port) {
> - case PORT_A:
> + switch (encoder->hpd_pin) {
> + case HPD_PORT_A:
> bit = SDE_PORTA_HOTPLUG_SPT;
> break;
> - case PORT_E:
> + case HPD_PORT_E:
> bit = SDE_PORTE_HOTPLUG_SPT;
> break;
> default:
> - return cpt_digital_port_connected(dev_priv, port);
> + return cpt_digital_port_connected(encoder);
> }
>
> return I915_READ(SDEISR) & bit;
> }
>
> -static bool g4x_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +static bool g4x_digital_port_connected(struct intel_encoder *encoder)
> {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> u32 bit;
>
> - switch (port->base.port) {
> - case PORT_B:
> + switch (encoder->hpd_pin) {
> + case HPD_PORT_B:
> bit = PORTB_HOTPLUG_LIVE_STATUS_G4X;
> break;
> - case PORT_C:
> + case HPD_PORT_C:
> bit = PORTC_HOTPLUG_LIVE_STATUS_G4X;
> break;
> - case PORT_D:
> + case HPD_PORT_D:
> bit = PORTD_HOTPLUG_LIVE_STATUS_G4X;
> break;
> default:
> - MISSING_CASE(port->base.port);
> + MISSING_CASE(encoder->hpd_pin);
> return false;
> }
>
> return I915_READ(PORT_HOTPLUG_STAT) & bit;
> }
>
> -static bool gm45_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +static bool gm45_digital_port_connected(struct intel_encoder *encoder)
> {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> u32 bit;
>
> - switch (port->base.port) {
> - case PORT_B:
> + switch (encoder->hpd_pin) {
> + case HPD_PORT_B:
> bit = PORTB_HOTPLUG_LIVE_STATUS_GM45;
> break;
> - case PORT_C:
> + case HPD_PORT_C:
> bit = PORTC_HOTPLUG_LIVE_STATUS_GM45;
> break;
> - case PORT_D:
> + case HPD_PORT_D:
> bit = PORTD_HOTPLUG_LIVE_STATUS_GM45;
> break;
> default:
> - MISSING_CASE(port->base.port);
> + MISSING_CASE(encoder->hpd_pin);
> return false;
> }
>
> return I915_READ(PORT_HOTPLUG_STAT) & bit;
> }
>
> -static bool ilk_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +static bool ilk_digital_port_connected(struct intel_encoder *encoder)
> {
> - if (port->base.port == PORT_A)
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
Style nit: missing newline here, and in several other functions.
> + if (encoder->hpd_pin == HPD_PORT_A)
> return I915_READ(DEISR) & DE_DP_A_HOTPLUG;
> else
> - return ibx_digital_port_connected(dev_priv, port);
> + return ibx_digital_port_connected(encoder);
> }
>
> -static bool snb_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +static bool snb_digital_port_connected(struct intel_encoder *encoder)
> {
> - if (port->base.port == PORT_A)
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + if (encoder->hpd_pin == HPD_PORT_A)
> return I915_READ(DEISR) & DE_DP_A_HOTPLUG;
> else
> - return cpt_digital_port_connected(dev_priv, port);
> + return cpt_digital_port_connected(encoder);
> }
>
> -static bool ivb_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +static bool ivb_digital_port_connected(struct intel_encoder *encoder)
> {
> - if (port->base.port == PORT_A)
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + if (encoder->hpd_pin == HPD_PORT_A)
> return I915_READ(DEISR) & DE_DP_A_HOTPLUG_IVB;
> else
> - return cpt_digital_port_connected(dev_priv, port);
> + return cpt_digital_port_connected(encoder);
> }
>
> -static bool bdw_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +static bool bdw_digital_port_connected(struct intel_encoder *encoder)
> {
> - if (port->base.port == PORT_A)
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + if (encoder->hpd_pin == HPD_PORT_A)
> return I915_READ(GEN8_DE_PORT_ISR) & GEN8_PORT_DP_A_HOTPLUG;
> else
> - return cpt_digital_port_connected(dev_priv, port);
> + return cpt_digital_port_connected(encoder);
> }
>
> -static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *intel_dig_port)
> +static bool bxt_digital_port_connected(struct intel_encoder *encoder)
> {
> - struct intel_encoder *intel_encoder = &intel_dig_port->base;
> - enum port port;
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> u32 bit;
>
> - port = intel_hpd_pin_to_port(intel_encoder->hpd_pin);
> - switch (port) {
> - case PORT_A:
> + switch (encoder->hpd_pin) {
> + case HPD_PORT_A:
> bit = BXT_DE_PORT_HP_DDIA;
> break;
> - case PORT_B:
> + case HPD_PORT_B:
> bit = BXT_DE_PORT_HP_DDIB;
> break;
> - case PORT_C:
> + case HPD_PORT_C:
> bit = BXT_DE_PORT_HP_DDIC;
> break;
> default:
> - MISSING_CASE(port);
> + MISSING_CASE(encoder->hpd_pin);
> return false;
> }
>
> @@ -4662,33 +4659,33 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
>
> /*
> * intel_digital_port_connected - is the specified port connected?
> - * @dev_priv: i915 private structure
> - * @port: the port to test
> + * @encoder: intel_encoder
> *
> * Return %true if @port is connected, %false otherwise.
^^^^^
Needs to be updated, assuming this kernel doc comment is actually
used. And if it's not used we should just nuke it.
Apart from those this looks good, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> */
> -bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port)
> +bool intel_digital_port_connected(struct intel_encoder *encoder)
> {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> if (HAS_GMCH_DISPLAY(dev_priv)) {
> if (IS_GM45(dev_priv))
> - return gm45_digital_port_connected(dev_priv, port);
> + return gm45_digital_port_connected(encoder);
> else
> - return g4x_digital_port_connected(dev_priv, port);
> + return g4x_digital_port_connected(encoder);
> }
>
> if (IS_GEN5(dev_priv))
> - return ilk_digital_port_connected(dev_priv, port);
> + return ilk_digital_port_connected(encoder);
> else if (IS_GEN6(dev_priv))
> - return snb_digital_port_connected(dev_priv, port);
> + return snb_digital_port_connected(encoder);
> else if (IS_GEN7(dev_priv))
> - return ivb_digital_port_connected(dev_priv, port);
> + return ivb_digital_port_connected(encoder);
> else if (IS_GEN8(dev_priv))
> - return bdw_digital_port_connected(dev_priv, port);
> + return bdw_digital_port_connected(encoder);
> else if (IS_GEN9_LP(dev_priv))
> - return bxt_digital_port_connected(dev_priv, port);
> + return bxt_digital_port_connected(encoder);
> else
> - return spt_digital_port_connected(dev_priv, port);
> + return spt_digital_port_connected(encoder);
> }
>
> static struct edid *
> @@ -4747,8 +4744,7 @@ intel_dp_long_pulse(struct intel_connector *connector)
> /* Can't disconnect eDP, but you can close the lid... */
> if (intel_dp_is_edp(intel_dp))
> status = edp_detect(intel_dp);
> - else if (intel_digital_port_connected(dev_priv,
> - dp_to_dig_port(intel_dp)))
> + else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
> status = intel_dp_detect_dpcd(intel_dp);
> else
> status = connector_status_disconnected;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a73a9f0d790f..9ae820f82235 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1671,8 +1671,7 @@ static inline unsigned int intel_dp_unused_lane_mask(int lane_count)
> bool intel_dp_read_dpcd(struct intel_dp *intel_dp);
> int intel_dp_link_required(int pixel_clock, int bpp);
> int intel_dp_max_data_rate(int max_link_clock, int max_lanes);
> -bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
> - struct intel_digital_port *port);
> +bool intel_digital_port_connected(struct intel_encoder *encoder);
>
> /* intel_dp_aux_backlight.c */
> int intel_dp_aux_init_backlight_funcs(struct intel_connector *intel_connector);
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index dcbc786479f9..8ae8f42f430a 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -167,11 +167,10 @@ static void lspcon_resume_in_pcon_wa(struct intel_lspcon *lspcon)
> {
> struct intel_dp *intel_dp = lspcon_to_intel_dp(lspcon);
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> - struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> unsigned long start = jiffies;
>
> while (1) {
> - if (intel_digital_port_connected(dev_priv, dig_port)) {
> + if (intel_digital_port_connected(&dig_port->base)) {
> DRM_DEBUG_KMS("LSPCON recovering in PCON mode after %u ms\n",
> jiffies_to_msecs(jiffies - start));
> return;
> --
> 2.13.6
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-01-22 16:40 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-20 0:05 [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Rodrigo Vivi
2018-01-20 0:05 ` [PATCH 02/10] drm/i915/cnl: Add AUX-F support Rodrigo Vivi
2018-01-22 23:42 ` Pandiyan, Dhinakaran
2018-01-22 23:59 ` [PATCH] " Rodrigo Vivi
2018-01-23 2:43 ` Pandiyan, Dhinakaran
2018-01-23 4:53 ` Pandiyan, Dhinakaran
2018-01-23 16:12 ` Lucas De Marchi
2018-01-23 16:30 ` Rodrigo Vivi
2018-01-23 18:35 ` Runyan, Arthur J
2018-01-23 21:57 ` [PATCH] drm/i915/cnl: Extend Wa 1178 to Aux F Rodrigo Vivi
2018-01-23 23:21 ` Lucas De Marchi
2018-01-23 21:10 ` [PATCH] drm/i915/cnl: Add AUX-F support Rodrigo Vivi
2018-01-20 0:05 ` [PATCH 03/10] drm/i915/cnl: Fix _CNL_PORT_TX_DW2_LN0_F definition Rodrigo Vivi
2018-01-20 0:05 ` [PATCH 04/10] drm/i915: Fix DPLCLKA_CFGCR0 bits for Port F Rodrigo Vivi
2018-01-22 21:44 ` Pandiyan, Dhinakaran
2018-01-22 23:08 ` Rodrigo Vivi
2018-01-20 0:05 ` [PATCH 05/10] drm/i915/cnl: Add right GMBUS pin number for HDMI on " Rodrigo Vivi
2018-01-20 0:05 ` [PATCH 06/10] drm/i915: For HPD connected port use hpd_pin instead of port Rodrigo Vivi
2018-01-22 16:40 ` Ville Syrjälä [this message]
2018-01-22 23:05 ` [PATCH] " Rodrigo Vivi
2018-01-20 0:05 ` [PATCH 07/10] drm/i915/cnl: Add HPD support for Port F Rodrigo Vivi
2018-01-22 16:51 ` Ville Syrjälä
2018-01-22 23:20 ` Rodrigo Vivi
2018-01-20 0:05 ` [PATCH 08/10] drm/i915/cnl: Enable DDI-F on Cannonlake Rodrigo Vivi
2018-01-23 3:12 ` Pandiyan, Dhinakaran
2018-01-23 16:29 ` Rodrigo Vivi
2018-01-20 0:05 ` [PATCH 09/10] drm/i915/cnl: Fix DP max rate for Cannonlake with port F Rodrigo Vivi
2018-01-22 16:46 ` Ville Syrjälä
2018-01-23 22:32 ` [PATCH] " Rodrigo Vivi
2018-01-23 22:45 ` Manasi Navare
2018-01-20 0:05 ` [PATCH 10/10] drm/i915/cnl: Don't try to manage Port F power wells on all CNL Rodrigo Vivi
2018-01-22 12:12 ` Imre Deak
2018-01-22 23:48 ` [PATCH] " Rodrigo Vivi
2018-01-23 3:03 ` Pandiyan, Dhinakaran
2018-01-23 16:27 ` Rodrigo Vivi
2018-01-23 23:11 ` Rodrigo Vivi
2018-01-20 0:30 ` ✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Patchwork
2018-01-20 8:40 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-01-22 16:56 ` [PATCH 01/10] " Ville Syrjälä
2018-01-22 23:00 ` Rodrigo Vivi
2018-01-23 0:11 ` ✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev4) Patchwork
2018-01-23 0:32 ` ✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev5) Patchwork
2018-01-23 6:54 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-01-23 22:16 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev6) Patchwork
2018-01-23 22:36 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev7) Patchwork
2018-01-23 23:15 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU. (rev8) Patchwork
2018-01-25 17:40 ` [PATCH 01/10] drm/i915/cnl: Add Cannonlake PCI IDs for another SKU Paulo Zanoni
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=20180122164056.GD5453@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=lucas.demarchi@intel.com \
--cc=rodrigo.vivi@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).