Re: [PATCH v2] gpu: drm: gma500: remove dead code

[Daniel Vetter]

On Tue, May 23, 2017 at 01:35:16AM +0200, Patrik Jakobsson wrote:
> On Fri, May 19, 2017 at 2:28 PM, Patrik Jakobsson
> <patrik.r.jakobsson@xxxxxxxxx> wrote:
> > On Fri, May 19, 2017 at 11:19 AM, Gustavo A. R. Silva
> > <garsilva@xxxxxxxxxxxxxx> wrote:
> >> Local variable use_gct is assigned to a constant value and it is never
> >> updated again. Remove this variable and the dead code it guards.
> >>
> >> Addresses-Coverity-ID: 145690
> >> Signed-off-by: Gustavo A. R. Silva <garsilva@xxxxxxxxxxxxxx>
> >
> > I believe the first version is already in drm-misc. Actually this
> > entire file can be removed. It was never hooked up and since nobody
> > every complained I feel confident we can remove it.
> 
> Sorry my bad, it is actually in use since it's hardcoded. I'll pick up
> Arnd's fix (unless he takes it through some other tree).

Arnd generally expects maintainers to pick his compile fixes up.
-Daniel

> 
> Thanks
> Patrik
> 
> > Cheers
> > Patrik
> >
> >> ---
> >> Changes in v2:
> >>  Remove variables ti and dev_priv, which was causing a compilation warning.
> >>
> >>  I have improved my testing to avoid similar issues in the future.
> >>  This is how I tested it this time:
> >>
> >>  $ make allmodconfig
> >>  $ make drivers/gpu/drm/gma500/mdfld_tpo_vid.o
> >>
> >>
> >>  drivers/gpu/drm/gma500/mdfld_tpo_vid.c | 53 ++++++----------------------------
> >>  1 file changed, 9 insertions(+), 44 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> >> index d8d4170..a9420bf 100644
> >> --- a/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> >> +++ b/drivers/gpu/drm/gma500/mdfld_tpo_vid.c
> >> @@ -30,55 +30,20 @@
> >>  static struct drm_display_mode *tpo_vid_get_config_mode(struct drm_device *dev)
> >>  {
> >>         struct drm_display_mode *mode;
> >> -       struct drm_psb_private *dev_priv = dev->dev_private;
> >> -       struct oaktrail_timing_info *ti = &dev_priv->gct_data.DTD;
> >> -       bool use_gct = false;
> >>
> >>         mode = kzalloc(sizeof(*mode), GFP_KERNEL);
> >>         if (!mode)
> >>                 return NULL;
> >>
> >> -       if (use_gct) {
> >> -               mode->hdisplay = (ti->hactive_hi << 8) | ti->hactive_lo;
> >> -               mode->vdisplay = (ti->vactive_hi << 8) | ti->vactive_lo;
> >> -               mode->hsync_start = mode->hdisplay +
> >> -                               ((ti->hsync_offset_hi << 8) |
> >> -                               ti->hsync_offset_lo);
> >> -               mode->hsync_end = mode->hsync_start +
> >> -                               ((ti->hsync_pulse_width_hi << 8) |
> >> -                               ti->hsync_pulse_width_lo);
> >> -               mode->htotal = mode->hdisplay + ((ti->hblank_hi << 8) |
> >> -                                                               ti->hblank_lo);
> >> -               mode->vsync_start =
> >> -                       mode->vdisplay + ((ti->vsync_offset_hi << 8) |
> >> -                                               ti->vsync_offset_lo);
> >> -               mode->vsync_end =
> >> -                       mode->vsync_start + ((ti->vsync_pulse_width_hi << 8) |
> >> -                                               ti->vsync_pulse_width_lo);
> >> -               mode->vtotal = mode->vdisplay +
> >> -                               ((ti->vblank_hi << 8) | ti->vblank_lo);
> >> -               mode->clock = ti->pixel_clock * 10;
> >> -
> >> -               dev_dbg(dev->dev, "hdisplay is %d\n", mode->hdisplay);
> >> -               dev_dbg(dev->dev, "vdisplay is %d\n", mode->vdisplay);
> >> -               dev_dbg(dev->dev, "HSS is %d\n", mode->hsync_start);
> >> -               dev_dbg(dev->dev, "HSE is %d\n", mode->hsync_end);
> >> -               dev_dbg(dev->dev, "htotal is %d\n", mode->htotal);
> >> -               dev_dbg(dev->dev, "VSS is %d\n", mode->vsync_start);
> >> -               dev_dbg(dev->dev, "VSE is %d\n", mode->vsync_end);
> >> -               dev_dbg(dev->dev, "vtotal is %d\n", mode->vtotal);
> >> -               dev_dbg(dev->dev, "clock is %d\n", mode->clock);
> >> -       } else {
> >> -               mode->hdisplay = 864;
> >> -               mode->vdisplay = 480;
> >> -               mode->hsync_start = 873;
> >> -               mode->hsync_end = 876;
> >> -               mode->htotal = 887;
> >> -               mode->vsync_start = 487;
> >> -               mode->vsync_end = 490;
> >> -               mode->vtotal = 499;
> >> -               mode->clock = 33264;
> >> -       }
> >> +       mode->hdisplay = 864;
> >> +       mode->vdisplay = 480;
> >> +       mode->hsync_start = 873;
> >> +       mode->hsync_end = 876;
> >> +       mode->htotal = 887;
> >> +       mode->vsync_start = 487;
> >> +       mode->vsync_end = 490;
> >> +       mode->vtotal = 499;
> >> +       mode->clock = 33264;
> >>
> >>         drm_mode_set_name(mode);
> >>         drm_mode_set_crtcinfo(mode, 0);
> >> --
> >> 2.5.0
> >>

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch