diff mbox

[13/14] drm/i915: Give encoders useful names

Message ID 1449592922-5545-14-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Dec. 8, 2015, 4:42 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather than let the core generate usless encoder names, let's pass in
something that actually identifies the piece of hardware we're dealing
with.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c    |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c    |  2 +-
 drivers/gpu/drm/i915/intel_dp.c     |  2 +-
 drivers/gpu/drm/i915/intel_dp_mst.c |  2 +-
 drivers/gpu/drm/i915/intel_dsi.c    | 15 ++++++++++++++-
 drivers/gpu/drm/i915/intel_dvo.c    | 18 ++++++++++++++++--
 drivers/gpu/drm/i915/intel_hdmi.c   |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c   |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c   |  2 +-
 drivers/gpu/drm/i915/intel_tv.c     |  2 +-
 10 files changed, 38 insertions(+), 11 deletions(-)

Comments

Jani Nikula Dec. 9, 2015, 8:35 a.m. UTC | #1 
On Tue, 08 Dec 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rather than let the core generate usless encoder names, let's pass in
> something that actually identifies the piece of hardware we're dealing
> with.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index fff9a66c32a1..9b064a15a8e2 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1111,6 +1111,19 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
>  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>  };
>  
> +static char intel_dsi_port_name(const struct drm_i915_private *dev_priv)
> +{
> +	switch (dev_priv->vbt.dsi.port) {
> +	case DVO_PORT_MIPIA:
> +		return 'A';
> +	case DVO_PORT_MIPIC:
> +		return 'C';
> +	default:
> +		MISSING_CASE(dev_priv->vbt.dsi.port);
> +		return '?';
> +	}
> +}
> +
>  void intel_dsi_init(struct drm_device *dev)
>  {
>  	struct intel_dsi *intel_dsi;
> @@ -1153,7 +1166,7 @@ void intel_dsi_init(struct drm_device *dev)
>  	connector = &intel_connector->base;
>  
>  	drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI,
> -			 NULL);
> +			 "MIPI %c", intel_dsi_port_name(dev_priv));

Did you plant that on purpose to tick me off? From here on out I'll
insist we refer to VESA, embedded VESA, and VESA MST. Only half
kidding. :/

BR,
Jani.


>  
>  	intel_encoder->compute_config = intel_dsi_compute_config;
>  	intel_encoder->pre_enable = intel_dsi_pre_enable;
Ville Syrjälä Dec. 9, 2015, 11:59 a.m. UTC | #2 
On Wed, Dec 09, 2015 at 10:35:24AM +0200, Jani Nikula wrote:
> On Tue, 08 Dec 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Rather than let the core generate usless encoder names, let's pass in
> > something that actually identifies the piece of hardware we're dealing
> > with.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index fff9a66c32a1..9b064a15a8e2 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -1111,6 +1111,19 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
> >  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> >  };
> >  
> > +static char intel_dsi_port_name(const struct drm_i915_private *dev_priv)
> > +{
> > +	switch (dev_priv->vbt.dsi.port) {
> > +	case DVO_PORT_MIPIA:
> > +		return 'A';
> > +	case DVO_PORT_MIPIC:
> > +		return 'C';
> > +	default:
> > +		MISSING_CASE(dev_priv->vbt.dsi.port);
> > +		return '?';
> > +	}
> > +}
> > +
> >  void intel_dsi_init(struct drm_device *dev)
> >  {
> >  	struct intel_dsi *intel_dsi;
> > @@ -1153,7 +1166,7 @@ void intel_dsi_init(struct drm_device *dev)
> >  	connector = &intel_connector->base;
> >  
> >  	drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI,
> > -			 NULL);
> > +			 "MIPI %c", intel_dsi_port_name(dev_priv));
> 
> Did you plant that on purpose to tick me off? From here on out I'll
> insist we refer to VESA, embedded VESA, and VESA MST. Only half
> kidding. :/

I was thinking it should match what the spec calls it in most cases
(which is why I almost left out the space too, but then I figured it's
easier to read with the space). I could make it "MIPI [sic] %c" :P

But, if you prefer to call it "DSI %c" instead, I can change it.

> 
> BR,
> Jani.
> 
> 
> >  
> >  	intel_encoder->compute_config = intel_dsi_compute_config;
> >  	intel_encoder->pre_enable = intel_dsi_pre_enable;
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
Jani Nikula Dec. 9, 2015, 1:50 p.m. UTC | #3 
On Wed, 09 Dec 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Dec 09, 2015 at 10:35:24AM +0200, Jani Nikula wrote:
>> On Tue, 08 Dec 2015, ville.syrjala@linux.intel.com wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Rather than let the core generate usless encoder names, let's pass in
>> > something that actually identifies the piece of hardware we're dealing
>> > with.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> 
>> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> > index fff9a66c32a1..9b064a15a8e2 100644
>> > --- a/drivers/gpu/drm/i915/intel_dsi.c
>> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> > @@ -1111,6 +1111,19 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
>> >  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> >  };
>> >  
>> > +static char intel_dsi_port_name(const struct drm_i915_private *dev_priv)
>> > +{
>> > +	switch (dev_priv->vbt.dsi.port) {
>> > +	case DVO_PORT_MIPIA:
>> > +		return 'A';
>> > +	case DVO_PORT_MIPIC:
>> > +		return 'C';
>> > +	default:
>> > +		MISSING_CASE(dev_priv->vbt.dsi.port);
>> > +		return '?';
>> > +	}
>> > +}
>> > +
>> >  void intel_dsi_init(struct drm_device *dev)
>> >  {
>> >  	struct intel_dsi *intel_dsi;
>> > @@ -1153,7 +1166,7 @@ void intel_dsi_init(struct drm_device *dev)
>> >  	connector = &intel_connector->base;
>> >  
>> >  	drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI,
>> > -			 NULL);
>> > +			 "MIPI %c", intel_dsi_port_name(dev_priv));
>> 
>> Did you plant that on purpose to tick me off? From here on out I'll
>> insist we refer to VESA, embedded VESA, and VESA MST. Only half
>> kidding. :/
>
> I was thinking it should match what the spec calls it in most cases
> (which is why I almost left out the space too, but then I figured it's
> easier to read with the space). I could make it "MIPI [sic] %c" :P

Yeah, also rename our register macros (that are straight from the spec)
as MIPI_SIC_FOO. ;)

>
> But, if you prefer to call it "DSI %c" instead, I can change it.

Yes, DSI or MIPI DSI please.

BR,
Jani.

>
>> 
>> BR,
>> Jani.
>> 
>> 
>> >  
>> >  	intel_encoder->compute_config = intel_dsi_compute_config;
>> >  	intel_encoder->pre_enable = intel_dsi_pre_enable;
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 9c89df1af036..2c5fbd06b470 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -824,7 +824,7 @@  void intel_crt_init(struct drm_device *dev)
 			   &intel_crt_connector_funcs, DRM_MODE_CONNECTOR_VGA);
 
 	drm_encoder_init(dev, &crt->base.base, &intel_crt_enc_funcs,
-			 DRM_MODE_ENCODER_DAC, NULL);
+			 DRM_MODE_ENCODER_DAC, "CRT");
 
 	intel_connector_attach_encoder(intel_connector, &crt->base);
 
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b0b1a4838cea..39c1e7149c05 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3290,7 +3290,7 @@  void intel_ddi_init(struct drm_device *dev, enum port port)
 	encoder = &intel_encoder->base;
 
 	drm_encoder_init(dev, encoder, &intel_ddi_funcs,
-			 DRM_MODE_ENCODER_TMDS, NULL);
+			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
 
 	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0f0573aa1b0d..b0c5944a6f45 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5989,7 +5989,7 @@  intel_dp_init(struct drm_device *dev,
 	encoder = &intel_encoder->base;
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
-			 DRM_MODE_ENCODER_TMDS, NULL);
+			 DRM_MODE_ENCODER_TMDS, "DP %c", port_name(port));
 
 	intel_encoder->compute_config = intel_dp_compute_config;
 	intel_encoder->disable = intel_disable_dp;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index e8d369d0a713..032882c2d693 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -536,7 +536,7 @@  intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
 	intel_mst->primary = intel_dig_port;
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_dp_mst_enc_funcs,
-			 DRM_MODE_ENCODER_DPMST, NULL);
+			 DRM_MODE_ENCODER_DPMST, "DP-MST %c", pipe_name(pipe));
 
 	intel_encoder->type = INTEL_OUTPUT_DP_MST;
 	intel_encoder->crtc_mask = 0x7;
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index fff9a66c32a1..9b064a15a8e2 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1111,6 +1111,19 @@  static const struct drm_connector_funcs intel_dsi_connector_funcs = {
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 };
 
+static char intel_dsi_port_name(const struct drm_i915_private *dev_priv)
+{
+	switch (dev_priv->vbt.dsi.port) {
+	case DVO_PORT_MIPIA:
+		return 'A';
+	case DVO_PORT_MIPIC:
+		return 'C';
+	default:
+		MISSING_CASE(dev_priv->vbt.dsi.port);
+		return '?';
+	}
+}
+
 void intel_dsi_init(struct drm_device *dev)
 {
 	struct intel_dsi *intel_dsi;
@@ -1153,7 +1166,7 @@  void intel_dsi_init(struct drm_device *dev)
 	connector = &intel_connector->base;
 
 	drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI,
-			 NULL);
+			 "MIPI %c", intel_dsi_port_name(dev_priv));
 
 	intel_encoder->compute_config = intel_dsi_compute_config;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 286baec979c8..a456f2eb68b6 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -406,6 +406,18 @@  intel_dvo_get_current_mode(struct drm_connector *connector)
 	return mode;
 }
 
+static char intel_dvo_port_name(i915_reg_t dvo_reg)
+{
+	if (i915_mmio_reg_equal(dvo_reg, DVOA))
+		return 'A';
+	else if (i915_mmio_reg_equal(dvo_reg, DVOB))
+		return 'B';
+	else if (i915_mmio_reg_equal(dvo_reg, DVOC))
+		return 'C';
+	else
+		return '?';
+}
+
 void intel_dvo_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -428,8 +440,6 @@  void intel_dvo_init(struct drm_device *dev)
 	intel_dvo->attached_connector = intel_connector;
 
 	intel_encoder = &intel_dvo->base;
-	drm_encoder_init(dev, &intel_encoder->base,
-			 &intel_dvo_enc_funcs, encoder_type, NULL);
 
 	intel_encoder->disable = intel_disable_dvo;
 	intel_encoder->enable = intel_enable_dvo;
@@ -496,6 +506,10 @@  void intel_dvo_init(struct drm_device *dev)
 		if (!dvoinit)
 			continue;
 
+		drm_encoder_init(dev, &intel_encoder->base,
+				 &intel_dvo_enc_funcs, encoder_type,
+				 "DVO %c", intel_dvo_port_name(dvo->dvo_reg));
+
 		intel_encoder->type = INTEL_OUTPUT_DVO;
 		intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
 		switch (dvo->type) {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 00d065fee506..b403c9d8feb5 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2164,7 +2164,7 @@  void intel_hdmi_init(struct drm_device *dev,
 	intel_encoder = &intel_dig_port->base;
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_hdmi_enc_funcs,
-			 DRM_MODE_ENCODER_TMDS, NULL);
+			 DRM_MODE_ENCODER_TMDS, "HDMI %c", port_name(port));
 
 	intel_encoder->compute_config = intel_hdmi_compute_config;
 	if (HAS_PCH_SPLIT(dev)) {
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 0da0240caf81..470e4eb3f5eb 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1025,7 +1025,7 @@  void intel_lvds_init(struct drm_device *dev)
 			   DRM_MODE_CONNECTOR_LVDS);
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_lvds_enc_funcs,
-			 DRM_MODE_ENCODER_LVDS, NULL);
+			 DRM_MODE_ENCODER_LVDS, "LVDS");
 
 	intel_encoder->enable = intel_enable_lvds;
 	intel_encoder->pre_enable = intel_pre_enable_lvds;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 2e1da060b0e1..d47dba52ab05 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2979,7 +2979,7 @@  bool intel_sdvo_init(struct drm_device *dev,
 	intel_encoder = &intel_sdvo->base;
 	intel_encoder->type = INTEL_OUTPUT_SDVO;
 	drm_encoder_init(dev, &intel_encoder->base, &intel_sdvo_enc_funcs, 0,
-			 NULL);
+			 "SDVO %c", port_name(port));
 
 	/* Read the regs to test if we can talk to the device */
 	for (i = 0; i < 0x40; i++) {
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 948cbff6c62e..6ffb7cda6e0b 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1645,7 +1645,7 @@  intel_tv_init(struct drm_device *dev)
 			   DRM_MODE_CONNECTOR_SVIDEO);
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_tv_enc_funcs,
-			 DRM_MODE_ENCODER_TVDAC, NULL);
+			 DRM_MODE_ENCODER_TVDAC, "TV");
 
 	intel_encoder->compute_config = intel_tv_compute_config;
 	intel_encoder->get_config = intel_tv_get_config;