On 12/04/2017 11:51, guangrong.xiao@xxxxxxxxx wrote:
> From: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxx>
>
> Currently, the timer is updated whenever RegA or RegB is written
> even if the periodic timer related configuration is not changed
>
> This patch optimizes it slightly to make the update happen only
> if its period or enable-status is changed, also later patches are
> depend on this optimization
>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxx>
> ---
> hw/timer/mc146818rtc.c | 31 +++++++++++++++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> index 4165450..749e206 100644
> --- a/hw/timer/mc146818rtc.c
> +++ b/hw/timer/mc146818rtc.c
> @@ -387,10 +387,25 @@ static void rtc_update_timer(void *opaque)
> check_update_timer(s);
> }
>
> +static bool rtc_periodic_timer_updated_by_regA(RTCState *s, uint64_t data)
> +{
> + uint8_t orig = s->cmos_data[RTC_REG_A];
> +
> + return (orig & 0x0f) != (data & 0x0f);
> +}
> +
> +static bool rtc_periodic_timer_updated_by_regB(RTCState *s, uint64_t data)
> +{
> + uint8_t orig = s->cmos_data[RTC_REG_B];
> +
> + return (orig & REG_B_PIE) != (data & REG_B_PIE);
> +}
> +
> static void cmos_ioport_write(void *opaque, hwaddr addr,
> uint64_t data, unsigned size)
> {
> RTCState *s = opaque;
> + bool update_periodic_timer;
>
> if ((addr & 1) == 0) {
> s->cmos_index = data & 0x7f;
> @@ -423,6 +438,8 @@ static void cmos_ioport_write(void *opaque, hwaddr addr,
> }
> break;
> case RTC_REG_A:
> + update_periodic_timer = rtc_periodic_timer_updated_by_regA(s, data);
I think you can change it to a...
> if ((data & 0x60) == 0x60) {
> if (rtc_running(s)) {
> rtc_update_time(s);
> @@ -445,10 +462,16 @@ static void cmos_ioport_write(void *opaque, hwaddr addr,
> /* UIP bit is read only */
... inlined update_periodic_timer assignment here:
update_periodic_timer = (s->cmos_data[RTC_REG_A] ^ data) & REG_A_PERIOD;
> s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) |
> (s->cmos_data[RTC_REG_A] & REG_A_UIP);
> - periodic_timer_update(s, qemu_clock_get_ns(rtc_clock));
> +
> + if (update_periodic_timer) {
> + periodic_timer_update(s, qemu_clock_get_ns(rtc_clock));
> + }
> +
> check_update_timer(s);
> break;
> case RTC_REG_B:
> + update_periodic_timer = rtc_periodic_timer_updated_by_regB(s, data);
> +
> if (data & REG_B_SET) {
> /* update cmos to when the rtc was stopping */
> if (rtc_running(s)) {
> @@ -475,7 +498,11 @@ static void cmos_ioport_write(void *opaque, hwaddr addr,
> qemu_irq_lower(s->irq);
> }
Same here:
update_periodic_timer = (s->cmos_data[RTC_REG_A] ^ data) & REG_B_PIE;
Thanks,
Paolo
> s->cmos_data[RTC_REG_B] = data;
> - periodic_timer_update(s, qemu_clock_get_ns(rtc_clock));
> +
> + if (update_periodic_timer) {
> + periodic_timer_update(s, qemu_clock_get_ns(rtc_clock));
> + }
> +
> check_update_timer(s);
> break;
> case RTC_REG_C:
>
