On Wed, Mar 23, 2016 at 11:35 AM, Pooja Dhannawat <dhannawatpoo...@gmail.com
> wrote:
>
>
> 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.
>
> so surface_bits_per_pixel() always returns 32 (just for reference :
Message-ID: <56f43a06.9090...@redhat.com> ) as discussed in this mail
chain. I will make desired changed for this patch too. is it ok to go ahead
with changes ?
> >
>> > 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
>>
>>
>
