Re: [Intel-gfx] [PATCH linux-next] drm/i915/display: Remove unused variable and its assignment.

2021-10-19 Thread luo.penghao
> This one we could use. For some reason we hardcode it to



> 1 now, which is correct for our use cases but I don't really> see a reason to 
> hardcode it here. We are supposed to calculate> it correctly after all, and 
> chv_crtc_clock_get() also just blindly> reads it out.> > >  bestm2_frac = 
> crtc_state->dpll.m2 & 0x3f;> > -bestm1 = crtc_state->dpll.m1;> > This 
> one is a bit trickier since I don't think the spec even> gives us other 
> values. But we could assert that it's correct.> > Some something along these 
> lines I think would be best:> + drm_WARN_ON(_priv->drm, bestm1 != 2);>   
> vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW1(port),>  
> DPIO_CHV_M1_DIV_BY_2 |> -  1 << DPIO_CHV_N_DIV_SHIFT);> +  
> bestn << DPIO_CHV_N_DIV_SHIFT);






Thanks for your kind response ! Does that means the variable will be


used by the hardware?if so as far as I see it, I don't seem to see the


relevant interface.

Re: [Intel-gfx] [PATCH linux-next] drm/i915/display: Remove unused variable and its assignment.

2021-10-19 Thread luo.penghao
> This one we could use. For some reason we hardcode it to



> 1 now, which is correct for our use cases but I don't really> see a reason to 
> hardcode it here. We are supposed to calculate> it correctly after all, and 
> chv_crtc_clock_get() also just blindly> reads it out.> > >  bestm2_frac = 
> crtc_state->dpll.m2 & 0x3f;> > -bestm1 = crtc_state->dpll.m1;> > This 
> one is a bit trickier since I don't think the spec even> gives us other 
> values. But we could assert that it's correct.> > Some something along these 
> lines I think would be best:> + drm_WARN_ON(_priv->drm, bestm1 != 2);>   
> vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW1(port),>  
> DPIO_CHV_M1_DIV_BY_2 |> -  1 << DPIO_CHV_N_DIV_SHIFT);> +  
> bestn << DPIO_CHV_N_DIV_SHIFT);






Thanks for your kind response ! Does that means the variable will be


used by the hardware?if so as far as I see it, I don't seem to see the


relevant interface.

Re: [Intel-gfx] [PATCH linux-next] drm/i915/display: Remove unused variable and its assignment.

2021-10-19 Thread luo.penghao
> This one we could use. For some reason we hardcode it to



> 1 now, which is correct for our use cases but I don't really> see a reason to 
> hardcode it here. We are supposed to calculate> it correctly after all, and 
> chv_crtc_clock_get() also just blindly> reads it out.> > >  bestm2_frac = 
> crtc_state->dpll.m2 & 0x3f;> > -bestm1 = crtc_state->dpll.m1;> > This 
> one is a bit trickier since I don't think the spec even> gives us other 
> values. But we could assert that it's correct.> > Some something along these 
> lines I think would be best:> + drm_WARN_ON(_priv->drm, bestm1 != 2);>   
> vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW1(port),>  
> DPIO_CHV_M1_DIV_BY_2 |> -  1 << DPIO_CHV_N_DIV_SHIFT);> +  
> bestn << DPIO_CHV_N_DIV_SHIFT);






Thanks for your kind response ! Does that means the variable will be


used by the hardware?if so as far as I see it, I don't seem to see the


relevant interface.

[Intel-gfx] [PATCH linux-next] drm/i915/display: Remove unused variable and its assignment.

2021-10-18 Thread luo penghao
Variable is not used in functions, and its assignment is redundant too.
So it should be deleted.

The clang_analyzer complains as follows:

drivers/gpu/drm/i915/display/intel_dpll.c:1653:2 warning:
Value stored to 'bestm1' is never read.

drivers/gpu/drm/i915/display/intel_dpll.c:1651:2 warning:
Value stored to 'bestn' is never read.

Reported-by: Zeal Robot 
Signed-off-by: luo penghao 
---
 drivers/gpu/drm/i915/display/intel_dpll.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c 
b/drivers/gpu/drm/i915/display/intel_dpll.c
index b84ed4a..28b1616 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll.c
@@ -1644,13 +1644,11 @@ static void chv_prepare_pll(const struct 
intel_crtc_state *crtc_state)
enum pipe pipe = crtc->pipe;
enum dpio_channel port = vlv_pipe_to_channel(pipe);
u32 loopfilter, tribuf_calcntr;
-   u32 bestn, bestm1, bestm2, bestp1, bestp2, bestm2_frac;
+   u32 bestm2, bestp1, bestp2, bestm2_frac;
u32 dpio_val;
int vco;
 
-   bestn = crtc_state->dpll.n;
bestm2_frac = crtc_state->dpll.m2 & 0x3f;
-   bestm1 = crtc_state->dpll.m1;
bestm2 = crtc_state->dpll.m2 >> 22;
bestp1 = crtc_state->dpll.p1;
bestp2 = crtc_state->dpll.p2;
-- 
2.15.2



Re: [Intel-gfx] [PATCH linux-next] drm/i915/display: Remove unused variable and its assignment.

2021-10-18 Thread Ville Syrjälä
On Mon, Oct 18, 2021 at 08:43:31AM +, luo penghao wrote:
> Variable is not used in functions, and its assignment is redundant too.
> So it should be deleted.
> 
> The clang_analyzer complains as follows:
> 
> drivers/gpu/drm/i915/display/intel_dpll.c:1653:2 warning:
> Value stored to 'bestm1' is never read.
> 
> drivers/gpu/drm/i915/display/intel_dpll.c:1651:2 warning:
> Value stored to 'bestn' is never read.
> 
> Reported-by: Zeal Robot 
> Signed-off-by: luo penghao 
> ---
>  drivers/gpu/drm/i915/display/intel_dpll.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c 
> b/drivers/gpu/drm/i915/display/intel_dpll.c
> index b84ed4a..28b1616 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll.c
> @@ -1644,13 +1644,11 @@ static void chv_prepare_pll(const struct 
> intel_crtc_state *crtc_state)
>   enum pipe pipe = crtc->pipe;
>   enum dpio_channel port = vlv_pipe_to_channel(pipe);
>   u32 loopfilter, tribuf_calcntr;
> - u32 bestn, bestm1, bestm2, bestp1, bestp2, bestm2_frac;
> + u32 bestm2, bestp1, bestp2, bestm2_frac;
>   u32 dpio_val;
>   int vco;
>  
> - bestn = crtc_state->dpll.n;

This one we could use. For some reason we hardcode it to
1 now, which is correct for our use cases but I don't really
see a reason to hardcode it here. We are supposed to calculate
it correctly after all, and chv_crtc_clock_get() also just blindly
reads it out.

>   bestm2_frac = crtc_state->dpll.m2 & 0x3f;
> - bestm1 = crtc_state->dpll.m1;

This one is a bit trickier since I don't think the spec even
gives us other values. But we could assert that it's correct.

Some something along these lines I think would be best:
+ drm_WARN_ON(_priv->drm, bestm1 != 2);
  vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW1(port),
 DPIO_CHV_M1_DIV_BY_2 |
-1 << DPIO_CHV_N_DIV_SHIFT);
+bestn << DPIO_CHV_N_DIV_SHIFT);

-- 
Ville Syrjälä
Intel