All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: <intel-gfx@lists.freedesktop.org>,
	<intel-xe@lists.freedesktop.org>, <lucas.demarchi@intel.com>
Subject: Re: [PATCH 4/6] drm/i915/display: switch to struct drm_device based pcode interface
Date: Wed, 18 Jun 2025 09:48:17 -0400	[thread overview]
Message-ID: <aFLDoTaYN8nETN-q@intel.com> (raw)
In-Reply-To: <8ee6ac6ffe87af00aedb5a7f18f402228f892be6@intel.com>

On Tue, Jun 17, 2025 at 11:29:54AM +0300, Jani Nikula wrote:
> On Mon, 16 Jun 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Thu, Jun 05, 2025 at 01:29:36PM +0300, Jani Nikula wrote:
> >> With the struct drm_device based pcode interface in place in both i915
> >> and xe, we can switch display code to use that, and ditch a number of
> >> struct drm_i915_private uses. Also drop the dependency on i915_drv.h
> >> from a couple of files.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/hsw_ips.c        | 13 ++---
> >>  drivers/gpu/drm/i915/display/intel_bw.c       | 23 ++++----
> >>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 56 +++++++++----------
> >>  .../drm/i915/display/intel_display_power.c    |  4 +-
> >>  .../i915/display/intel_display_power_well.c   |  4 +-
> >>  drivers/gpu/drm/i915/display/intel_hdcp.c     |  3 +-
> >>  drivers/gpu/drm/i915/display/skl_watermark.c  | 30 +++++-----
> >>  7 files changed, 58 insertions(+), 75 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/hsw_ips.c b/drivers/gpu/drm/i915/display/hsw_ips.c
> >> index 4307e2ed03d9..19b020a4ec22 100644
> >> --- a/drivers/gpu/drm/i915/display/hsw_ips.c
> >> +++ b/drivers/gpu/drm/i915/display/hsw_ips.c
> >> @@ -5,8 +5,9 @@
> >>  
> >>  #include <linux/debugfs.h>
> >>  
> >> +#include <drm/drm_print.h>
> >
> > I'm afraid I didn't understood why you are adding these includes here
> > and also below...
> 
> While switching to the new interface, we can drop the dependency on
> i915_drv.h, as mentioned in the commit message. And dropping that
> reveals missing includes, which wasn't mentioned.
> 
> Yeah, it could've been a separate patch, I guess. :/

Ah, great. No worries then. On that I will trust your compiler more than
my eyes ;)

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Btw, Jani Saarinen just pointed out that I had put the rv-b 3 times in
the same patch, while I had reviewed all the patches and was attempting
to add the rv-b individually to make your life easier with b4-shazam.
(facepalm)

> 
> BR,
> Jani.
> 
> 
> 
> >
> >> +
> >>  #include "hsw_ips.h"
> >> -#include "i915_drv.h"
> >>  #include "i915_reg.h"
> >>  #include "intel_color_regs.h"
> >>  #include "intel_de.h"
> >> @@ -17,8 +18,6 @@
> >>  static void hsw_ips_enable(const struct intel_crtc_state *crtc_state)
> >>  {
> >>  	struct intel_display *display = to_intel_display(crtc_state);
> >> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> >> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> >>  	u32 val;
> >>  
> >>  	if (!crtc_state->ips_enabled)
> >> @@ -39,8 +38,8 @@ static void hsw_ips_enable(const struct intel_crtc_state *crtc_state)
> >>  
> >>  	if (display->platform.broadwell) {
> >>  		drm_WARN_ON(display->drm,
> >> -			    snb_pcode_write(&i915->uncore, DISPLAY_IPS_CONTROL,
> >> -					    val | IPS_PCODE_CONTROL));
> >> +			    intel_pcode_write(display->drm, DISPLAY_IPS_CONTROL,
> >> +					      val | IPS_PCODE_CONTROL));
> >>  		/*
> >>  		 * Quoting Art Runyan: "its not safe to expect any particular
> >>  		 * value in IPS_CTL bit 31 after enabling IPS through the
> >> @@ -65,8 +64,6 @@ static void hsw_ips_enable(const struct intel_crtc_state *crtc_state)
> >>  bool hsw_ips_disable(const struct intel_crtc_state *crtc_state)
> >>  {
> >>  	struct intel_display *display = to_intel_display(crtc_state);
> >> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> >> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> >>  	bool need_vblank_wait = false;
> >>  
> >>  	if (!crtc_state->ips_enabled)
> >> @@ -74,7 +71,7 @@ bool hsw_ips_disable(const struct intel_crtc_state *crtc_state)
> >>  
> >>  	if (display->platform.broadwell) {
> >>  		drm_WARN_ON(display->drm,
> >> -			    snb_pcode_write(&i915->uncore, DISPLAY_IPS_CONTROL, 0));
> >> +			    intel_pcode_write(display->drm, DISPLAY_IPS_CONTROL, 0));
> >>  		/*
> >>  		 * Wait for PCODE to finish disabling IPS. The BSpec specified
> >>  		 * 42ms timeout value leads to occasional timeouts so use 100ms
> >> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> >> index 97aef729f7d4..82f131c3f8d3 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> >> @@ -80,14 +80,13 @@ static int icl_pcode_read_qgv_point_info(struct intel_display *display,
> >>  					 struct intel_qgv_point *sp,
> >>  					 int point)
> >>  {
> >> -	struct drm_i915_private *i915 = to_i915(display->drm);
> >>  	u32 val = 0, val2 = 0;
> >>  	u16 dclk;
> >>  	int ret;
> >>  
> >> -	ret = snb_pcode_read(&i915->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> >> -			     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point),
> >> -			     &val, &val2);
> >> +	ret = intel_pcode_read(display->drm, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> >> +			       ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point),
> >> +			       &val, &val2);
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> @@ -108,13 +107,12 @@ static int icl_pcode_read_qgv_point_info(struct intel_display *display,
> >>  static int adls_pcode_read_psf_gv_point_info(struct intel_display *display,
> >>  					     struct intel_psf_gv_point *points)
> >>  {
> >> -	struct drm_i915_private *i915 = to_i915(display->drm);
> >>  	u32 val = 0;
> >>  	int ret;
> >>  	int i;
> >>  
> >> -	ret = snb_pcode_read(&i915->uncore, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> >> -			     ADL_PCODE_MEM_SS_READ_PSF_GV_INFO, &val, NULL);
> >> +	ret = intel_pcode_read(display->drm, ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> >> +			       ADL_PCODE_MEM_SS_READ_PSF_GV_INFO, &val, NULL);
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> @@ -155,18 +153,17 @@ static bool is_sagv_enabled(struct intel_display *display, u16 points_mask)
> >>  int icl_pcode_restrict_qgv_points(struct intel_display *display,
> >>  				  u32 points_mask)
> >>  {
> >> -	struct drm_i915_private *i915 = to_i915(display->drm);
> >>  	int ret;
> >>  
> >>  	if (DISPLAY_VER(display) >= 14)
> >>  		return 0;
> >>  
> >>  	/* bspec says to keep retrying for at least 1 ms */
> >> -	ret = skl_pcode_request(&i915->uncore, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
> >> -				points_mask,
> >> -				ICL_PCODE_REP_QGV_MASK | ADLS_PCODE_REP_PSF_MASK,
> >> -				ICL_PCODE_REP_QGV_SAFE | ADLS_PCODE_REP_PSF_SAFE,
> >> -				1);
> >> +	ret = intel_pcode_request(display->drm, ICL_PCODE_SAGV_DE_MEM_SS_CONFIG,
> >> +				  points_mask,
> >> +				  ICL_PCODE_REP_QGV_MASK | ADLS_PCODE_REP_PSF_MASK,
> >> +				  ICL_PCODE_REP_QGV_SAFE | ADLS_PCODE_REP_PSF_SAFE,
> >> +				  1);
> >>  
> >>  	if (ret < 0) {
> >>  		drm_err(display->drm,
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> index 7ad506da7d3d..f60bf8a06541 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> @@ -840,7 +840,6 @@ static void bdw_set_cdclk(struct intel_display *display,
> >>  			  const struct intel_cdclk_config *cdclk_config,
> >>  			  enum pipe pipe)
> >>  {
> >> -	struct drm_i915_private *dev_priv = to_i915(display->drm);
> >>  	int cdclk = cdclk_config->cdclk;
> >>  	int ret;
> >>  
> >> @@ -853,7 +852,7 @@ static void bdw_set_cdclk(struct intel_display *display,
> >>  		     "trying to change cdclk frequency with cdclk not enabled\n"))
> >>  		return;
> >>  
> >> -	ret = snb_pcode_write(&dev_priv->uncore, BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
> >> +	ret = intel_pcode_write(display->drm, BDW_PCODE_DISPLAY_FREQ_CHANGE_REQ, 0x0);
> >>  	if (ret) {
> >>  		drm_err(display->drm,
> >>  			"failed to inform pcode about cdclk change\n");
> >> @@ -881,8 +880,8 @@ static void bdw_set_cdclk(struct intel_display *display,
> >>  			 LCPLL_CD_SOURCE_FCLK_DONE) == 0, 1))
> >>  		drm_err(display->drm, "Switching back to LCPLL failed\n");
> >>  
> >> -	snb_pcode_write(&dev_priv->uncore, HSW_PCODE_DE_WRITE_FREQ_REQ,
> >> -			cdclk_config->voltage_level);
> >> +	intel_pcode_write(display->drm, HSW_PCODE_DE_WRITE_FREQ_REQ,
> >> +			  cdclk_config->voltage_level);
> >>  
> >>  	intel_de_write(display, CDCLK_FREQ,
> >>  		       DIV_ROUND_CLOSEST(cdclk, 1000) - 1);
> >> @@ -1122,7 +1121,6 @@ static void skl_set_cdclk(struct intel_display *display,
> >>  			  const struct intel_cdclk_config *cdclk_config,
> >>  			  enum pipe pipe)
> >>  {
> >> -	struct drm_i915_private *dev_priv = to_i915(display->drm);
> >>  	int cdclk = cdclk_config->cdclk;
> >>  	int vco = cdclk_config->vco;
> >>  	u32 freq_select, cdclk_ctl;
> >> @@ -1139,10 +1137,10 @@ static void skl_set_cdclk(struct intel_display *display,
> >>  	drm_WARN_ON_ONCE(display->drm,
> >>  			 display->platform.skylake && vco == 8640000);
> >>  
> >> -	ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
> >> -				SKL_CDCLK_PREPARE_FOR_CHANGE,
> >> -				SKL_CDCLK_READY_FOR_CHANGE,
> >> -				SKL_CDCLK_READY_FOR_CHANGE, 3);
> >> +	ret = intel_pcode_request(display->drm, SKL_PCODE_CDCLK_CONTROL,
> >> +				  SKL_CDCLK_PREPARE_FOR_CHANGE,
> >> +				  SKL_CDCLK_READY_FOR_CHANGE,
> >> +				  SKL_CDCLK_READY_FOR_CHANGE, 3);
> >>  	if (ret) {
> >>  		drm_err(display->drm,
> >>  			"Failed to inform PCU about cdclk change (%d)\n", ret);
> >> @@ -1185,8 +1183,8 @@ static void skl_set_cdclk(struct intel_display *display,
> >>  	intel_de_posting_read(display, CDCLK_CTL);
> >>  
> >>  	/* inform PCU of the change */
> >> -	snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
> >> -			cdclk_config->voltage_level);
> >> +	intel_pcode_write(display->drm, SKL_PCODE_CDCLK_CONTROL,
> >> +			  cdclk_config->voltage_level);
> >>  
> >>  	intel_update_cdclk(display);
> >>  }
> >> @@ -2122,7 +2120,6 @@ static void bxt_set_cdclk(struct intel_display *display,
> >>  			  const struct intel_cdclk_config *cdclk_config,
> >>  			  enum pipe pipe)
> >>  {
> >> -	struct drm_i915_private *dev_priv = to_i915(display->drm);
> >>  	struct intel_cdclk_config mid_cdclk_config;
> >>  	int cdclk = cdclk_config->cdclk;
> >>  	int ret = 0;
> >> @@ -2136,18 +2133,18 @@ static void bxt_set_cdclk(struct intel_display *display,
> >>  	if (DISPLAY_VER(display) >= 14 || display->platform.dg2)
> >>  		; /* NOOP */
> >>  	else if (DISPLAY_VER(display) >= 11)
> >> -		ret = skl_pcode_request(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
> >> -					SKL_CDCLK_PREPARE_FOR_CHANGE,
> >> -					SKL_CDCLK_READY_FOR_CHANGE,
> >> -					SKL_CDCLK_READY_FOR_CHANGE, 3);
> >> +		ret = intel_pcode_request(display->drm, SKL_PCODE_CDCLK_CONTROL,
> >> +					  SKL_CDCLK_PREPARE_FOR_CHANGE,
> >> +					  SKL_CDCLK_READY_FOR_CHANGE,
> >> +					  SKL_CDCLK_READY_FOR_CHANGE, 3);
> >>  	else
> >>  		/*
> >>  		 * BSpec requires us to wait up to 150usec, but that leads to
> >>  		 * timeouts; the 2ms used here is based on experiment.
> >>  		 */
> >> -		ret = snb_pcode_write_timeout(&dev_priv->uncore,
> >> -					      HSW_PCODE_DE_WRITE_FREQ_REQ,
> >> -					      0x80000000, 2);
> >> +		ret = intel_pcode_write_timeout(display->drm,
> >> +						HSW_PCODE_DE_WRITE_FREQ_REQ,
> >> +						0x80000000, 2);
> >>  
> >>  	if (ret) {
> >>  		drm_err(display->drm,
> >> @@ -2176,8 +2173,8 @@ static void bxt_set_cdclk(struct intel_display *display,
> >>  		 * Display versions 14 and beyond
> >>  		 */;
> >>  	else if (DISPLAY_VER(display) >= 11 && !display->platform.dg2)
> >> -		ret = snb_pcode_write(&dev_priv->uncore, SKL_PCODE_CDCLK_CONTROL,
> >> -				      cdclk_config->voltage_level);
> >> +		ret = intel_pcode_write(display->drm, SKL_PCODE_CDCLK_CONTROL,
> >> +					cdclk_config->voltage_level);
> >>  	if (DISPLAY_VER(display) < 11) {
> >>  		/*
> >>  		 * The timeout isn't specified, the 2ms used here is based on
> >> @@ -2185,9 +2182,9 @@ static void bxt_set_cdclk(struct intel_display *display,
> >>  		 * FIXME: Waiting for the request completion could be delayed
> >>  		 * until the next PCODE request based on BSpec.
> >>  		 */
> >> -		ret = snb_pcode_write_timeout(&dev_priv->uncore,
> >> -					      HSW_PCODE_DE_WRITE_FREQ_REQ,
> >> -					      cdclk_config->voltage_level, 2);
> >> +		ret = intel_pcode_write_timeout(display->drm,
> >> +						HSW_PCODE_DE_WRITE_FREQ_REQ,
> >> +						cdclk_config->voltage_level, 2);
> >>  	}
> >>  	if (ret) {
> >>  		drm_err(display->drm,
> >> @@ -2473,7 +2470,6 @@ static void intel_pcode_notify(struct intel_display *display,
> >>  			       bool cdclk_update_valid,
> >>  			       bool pipe_count_update_valid)
> >>  {
> >> -	struct drm_i915_private *i915 = to_i915(display->drm);
> >>  	int ret;
> >>  	u32 update_mask = 0;
> >>  
> >> @@ -2488,11 +2484,11 @@ static void intel_pcode_notify(struct intel_display *display,
> >>  	if (pipe_count_update_valid)
> >>  		update_mask |= DISPLAY_TO_PCODE_PIPE_COUNT_VALID;
> >>  
> >> -	ret = skl_pcode_request(&i915->uncore, SKL_PCODE_CDCLK_CONTROL,
> >> -				SKL_CDCLK_PREPARE_FOR_CHANGE |
> >> -				update_mask,
> >> -				SKL_CDCLK_READY_FOR_CHANGE,
> >> -				SKL_CDCLK_READY_FOR_CHANGE, 3);
> >> +	ret = intel_pcode_request(display->drm, SKL_PCODE_CDCLK_CONTROL,
> >> +				  SKL_CDCLK_PREPARE_FOR_CHANGE |
> >> +				  update_mask,
> >> +				  SKL_CDCLK_READY_FOR_CHANGE,
> >> +				  SKL_CDCLK_READY_FOR_CHANGE, 3);
> >>  	if (ret)
> >>  		drm_err(display->drm,
> >>  			"Failed to inform PCU about display config (err %d)\n",
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> >> index 8e8c3a2f401b..562d15f8c38c 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> >> @@ -1255,10 +1255,8 @@ static u32 hsw_read_dcomp(struct intel_display *display)
> >>  
> >>  static void hsw_write_dcomp(struct intel_display *display, u32 val)
> >>  {
> >> -	struct drm_i915_private *dev_priv = to_i915(display->drm);
> >> -
> >>  	if (display->platform.haswell) {
> >> -		if (snb_pcode_write(&dev_priv->uncore, GEN6_PCODE_WRITE_D_COMP, val))
> >> +		if (intel_pcode_write(display->drm, GEN6_PCODE_WRITE_D_COMP, val))
> >>  			drm_dbg_kms(display->drm, "Failed to write to D_COMP\n");
> >>  	} else {
> >>  		intel_de_write(display, D_COMP_BDW, val);
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> >> index e60f60ddbff7..c05b9349d806 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> >> @@ -485,7 +485,7 @@ static void icl_tc_cold_exit(struct intel_display *display)
> >>  	int ret, tries = 0;
> >>  
> >>  	while (1) {
> >> -		ret = snb_pcode_write(&i915->uncore, ICL_PCODE_EXIT_TCCOLD, 0);
> >> +		ret = intel_pcode_write(display->drm, ICL_PCODE_EXIT_TCCOLD, 0);
> >>  		if (ret != -EAGAIN || ++tries == 3)
> >>  			break;
> >>  		msleep(1);
> >> @@ -1764,7 +1764,7 @@ tgl_tc_cold_request(struct intel_display *display, bool block)
> >>  		 * Spec states that we should timeout the request after 200us
> >>  		 * but the function below will timeout after 500us
> >>  		 */
> >> -		ret = snb_pcode_read(&i915->uncore, TGL_PCODE_TCCOLD, &low_val, &high_val);
> >> +		ret = intel_pcode_read(display->drm, TGL_PCODE_TCCOLD, &low_val, &high_val);
> >>  		if (ret == 0) {
> >>  			if (block &&
> >>  			    (low_val & TGL_PCODE_EXIT_TCCOLD_DATA_L_EXIT_FAILED))
> >> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> index 3e3038f4ee1f..52808cab95dd 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> >> @@ -373,7 +373,6 @@ static void intel_hdcp_clear_keys(struct intel_display *display)
> >>  
> >>  static int intel_hdcp_load_keys(struct intel_display *display)
> >>  {
> >> -	struct drm_i915_private *i915 = to_i915(display->drm);
> >>  	int ret;
> >>  	u32 val;
> >>  
> >> @@ -398,7 +397,7 @@ static int intel_hdcp_load_keys(struct intel_display *display)
> >>  	 * Mailbox interface.
> >>  	 */
> >>  	if (DISPLAY_VER(display) == 9 && !display->platform.broxton) {
> >> -		ret = snb_pcode_write(&i915->uncore, SKL_PCODE_LOAD_HDCP_KEYS, 1);
> >> +		ret = intel_pcode_write(display->drm, SKL_PCODE_LOAD_HDCP_KEYS, 1);
> >>  		if (ret) {
> >>  			drm_err(display->drm,
> >>  				"Failed to initiate HDCP key load (%d)\n",
> >> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> >> index 817939f6d4dd..df5522511dda 100644
> >> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> >> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> >> @@ -6,10 +6,12 @@
> >>  #include <linux/debugfs.h>
> >>  
> >>  #include <drm/drm_blend.h>
> >> +#include <drm/drm_file.h>
> >> +#include <drm/drm_print.h>
> >>  
> >>  #include "soc/intel_dram.h"
> >> -#include "i915_drv.h"
> >>  #include "i915_reg.h"
> >> +#include "i915_utils.h"
> >>  #include "i9xx_wm.h"
> >>  #include "intel_atomic.h"
> >>  #include "intel_atomic_plane.h"
> >> @@ -85,8 +87,6 @@ intel_has_sagv(struct intel_display *display)
> >>  static u32
> >>  intel_sagv_block_time(struct intel_display *display)
> >>  {
> >> -	struct drm_i915_private *i915 = to_i915(display->drm);
> >> -
> >>  	if (DISPLAY_VER(display) >= 14) {
> >>  		u32 val;
> >>  
> >> @@ -97,9 +97,9 @@ intel_sagv_block_time(struct intel_display *display)
> >>  		u32 val = 0;
> >>  		int ret;
> >>  
> >> -		ret = snb_pcode_read(&i915->uncore,
> >> -				     GEN12_PCODE_READ_SAGV_BLOCK_TIME_US,
> >> -				     &val, NULL);
> >> +		ret = intel_pcode_read(display->drm,
> >> +				       GEN12_PCODE_READ_SAGV_BLOCK_TIME_US,
> >> +				       &val, NULL);
> >>  		if (ret) {
> >>  			drm_dbg_kms(display->drm, "Couldn't read SAGV block time!\n");
> >>  			return 0;
> >> @@ -157,7 +157,6 @@ static void intel_sagv_init(struct intel_display *display)
> >>   */
> >>  static void skl_sagv_enable(struct intel_display *display)
> >>  {
> >> -	struct drm_i915_private *i915 = to_i915(display->drm);
> >>  	int ret;
> >>  
> >>  	if (!intel_has_sagv(display))
> >> @@ -167,8 +166,8 @@ static void skl_sagv_enable(struct intel_display *display)
> >>  		return;
> >>  
> >>  	drm_dbg_kms(display->drm, "Enabling SAGV\n");
> >> -	ret = snb_pcode_write(&i915->uncore, GEN9_PCODE_SAGV_CONTROL,
> >> -			      GEN9_SAGV_ENABLE);
> >> +	ret = intel_pcode_write(display->drm, GEN9_PCODE_SAGV_CONTROL,
> >> +				GEN9_SAGV_ENABLE);
> >>  
> >>  	/* We don't need to wait for SAGV when enabling */
> >>  
> >> @@ -190,7 +189,6 @@ static void skl_sagv_enable(struct intel_display *display)
> >>  
> >>  static void skl_sagv_disable(struct intel_display *display)
> >>  {
> >> -	struct drm_i915_private *i915 = to_i915(display->drm);
> >>  	int ret;
> >>  
> >>  	if (!intel_has_sagv(display))
> >> @@ -201,10 +199,9 @@ static void skl_sagv_disable(struct intel_display *display)
> >>  
> >>  	drm_dbg_kms(display->drm, "Disabling SAGV\n");
> >>  	/* bspec says to keep retrying for at least 1 ms */
> >> -	ret = skl_pcode_request(&i915->uncore, GEN9_PCODE_SAGV_CONTROL,
> >> -				GEN9_SAGV_DISABLE,
> >> -				GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED,
> >> -				1);
> >> +	ret = intel_pcode_request(display->drm, GEN9_PCODE_SAGV_CONTROL,
> >> +				  GEN9_SAGV_DISABLE,
> >> +				  GEN9_SAGV_IS_DISABLED, GEN9_SAGV_IS_DISABLED, 1);
> >>  	/*
> >>  	 * Some skl systems, pre-release machines in particular,
> >>  	 * don't actually have SAGV.
> >> @@ -3277,7 +3274,6 @@ static void mtl_read_wm_latency(struct intel_display *display, u16 wm[])
> >>  
> >>  static void skl_read_wm_latency(struct intel_display *display, u16 wm[])
> >>  {
> >> -	struct drm_i915_private *i915 = to_i915(display->drm);
> >>  	int num_levels = display->wm.num_levels;
> >>  	int read_latency = DISPLAY_VER(display) >= 12 ? 3 : 2;
> >>  	int mult = display->platform.dg2 ? 2 : 1;
> >> @@ -3286,7 +3282,7 @@ static void skl_read_wm_latency(struct intel_display *display, u16 wm[])
> >>  
> >>  	/* read the first set of memory latencies[0:3] */
> >>  	val = 0; /* data0 to be programmed to 0 for first set */
> >> -	ret = snb_pcode_read(&i915->uncore, GEN9_PCODE_READ_MEM_LATENCY, &val, NULL);
> >> +	ret = intel_pcode_read(display->drm, GEN9_PCODE_READ_MEM_LATENCY, &val, NULL);
> >>  	if (ret) {
> >>  		drm_err(display->drm, "SKL Mailbox read error = %d\n", ret);
> >>  		return;
> >> @@ -3299,7 +3295,7 @@ static void skl_read_wm_latency(struct intel_display *display, u16 wm[])
> >>  
> >>  	/* read the second set of memory latencies[4:7] */
> >>  	val = 1; /* data0 to be programmed to 1 for second set */
> >> -	ret = snb_pcode_read(&i915->uncore, GEN9_PCODE_READ_MEM_LATENCY, &val, NULL);
> >> +	ret = intel_pcode_read(display->drm, GEN9_PCODE_READ_MEM_LATENCY, &val, NULL);
> >>  	if (ret) {
> >>  		drm_err(display->drm, "SKL Mailbox read error = %d\n", ret);
> >>  		return;
> >> -- 
> >> 2.39.5
> >> 
> 
> -- 
> Jani Nikula, Intel

  reply	other threads:[~2025-06-18 13:48 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-05 10:29 [PATCH 0/6] drm/i915, drm/xe: add drm device based pcode interface for display Jani Nikula
2025-06-05 10:29 ` [PATCH 1/6] drm/i915/pcode: drop fast wait from snb_pcode_write_timeout() Jani Nikula
2025-06-16 20:41   ` Rodrigo Vivi
2025-06-16 20:48   ` Rodrigo Vivi
2025-06-16 20:48   ` Rodrigo Vivi
2025-06-05 10:29 ` [PATCH 2/6] drm/i915/pcode: add struct drm_device based interface Jani Nikula
2025-06-16 20:45   ` Rodrigo Vivi
2025-06-05 10:29 ` [PATCH 3/6] drm/xe/pcode: " Jani Nikula
2025-06-16 20:44   ` Rodrigo Vivi
2025-06-05 10:29 ` [PATCH 4/6] drm/i915/display: switch to struct drm_device based pcode interface Jani Nikula
2025-06-16 20:47   ` Rodrigo Vivi
2025-06-17  8:29     ` Jani Nikula
2025-06-18 13:48       ` Rodrigo Vivi [this message]
2025-06-05 10:29 ` [PATCH 5/6] drm/i915/dram: " Jani Nikula
2025-06-18 13:49   ` Rodrigo Vivi
2025-06-05 10:29 ` [PATCH 6/6] drm/xe/compat: remove old pcode compat interface Jani Nikula
2025-06-18 13:49   ` Rodrigo Vivi
2025-06-05 12:33 ` ✗ Fi.CI.SPARSE: warning for drm/i915, drm/xe: add drm device based pcode interface for display Patchwork
2025-06-05 12:56 ` ✓ i915.CI.BAT: success " Patchwork
2025-06-18 15:27   ` Jani Nikula
2025-06-05 14:51 ` ✓ CI.Patch_applied: " Patchwork
2025-06-05 14:51 ` ✓ CI.checkpatch: " Patchwork
2025-06-05 14:52 ` ✓ CI.KUnit: " Patchwork
2025-06-05 15:03 ` ✓ CI.Build: " Patchwork
2025-06-05 15:06 ` ✓ CI.Hooks: " Patchwork
2025-06-05 15:08 ` ✗ CI.checksparse: warning " Patchwork
2025-06-06  5:29 ` ✓ Xe.CI.BAT: success " Patchwork
2025-06-06 23:06 ` ✗ Xe.CI.Full: failure " Patchwork
2025-06-23 10:49 ` ✗ Fi.CI.BUILD: failure for drm/i915, drm/xe: add drm device based pcode interface for display (rev2) Patchwork

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=aFLDoTaYN8nETN-q@intel.com \
    --to=rodrigo.vivi@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=lucas.demarchi@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.