Re: [Intel-gfx] [PATCH 1/4] drm/i915/hti: abstract hti handling
On Thu, 17 Nov 2022, Ville Syrjälä 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
>> ---
>> 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
Re: [Intel-gfx] [PATCH 1/4] drm/i915/hti: abstract hti handling
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
> ---
> 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)
> {
> @@ -3
