On Wed, Sep 04, 2019 at 06:20:37PM -0700, Matt Roper wrote:
> ICL+ DSI outputs operate a bit differently than DP/HDMI/eDP and are more
> closely tied to the PHY than to the DDI. Since we've separated PHY's
> out into their own namespace it makes sense to operate on 'enum phy'
> throughout most of the ICL DSI code rather than 'enum port' which we
> generally use to refer to the DDI these days. Part of this transition
> has already been done as part of commit dc867bc7d887 ("drm/i915/gen11:
> Convert combo PHY logic to use new 'enum phy' namespace"), although that
> patch only migrated the parts of the code that were specifically
> updating the combo PHY registers themselves. Moving the rest of the
> code over to the PHY namespace as well will hopefully make things more
> consistent and less confusing.
>
> Note that the change here is really just a naming change. On all of the
> platforms that this code currently applies to, the DSI outputs always
> have port==phy so we're still passing the same integer values around no
> matter which type of enum they belong to..
>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c | 236 ++++++++++++-----------
> drivers/gpu/drm/i915/display/intel_dsi.h | 5 +-
> 2 files changed, 126 insertions(+), 115 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6e398c33a524..5cf32032e795 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -65,9 +65,9 @@ static void wait_for_payload_credits(struct drm_i915_private *dev_priv,
> DRM_ERROR("DSI payload credits not released\n");
> }
>
> -static enum transcoder dsi_port_to_transcoder(enum port port)
> +static enum transcoder dsi_phy_to_transcoder(enum phy phy)
> {
> - if (port == PORT_A)
> + if (phy == PHY_A)
> return TRANSCODER_DSI_0;
> else
> return TRANSCODER_DSI_1;
> @@ -78,20 +78,20 @@ static void wait_for_cmds_dispatched_to_panel(struct intel_encoder *encoder)
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> struct mipi_dsi_device *dsi;
> - enum port port;
> + enum phy phy;
> enum transcoder dsi_trans;
> int ret;
>
> /* wait for header/payload credits to be released */
> - for_each_dsi_port(port, intel_dsi->ports) {
> - dsi_trans = dsi_port_to_transcoder(port);
> + for_each_dsi_phy(phy, intel_dsi->phys) {
> + dsi_trans = dsi_phy_to_transcoder(phy);
> wait_for_header_credits(dev_priv, dsi_trans);
> wait_for_payload_credits(dev_priv, dsi_trans);
> }
>
> /* send nop DCS command */
> - for_each_dsi_port(port, intel_dsi->ports) {
> - dsi = intel_dsi->dsi_hosts[port]->device;
> + for_each_dsi_phy(phy, intel_dsi->phys) {
> + dsi = intel_dsi->dsi_hosts[phy]->device;
> dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> dsi->channel = 0;
> ret = mipi_dsi_dcs_nop(dsi);
> @@ -100,14 +100,14 @@ static void wait_for_cmds_dispatched_to_panel(struct intel_encoder *encoder)
> }
>
> /* wait for header credits to be released */
> - for_each_dsi_port(port, intel_dsi->ports) {
> - dsi_trans = dsi_port_to_transcoder(port);
> + for_each_dsi_phy(phy, intel_dsi->phys) {
> + dsi_trans = dsi_phy_to_transcoder(phy);
> wait_for_header_credits(dev_priv, dsi_trans);
> }
>
> /* wait for LP TX in progress bit to be cleared */
> - for_each_dsi_port(port, intel_dsi->ports) {
> - dsi_trans = dsi_port_to_transcoder(port);
> + for_each_dsi_phy(phy, intel_dsi->phys) {
> + dsi_trans = dsi_phy_to_transcoder(phy);
> if (wait_for_us(!(I915_READ(DSI_LP_MSG(dsi_trans)) &
> LPTX_IN_PROGRESS), 20))
> DRM_ERROR("LPTX bit not cleared\n");
> @@ -119,7 +119,7 @@ static bool add_payld_to_queue(struct intel_dsi_host *host, const u8 *data,
> {
> struct intel_dsi *intel_dsi = host->intel_dsi;
> struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
> - enum transcoder dsi_trans = dsi_port_to_transcoder(host->port);
> + enum transcoder dsi_trans = dsi_phy_to_transcoder(host->phy);
> int free_credits;
> int i, j;
>
> @@ -146,7 +146,7 @@ static int dsi_send_pkt_hdr(struct intel_dsi_host *host,
> {
> struct intel_dsi *intel_dsi = host->intel_dsi;
> struct drm_i915_private *dev_priv = to_i915(intel_dsi->base.base.dev);
> - enum transcoder dsi_trans = dsi_port_to_transcoder(host->port);
> + enum transcoder dsi_trans = dsi_phy_to_transcoder(host->phy);
> u32 tmp;
> int free_credits;
>
> @@ -305,7 +305,7 @@ static void gen11_dsi_program_esc_clk_div(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> - enum port port;
> + enum phy phy;
> u32 bpp = mipi_dsi_pixel_format_to_bpp(intel_dsi->pixel_format);
> u32 afe_clk_khz; /* 8X Clock */
> u32 esc_clk_div_m;
> @@ -315,29 +315,29 @@ static void gen11_dsi_program_esc_clk_div(struct intel_encoder *encoder)
>
> esc_clk_div_m = DIV_ROUND_UP(afe_clk_khz, DSI_MAX_ESC_CLK);
>
> - for_each_dsi_port(port, intel_dsi->ports) {
> - I915_WRITE(ICL_DSI_ESC_CLK_DIV(port),
> + for_each_dsi_phy(phy, intel_dsi->phys) {
> + I915_WRITE(ICL_DSI_ESC_CLK_DIV(phy),
> esc_clk_div_m & ICL_ESC_CLK_DIV_MASK);
> - POSTING_READ(ICL_DSI_ESC_CLK_DIV(port));
> + POSTING_READ(ICL_DSI_ESC_CLK_DIV(phy));
> }
>
> - for_each_dsi_port(port, intel_dsi->ports) {
> - I915_WRITE(ICL_DPHY_ESC_CLK_DIV(port),
> + for_each_dsi_phy(phy, intel_dsi->phys) {
> + I915_WRITE(ICL_DPHY_ESC_CLK_DIV(phy),
> esc_clk_div_m & ICL_ESC_CLK_DIV_MASK);
> - POSTING_READ(ICL_DPHY_ESC_CLK_DIV(port));
> + POSTING_READ(ICL_DPHY_ESC_CLK_DIV(phy));
> }
> }
>
> static void get_dsi_io_power_domains(struct drm_i915_private *dev_priv,
> struct intel_dsi *intel_dsi)
> {
> - enum port port;
> + enum phy phy;
>
> - for_each_dsi_port(port, intel_dsi->ports) {
> - WARN_ON(intel_dsi->io_wakeref[port]);
> - intel_dsi->io_wakeref[port] =
> + for_each_dsi_phy(phy, intel_dsi->phys) {
> + WARN_ON(intel_dsi->io_wakeref[phy]);
> + intel_dsi->io_wakeref[phy] =
> intel_display_power_get(dev_priv,
> - port == PORT_A ?
> + phy == PHY_A ?
> POWER_DOMAIN_PORT_DDI_A_IO :
> POWER_DOMAIN_PORT_DDI_B_IO);
I think this is now getting a bit confusing w.r.t. the normal DDI code.
With normal DDI The IO power request is based on the DDI not the PHY,
and the same goes for stuff like DDI_BUF_CTL below. IMO would be nice
if we could keep these things consistent between the two paths.
As for the DSI specific registers/etc. I'm not sure if port or phy would
be more appropriate.
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
