On Wed, Mar 23, 2016 at 3:55 AM, Eric Blake <ebl...@redhat.com> wrote:
> On 03/12/2016 03:23 AM, Pooja Dhannawat wrote:
> > As only DEPTH ==32 case is used, removing other dead code
> > which is based on DEPTH !== 32
> >
> > Signed-off-by: Pooja Dhannawat <dhannawatpoo...@gmail.com>
> > ---
>
> > +++ b/hw/display/omap_lcdc.c
> > @@ -71,44 +71,23 @@ static void omap_lcd_interrupts(struct
> omap_lcd_panel_s *s)
> >
> > #define draw_line_func drawfn
> >
> > -#define DEPTH 8
> > -#include "omap_lcd_template.h"
> > -#define DEPTH 15
> > -#include "omap_lcd_template.h"
> > -#define DEPTH 16
> > -#include "omap_lcd_template.h"
> > #define DEPTH 32
> > #include "omap_lcd_template.h"
>
> Umm, the old code WAS using DEPTH != 32. I'm not a display expert, so
> there may be justification in nuking that code; but the commit message
> needs a better argument than "the code wasn't used" when it sure seems
> to be used. If the argument is that surface_bits_per_pixel() always
> returns 32, that would be a good argument.
>
> Correct me If I am wrong, I dig down on this and I found internally it
used PIXMAN_FORMAT which uses bits_per_pixel = 32 only.
>
> > static draw_line_func draw_line_table2[33] = {
> > [0 ... 32] = NULL,
> > - [8] = draw_line2_8,
> > - [15] = draw_line2_15,
> > - [16] = draw_line2_16,
> > [32] = draw_line2_32,
>
> This array is now wasteful. If surface_bits_per_pixel() always returns
> 32, then just ditch this array, and later on, change:
>
> Yes sure.
> /* Colour depth */
> switch ((omap_lcd->palette[0] >> 12) & 7) {
> case 1:
> - draw_line = draw_line_table2[surface_bits_per_pixel(surface)];
> + draw_line = draw_line2_32;
> bpp = 2;
> break;
>
> In other words, your cleanup, if justified, is incomplete.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>
