from:"David Binderman"

include/drm/i915_drm.h:96: possible bad bitmask ?

2016-08-08 Thread David Binderman

Hello there,

Recent versions of gcc say this:

include/drm/i915_drm.h:96:34: warning: result of â65535 << 20â
requires 37 bits to represent, but âintâ only has 32 bits
[-Wshift-overflow=]

Source code is

#define   INTEL_BSM_MASK (0x << 20)

Maybe something like

#define   INTEL_BSM_MASK (0xUL<< 20)

might be better.


Regards

David Binderman

linux-5.1/drivers/video/fbdev/via/viafbdev.c:233: bad assignment ?

2019-05-08 Thread David Binderman

Hello there,

[linux-5.1/drivers/video/fbdev/via/viafbdev.c:233]: (style) Assignment 
'depth=30' is redundant with condition 'depth==30'.

Source code is

else if (depth == 30)
depth = 30;

Suggest code rework.

Regards

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

linux-5.4-rc2/drivers/video/fbdev/amifb.c:1875: bad expression ?

2019-10-08 Thread David Binderman

Hello there,

linux-5.4-rc2/drivers/video/fbdev/amifb.c:1875:41: error: Expression 
'(*(lspr+delta)<<16)|(*lspr++)' depends on order of evaluation of side effects 
[unknownEvaluationOrder]

Source code is

  datawords = (*(lspr + delta) << 16) | (*lspr++);

Maybe better code:

  datawords = (*(lspr + delta) << 16) | (*lspr);
  ++lspr;

Regards

David Binderman

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

linux-5.6-rc1/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:8411:45: error: Division by zero.

2020-02-10 Thread David Binderman

Hello there,

Source code is

unsigned int vsync_rate_hz = 0;
struct dc_static_screen_params params = {0};
/* Calculate number of static frames before generating interrupt to
 * enter PSR.
 */
unsigned int frame_time_microsec = 100 / vsync_rate_hz;

Suggest code rework.

Regards

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

drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c:33:33: error: Buffer is accessed out of bounds

2020-02-10 Thread David Binderman

Hello there,

linux-5.6-rc1/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c:33:33: 
error: Buffer is accessed out of bounds: hdcp->auth.msg.hdcp1.bksv 
[bufferAccessOutOfBounds]

Source code is

   memcpy(&n, hdcp->auth.msg.hdcp1.bksv, sizeof(uint64_t));

Field bksv is only five bytes long. Suggest code rework.

Regards

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

linux-5.3-rc3 bug report

2019-08-06 Thread David Binderman

Hello there,

linux-5.3-rc3/drivers/gpu/drm/nouveau/nouveau_svm.c:763]: (style) Array index 
'i' is used before limits check.

Source code is

for (i = 0; buffer->fault[i] && i < buffer->entries; i++)

Maybe better code:

for (i = 0; (i < buffer->entries) && buffer->fault[i]; i++)

Regards

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

drivers/gpu/drm/bridge/fsl-ldb.c:101: possible loss of information.

2023-03-07 Thread David Binderman

Hello there,

I just ran the static analyser "cppcheck" over the source code of 
linux-6.2-rc1. It said:

linux-6.3-rc1/drivers/gpu/drm/bridge/fsl-ldb.c:101:3: style: int result is 
returned as long value. If the return value is long to avoid loss of 
information, then you have loss of information. [truncLongCastReturn]

Source code is

static unsigned long fsl_ldb_link_frequency(struct fsl_ldb *fsl_ldb, int clock)
{
if (fsl_ldb->lvds_dual_link)
return clock * 3500;
else
return clock * 7000;
}

Depending on the range of the value of clock, maybe unsigned long literals, 
like 3500UL, should
have been used ?

Regards

David Binderman

dcn321_fpu.c:626: Array index check in wrong place ?

2022-11-07 Thread David Binderman

Hello there,

Static analyser cppcheck says:

linux-6.1-rc4/drivers/gpu/drm/amd/display/dc/dml/dcn321/dcn321_fpu.c:626:27: 
style: Array index 'i' is used before limits check. [arrayIndexThenCheck]

Source code is

   if (dcfclk_sta_targets[i] < optimal_dcfclk_for_uclk[j] && i < 
num_dcfclk_sta_targets) {

It might be wise to move the limits check to before use.

Regards

David Binderman

Re: drivers/gpu/drm/bridge/fsl-ldb.c:101: possible loss of information.

2023-03-08 Thread David Binderman

Hello there Laurent,

>We could, but I don't think it will make any difference in practice as
>the maximum pixel clock frequency supported by the SoC is 80MHz (per
>LVDS channel). That would result in a 560MHz frequency returned by this
>function, well below the 31 bits limit.

Thanks for your explanation. I have a couple of suggestions for possible 
improvements:

1. Your explanatory text in a comment nearby. This helps all readers of the 
code.

2. Might the frequency go up to 300 MHz anytime soon ? The code will stop 
working then. 
In this case, I would suggest to put in a run time sanity check to make sure no 
31 bit overflow. 

Just a couple of ideas for the code.

Regards

David Binderman

Re: drivers/gpu/drm/bridge/fsl-ldb.c:101: possible loss of information.

2023-03-09 Thread David Binderman

Hello there Laurent,

>Would you be able to send a patch to fix this ?

Sadly, no. My success rate with kernel patches is low enough to make it not 
worth trying.

Regards

David Binderman


From: Laurent Pinchart 
Sent: 09 March 2023 09:26
To: David Binderman 
Cc: andrzej.ha...@intel.com ; 
neil.armstr...@linaro.org ; rf...@kernel.org 
; jo...@kwiboo.se ; jernej.skra...@gmail.com 
; airl...@gmail.com ; 
dan...@ffwll.ch ; dri-devel@lists.freedesktop.org 
; linux-ker...@vger.kernel.org 

Subject: Re: drivers/gpu/drm/bridge/fsl-ldb.c:101: possible loss of 
information. 
 
Hi David,

On Thu, Mar 09, 2023 at 07:59:34AM +0000, David Binderman wrote:
> Hello there Laurent,
> 
> >We could, but I don't think it will make any difference in practice as
> >the maximum pixel clock frequency supported by the SoC is 80MHz (per
> >LVDS channel). That would result in a 560MHz frequency returned by this
> >function, well below the 31 bits limit.
> 
> Thanks for your explanation. I have a couple of suggestions for possible 
> improvements:
> 
> 1. Your explanatory text in a comment nearby. This helps all readers of the 
> code.
> 
> 2. Might the frequency go up to 300 MHz anytime soon ? The code will stop 
> working then. 
> In this case, I would suggest to put in a run time sanity check to make sure 
> no 31 bit overflow. 

As it's a hardware limit of the SoC, I wouldn't expect so.

This being said, I think adding a UL suffix to the constants would be
better than a comment as it will please static checkers and serve as
documentation to humans. Would you be able to send a patch to fix this ?

> Just a couple of ideas for the code.

Thanks for taking the time to share those.

-- 
Regards,

Laurent Pinchart

drivers/gpu/drm/radeon/r100.c:3303]: (style) Redundant condition

2017-03-29 Thread David Binderman


Hello there,

linux-4.11-rc4/drivers/gpu/drm/radeon/r100.c:3303]: (style) Redundant 
condition: If 'EXPR == 11', the comparison 'EXPR <= 12' is always true.

Source code is

} else if (rdev->family == CHIP_RV350 ||
   rdev->family <= CHIP_RV380) {

Also in the same file:

[drivers/gpu/drm/radeon/r100.c:2550]: (style) Variable 'tmp' is assigned a value
 that is never used.
[drivers/gpu/drm/radeon/r100.c:2875]: (style) Variable 'tmp' is assigned a value
 that is never used.

Regards

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

linux-4.11/drivers/gpu/drm/bridge/tc358767.c:715]: (error) Signed integer overflow for expression 'max_tu_symbol<<23'.

2017-05-02 Thread David Binderman

Hello there,

Source code is

    max_tu_symbol = TU_SIZE_RECOMMENDED - 1;
    tc_write(DP0_MISC, (max_tu_symbol << 23) | TU_SIZE_RECOMMENDED | BPC_8);

and

#define TU_SIZE_RECOMMENDED (0x3f << 16) /* LSCLK cycles per TU */

So it looks to me like 44 bits are required. Suggest use type long ?

Regards

David Binderman
 

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

drivers/gpu/drm/nouveau/nvkm/engine/dma/usernv04.c:124:: possible unintended fallthrough ?

2017-05-02 Thread David Binderman

Hello there,

drivers/gpu/drm/nouveau/nvkm/engine/dma/usernv04.c:124:18: warning: this 
statement may fall through [-Wimplicit-fallthrough=]

Source code is

switch (dmaobj->base.access) {
case NV_MEM_ACCESS_RO:
dmaobj->flags0 |= 0x4000;
break;
case NV_MEM_ACCESS_WO:
dmaobj->flags0 |= 0x8000;
case NV_MEM_ACCESS_RW:
dmaobj->flags2 |= 0x0002;
break;
default:
return -EINVAL;
}

Suggest either document the fallthrough or add the missing break.

Regards

David Binderman

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

linux-4.11-rc1/drivers/gpu/drm/exynos/exynos5433_drm_decon.c:681: suspicious mask ?

2017-03-06 Thread David Binderman

Hello there,

linux-4.11-rc1/drivers/gpu/drm/exynos/exynos5433_drm_decon.c:681]: (warning) 
Result of operator '|' is always true if one operand is non-zero. Did you 
intend to use '&'?

Source code is

if (ctx->out_type | I80_HW_TRG) {

Also in the same file:

[drivers/gpu/drm/exynos/exynos5433_drm_decon.c:131]: (style) Same expression on 
both sides of '|'.

Source code is

   writel(TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN
   | TRIGCON_TE_AUTO_MASK | TRIGCON_SWTRIGEN,

Regards

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

linux-4.11-rc1/drivers/gpu/drm/amd/amdgpu/vi.c: 3 bugs

2017-03-06 Thread David Binderman


Hello there,
1

[linux-4.11-rc1/drivers/gpu/drm/amd/amdgpu/vi.c:1041] -> 
[linux-4.11-rc1/drivers/gpu/drm/amd/amdgpu/vi.c:1037]: (style) Same expression 
on both sides of '|'.

Maybe the macro AMD_CG_SUPPORT_GFX_MGLS is used twice ?

2.

[linux-4.11-rc1/drivers/gpu/drm/amd/amdgpu/vi.c:1070] -> 
[linux-4.11-rc1/drivers/gpu/drm/amd/amdgpu/vi.c:1066]: (style) Same expression 
on both sides of '|'.

Duplicate.

In the same file:

linux-4.11-rc1/drivers/gpu/drm/amd/amdgpu/vi.c:792]: (style) Variable 'r' is 
assigned a value that is never used.

Source code is

r = vi_set_uvd_clock(adev, dclk, ixCG_DCLK_CNTL, ixCG_DCLK_STATUS);

    return 0;

Regards

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

drivers/gpu/drm/msm/hdmi/hdmi_hdcp.c:1319]: (style) Redundant condition

2017-03-27 Thread David Binderman

Hello there,

linux-4.11-rc4/drivers/gpu/drm/msm/hdmi/hdmi_hdcp.c:1319]: (style) Redundant 
condition: If 'EXPR == 0', the comparison 'EXPR != 1' is always true.

Source code is

if ((HDCP_STATE_INACTIVE != hdcp_ctrl->hdcp_state) ||
(HDCP_STATE_NO_AKSV == hdcp_ctrl->hdcp_state)) {

Suggest code rework.

Regards

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

drivers/gpu/drm/i915/i915_reg.h:90:shift-overflow problem ?

2016-05-30 Thread David Binderman

Hello there,

drivers/gpu/drm/i915/i915_reg.h:90:28: warning: result of â65535 <<
20â requires 37 bits to represent, but âintâ only has 32 bits
[-Wshift-overflow=]

Source code is

#define   BSM_MASK (0x << 20)

Maybe better code

#define   BSM_MASK (((unsigned long) 0x) << 20)


Regards

David Binderman

linux-5.7-rc4/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c:1393: bad loop termination condition ?

2020-05-04 Thread David Binderman

Hello there,

linux-5.7-rc4/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c:1393:63: 
style: Unsigned expression 'j' can't be negative so it is unnecessary to test 
it. [unsignedPositive]

Source code is 

for (closest_clk_lvl = 0, j = dcn2_1_soc.num_states - 1; j >= 0; 
j--) {

but

unsigned int i, j, closest_clk_lvl;

so it looks like the loop never terminates. Suggest code rework.

Regards

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

linux-4.7-rc3/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:4836: wierd condition ?

2016-06-13 Thread David Binderman

Hello there,

linux-4.7-rc3/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:4836]: (style)
Boolean result is used in bitwise operation. Clarify expression with
parentheses.

Source code is

if ((ring->me == me_id) & (ring->pipe == pipe_id))

Maybe better code

if ((ring->me == me_id) && (ring->pipe == pipe_id))

Also in the same file:

[drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:3866]: (style) Variable 'data'
is assigned a value that is never used.
[drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:4321]: (style) Variable
'mc_shared_chmap' is assigned a value that is never used.
[drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:4657]: (style) Variable 'tmp'
is assigned a value that is never used.

Regards

David Binderman

linux-4.7-rc3/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c: 2 * pointless tests ?

2016-06-13 Thread David Binderman

Hello there,

1.

linux-4.7-rc3/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c:545]: (style)
Checking if unsigned variable 'pipe' is less than zero.

Source code is

if (pipe < 0 || pipe >= priv->num_crtcs) {

2.

linux-4.7-rc3/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c:567]: (style)
Checking if unsigned variable 'pipe' is less than zero.

Duplicate a few lines further down.


Regards

David Binderman

linux-4.7-rc3/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c: 2 * pointless tests ?

2016-06-15 Thread David Binderman

Hello there,

>yup, looks like we can drop the two pipe<0 checks.

Righto.

>Care to send a patch?

Oh dear. My success rate with patches is near zero. Maybe something
like this might be suitable:

*** linux-3.7-rc3/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c.sav
2016-06-15 10:58:04.868619030 +0100
--- linux-4.7-rc3/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   2016-06-15
10:58:25.401942157 +0100
***
*** 542,548 
struct msm_drm_private *priv = dev->dev_private;
struct drm_crtc *crtc;

!   if (pipe < 0 || pipe >= priv->num_crtcs) {
DRM_ERROR("Invalid crtc %d\n", pipe);
return -EINVAL;
}
--- 542,548 
struct msm_drm_private *priv = dev->dev_private;
struct drm_crtc *crtc;

!   if (pipe >= priv->num_crtcs) {
DRM_ERROR("Invalid crtc %d\n", pipe);
return -EINVAL;
}
***
*** 564,570 
struct drm_crtc *crtc;
struct drm_encoder *encoder;

!   if (pipe < 0 || pipe >= priv->num_crtcs)
return 0;

crtc = priv->crtcs[pipe];
--- 564,570 
struct drm_crtc *crtc;
struct drm_encoder *encoder;

!   if (pipe >= priv->num_crtcs)
return 0;

crtc = priv->crtcs[pipe];

Feel free to mess this tentative patch about in any way you see fit.


Regards

David Binderman


On Mon, Jun 13, 2016 at 4:27 PM, Rob Clark  wrote:
> yup, looks like we can drop the two pipe<0 checks.  Care to send a patch?
>
> BR,
> -R
>
> On Mon, Jun 13, 2016 at 10:51 AM, David Binderman
>  wrote:
>> Hello there,
>>
>> 1.
>>
>> linux-4.7-rc3/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c:545]: (style)
>> Checking if unsigned variable 'pipe' is less than zero.
>>
>> Source code is
>>
>> if (pipe < 0 || pipe >= priv->num_crtcs) {
>>
>> 2.
>>
>> linux-4.7-rc3/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c:567]: (style)
>> Checking if unsigned variable 'pipe' is less than zero.
>>
>> Duplicate a few lines further down.
>>
>>
>> Regards
>>
>> David Binderman

drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c:1812: possible pointless variable ?

2016-06-27 Thread David Binderman

Hello there,

linux-4.7-rc5/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c:1812]:
(style) Variable 'stretch_amount2' is assigned a value that is never
used.

Source code is

else if (stretch_amount == 3 || stretch_amount == 4)
stretch_amount2 = 1;

Suggest either use the variable in some way, or delete it.

Regards

David Binderman

linux-next/drivers/gpu/drm/rockchip/dw-mipi-dsi.c:470: poor error checking ?

2016-01-18 Thread David Binderman

Hello there,

[linux-next/drivers/gpu/drm/rockchip/dw-mipi-dsi.c:470]: (style) Checking if 
unsigned variable 'bpp' is less than zero.

Â Â Â  bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
Â Â Â  if (bpp < 0) {

but

Â  unsigned int bpp, i, pre;

Suggest code rework.


Regards

David Binderman

linux-4.6-rc2/drivers/gpu/drm/radeon/si_dpm.c: multiple problems ?

2016-04-04 Thread David Binderman

Hello there,

1.

linux-4.6-rc2/drivers/gpu/drm/radeon/si_dpm.c:3788]: (style) Condition 'td==0' 
is always true

Source code is

Â Â Â  enum r600_td td = R600_TD_DFLT;

Â Â Â  for (i = 0; i < R600_PM_NUMBER_OF_TC; i++)
Â Â Â Â Â Â Â  WREG32(CG_FFCT_0 + (i * 4), (UTC_0(r600_utc[i]) | 
DTC_0(r600_dtc[i])));

Â Â Â  if (td == R600_TD_AUTO)

2.

[linux-4.6-rc2/drivers/gpu/drm/radeon/si_dpm.c:2838]: (style) Variable 
'smc_result' is assigned a value that is never used.
[linux-4.6-rc2/drivers/gpu/drm/radeon/si_dpm.c:3661]: (style) Variable 
'backbias_response_time' is assigned a value that is never used.
[linux-4.6-rc2/drivers/gpu/drm/radeon/si_dpm.c:4190]: (style) Variable 
'voltage_found' is assigned a value that is never used.

Regards

David Binderman

linux-4.18-rc1/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:207: broken nested if ?

2018-06-18 Thread David Binderman

Hello there,

[linux-4.18-rc1/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c:208]: (warning) 
Identical inner 'if' condition is always true.

Source code is

if (set_clocks && adev->pm.dpm_enabled) {
if (adev->pm.dpm_enabled)
amdgpu_dpm_enable_uvd(adev, true);
else
amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
   
AMD_PG_STATE_UNGATE);

So second function call never happens. Suggest code rework.

Regards

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

linux-6.6-rc1/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c:2175:26: style: Array index 'i' is used before limits check. [arrayIndexThenCheck]

2023-09-11 Thread David Binderman

Hello there,

Static analyser cppcheck noticed the above problem. Source code is

if (dcfclk_sta_targets[i] < optimal_dcfclk_for_uclk[j] && i < 
num_dcfclk_sta_targets) {

Suggest new code:

if (i < num_dcfclk_sta_targets && dcfclk_sta_targets[i] < 
optimal_dcfclk_for_uclk[j]) {

Regards

David Binderman

drivers/gpu/drm/gma500/mdfld_intel_display.c:102:37: warning: bitwise comparison always evaluates to false [-Wtautological-compare]

2017-10-24 Thread David Binderman


Hello there,

Source code is

if ((temp & PIPEACONF_PIPE_STATE) == 1)
break;

but

$ fgrep PIPEACONF_PIPE_STATE `find drivers/gpu/drm/gma500 -name \*.h -print`
drivers/gpu/drm/gma500/psb_intel_reg.h:#define PIPEACONF_PIPE_STATE 
(1 << 30)
$ 

Suggest new code

if ((temp & PIPEACONF_PIPE_STATE) != 0)
break;

Regards

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

Re: drivers/gpu/drm/gma500/mdfld_intel_display.c:102:37: warning: bitwise comparison always evaluates to false [-Wtautological-compare]

2017-10-25 Thread David Binderman

Hello there,


Code I looked at is in linux-4.14-rc6, released 20171023, so reasonably

up to date.


I did a further check on github.com/torvalds/linux and the code

looks wrong there, too.


So I don't see the fix you mentioned in either of the places I looked.


Regards


David Binderman


From: Jani Nikula 
Sent: 24 October 2017 08:54
To: David Binderman; patrik.r.jakobs...@gmail.com; airl...@linux.ie; 
dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org
Subject: Re: drivers/gpu/drm/gma500/mdfld_intel_display.c:102:37: warning: 
bitwise comparison always evaluates to false [-Wtautological-compare]

On Mon, 23 Oct 2017, David Binderman  wrote:
> Hello there,
>
> Source code is
>
> if ((temp & PIPEACONF_PIPE_STATE) == 1)
> break;
>
> but
>
> $ fgrep PIPEACONF_PIPE_STATE `find drivers/gpu/drm/gma500 -name \*.h -print`
> drivers/gpu/drm/gma500/psb_intel_reg.h:#define PIPEACONF_PIPE_STATE   
> (1 << 30)
> $
>
> Suggest new code
>
> if ((temp & PIPEACONF_PIPE_STATE) != 0)
> break;

Suggest looking at latest sources. ;)

Fixed by 67a3b63a54cb ("drm: gma500: fix logic error").

BR,
Jani.

--
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

linux-4.16-rc5/drivers/gpu/drm/amd/display/dc/dce110/dce110_hw_sequencer.c:1723]: (warning) Possible null pointer dereference: pipe_ctx

2018-03-13 Thread David Binderman


hello there,

Source code is


    for (i = 0; i < dc->res_pool->pipe_count; i++) {
    if (res_ctx->pipe_ctx[i].stream) {
    pipe_ctx = &res_ctx->pipe_ctx[i];
    *pipe_idx = i;
    break;
    }
    }

    /* Only supports eDP */
    if (pipe_ctx->stream->sink->link->connector_signal != SIGNAL_TYPE_EDP)
    return false;

Suggest add some code to deal with the case that the for loop doesn't find
what it is looking for and so pipe_ctx is NULL.

Regards

David Binderman

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

linux-4.13/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fan.c:86: possible faulty logic ?

2017-09-04 Thread David Binderman

Hello there,

[linux-4.13/drivers/gpu/drm/nouveau/nvkm/subdev/therm/fan.c:93]: (warning) 
Opposite inner 'if' condition leads to a dead code block.

Source code is

if (target != duty) {
u16 bump_period = fan->bios.bump_period;
u16 slow_down_period = fan->bios.slow_down_period;
u64 delay;

if (duty > target)
delay = slow_down_period;
else if (duty == target)
delay = min(bump_period, slow_down_period) ;

Last if can never be true, so call to min can never execute. Suggest code 
rework.

Regards

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

drivers/gpu/drm/i915/intel_pm.c:4467: bad comparison ?

2017-07-19 Thread David Binderman


Hello there,

drivers/gpu/drm/i915/intel_pm.c:4467]: (warning) Comparison of a boolean 
expression with an integer other than 0 or 1.

Source code is

else if ((ddb_allocation && ddb_allocation /
fixed_16_16_to_u32_round_up(plane_blocks_per_line)) >= 1)

Regards

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

linux-4.15-rc1/drivers/gpu/drm/i915/gvt/cmd_parser.c:1640: poor error checking ?

2017-11-28 Thread David Binderman


Hello there,

linux-4.15-rc1/drivers/gpu/drm/i915/gvt/cmd_parser.c:1640]: (style) Checking if 
unsigned variable 'bb_size' is less than zero.

Source code is

/* get the size of the batch buffer */
bb_size = find_bb_size(s);
if (bb_size < 0)
return -EINVAL;

but

static int find_bb_size(struct parser_exec_state *s)

so the code isn't properly checking the return value. Suggest code rework.

Regards

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

linux-4.5/drivers/gpu/drm/gma500/oaktrail_crtc.c: 5 * uninit data ?

2016-03-14 Thread David Binderman

Hello there,

[linux-4.5/drivers/gpu/drm/gma500/oaktrail_crtc.c:175]: (error) Uninitialized 
struct member: clock.dot
[linux-4.5/drivers/gpu/drm/gma500/oaktrail_crtc.c:175]: (error) Uninitialized 
struct member: clock.m1
[linux-4.5/drivers/gpu/drm/gma500/oaktrail_crtc.c:175]: (error) Uninitialized 
struct member: clock.m2
[linux-4.5/drivers/gpu/drm/gma500/oaktrail_crtc.c:175]: (error) Uninitialized 
struct member: clock.p2
[linux-4.5/drivers/gpu/drm/gma500/oaktrail_crtc.c:175]: (error) Uninitialized 
struct member: clock.vco

Source code is

Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  *best_clock = clock;

Suggest init all fields of local variable clock shortly after declaration, that
way the function mrst_sdvo_find_best_pll can only ever return init'ed data.

Regards

David Binderman

[linux-4.4-rc1/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c:206: possible missing comma ?

2015-11-17 Thread David Binderman

Hello there,

[linux-4.4-rc1/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c:206]: (error) 
Array 'dsi_errors[31]' accessed at index 31, which is out of bounds.

I looks to me like array dsi_errors is badly laid out:

Â Â Â  "LP Generic Write FIFO Full",
Â Â Â  "Generic Read Data Avail"
Â Â Â  "Special Packet Sent",

Missing comma means the consecutive strings are concatenated and so there are 
only
31 elements in the array, not the presumably intended 32.

Regards

David Binderman

linux-4.2-rc1/drivers/gpu/drm/radeon/cik.c:1912: dead code block ?

2015-07-07 Thread David Binderman

Hello there,

[linux-4.2-rc1/drivers/gpu/drm/radeon/cik.c:1912] -> 
[linux-4.2-rc1/drivers/gpu/drm/radeon/cik.c:1913]: (warning) Opposite 
conditions in nested 'if' blocks lead to a dead code block.

Source code is

Â Â  if (running == 0) {
Â Â Â Â Â Â Â  if (running) {
Â Â Â Â Â Â Â Â Â Â Â  blackout = RREG32(MC_SHARED_BLACKOUT_CNTL);
Â Â Â Â Â Â Â Â Â Â Â  WREG32(MC_SHARED_BLACKOUT_CNTL, blackout | 1);
Â Â Â Â Â Â Â  }

Regards

David Binderman

linux-4.1-rc1/drivers/gpu/drm/msm/dsi/dsi_host.c:1799: possible missing break ?

2015-04-27 Thread David Binderman

Hello there,

[linux-4.1-rc1/drivers/gpu/drm/msm/dsi/dsi_host.c:1799] -> 
[linux-4.1-rc1/drivers/gpu/drm/msm/dsi/dsi_host.c:1802]: (warning) Variable 
'ret' is reassigned a value before the old one has been used. 'break;' missing?

Â Â Â  switch (cmd) {
Â Â Â  case MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT:
Â Â Â Â Â Â Â  pr_err("%s: rx ACK_ERR_PACLAGE\n", __func__);
Â Â Â Â Â Â Â  ret = 0;
Â Â Â  case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_1BYTE:
Â Â Â  case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_1BYTE:
Â Â Â Â Â Â Â  ret = dsi_short_read1_resp(buf, msg);
Â Â Â Â Â Â Â  break;

Regards

David Binderman

linux-6.12-rc1/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c:653: Possible &/&& mixup ?

2024-09-30 Thread David Binderman

Hello there,

Static analyser cppcheck says:

linux-6.12-rc1/drivers/gpu/drm/amd/display/dc/dml/dcn20/display_rq_dlg_calc_20.c:653:19:
 style: Boolean result is used in bitwise operation. Clarify expression with 
parentheses. [clarifyCondition]

Source code is

if (!surf_linear & (log2_dpte_req_height_ptes == 0) & surf_vert) //reduced, 
in this case, will have page fault within a group

IMHO, looks odd. Was && intended ?

Regards

David Binderman

linux-6.15-rc6/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c: possible cut'n'paste error ?

2025-05-16 Thread David Binderman

Hello there,

Static analyser cppcheck says:

linux-6.15-rc6/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c:595:45:
 style: Variable 'p->out_states->state_array[i].dtbclk_mhz' is reassigned a 
value before the old one has been used. [redundantAssignment]
linux-6.15-rc6/drivers/gpu/drm/amd/display/dc/dml2/dml2_translation_helper.c:594:45:
 style: Variable 'p->out_states->state_array[i].phyclk_mhz' is reassigned a 
value before the old one has been used. [redundantAssignment]

Source code is

p->out_states->state_array[i].dppclk_mhz = max_dppclk_mhz;
p->out_states->state_array[i].dtbclk_mhz = max_dtbclk_mhz;
p->out_states->state_array[i].phyclk_mhz = max_phyclk_mhz;

p->out_states->state_array[i].dscclk_mhz = max_dispclk_mhz / 3.0;
p->out_states->state_array[i].phyclk_mhz = max_phyclk_mhz;
p->out_states->state_array[i].dtbclk_mhz = max_dtbclk_mhz;

Suggest code rework.

Regards

David Binderman

38 matches

Mail list logo