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

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 1/25/2022 3:15 PM, Jani Nikula wrote:
> 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:".
Thanks for pointing it out. Will fix this.
>
>> Fixes: 6e6cb758e035 ("drm/i915: Add helper functions for calculating DSC
>> parameters for HDMI2.1")
> The Fixes: tag should not be wrapped.
Noted.
>
> Superfluous blank line.

I think I just copy pasted from drm fixes, without taking care of this. 
Will fix this.

>
>> 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.
Noted. Will take care in next version.
>
>> 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.
Got it.
> Superfluous blank line.
Thanks for catching this, will remove.
>> 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.

Right that make sense, will get rid of the header-file and forward 
declare the enum.

Thanks for the inputs Jani. Will address all of these in the next version.

Regards,

Ankit

>
>>   
>>   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);