All of lore.kernel.org
 help / color / mirror / Atom feed
From: Manasi Navare <manasi.d.navare@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v3 6/6] drm/i915: Have pfit calculations return an error code
Date: Wed, 22 Apr 2020 14:42:03 -0700	[thread overview]
Message-ID: <20200422214203.GD28167@intel.com> (raw)
In-Reply-To: <20200422161917.17389-6-ville.syrjala@linux.intel.com>

On Wed, Apr 22, 2020 at 07:19:17PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Change intel_{gmch,pch}_panel_fitting() to return a normal
> error vs. success int. We'll need this later to validate that
> the margin properties aren't misconfigured.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yes makes total sense to return an ERR Value

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c     | 10 +++++++---
>  drivers/gpu/drm/i915/display/intel_dp.c    | 10 +++++-----
>  drivers/gpu/drm/i915/display/intel_hdmi.c  | 23 +++++++++++-----------
>  drivers/gpu/drm/i915/display/intel_lvds.c  | 13 +++++++-----
>  drivers/gpu/drm/i915/display/intel_panel.c | 19 ++++++++++--------
>  drivers/gpu/drm/i915/display/intel_panel.h |  6 +++---
>  drivers/gpu/drm/i915/display/vlv_dsi.c     |  6 ++++--
>  7 files changed, 49 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 6650590f18fd..4fec5bd64920 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1526,13 +1526,17 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  						   base);
>  	struct intel_connector *intel_connector = intel_dsi->attached_connector;
>  	const struct drm_display_mode *fixed_mode =
> -					intel_connector->panel.fixed_mode;
> +		intel_connector->panel.fixed_mode;
>  	struct drm_display_mode *adjusted_mode =
> -					&pipe_config->hw.adjusted_mode;
> +		&pipe_config->hw.adjusted_mode;
> +	int ret;
>  
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  	intel_fixed_panel_mode(fixed_mode, adjusted_mode);
> -	intel_pch_panel_fitting(pipe_config, conn_state);
> +
> +	ret = intel_pch_panel_fitting(pipe_config, conn_state);
> +	if (ret)
> +		return ret;
>  
>  	adjusted_mode->flags = 0;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7c4c9a4f4238..5c7009b74c24 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2354,9 +2354,7 @@ intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
>  
>  	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>  
> -	intel_pch_panel_fitting(crtc_state, conn_state);
> -
> -	return 0;
> +	return intel_pch_panel_fitting(crtc_state, conn_state);
>  }
>  
>  bool intel_dp_limited_color_range(const struct intel_crtc_state *crtc_state,
> @@ -2568,9 +2566,11 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  				       adjusted_mode);
>  
>  		if (HAS_GMCH(dev_priv))
> -			intel_gmch_panel_fitting(pipe_config, conn_state);
> +			ret = intel_gmch_panel_fitting(pipe_config, conn_state);
>  		else
> -			intel_pch_panel_fitting(pipe_config, conn_state);
> +			ret = intel_pch_panel_fitting(pipe_config, conn_state);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 0edbdd39f462..010f37240710 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2321,24 +2321,27 @@ static bool hdmi_deep_color_possible(const struct intel_crtc_state *crtc_state,
>  	return true;
>  }
>  
> -static bool
> +static int
>  intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
>  			   const struct drm_connector_state *conn_state)
>  {
>  	struct drm_connector *connector = conn_state->connector;
>  	struct drm_i915_private *i915 = to_i915(connector->dev);
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->hw.adjusted_mode;
> +
> +	if (!drm_mode_is_420_only(&connector->display_info, adjusted_mode))
> +		return 0;
>  
>  	if (!connector->ycbcr_420_allowed) {
>  		drm_err(&i915->drm,
>  			"Platform doesn't support YCBCR420 output\n");
> -		return false;
> +		return -EINVAL;
>  	}
>  
>  	crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
>  
> -	intel_pch_panel_fitting(crtc_state, conn_state);
> -
> -	return true;
> +	return intel_pch_panel_fitting(crtc_state, conn_state);
>  }
>  
>  static int intel_hdmi_port_clock(int clock, int bpc)
> @@ -2466,13 +2469,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		pipe_config->pixel_multiplier = 2;
>  
> -	if (drm_mode_is_420_only(&connector->display_info, adjusted_mode)) {
> -		if (!intel_hdmi_ycbcr420_config(pipe_config, conn_state)) {
> -			drm_err(&dev_priv->drm,
> -				"Can't support YCBCR420 output\n");
> -			return -EINVAL;
> -		}
> -	}
> +	ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
> +	if (ret)
> +		return ret;
>  
>  	pipe_config->limited_color_range =
>  		intel_hdmi_limited_color_range(pipe_config, conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> index ae658d9354b7..872f2a489339 100644
> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> @@ -403,6 +403,7 @@ static int intel_lvds_compute_config(struct intel_encoder *intel_encoder,
>  	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->uapi.crtc);
>  	unsigned int lvds_bpp;
> +	int ret;
>  
>  	/* Should never happen!! */
>  	if (INTEL_GEN(dev_priv) < 4 && intel_crtc->pipe == 0) {
> @@ -436,13 +437,15 @@ static int intel_lvds_compute_config(struct intel_encoder *intel_encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
>  		return -EINVAL;
>  
> -	if (HAS_PCH_SPLIT(dev_priv)) {
> +	if (HAS_PCH_SPLIT(dev_priv))
>  		pipe_config->has_pch_encoder = true;
>  
> -		intel_pch_panel_fitting(pipe_config, conn_state);
> -	} else {
> -		intel_gmch_panel_fitting(pipe_config, conn_state);
> -	}
> +	if (HAS_GMCH(dev_priv))
> +		ret = intel_gmch_panel_fitting(pipe_config, conn_state);
> +	else
> +		ret = intel_pch_panel_fitting(pipe_config, conn_state);
> +	if (ret)
> +		return ret;
>  
>  	/*
>  	 * XXX: It would be nice to support lower refresh rates on the
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index b4bb1cfc54a9..aa931f9f0d6a 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -176,9 +176,8 @@ intel_panel_vbt_fixed_mode(struct intel_connector *connector)
>  }
>  
>  /* adjusted_mode has been preset to be the panel's fixed mode */
> -void
> -intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
> -			const struct drm_connector_state *conn_state)
> +int intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
> +			    const struct drm_connector_state *conn_state)
>  {
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->hw.adjusted_mode;
> @@ -188,7 +187,7 @@ intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
>  	if (adjusted_mode->crtc_hdisplay == crtc_state->pipe_src_w &&
>  	    adjusted_mode->crtc_vdisplay == crtc_state->pipe_src_h &&
>  	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420)
> -		return;
> +		return 0;
>  
>  	switch (conn_state->scaling_mode) {
>  	case DRM_MODE_SCALE_CENTER:
> @@ -239,12 +238,14 @@ intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
>  
>  	default:
>  		MISSING_CASE(conn_state->scaling_mode);
> -		return;
> +		return -EINVAL;
>  	}
>  
>  	drm_rect_init(&crtc_state->pch_pfit.dst,
>  		      x, y, width, height);
>  	crtc_state->pch_pfit.enabled = true;
> +
> +	return 0;
>  }
>  
>  static void
> @@ -381,8 +382,8 @@ static void i9xx_scale_aspect(struct intel_crtc_state *crtc_state,
>  	}
>  }
>  
> -void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
> -			      const struct drm_connector_state *conn_state)
> +int intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
> +			     const struct drm_connector_state *conn_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -431,7 +432,7 @@ void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
>  		break;
>  	default:
>  		MISSING_CASE(conn_state->scaling_mode);
> -		return;
> +		return -EINVAL;
>  	}
>  
>  	/* 965+ wants fuzzy fitting */
> @@ -452,6 +453,8 @@ void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
>  	crtc_state->gmch_pfit.control = pfit_control;
>  	crtc_state->gmch_pfit.pgm_ratios = pfit_pgm_ratios;
>  	crtc_state->gmch_pfit.lvds_border_bits = border;
> +
> +	return 0;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
> index a26db895038e..968b95281cb4 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.h
> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
> @@ -25,10 +25,10 @@ int intel_panel_init(struct intel_panel *panel,
>  void intel_panel_fini(struct intel_panel *panel);
>  void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
>  			    struct drm_display_mode *adjusted_mode);
> -void intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
> +int intel_pch_panel_fitting(struct intel_crtc_state *crtc_state,
> +			    const struct drm_connector_state *conn_state);
> +int intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
>  			     const struct drm_connector_state *conn_state);
> -void intel_gmch_panel_fitting(struct intel_crtc_state *crtc_state,
> -			      const struct drm_connector_state *conn_state);
>  void intel_panel_set_backlight_acpi(const struct drm_connector_state *conn_state,
>  				    u32 level, u32 max);
>  int intel_panel_setup_backlight(struct drm_connector *connector,
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 9c9ea89d2107..f582ab52f0b0 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -278,9 +278,11 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
>  		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
>  
>  		if (HAS_GMCH(dev_priv))
> -			intel_gmch_panel_fitting(pipe_config, conn_state);
> +			ret = intel_gmch_panel_fitting(pipe_config, conn_state);
>  		else
> -			intel_pch_panel_fitting(pipe_config, conn_state);
> +			ret = intel_pch_panel_fitting(pipe_config, conn_state);
> +		if (ret)
> +			return ret;
>  	}
>  
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
> -- 
> 2.24.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2020-04-22 21:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-22 16:19 [Intel-gfx] [PATCH v3 1/6] drm/i915: Fix skl+ non-scaled pfit modes Ville Syrjala
2020-04-22 16:19 ` [Intel-gfx] [PATCH v3 2/6] drm/i915: Flatten a bunch of the pfit functions Ville Syrjala
2020-04-22 16:19 ` [Intel-gfx] [PATCH v3 3/6] drm/i915: Use drm_rect to store the pfit window pos/size Ville Syrjala
2020-04-22 19:20   ` Manasi Navare
2020-04-23 15:38     ` Ville Syrjälä
2020-04-23 18:51       ` Manasi Navare
2020-04-22 16:19 ` [Intel-gfx] [PATCH v3 4/6] drm/i915: s/pipe_config/crtc_state/ in pfit functions Ville Syrjala
2020-04-22 19:26   ` Manasi Navare
2020-04-22 16:19 ` [Intel-gfx] [PATCH v3 5/6] drm/i915: Pass connector state to pfit calculations Ville Syrjala
2020-04-22 21:35   ` Manasi Navare
2020-04-22 16:19 ` [Intel-gfx] [PATCH v3 6/6] drm/i915: Have pfit calculations return an error code Ville Syrjala
2020-04-22 21:42   ` Manasi Navare [this message]
2020-04-22 18:21 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/6] drm/i915: Fix skl+ non-scaled pfit modes Patchwork
2020-04-22 18:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-04-22 22:44 ` [Intel-gfx] ✓ Fi.CI.IGT: " 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=20200422214203.GD28167@intel.com \
    --to=manasi.d.navare@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.