Re: [Qemu-devel] [Patch 1/1] omap_lcdc: Remove support for DEPTH != 32

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1669 bytes --]

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 <dhannawatpooja1@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.

>  
>  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:

    /* 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]