Re: [Intel-gfx] [PATCH] drm/i915: add Type-C port to encoder name
On Fri, Aug 30, 2019 at 09:57:00AM +0300, Jani Nikula wrote: > On Thu, 29 Aug 2019, Simon Ser wrote: > > This patch adds the Type-C port number to the encoder name. This is an > > alternative to [1]. > > > > [1]: https://patchwork.freedesktop.org/series/65695/ > > > > Signed-off-by: Simon Ser > > Cc: Imre Deak > > Cc: Manasi Navare > > Cc: Ville Syrjälä > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 11 ++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > index 8eb2b3ec01ed..e543d44a7105 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -4278,6 +4278,8 @@ void intel_ddi_init(struct drm_i915_private > > *dev_priv, enum port port) > > bool init_hdmi, init_dp, init_lspcon = false; > > enum pipe pipe; > > enum phy phy = intel_port_to_phy(dev_priv, port); > > + enum tc_port tc_port; > > + char tc_suffix[128]; > > Please let's try to avoid arbitrary large-ish buffers on stack in the > kernel. > > > > > init_hdmi = port_info->supports_dvi || port_info->supports_hdmi; > > init_dp = port_info->supports_dp; > > @@ -4307,8 +4309,15 @@ void intel_ddi_init(struct drm_i915_private > > *dev_priv, enum port port) > > intel_encoder = _dig_port->base; > > encoder = _encoder->base; > > > > + tc_port = intel_port_to_tc(dev_priv, port); > > + if (tc_port != PORT_TC_NONE) > > + snprintf(tc_suffix, sizeof(tc_suffix), "/TC#%d", tc_port + 1); > > + else > > + tc_suffix[0] = '\0'; > > I think I'd add a > > const char *tc_port_name(enum tc_port) > > similar to transcoder_name(). The magic +1 goes away as well. > > > + > > drm_encoder_init(_priv->drm, encoder, _ddi_funcs, > > -DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port)); > > +DRM_MODE_ENCODER_TMDS, "DDI %c%s", port_name(port), > > +tc_suffix); > > This will be a bit tedious, but straightforward: > > "DDI %c%s%s", port_name(port), tc_port == PORT_TC_NONE ? "" : "/", > tc_port == PORT_TC_NONE ? "" : tc_port_name(tc_port) > > The long-term idea would be to make port_name() handle all this > internally, but it seems like a fairly intrusive change, and AFAICT you > can't trivially have const strings for that. I think we just need to stop using port_name() and prefer encoder->name instead. -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add Type-C port to encoder name
On Thu, 29 Aug 2019, Simon Ser wrote: > This patch adds the Type-C port number to the encoder name. This is an > alternative to [1]. > > [1]: https://patchwork.freedesktop.org/series/65695/ > > Signed-off-by: Simon Ser > Cc: Imre Deak > Cc: Manasi Navare > Cc: Ville Syrjälä > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 8eb2b3ec01ed..e543d44a7105 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -4278,6 +4278,8 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, > enum port port) > bool init_hdmi, init_dp, init_lspcon = false; > enum pipe pipe; > enum phy phy = intel_port_to_phy(dev_priv, port); > + enum tc_port tc_port; > + char tc_suffix[128]; Please let's try to avoid arbitrary large-ish buffers on stack in the kernel. > > init_hdmi = port_info->supports_dvi || port_info->supports_hdmi; > init_dp = port_info->supports_dp; > @@ -4307,8 +4309,15 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, > enum port port) > intel_encoder = _dig_port->base; > encoder = _encoder->base; > > + tc_port = intel_port_to_tc(dev_priv, port); > + if (tc_port != PORT_TC_NONE) > + snprintf(tc_suffix, sizeof(tc_suffix), "/TC#%d", tc_port + 1); > + else > + tc_suffix[0] = '\0'; I think I'd add a const char *tc_port_name(enum tc_port) similar to transcoder_name(). The magic +1 goes away as well. > + > drm_encoder_init(_priv->drm, encoder, _ddi_funcs, > - DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port)); > + DRM_MODE_ENCODER_TMDS, "DDI %c%s", port_name(port), > + tc_suffix); This will be a bit tedious, but straightforward: "DDI %c%s%s", port_name(port), tc_port == PORT_TC_NONE ? "" : "/", tc_port == PORT_TC_NONE ? "" : tc_port_name(tc_port) The long-term idea would be to make port_name() handle all this internally, but it seems like a fairly intrusive change, and AFAICT you can't trivially have const strings for that. BR, Jani. > > intel_encoder->hotplug = intel_ddi_hotplug; > intel_encoder->compute_output_type = intel_ddi_compute_output_type; -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx