From: Jani Nikula <jani.nikula@intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/4] drm/i915/hti: abstract hti handling
Date: Thu, 17 Nov 2022 16:30:38 +0200 [thread overview]
Message-ID: <87cz9l1wmp.fsf@intel.com> (raw)
In-Reply-To: <Y3Y0rDpgIn2F15fd@intel.com>
On Thu, 17 Nov 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Wed, Nov 09, 2022 at 04:42:06PM +0200, Jani Nikula wrote:
>> The HTI or HDPORT handling is sprinkled around. Centralize to one place.
>>
>> Add a note about how subtle the mapping from HDPORT_STATE register to
>> dpll mask actually is.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>> drivers/gpu/drm/i915/Makefile | 1 +
>> drivers/gpu/drm/i915/display/intel_ddi.c | 9 +----
>> drivers/gpu/drm/i915/display/intel_display.c | 8 +---
>> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 11 +-----
>> drivers/gpu/drm/i915/display/intel_hti.c | 39 +++++++++++++++++++
>> drivers/gpu/drm/i915/display/intel_hti.h | 18 +++++++++
>> drivers/gpu/drm/i915/display/intel_hti_regs.h | 16 ++++++++
>> drivers/gpu/drm/i915/i915_reg.h | 5 ---
>> 8 files changed, 80 insertions(+), 27 deletions(-)
>> create mode 100644 drivers/gpu/drm/i915/display/intel_hti.c
>> create mode 100644 drivers/gpu/drm/i915/display/intel_hti.h
>> create mode 100644 drivers/gpu/drm/i915/display/intel_hti_regs.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 51704b54317c..cb8232bd315b 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -247,6 +247,7 @@ i915-y += \
>> display/intel_global_state.o \
>> display/intel_hdcp.o \
>> display/intel_hotplug.o \
>> + display/intel_hti.o \
>> display/intel_lpe_audio.o \
>> display/intel_modeset_verify.o \
>> display/intel_modeset_setup.o \
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index e95bde5cf060..5be9573bf65c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -56,6 +56,7 @@
>> #include "intel_hdcp.h"
>> #include "intel_hdmi.h"
>> #include "intel_hotplug.h"
>> +#include "intel_hti.h"
>> #include "intel_lspcon.h"
>> #include "intel_mg_phy_regs.h"
>> #include "intel_pps.h"
>> @@ -4113,12 +4114,6 @@ intel_ddi_max_lanes(struct intel_digital_port *dig_port)
>> return max_lanes;
>> }
>>
>> -static bool hti_uses_phy(struct drm_i915_private *i915, enum phy phy)
>> -{
>> - return i915->hti_state & HDPORT_ENABLED &&
>> - i915->hti_state & HDPORT_DDI_USED(phy);
>> -}
>> -
>> static enum hpd_pin xelpd_hpd_pin(struct drm_i915_private *dev_priv,
>> enum port port)
>> {
>> @@ -4247,7 +4242,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>> * driver. In that case we should skip initializing the corresponding
>> * outputs.
>> */
>> - if (hti_uses_phy(dev_priv, phy)) {
>> + if (intel_hti_uses_phy(dev_priv, phy)) {
>> drm_dbg_kms(&dev_priv->drm, "PORT %c / PHY %c reserved by HTI\n",
>> port_name(port), phy_name(phy));
>> return;
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 0d3353c403af..90219e7abc7c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -100,6 +100,7 @@
>> #include "intel_frontbuffer.h"
>> #include "intel_hdcp.h"
>> #include "intel_hotplug.h"
>> +#include "intel_hti.h"
>> #include "intel_modeset_verify.h"
>> #include "intel_modeset_setup.h"
>> #include "intel_overlay.h"
>> @@ -8735,12 +8736,7 @@ int intel_modeset_init_nogem(struct drm_i915_private *i915)
>> if (i915->display.cdclk.max_cdclk_freq == 0)
>> intel_update_max_cdclk(i915);
>>
>> - /*
>> - * If the platform has HTI, we need to find out whether it has reserved
>> - * any display resources before we create our display outputs.
>> - */
>> - if (INTEL_INFO(i915)->display.has_hti)
>> - i915->hti_state = intel_de_read(i915, HDPORT_STATE);
>> + intel_hti_init(i915);
>>
>> /* Just disable it once at startup */
>> intel_vga_disable(i915);
>> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> index 7c6c094a0a01..28f7dcb170c7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
>> @@ -30,6 +30,7 @@
>> #include "intel_dpio_phy.h"
>> #include "intel_dpll.h"
>> #include "intel_dpll_mgr.h"
>> +#include "intel_hti.h"
>> #include "intel_mg_phy_regs.h"
>> #include "intel_pch_refclk.h"
>> #include "intel_tc.h"
>> @@ -3163,14 +3164,6 @@ static void icl_update_active_dpll(struct intel_atomic_state *state,
>> icl_set_active_port_dpll(crtc_state, port_dpll_id);
>> }
>>
>> -static u32 intel_get_hti_plls(struct drm_i915_private *i915)
>> -{
>> - if (!(i915->hti_state & HDPORT_ENABLED))
>> - return 0;
>> -
>> - return REG_FIELD_GET(HDPORT_DPLL_USED_MASK, i915->hti_state);
>> -}
>> -
>> static int icl_compute_combo_phy_dpll(struct intel_atomic_state *state,
>> struct intel_crtc *crtc)
>> {
>> @@ -3245,7 +3238,7 @@ static int icl_get_combo_phy_dpll(struct intel_atomic_state *state,
>> }
>>
>> /* Eliminate DPLLs from consideration if reserved by HTI */
>> - dpll_mask &= ~intel_get_hti_plls(dev_priv);
>> + dpll_mask &= ~intel_hti_dpll_mask(dev_priv);
>>
>> port_dpll->pll = intel_find_shared_dpll(state, crtc,
>> &port_dpll->hw_state,
>> diff --git a/drivers/gpu/drm/i915/display/intel_hti.c b/drivers/gpu/drm/i915/display/intel_hti.c
>> new file mode 100644
>> index 000000000000..e2b09e96f9a9
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/display/intel_hti.c
>> @@ -0,0 +1,39 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#include "i915_drv.h"
>> +#include "intel_de.h"
>> +#include "intel_display.h"
>> +#include "intel_hti.h"
>> +#include "intel_hti_regs.h"
>> +
>> +void intel_hti_init(struct drm_i915_private *i915)
>> +{
>> + /*
>> + * If the platform has HTI, we need to find out whether it has reserved
>> + * any display resources before we create our display outputs.
>> + */
>> + if (INTEL_INFO(i915)->display.has_hti)
>> + i915->hti_state = intel_de_read(i915, HDPORT_STATE);
>> +}
>> +
>> +bool intel_hti_uses_phy(struct drm_i915_private *i915, enum phy phy)
>> +{
>> + return i915->hti_state & HDPORT_ENABLED &&
>> + i915->hti_state & HDPORT_DDI_USED(phy);
>> +}
>> +
>> +u32 intel_hti_dpll_mask(struct drm_i915_private *i915)
>> +{
>> + if (!(i915->hti_state & HDPORT_ENABLED))
>> + return 0;
>> +
>> + /*
>> + * Note: This is subtle. The values must coincide with what's defined
>> + * for the platform.
>> + */
>> + return REG_FIELD_GET(HDPORT_DPLL_USED_MASK, i915->hti_state);
>
> o_O
>
>> +}
>> +
>
> stray newline?
>
> For the series
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks for the review, pushed to drm-intel-next, and axed the newline
while pushing.
BR,
Jani.
>
>> diff --git a/drivers/gpu/drm/i915/display/intel_hti.h b/drivers/gpu/drm/i915/display/intel_hti.h
>> new file mode 100644
>> index 000000000000..2893d6668657
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/display/intel_hti.h
>> @@ -0,0 +1,18 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#ifndef __INTEL_HTI_H__
>> +#define __INTEL_HTI_H__
>> +
>> +#include <linux/types.h>
>> +
>> +struct drm_i915_private;
>> +enum phy;
>> +
>> +void intel_hti_init(struct drm_i915_private *i915);
>> +bool intel_hti_uses_phy(struct drm_i915_private *i915, enum phy phy);
>> +u32 intel_hti_dpll_mask(struct drm_i915_private *i915);
>> +
>> +#endif /* __INTEL_HTI_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_hti_regs.h b/drivers/gpu/drm/i915/display/intel_hti_regs.h
>> new file mode 100644
>> index 000000000000..e206f2837fc8
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/display/intel_hti_regs.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#ifndef __INTEL_HTI_REGS_H__
>> +#define __INTEL_HTI_REGS_H__
>> +
>> +#include "i915_reg_defs.h"
>> +
>> +#define HDPORT_STATE _MMIO(0x45050)
>> +#define HDPORT_DPLL_USED_MASK REG_GENMASK(15, 12)
>> +#define HDPORT_DDI_USED(phy) REG_BIT(2 * (phy) + 1)
>> +#define HDPORT_ENABLED REG_BIT(0)
>> +
>> +#endif /* __INTEL_HTI_REGS_H__ */
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index a37ed0c61f20..fd60d335d9cb 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1148,11 +1148,6 @@
>> #define MBUS_JOIN_PIPE_SELECT(pipe) REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
>> #define MBUS_JOIN_PIPE_SELECT_NONE MBUS_JOIN_PIPE_SELECT(7)
>>
>> -#define HDPORT_STATE _MMIO(0x45050)
>> -#define HDPORT_DPLL_USED_MASK REG_GENMASK(15, 12)
>> -#define HDPORT_DDI_USED(phy) REG_BIT(2 * (phy) + 1)
>> -#define HDPORT_ENABLED REG_BIT(0)
>> -
>> /* Make render/texture TLB fetches lower priorty than associated data
>> * fetches. This is not turned on by default
>> */
>> --
>> 2.34.1
--
Jani Nikula, Intel Open Source Graphics Center
prev parent reply other threads:[~2022-11-17 14:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-09 14:42 [Intel-gfx] [PATCH 1/4] drm/i915/hti: abstract hti handling Jani Nikula
2022-11-09 14:42 ` [Intel-gfx] [PATCH 2/4] drm/i915/display: move hti under display sub-struct Jani Nikula
2022-11-09 14:42 ` [Intel-gfx] [PATCH 3/4] drm/i915/display: move global_obj_list " Jani Nikula
2022-11-09 14:42 ` [Intel-gfx] [PATCH 4/4] drm/i915/display: move restore state and ctx " Jani Nikula
2022-11-10 17:01 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/i915/hti: abstract hti handling Patchwork
2022-11-10 17:01 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2022-11-10 17:28 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-11-10 20:14 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-11-17 13:18 ` [Intel-gfx] [PATCH 1/4] " Ville Syrjälä
2022-11-17 14:30 ` Jani Nikula [this message]
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=87cz9l1wmp.fsf@intel.com \
--to=jani.nikula@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.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.