diff mbox series

[05/19] drm/i915/dp: Update Bigjoiner interface bits for computing compressed bpp

Message ID 20230713103346.1163315-6-ankit.k.nautiyal@intel.com (mailing list archive)
State New, archived
Headers show
Series DSC misc fixes | expand

Commit Message

Nautiyal, Ankit K July 13, 2023, 10:33 a.m. UTC 
In Bigjoiner check for DSC, bigjoiner interface bits for DP for
DISPLAY > 13 is 36 (Bspec: 49259).

v2: Corrected Display ver to 13.

v3: Follow convention for conditional statement. (Ville)

v4: Fix check for display ver. (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Stanislav Lisovskiy July 20, 2023, 9:29 a.m. UTC | #1 
On Thu, Jul 13, 2023 at 04:03:32PM +0530, Ankit Nautiyal wrote:
> In Bigjoiner check for DSC, bigjoiner interface bits for DP for
> DISPLAY > 13 is 36 (Bspec: 49259).
> 
> v2: Corrected Display ver to 13.
> 
> v3: Follow convention for conditional statement. (Ville)
> 
> v4: Fix check for display ver. (Ville)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 19768ac658ba..c1fd448d80e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -802,8 +802,9 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
>  	bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
>  
>  	if (bigjoiner) {
> +		int bigjoiner_interface_bits = DISPLAY_VER(i915) >= 14 ? 36 : 24;
>  		u32 max_bpp_bigjoiner =
> -			i915->display.cdclk.max_cdclk_freq * 48 /
> +			i915->display.cdclk.max_cdclk_freq * 2 * bigjoiner_interface_bits /

Probably "num_vdsc_instances = intel_dsc_get_num_vdsc_instances(crtc_state);" again,
instead of "2"? 

With that clarified, 

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

>  			intel_dp_mode_to_fec_clock(mode_clock);
>  
>  		bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);
> -- 
> 2.40.1
>
Nautiyal, Ankit K July 24, 2023, 12:19 p.m. UTC | #2 
Hi Stan,

Thanks for the reviews ans suggestions. Please my response inline:


On 7/20/2023 2:59 PM, Lisovskiy, Stanislav wrote:
> On Thu, Jul 13, 2023 at 04:03:32PM +0530, Ankit Nautiyal wrote:
>> In Bigjoiner check for DSC, bigjoiner interface bits for DP for
>> DISPLAY > 13 is 36 (Bspec: 49259).
>>
>> v2: Corrected Display ver to 13.
>>
>> v3: Follow convention for conditional statement. (Ville)
>>
>> v4: Fix check for display ver. (Ville)
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 19768ac658ba..c1fd448d80e1 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -802,8 +802,9 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
>>   	bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
>>   
>>   	if (bigjoiner) {
>> +		int bigjoiner_interface_bits = DISPLAY_VER(i915) >= 14 ? 36 : 24;
>>   		u32 max_bpp_bigjoiner =
>> -			i915->display.cdclk.max_cdclk_freq * 48 /
>> +			i915->display.cdclk.max_cdclk_freq * 2 * bigjoiner_interface_bits /
> Probably "num_vdsc_instances = intel_dsc_get_num_vdsc_instances(crtc_state);" again,
> instead of "2"?

Currently intel_dsc_get_num_vdsc_instances will give total number of 
vdsc_engines including joined pipes.

So with bigjoiner the function will return 4.

This was good to calculate Pipe BW check: (Pixel clock < PPC * CDCLK 
frequency * Number of pipes joined

as we get PPC * number of pipes joined from 
intel_dsc_get_num_vdsc_instances)

Or to calculate DSC_PIC_WIDTH PPS parameter.

But here we perhaps need intel_dsc_get_vdsc_engines_per_pipe that will 
just return 2 if dsc_split 1 otherwise.

Thanks & Regards,

Ankit

>
> With that clarified,
>
> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>
>>   			intel_dp_mode_to_fec_clock(mode_clock);
>>   
>>   		bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);
>> -- 
>> 2.40.1
>>
Stanislav Lisovskiy July 25, 2023, 10:13 a.m. UTC | #3 
On Mon, Jul 24, 2023 at 05:49:11PM +0530, Nautiyal, Ankit K wrote:
> Hi Stan,
> 
> Thanks for the reviews ans suggestions. Please my response inline:
> 
> 
> On 7/20/2023 2:59 PM, Lisovskiy, Stanislav wrote:
> > On Thu, Jul 13, 2023 at 04:03:32PM +0530, Ankit Nautiyal wrote:
> > > In Bigjoiner check for DSC, bigjoiner interface bits for DP for
> > > DISPLAY > 13 is 36 (Bspec: 49259).
> > > 
> > > v2: Corrected Display ver to 13.
> > > 
> > > v3: Follow convention for conditional statement. (Ville)
> > > 
> > > v4: Fix check for display ver. (Ville)
> > > 
> > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 19768ac658ba..c1fd448d80e1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -802,8 +802,9 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
> > >   	bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
> > >   	if (bigjoiner) {
> > > +		int bigjoiner_interface_bits = DISPLAY_VER(i915) >= 14 ? 36 : 24;
> > >   		u32 max_bpp_bigjoiner =
> > > -			i915->display.cdclk.max_cdclk_freq * 48 /
> > > +			i915->display.cdclk.max_cdclk_freq * 2 * bigjoiner_interface_bits /
> > Probably "num_vdsc_instances = intel_dsc_get_num_vdsc_instances(crtc_state);" again,
> > instead of "2"?
> 
> Currently intel_dsc_get_num_vdsc_instances will give total number of
> vdsc_engines including joined pipes.
> 
> So with bigjoiner the function will return 4.
> 
> This was good to calculate Pipe BW check: (Pixel clock < PPC * CDCLK
> frequency * Number of pipes joined
> 
> as we get PPC * number of pipes joined from
> intel_dsc_get_num_vdsc_instances)
> 
> Or to calculate DSC_PIC_WIDTH PPS parameter.
> 
> But here we perhaps need intel_dsc_get_vdsc_engines_per_pipe that will just
> return 2 if dsc_split 1 otherwise.
> 
> Thanks & Regards,
> 
> Ankit

Yes, I agree, unfortunately not applicable here.
May be yeah we need some function like that and then refactor
also the intel_dsc_get_num_vdsc_instances to use that one..

Stan

> 
> > 
> > With that clarified,
> > 
> > Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > 
> > >   			intel_dp_mode_to_fec_clock(mode_clock);
> > >   		bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);
> > > -- 
> > > 2.40.1
> > >
Nautiyal, Ankit K July 25, 2023, 11:19 a.m. UTC | #4 
On 7/25/2023 3:43 PM, Lisovskiy, Stanislav wrote:
> On Mon, Jul 24, 2023 at 05:49:11PM +0530, Nautiyal, Ankit K wrote:
>> Hi Stan,
>>
>> Thanks for the reviews ans suggestions. Please my response inline:
>>
>>
>> On 7/20/2023 2:59 PM, Lisovskiy, Stanislav wrote:
>>> On Thu, Jul 13, 2023 at 04:03:32PM +0530, Ankit Nautiyal wrote:
>>>> In Bigjoiner check for DSC, bigjoiner interface bits for DP for
>>>> DISPLAY > 13 is 36 (Bspec: 49259).
>>>>
>>>> v2: Corrected Display ver to 13.
>>>>
>>>> v3: Follow convention for conditional statement. (Ville)
>>>>
>>>> v4: Fix check for display ver. (Ville)
>>>>
>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> index 19768ac658ba..c1fd448d80e1 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>> @@ -802,8 +802,9 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
>>>>    	bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
>>>>    	if (bigjoiner) {
>>>> +		int bigjoiner_interface_bits = DISPLAY_VER(i915) >= 14 ? 36 : 24;
>>>>    		u32 max_bpp_bigjoiner =
>>>> -			i915->display.cdclk.max_cdclk_freq * 48 /
>>>> +			i915->display.cdclk.max_cdclk_freq * 2 * bigjoiner_interface_bits /
>>> Probably "num_vdsc_instances = intel_dsc_get_num_vdsc_instances(crtc_state);" again,
>>> instead of "2"?
>> Currently intel_dsc_get_num_vdsc_instances will give total number of
>> vdsc_engines including joined pipes.
>>
>> So with bigjoiner the function will return 4.
>>
>> This was good to calculate Pipe BW check: (Pixel clock < PPC * CDCLK
>> frequency * Number of pipes joined
>>
>> as we get PPC * number of pipes joined from
>> intel_dsc_get_num_vdsc_instances)
>>
>> Or to calculate DSC_PIC_WIDTH PPS parameter.
>>
>> But here we perhaps need intel_dsc_get_vdsc_engines_per_pipe that will just
>> return 2 if dsc_split 1 otherwise.
>>
>> Thanks & Regards,
>>
>> Ankit
> Yes, I agree, unfortunately not applicable here.
> May be yeah we need some function like that and then refactor
> also the intel_dsc_get_num_vdsc_instances to use that one..
>
> Stan

Alright, let me make the change in a separate patch and add to this series.

Thanks & Regards,

Ankit


>
>>> With that clarified,
>>>
>>> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>>>
>>>>    			intel_dp_mode_to_fec_clock(mode_clock);
>>>>    		bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);
>>>> -- 
>>>> 2.40.1
>>>>
Nautiyal, Ankit K July 28, 2023, 4:18 a.m. UTC | #5 
On 7/25/2023 4:49 PM, Nautiyal, Ankit K wrote:
>
> On 7/25/2023 3:43 PM, Lisovskiy, Stanislav wrote:
>> On Mon, Jul 24, 2023 at 05:49:11PM +0530, Nautiyal, Ankit K wrote:
>>> Hi Stan,
>>>
>>> Thanks for the reviews ans suggestions. Please my response inline:
>>>
>>>
>>> On 7/20/2023 2:59 PM, Lisovskiy, Stanislav wrote:
>>>> On Thu, Jul 13, 2023 at 04:03:32PM +0530, Ankit Nautiyal wrote:
>>>>> In Bigjoiner check for DSC, bigjoiner interface bits for DP for
>>>>> DISPLAY > 13 is 36 (Bspec: 49259).
>>>>>
>>>>> v2: Corrected Display ver to 13.
>>>>>
>>>>> v3: Follow convention for conditional statement. (Ville)
>>>>>
>>>>> v4: Fix check for display ver. (Ville)
>>>>>
>>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
>>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
>>>>> b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> index 19768ac658ba..c1fd448d80e1 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>>>>> @@ -802,8 +802,9 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct 
>>>>> drm_i915_private *i915,
>>>>>        bits_per_pixel = min(bits_per_pixel, 
>>>>> max_bpp_small_joiner_ram);
>>>>>        if (bigjoiner) {
>>>>> +        int bigjoiner_interface_bits = DISPLAY_VER(i915) >= 14 ? 
>>>>> 36 : 24;
>>>>>            u32 max_bpp_bigjoiner =
>>>>> -            i915->display.cdclk.max_cdclk_freq * 48 /
>>>>> +            i915->display.cdclk.max_cdclk_freq * 2 * 
>>>>> bigjoiner_interface_bits /
>>>> Probably "num_vdsc_instances = 
>>>> intel_dsc_get_num_vdsc_instances(crtc_state);" again,
>>>> instead of "2"?
>>> Currently intel_dsc_get_num_vdsc_instances will give total number of
>>> vdsc_engines including joined pipes.
>>>
>>> So with bigjoiner the function will return 4.
>>>
>>> This was good to calculate Pipe BW check: (Pixel clock < PPC * CDCLK
>>> frequency * Number of pipes joined
>>>
>>> as we get PPC * number of pipes joined from
>>> intel_dsc_get_num_vdsc_instances)
>>>
>>> Or to calculate DSC_PIC_WIDTH PPS parameter.
>>>
>>> But here we perhaps need intel_dsc_get_vdsc_engines_per_pipe that 
>>> will just
>>> return 2 if dsc_split 1 otherwise.
>>>
>>> Thanks & Regards,
>>>
>>> Ankit
>> Yes, I agree, unfortunately not applicable here.
>> May be yeah we need some function like that and then refactor
>> also the intel_dsc_get_num_vdsc_instances to use that one..
>>
>> Stan
>
> Alright, let me make the change in a separate patch and add to this 
> series.
>
> Thanks & Regards,
>
> Ankit
>
>
Since we call this function during mode valid too, so cannot directly 
use intel_dsc_get_num_vdsc_instance,

so I have put a comment for clarification about PPC in the latest version.

Regard,

Ankit


>>
>>>> With that clarified,
>>>>
>>>> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>>>>
>>>>> intel_dp_mode_to_fec_clock(mode_clock);
>>>>>            bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);
>>>>> -- 
>>>>> 2.40.1
>>>>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 19768ac658ba..c1fd448d80e1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -802,8 +802,9 @@  u16 intel_dp_dsc_get_max_compressed_bpp(struct drm_i915_private *i915,
 	bits_per_pixel = min(bits_per_pixel, max_bpp_small_joiner_ram);
 
 	if (bigjoiner) {
+		int bigjoiner_interface_bits = DISPLAY_VER(i915) >= 14 ? 36 : 24;
 		u32 max_bpp_bigjoiner =
-			i915->display.cdclk.max_cdclk_freq * 48 /
+			i915->display.cdclk.max_cdclk_freq * 2 * bigjoiner_interface_bits /
 			intel_dp_mode_to_fec_clock(mode_clock);
 
 		bits_per_pixel = min(bits_per_pixel, max_bpp_bigjoiner);