diff mbox

[02/10] sm501: Use defines instead of constants where available

Message ID 84d7cdceac575c23bd4ee4bdcb80a03966b96db8.1487522123.git.balaton@eik.bme.hu
State New
Headers show

Commit Message

BALATON Zoltan Feb. 19, 2017, 4:35 p.m. UTC
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/sm501.c          | 8 ++++----
 hw/display/sm501_template.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Peter Maydell Feb. 24, 2017, 2:28 p.m. UTC | #1
On 19 February 2017 at 16:35, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/display/sm501.c          | 8 ++++----
>  hw/display/sm501_template.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/display/sm501.c b/hw/display/sm501.c
> index 4f40dee..4eb085c 100644
> --- a/hw/display/sm501.c
> +++ b/hw/display/sm501.c
> @@ -555,7 +555,7 @@ static uint32_t get_local_mem_size_index(uint32_t size)
>  static inline int is_hwc_enabled(SM501State *state, int crt)
>  {
>      uint32_t addr = crt ? state->dc_crt_hwc_addr : state->dc_panel_hwc_addr;
> -    return addr & 0x80000000;
> +    return addr & SM501_HWC_EN;
>  }
>
>  /**
> @@ -1411,9 +1411,9 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base,
>      s->local_mem_size_index = get_local_mem_size_index(local_mem_bytes);
>      SM501_DPRINTF("local mem size=%x. index=%d\n", get_local_mem_size(s),
>                    s->local_mem_size_index);
> -    s->system_control = 0x00100000;
> -    s->misc_control = 0x00001000; /* assumes SH, active=low */
> -    s->dc_panel_control = 0x00010000;
> +    s->system_control = 0x00100000; /* 2D engine FIFO empty */
> +    s->misc_control = SM501_MISC_IRQ_INVERT; /* assumes SH, active=low */
> +    s->dc_panel_control = 0x00010000; /* FIFO level 3 */

s->misc_control was set to 0x1000, but is now set to
SM501_MISC_IRQ_INVERT, which is (1 << 16), which is not the
same value... 0x1000 would be SM501_MISC_DAC_POWER.

thanks
-- PMM
BALATON Zoltan Feb. 24, 2017, 8:18 p.m. UTC | #2
On Fri, 24 Feb 2017, Peter Maydell wrote:
> On 19 February 2017 at 16:35, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/display/sm501.c          | 8 ++++----
>>  hw/display/sm501_template.h | 2 +-
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/display/sm501.c b/hw/display/sm501.c
>> index 4f40dee..4eb085c 100644
>> --- a/hw/display/sm501.c
>> +++ b/hw/display/sm501.c
>> @@ -555,7 +555,7 @@ static uint32_t get_local_mem_size_index(uint32_t size)
>>  static inline int is_hwc_enabled(SM501State *state, int crt)
>>  {
>>      uint32_t addr = crt ? state->dc_crt_hwc_addr : state->dc_panel_hwc_addr;
>> -    return addr & 0x80000000;
>> +    return addr & SM501_HWC_EN;
>>  }
>>
>>  /**
>> @@ -1411,9 +1411,9 @@ void sm501_init(MemoryRegion *address_space_mem, uint32_t base,
>>      s->local_mem_size_index = get_local_mem_size_index(local_mem_bytes);
>>      SM501_DPRINTF("local mem size=%x. index=%d\n", get_local_mem_size(s),
>>                    s->local_mem_size_index);
>> -    s->system_control = 0x00100000;
>> -    s->misc_control = 0x00001000; /* assumes SH, active=low */
>> -    s->dc_panel_control = 0x00010000;
>> +    s->system_control = 0x00100000; /* 2D engine FIFO empty */
>> +    s->misc_control = SM501_MISC_IRQ_INVERT; /* assumes SH, active=low */
>> +    s->dc_panel_control = 0x00010000; /* FIFO level 3 */
>
> s->misc_control was set to 0x1000, but is now set to
> SM501_MISC_IRQ_INVERT, which is (1 << 16), which is not the
> same value... 0x1000 would be SM501_MISC_DAC_POWER.

Thanks for reviewing these patches.

The comment suggests that the intended bit was the IRQ invert bit so the 
constant used before may have been a bug. (In my understanding nothing 
really uses this bit so I could set it either way you prefer but I think 
this is the correct one.) What should I do?
1. Leave it as it is
2. Set it to the old constant contradicting the comment
3. Make it a separate patch?
diff mbox

Patch

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 4f40dee..4eb085c 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -555,7 +555,7 @@  static uint32_t get_local_mem_size_index(uint32_t size)
 static inline int is_hwc_enabled(SM501State *state, int crt)
 {
     uint32_t addr = crt ? state->dc_crt_hwc_addr : state->dc_panel_hwc_addr;
-    return addr & 0x80000000;
+    return addr & SM501_HWC_EN;
 }
 
 /**
@@ -1411,9 +1411,9 @@  void sm501_init(MemoryRegion *address_space_mem, uint32_t base,
     s->local_mem_size_index = get_local_mem_size_index(local_mem_bytes);
     SM501_DPRINTF("local mem size=%x. index=%d\n", get_local_mem_size(s),
                   s->local_mem_size_index);
-    s->system_control = 0x00100000;
-    s->misc_control = 0x00001000; /* assumes SH, active=low */
-    s->dc_panel_control = 0x00010000;
+    s->system_control = 0x00100000; /* 2D engine FIFO empty */
+    s->misc_control = SM501_MISC_IRQ_INVERT; /* assumes SH, active=low */
+    s->dc_panel_control = 0x00010000; /* FIFO level 3 */
     s->dc_crt_control = 0x00010000;
 
     /* allocate local memory */
diff --git a/hw/display/sm501_template.h b/hw/display/sm501_template.h
index aeeac5d..16e500b 100644
--- a/hw/display/sm501_template.h
+++ b/hw/display/sm501_template.h
@@ -108,7 +108,7 @@  static void glue(draw_hwc_line_, PIXEL_NAME)(SM501State *s, int crt,
     /* get hardware cursor pattern */
     uint32_t cursor_addr = get_hwc_address(s, crt);
     assert(0 <= c_y && c_y < SM501_HWC_HEIGHT);
-    cursor_addr += 64 * c_y / 4;  /* 4 pixels per byte */
+    cursor_addr += SM501_HWC_WIDTH * c_y / 4;  /* 4 pixels per byte */
     cursor_addr += s->base;
 
     /* get cursor position */