Re: [PATCH] drm/dsc: Return unsigned long on compute offset

2019-11-20 Thread Mikita Lipski



On 20/11/2019 05:17, Ville Syrjälä wrote:

On Tue, Nov 19, 2019 at 04:11:43PM -0500, Mikita Lipski wrote:



On 19/11/2019 16:09, Mikita Lipski wrote:



On 19/11/2019 12:11, Ville Syrjälä wrote:

On Tue, Nov 19, 2019 at 04:59:40PM +, Cornij, Nikola wrote:

If you're going to make all of them the same, then u64, please.

This is because I'm not sure if calculations require 64-bit at some
stage.


If it does then it's already broken. Someone should probably figure out
what's actally needed instead of shooting ducks with an icbm.




Sorry made a type below. Supposed to be "I don't think it is broken"


I mean that it's broken if it actually needs u64 when it's
currently using unsigned long. So u64 is either overkill or the
code is currently broken.



None of the calculations exceed u32, so u64 would be an overkill, since 
none of the variables in the structure exceed 16 bits. Therefore u32 is 
enough.





I don't think it is not broken, cause I'm currently testing DSC.
The patch I sent early simply fixes the error of comparing  signed and
unsigned variables.

We can then submit a second patch addressing the issue of using unsigned
long int instead of u32. Also, since the variables in drm_dsc_config
structure are all of type u8 and u16, the calculation values shouldn't
exceed the size of u32.

Thanks



-Original Message-
From: Lipski, Mikita 
Sent: November 19, 2019 10:08 AM
To: Ville Syrjälä ; Lipski, Mikita

Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
Cornij, Nikola 
Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset



On 19/11/2019 09:56, Ville Syrjälä wrote:

On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote:

From: Mikita Lipski 

We shouldn't compare int with unsigned long to find the max value and
since we are not expecting negative value returned from
compute_offset we should make this function return unsigned long so
we can compare the values when computing rc parameters.


Why are there other unsigned longs in dsc parameter computation in the
first place?


I believe it was initially set to be unsigned long for variable
consistency, when we ported intel_compute_rc_parameters into
drm_dsc_compute_rc_parameters. But now that I look at it, we can
actually just set them to u32 or u64, as nothing should exceed that.




Cc: Nikola Cornij 
Cc: Harry Wentland 
Signed-off-by: Mikita Lipski 
---
    drivers/gpu/drm/drm_dsc.c | 6 +++---
    1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index 74f3527f567d..ec40604ab6a2 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct
drm_dsc_picture_parameter_set *pps_payload,
    }
    EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
-static int compute_offset(struct drm_dsc_config *vdsc_cfg, int
pixels_per_group,
+static unsigned long compute_offset(struct drm_dsc_config
*vdsc_cfg, int pixels_per_group,
    int groups_per_line, int grpcnt)
    {
-    int offset = 0;
-    int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay,
pixels_per_group);
+    unsigned long offset = 0;
+    unsigned long grpcnt_id =
DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group);
    if (grpcnt <= grpcnt_id)
    offset = DIV_ROUND_UP(grpcnt * pixels_per_group *
vdsc_cfg->bits_per_pixel, 16);
--
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel




--
Thanks,
Mikita Lipski
Software Engineer 2, AMD
mikita.lip...@amd.com






--
Thanks,
Mikita Lipski
Software Engineer 2, AMD
mikita.lip...@amd.com




--
Thanks,
Mikita Lipski
Software Engineer 2, AMD
mikita.lip...@amd.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/dsc: Return unsigned long on compute offset

2019-11-20 Thread Ville Syrjälä
On Tue, Nov 19, 2019 at 04:11:43PM -0500, Mikita Lipski wrote:
> 
> 
> On 19/11/2019 16:09, Mikita Lipski wrote:
> > 
> > 
> > On 19/11/2019 12:11, Ville Syrjälä wrote:
> >> On Tue, Nov 19, 2019 at 04:59:40PM +, Cornij, Nikola wrote:
> >>> If you're going to make all of them the same, then u64, please.
> >>>
> >>> This is because I'm not sure if calculations require 64-bit at some 
> >>> stage.
> >>
> >> If it does then it's already broken. Someone should probably figure out
> >> what's actally needed instead of shooting ducks with an icbm.
> >>
> 
> 
> Sorry made a type below. Supposed to be "I don't think it is broken"

I mean that it's broken if it actually needs u64 when it's
currently using unsigned long. So u64 is either overkill or the
code is currently broken.

> 
> > I don't think it is not broken, cause I'm currently testing DSC.
> > The patch I sent early simply fixes the error of comparing  signed and 
> > unsigned variables.
> > 
> > We can then submit a second patch addressing the issue of using unsigned 
> > long int instead of u32. Also, since the variables in drm_dsc_config 
> > structure are all of type u8 and u16, the calculation values shouldn't 
> > exceed the size of u32.
> > 
> > Thanks
> > 
> >>>
> >>> -Original Message-----
> >>> From: Lipski, Mikita 
> >>> Sent: November 19, 2019 10:08 AM
> >>> To: Ville Syrjälä ; Lipski, Mikita 
> >>> 
> >>> Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; 
> >>> Cornij, Nikola 
> >>> Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset
> >>>
> >>>
> >>>
> >>> On 19/11/2019 09:56, Ville Syrjälä wrote:
> >>>> On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote:
> >>>>> From: Mikita Lipski 
> >>>>>
> >>>>> We shouldn't compare int with unsigned long to find the max value and
> >>>>> since we are not expecting negative value returned from
> >>>>> compute_offset we should make this function return unsigned long so
> >>>>> we can compare the values when computing rc parameters.
> >>>>
> >>>> Why are there other unsigned longs in dsc parameter computation in the
> >>>> first place?
> >>>
> >>> I believe it was initially set to be unsigned long for variable 
> >>> consistency, when we ported intel_compute_rc_parameters into 
> >>> drm_dsc_compute_rc_parameters. But now that I look at it, we can 
> >>> actually just set them to u32 or u64, as nothing should exceed that.
> >>>>
> >>>>>
> >>>>> Cc: Nikola Cornij 
> >>>>> Cc: Harry Wentland 
> >>>>> Signed-off-by: Mikita Lipski 
> >>>>> ---
> >>>>>    drivers/gpu/drm/drm_dsc.c | 6 +++---
> >>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> >>>>> index 74f3527f567d..ec40604ab6a2 100644
> >>>>> --- a/drivers/gpu/drm/drm_dsc.c
> >>>>> +++ b/drivers/gpu/drm/drm_dsc.c
> >>>>> @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct 
> >>>>> drm_dsc_picture_parameter_set *pps_payload,
> >>>>>    }
> >>>>>    EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
> >>>>> -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int 
> >>>>> pixels_per_group,
> >>>>> +static unsigned long compute_offset(struct drm_dsc_config 
> >>>>> *vdsc_cfg, int pixels_per_group,
> >>>>>    int groups_per_line, int grpcnt)
> >>>>>    {
> >>>>> -    int offset = 0;
> >>>>> -    int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
> >>>>> pixels_per_group);
> >>>>> +    unsigned long offset = 0;
> >>>>> +    unsigned long grpcnt_id = 
> >>>>> DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group);
> >>>>>    if (grpcnt <= grpcnt_id)
> >>>>>    offset = DIV_ROUND_UP(grpcnt * pixels_per_group * 
> >>>>> vdsc_cfg->bits_per_pixel, 16);
> >>>>> -- 
> >>>>> 2.17.1
> >>>>>
> >>>>> ___
> >>>>> dri-devel mailing list
> >>>>> dri-devel@lists.freedesktop.org
> >>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>>
> >>>
> >>> -- 
> >>> Thanks,
> >>> Mikita Lipski
> >>> Software Engineer 2, AMD
> >>> mikita.lip...@amd.com
> >>
> > 
> 
> -- 
> Thanks,
> Mikita Lipski
> Software Engineer 2, AMD
> mikita.lip...@amd.com

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/dsc: Return unsigned long on compute offset

2019-11-19 Thread Mikita Lipski



On 19/11/2019 16:09, Mikita Lipski wrote:



On 19/11/2019 12:11, Ville Syrjälä wrote:

On Tue, Nov 19, 2019 at 04:59:40PM +, Cornij, Nikola wrote:

If you're going to make all of them the same, then u64, please.

This is because I'm not sure if calculations require 64-bit at some 
stage.


If it does then it's already broken. Someone should probably figure out
what's actally needed instead of shooting ducks with an icbm.




Sorry made a type below. Supposed to be "I don't think it is broken"


I don't think it is not broken, cause I'm currently testing DSC.
The patch I sent early simply fixes the error of comparing  signed and 
unsigned variables.


We can then submit a second patch addressing the issue of using unsigned 
long int instead of u32. Also, since the variables in drm_dsc_config 
structure are all of type u8 and u16, the calculation values shouldn't 
exceed the size of u32.


Thanks



-Original Message-
From: Lipski, Mikita 
Sent: November 19, 2019 10:08 AM
To: Ville Syrjälä ; Lipski, Mikita 

Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; 
Cornij, Nikola 

Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset



On 19/11/2019 09:56, Ville Syrjälä wrote:

On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote:

From: Mikita Lipski 

We shouldn't compare int with unsigned long to find the max value and
since we are not expecting negative value returned from
compute_offset we should make this function return unsigned long so
we can compare the values when computing rc parameters.


Why are there other unsigned longs in dsc parameter computation in the
first place?


I believe it was initially set to be unsigned long for variable 
consistency, when we ported intel_compute_rc_parameters into 
drm_dsc_compute_rc_parameters. But now that I look at it, we can 
actually just set them to u32 or u64, as nothing should exceed that.




Cc: Nikola Cornij 
Cc: Harry Wentland 
Signed-off-by: Mikita Lipski 
---
   drivers/gpu/drm/drm_dsc.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index 74f3527f567d..ec40604ab6a2 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct 
drm_dsc_picture_parameter_set *pps_payload,

   }
   EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
-static int compute_offset(struct drm_dsc_config *vdsc_cfg, int 
pixels_per_group,
+static unsigned long compute_offset(struct drm_dsc_config 
*vdsc_cfg, int pixels_per_group,

   int groups_per_line, int grpcnt)
   {
-    int offset = 0;
-    int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
pixels_per_group);

+    unsigned long offset = 0;
+    unsigned long grpcnt_id = 
DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, pixels_per_group);

   if (grpcnt <= grpcnt_id)
   offset = DIV_ROUND_UP(grpcnt * pixels_per_group * 
vdsc_cfg->bits_per_pixel, 16);

--
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel




--
Thanks,
Mikita Lipski
Software Engineer 2, AMD
mikita.lip...@amd.com






--
Thanks,
Mikita Lipski
Software Engineer 2, AMD
mikita.lip...@amd.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/dsc: Return unsigned long on compute offset

2019-11-19 Thread Mikita Lipski



On 19/11/2019 12:11, Ville Syrjälä wrote:

On Tue, Nov 19, 2019 at 04:59:40PM +, Cornij, Nikola wrote:

If you're going to make all of them the same, then u64, please.

This is because I'm not sure if calculations require 64-bit at some stage.


If it does then it's already broken. Someone should probably figure out
what's actally needed instead of shooting ducks with an icbm.


I don't think it is not broken, cause I'm currently testing DSC.
The patch I sent early simply fixes the error of comparing  signed and 
unsigned variables.


We can then submit a second patch addressing the issue of using unsigned 
long int instead of u32. Also, since the variables in drm_dsc_config 
structure are all of type u8 and u16, the calculation values shouldn't 
exceed the size of u32.


Thanks



-Original Message-
From: Lipski, Mikita 
Sent: November 19, 2019 10:08 AM
To: Ville Syrjälä ; Lipski, Mikita 

Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Cornij, Nikola 

Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset



On 19/11/2019 09:56, Ville Syrjälä wrote:

On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote:

From: Mikita Lipski 

We shouldn't compare int with unsigned long to find the max value and
since we are not expecting negative value returned from
compute_offset we should make this function return unsigned long so
we can compare the values when computing rc parameters.


Why are there other unsigned longs in dsc parameter computation in the
first place?


I believe it was initially set to be unsigned long for variable consistency, 
when we ported intel_compute_rc_parameters into drm_dsc_compute_rc_parameters. 
But now that I look at it, we can actually just set them to u32 or u64, as 
nothing should exceed that.




Cc: Nikola Cornij 
Cc: Harry Wentland 
Signed-off-by: Mikita Lipski 
---
   drivers/gpu/drm/drm_dsc.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index 74f3527f567d..ec40604ab6a2 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct 
drm_dsc_picture_parameter_set *pps_payload,
   }
   EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
   
-static int compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group,

+static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int 
pixels_per_group,
int groups_per_line, int grpcnt)
   {
-   int offset = 0;
-   int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
pixels_per_group);
+   unsigned long offset = 0;
+   unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
pixels_per_group);
   
   	if (grpcnt <= grpcnt_id)

offset = DIV_ROUND_UP(grpcnt * pixels_per_group * 
vdsc_cfg->bits_per_pixel, 16);
--
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel




--
Thanks,
Mikita Lipski
Software Engineer 2, AMD
mikita.lip...@amd.com




--
Thanks,
Mikita Lipski
Software Engineer 2, AMD
mikita.lip...@amd.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/dsc: Return unsigned long on compute offset

2019-11-19 Thread Ville Syrjälä
On Tue, Nov 19, 2019 at 04:59:40PM +, Cornij, Nikola wrote:
> If you're going to make all of them the same, then u64, please.
> 
> This is because I'm not sure if calculations require 64-bit at some stage.

If it does then it's already broken. Someone should probably figure out
what's actally needed instead of shooting ducks with an icbm.

> 
> -Original Message-
> From: Lipski, Mikita  
> Sent: November 19, 2019 10:08 AM
> To: Ville Syrjälä ; Lipski, Mikita 
> 
> Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Cornij, 
> Nikola 
> Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset
> 
> 
> 
> On 19/11/2019 09:56, Ville Syrjälä wrote:
> > On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote:
> >> From: Mikita Lipski 
> >>
> >> We shouldn't compare int with unsigned long to find the max value and 
> >> since we are not expecting negative value returned from 
> >> compute_offset we should make this function return unsigned long so 
> >> we can compare the values when computing rc parameters.
> > 
> > Why are there other unsigned longs in dsc parameter computation in the 
> > first place?
> 
> I believe it was initially set to be unsigned long for variable consistency, 
> when we ported intel_compute_rc_parameters into 
> drm_dsc_compute_rc_parameters. But now that I look at it, we can actually 
> just set them to u32 or u64, as nothing should exceed that.
> > 
> >>
> >> Cc: Nikola Cornij 
> >> Cc: Harry Wentland 
> >> Signed-off-by: Mikita Lipski 
> >> ---
> >>   drivers/gpu/drm/drm_dsc.c | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> >> index 74f3527f567d..ec40604ab6a2 100644
> >> --- a/drivers/gpu/drm/drm_dsc.c
> >> +++ b/drivers/gpu/drm/drm_dsc.c
> >> @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct 
> >> drm_dsc_picture_parameter_set *pps_payload,
> >>   }
> >>   EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
> >>   
> >> -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int 
> >> pixels_per_group,
> >> +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int 
> >> pixels_per_group,
> >>int groups_per_line, int grpcnt)
> >>   {
> >> -  int offset = 0;
> >> -  int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
> >> pixels_per_group);
> >> +  unsigned long offset = 0;
> >> +  unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
> >> pixels_per_group);
> >>   
> >>if (grpcnt <= grpcnt_id)
> >>offset = DIV_ROUND_UP(grpcnt * pixels_per_group * 
> >> vdsc_cfg->bits_per_pixel, 16);
> >> -- 
> >> 2.17.1
> >>
> >> ___
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> -- 
> Thanks,
> Mikita Lipski
> Software Engineer 2, AMD
> mikita.lip...@amd.com

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH] drm/dsc: Return unsigned long on compute offset

2019-11-19 Thread Cornij, Nikola
If you're going to make all of them the same, then u64, please.

This is because I'm not sure if calculations require 64-bit at some stage.

-Original Message-
From: Lipski, Mikita  
Sent: November 19, 2019 10:08 AM
To: Ville Syrjälä ; Lipski, Mikita 

Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Cornij, 
Nikola 
Subject: Re: [PATCH] drm/dsc: Return unsigned long on compute offset



On 19/11/2019 09:56, Ville Syrjälä wrote:
> On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote:
>> From: Mikita Lipski 
>>
>> We shouldn't compare int with unsigned long to find the max value and 
>> since we are not expecting negative value returned from 
>> compute_offset we should make this function return unsigned long so 
>> we can compare the values when computing rc parameters.
> 
> Why are there other unsigned longs in dsc parameter computation in the 
> first place?

I believe it was initially set to be unsigned long for variable consistency, 
when we ported intel_compute_rc_parameters into drm_dsc_compute_rc_parameters. 
But now that I look at it, we can actually just set them to u32 or u64, as 
nothing should exceed that.
> 
>>
>> Cc: Nikola Cornij 
>> Cc: Harry Wentland 
>> Signed-off-by: Mikita Lipski 
>> ---
>>   drivers/gpu/drm/drm_dsc.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
>> index 74f3527f567d..ec40604ab6a2 100644
>> --- a/drivers/gpu/drm/drm_dsc.c
>> +++ b/drivers/gpu/drm/drm_dsc.c
>> @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct 
>> drm_dsc_picture_parameter_set *pps_payload,
>>   }
>>   EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
>>   
>> -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int 
>> pixels_per_group,
>> +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int 
>> pixels_per_group,
>>  int groups_per_line, int grpcnt)
>>   {
>> -int offset = 0;
>> -int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
>> pixels_per_group);
>> +unsigned long offset = 0;
>> +unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
>> pixels_per_group);
>>   
>>  if (grpcnt <= grpcnt_id)
>>  offset = DIV_ROUND_UP(grpcnt * pixels_per_group * 
>> vdsc_cfg->bits_per_pixel, 16);
>> -- 
>> 2.17.1
>>
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thanks,
Mikita Lipski
Software Engineer 2, AMD
mikita.lip...@amd.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/dsc: Return unsigned long on compute offset

2019-11-19 Thread Mikita Lipski



On 19/11/2019 09:56, Ville Syrjälä wrote:

On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote:

From: Mikita Lipski 

We shouldn't compare int with unsigned long to find the max value
and since we are not expecting negative value returned from
compute_offset we should make this function return unsigned long
so we can compare the values when computing rc parameters.


Why are there other unsigned longs in dsc parameter computation
in the first place?


I believe it was initially set to be unsigned long for variable 
consistency, when we ported intel_compute_rc_parameters into 
drm_dsc_compute_rc_parameters. But now that I look at it, we can 
actually just set them to u32 or u64, as nothing should exceed that.




Cc: Nikola Cornij 
Cc: Harry Wentland 
Signed-off-by: Mikita Lipski 
---
  drivers/gpu/drm/drm_dsc.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
index 74f3527f567d..ec40604ab6a2 100644
--- a/drivers/gpu/drm/drm_dsc.c
+++ b/drivers/gpu/drm/drm_dsc.c
@@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct 
drm_dsc_picture_parameter_set *pps_payload,
  }
  EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
  
-static int compute_offset(struct drm_dsc_config *vdsc_cfg, int pixels_per_group,

+static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int 
pixels_per_group,
int groups_per_line, int grpcnt)
  {
-   int offset = 0;
-   int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
pixels_per_group);
+   unsigned long offset = 0;
+   unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
pixels_per_group);
  
  	if (grpcnt <= grpcnt_id)

offset = DIV_ROUND_UP(grpcnt * pixels_per_group * 
vdsc_cfg->bits_per_pixel, 16);
--
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel




--
Thanks,
Mikita Lipski
Software Engineer 2, AMD
mikita.lip...@amd.com
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/dsc: Return unsigned long on compute offset

2019-11-19 Thread Ville Syrjälä
On Tue, Nov 19, 2019 at 09:45:26AM -0500, mikita.lip...@amd.com wrote:
> From: Mikita Lipski 
> 
> We shouldn't compare int with unsigned long to find the max value
> and since we are not expecting negative value returned from
> compute_offset we should make this function return unsigned long
> so we can compare the values when computing rc parameters.

Why are there other unsigned longs in dsc parameter computation
in the first place?

> 
> Cc: Nikola Cornij 
> Cc: Harry Wentland 
> Signed-off-by: Mikita Lipski 
> ---
>  drivers/gpu/drm/drm_dsc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dsc.c b/drivers/gpu/drm/drm_dsc.c
> index 74f3527f567d..ec40604ab6a2 100644
> --- a/drivers/gpu/drm/drm_dsc.c
> +++ b/drivers/gpu/drm/drm_dsc.c
> @@ -245,11 +245,11 @@ void drm_dsc_pps_payload_pack(struct 
> drm_dsc_picture_parameter_set *pps_payload,
>  }
>  EXPORT_SYMBOL(drm_dsc_pps_payload_pack);
>  
> -static int compute_offset(struct drm_dsc_config *vdsc_cfg, int 
> pixels_per_group,
> +static unsigned long compute_offset(struct drm_dsc_config *vdsc_cfg, int 
> pixels_per_group,
>   int groups_per_line, int grpcnt)
>  {
> - int offset = 0;
> - int grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
> pixels_per_group);
> + unsigned long offset = 0;
> + unsigned long grpcnt_id = DIV_ROUND_UP(vdsc_cfg->initial_xmit_delay, 
> pixels_per_group);
>  
>   if (grpcnt <= grpcnt_id)
>   offset = DIV_ROUND_UP(grpcnt * pixels_per_group * 
> vdsc_cfg->bits_per_pixel, 16);
> -- 
> 2.17.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel