* [Qemu-devel] [Patch 1/1] omap_lcdc: Remove support for DEPTH != 32
@ 2016-03-12 10:23 Pooja Dhannawat
2016-03-22 22:25 ` Eric Blake
0 siblings, 1 reply; 4+ messages in thread
From: Pooja Dhannawat @ 2016-03-12 10:23 UTC (permalink / raw)
To: qemu-devel
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>
---
hw/display/omap_lcd_template.h | 10 ++--------
hw/display/omap_lcdc.c | 21 ---------------------
2 files changed, 2 insertions(+), 29 deletions(-)
diff --git a/hw/display/omap_lcd_template.h b/hw/display/omap_lcd_template.h
index f0ce71f..1025ff3 100644
--- a/hw/display/omap_lcd_template.h
+++ b/hw/display/omap_lcd_template.h
@@ -27,13 +27,7 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-#if DEPTH == 8
-# define BPP 1
-# define PIXEL_TYPE uint8_t
-#elif DEPTH == 15 || DEPTH == 16
-# define BPP 2
-# define PIXEL_TYPE uint16_t
-#elif DEPTH == 32
+#if DEPTH == 32
# define BPP 4
# define PIXEL_TYPE uint32_t
#else
@@ -152,7 +146,7 @@ static void glue(draw_line12_, DEPTH)(void *opaque,
static void glue(draw_line16_, DEPTH)(void *opaque,
uint8_t *d, const uint8_t *s, int width, int deststep)
{
-#if DEPTH == 16 && defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
+#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
memcpy(d, s, width * 2);
#else
uint16_t v;
diff --git a/hw/display/omap_lcdc.c b/hw/display/omap_lcdc.c
index ce1058b..4b3b810 100644
--- a/hw/display/omap_lcdc.c
+++ 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"
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,
}, draw_line_table4[33] = {
[0 ... 32] = NULL,
- [8] = draw_line4_8,
- [15] = draw_line4_15,
- [16] = draw_line4_16,
[32] = draw_line4_32,
}, draw_line_table8[33] = {
[0 ... 32] = NULL,
- [8] = draw_line8_8,
- [15] = draw_line8_15,
- [16] = draw_line8_16,
[32] = draw_line8_32,
}, draw_line_table12[33] = {
[0 ... 32] = NULL,
- [8] = draw_line12_8,
- [15] = draw_line12_15,
- [16] = draw_line12_16,
[32] = draw_line12_32,
}, draw_line_table16[33] = {
[0 ... 32] = NULL,
- [8] = draw_line16_8,
- [15] = draw_line16_15,
- [16] = draw_line16_16,
[32] = draw_line16_32,
};
--
2.5.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [Patch 1/1] omap_lcdc: Remove support for DEPTH != 32
2016-03-12 10:23 [Qemu-devel] [Patch 1/1] omap_lcdc: Remove support for DEPTH != 32 Pooja Dhannawat
@ 2016-03-22 22:25 ` Eric Blake
2016-03-23 6:05 ` Pooja Dhannawat
0 siblings, 1 reply; 4+ messages in thread
From: Eric Blake @ 2016-03-22 22:25 UTC (permalink / raw)
To: Pooja Dhannawat, qemu-devel
[-- 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 --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [Patch 1/1] omap_lcdc: Remove support for DEPTH != 32
2016-03-22 22:25 ` Eric Blake
@ 2016-03-23 6:05 ` Pooja Dhannawat
2016-03-28 12:45 ` Pooja Dhannawat
0 siblings, 1 reply; 4+ messages in thread
From: Pooja Dhannawat @ 2016-03-23 6:05 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU Developers, Gerd Hoffmann
[-- Attachment #1: Type: text/plain, Size: 1956 bytes --]
On Wed, Mar 23, 2016 at 3:55 AM, Eric Blake <eblake@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 <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.
>
> 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
>
>
[-- Attachment #2: Type: text/html, Size: 3346 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [Patch 1/1] omap_lcdc: Remove support for DEPTH != 32
2016-03-23 6:05 ` Pooja Dhannawat
@ 2016-03-28 12:45 ` Pooja Dhannawat
0 siblings, 0 replies; 4+ messages in thread
From: Pooja Dhannawat @ 2016-03-28 12:45 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU Developers, Gerd Hoffmann
[-- Attachment #1: Type: text/plain, Size: 2351 bytes --]
On Wed, Mar 23, 2016 at 11:35 AM, Pooja Dhannawat <dhannawatpooja1@gmail.com
> wrote:
>
>
> On Wed, Mar 23, 2016 at 3:55 AM, Eric Blake <eblake@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 <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.
>>
>> 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.9090700@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
>>
>>
>
[-- Attachment #2: Type: text/html, Size: 4297 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-28 12:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-12 10:23 [Qemu-devel] [Patch 1/1] omap_lcdc: Remove support for DEPTH != 32 Pooja Dhannawat
2016-03-22 22:25 ` Eric Blake
2016-03-23 6:05 ` Pooja Dhannawat
2016-03-28 12:45 ` Pooja Dhannawat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).