Re: [Intel-gfx] [PATCH 1/3] drm/i915_hdmi: Fix the definition of intel_hdmi_dsc_get_bpp

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 25 Jan 2022, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
> Fix the data-type of the argument output_format to enum, for the
> function intel_hdmi_dsc_get_bpp.

The subject prefix should be "drm/i915/hdmi:".

>
> Fixes: 6e6cb758e035 ("drm/i915: Add helper functions for calculating DSC
> parameters for HDMI2.1")

The Fixes: tag should not be wrapped.

>

Superfluous blank line.

> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: "Ville Syrj_l_" <ville.syrjala@linux.intel.com>
> Cc: "Jos_ Roberto de Souza" <jose.souza@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Werner Sembach <wse@tuxedocomputers.com>
> Cc: Aditya Swarup <aditya.swarup@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Please consider trimming the Cc lines manually, this is quite excessive
for a patch this simple.

> Cc: <stable@vger.kernel.org> # v5.12+

No, we don't really want to backport this stuff to stable. There's no
functional impact here.

>

Superfluous blank line.

> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 4 ++--
>  drivers/gpu/drm/i915/display/intel_hdmi.h | 5 +++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 45cf0ab04009..381a9de3a015 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3126,8 +3126,8 @@ intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
>   */
>  int
>  intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width, int num_slices,
> -		       int output_format, bool hdmi_all_bpp,
> -		       int hdmi_max_chunk_bytes)
> +		       enum intel_output_format output_format,
> +		       bool hdmi_all_bpp, int hdmi_max_chunk_bytes)
>  {
>  	int max_dsc_bpp, min_dsc_bpp;
>  	int target_bytes;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.h b/drivers/gpu/drm/i915/display/intel_hdmi.h
> index b577c38fa90c..fe40e49d2962 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.h
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.h
> @@ -10,6 +10,7 @@
>  #include <linux/types.h>
>  
>  #include "i915_reg.h"
> +#include "intel_display_types.h"

Please avoid including headers from other headers if at all possible.

Here we can forward declare

enum intel_output_format;

along with the other forward declarations below. Strictly speaking it's
not standards compliant to forward declare enums but it does work and we
use it all over the place.

BR,
Jani.

>  
>  struct drm_connector;
>  struct drm_encoder;
> @@ -49,8 +50,8 @@ bool intel_hdmi_limited_color_range(const struct intel_crtc_state *crtc_state,
>  bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
>  			     int bpc, bool has_hdmi_sink, bool ycbcr420_output);
>  int intel_hdmi_dsc_get_bpp(int src_fractional_bpp, int slice_width,
> -			   int num_slices, int output_format, bool hdmi_all_bpp,
> -			   int hdmi_max_chunk_bytes);
> +			   int num_slices, enum intel_output_format output_format,
> +			   bool hdmi_all_bpp, int hdmi_max_chunk_bytes);
>  int intel_hdmi_dsc_get_num_slices(const struct intel_crtc_state *crtc_state,
>  				  int src_max_slices, int src_max_slice_width,
>  				  int hdmi_max_slices, int hdmi_throughput);

-- 
Jani Nikula, Intel Open Source Graphics Center