From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 80FAAC432BE for ; Sat, 28 Aug 2021 00:48:07 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AC20E60F14 for ; Sat, 28 Aug 2021 00:48:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org AC20E60F14 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-m68k.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:49304 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mJmVx-0003z3-PA for qemu-devel@archiver.kernel.org; Fri, 27 Aug 2021 20:48:05 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53280) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mJmUV-0003JR-MV; Fri, 27 Aug 2021 20:46:35 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:59899) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mJmUT-00044x-OU; Fri, 27 Aug 2021 20:46:35 -0400 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id E88F65C00D6; Fri, 27 Aug 2021 20:46:29 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Fri, 27 Aug 2021 20:46:29 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=ijogH1 ZPsNoeQRumMw6phMVHhhmlNC+6ocOLzlFLzdQ=; b=IhVAVJ+yqANTj1m/B9gegp nGRpOPLOkm8E1Op6AQvpVWvDjy0MjwjlRG+xGR1gKkhHIV4SErA+kNic/07QwMan 03QAtLuV2Mba2DjDXd5ecyu5RS7t//1ZXGHpU0GNc9LQy6Md0Dp/9Hk0C+JEogvs 6EcF1LeHlpGaaAJcTwrlriljhkHKKRJpTpWwXnIMINZ5XJkdqx3BZuQRB/ZEyJKI NHq9EPK2Bph97v/i6Sqn+ovnBMVVLmd+eJQkenuGmMiiPyzdAKuml7tKKgjRYlmt tezMEvEhQqfmmD0PRAtoTOoZf0BQu0nN9PbKv21tPyl9Dn8+nJ9FtpdsC12XO0EQ == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddruddugedgfeekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffujgfkfhggtgesthdtredttddtvdenucfhrhhomhephfhinhhnucfv hhgrihhnuceofhhthhgrihhnsehlihhnuhigqdhmieekkhdrohhrgheqnecuggftrfgrth htvghrnhepiedvieetledvleevvdeiffetieekfffhleetjeeijeelffejudduteeihfff kedtnecuffhomhgrihhnpeeihedtvddrohhrghenucevlhhushhtvghrufhiiigvpedtne curfgrrhgrmhepmhgrihhlfhhrohhmpehfthhhrghinheslhhinhhugidqmheikehkrdho rhhg X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 27 Aug 2021 20:46:24 -0400 (EDT) Date: Sat, 28 Aug 2021 10:46:10 +1000 (AEST) From: Finn Thain To: Mark Cave-Ayland Subject: Re: [RFC 07/10] hw/mos6522: Fix initial timer counter reload In-Reply-To: Message-ID: <4721eb37-afb5-a55-35cd-f4d585a6c5f1@linux-m68k.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Received-SPF: none client-ip=66.111.4.25; envelope-from=fthain@linux-m68k.org; helo=out1-smtp.messagingengine.com X-Spam_score_int: -25 X-Spam_score: -2.6 X-Spam_bar: -- X-Spam_report: (-2.6 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-devel@nongnu.org, Laurent Vivier , qemu-ppc@nongnu.org, Greg Kurz , David Gibson Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Wed, 25 Aug 2021, Mark Cave-Ayland wrote: > On 24/08/2021 11:09, Finn Thain wrote: > > > The first reload of timer 1 is early by half of a clock cycle as it gets > > measured from a falling edge. By contrast, the succeeding reloads are > > measured from rising edge to rising edge. > > > > Neglecting that complication, the behaviour of the counter should be the > > same from one reload to the next. The sequence is always: > > > > N, N-1, N-2, ... 2, 1, 0, -1, N, N-1, N-2, ... > > > > But at the first reload, the present driver does this instead: > > > > N, N-1, N-2, ... 2, 1, 0, -1, N-1, N-2, ... > > > > Fix this deviation for both timer 1 and timer 2, and allow for the fact > > that on a real 6522 the timer 2 counter is not reloaded when it wraps. > > > > Signed-off-by: Finn Thain > > --- > > hw/misc/mos6522.c | 19 +++++++++++-------- > > 1 file changed, 11 insertions(+), 8 deletions(-) > > > > diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c > > index 5b1657ac0d..0a241fe9f8 100644 > > --- a/hw/misc/mos6522.c > > +++ b/hw/misc/mos6522.c > > @@ -63,15 +63,16 @@ static unsigned int get_counter(MOS6522State *s, > > MOS6522Timer *ti) > > if (ti->index == 0) { > > /* the timer goes down from latch to -1 (period of latch + 2) */ > > if (d <= (ti->counter_value + 1)) { > > - counter = (ti->counter_value - d) & 0xffff; > > + counter = ti->counter_value - d; > > } else { > > - counter = (d - (ti->counter_value + 1)) % (ti->latch + 2); > > - counter = (ti->latch - counter) & 0xffff; > > + int64_t d_post_reload = d - (ti->counter_value + 2); > > + /* XXX this calculation assumes that ti->latch has not changed > > */ > > + counter = ti->latch - (d_post_reload % (ti->latch + 2)); > > } > > } else { > > - counter = (ti->counter_value - d) & 0xffff; > > + counter = ti->counter_value - d; > > } > > - return counter; > > + return counter & 0xffff; > > } > > static void set_counter(MOS6522State *s, MOS6522Timer *ti, unsigned int > > val) > > @@ -103,11 +104,13 @@ static int64_t get_next_irq_time(MOS6522State *s, > > MOS6522Timer *ti, > > /* the timer goes down from latch to -1 (period of latch + 2) */ > > if (d <= (ti->counter_value + 1)) { > > - counter = (ti->counter_value - d) & 0xffff; > > + counter = ti->counter_value - d; > > } else { > > - counter = (d - (ti->counter_value + 1)) % (ti->latch + 2); > > - counter = (ti->latch - counter) & 0xffff; > > + int64_t d_post_reload = d - (ti->counter_value + 2); > > + /* XXX this calculation assumes that ti->latch has not changed */ > > + counter = ti->latch - (d_post_reload % (ti->latch + 2)); > > } > > + counter &= 0xffff; > > /* Note: we consider the irq is raised on 0 */ > > if (counter == 0xffff) { > > I think the code looks right, but I couldn't see an explicit reference to this > behaviour in > http://archive.6502.org/datasheets/mos_6522_preliminary_nov_1977.pdf. Yes, that datasheet is missing a lot of information. > Presumably this matches what you've observed on real hardware? > Yes.