All of lore.kernel.org
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com>,
	"r.verdejo@samsung.com" <r.verdejo@samsung.com>,
	"nicolas@ndufresne.ca" <nicolas@ndufresne.ca>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	"skhan@linuxfoundation.org" <skhan@linuxfoundation.org>,
	"linux-kernel-mentees@lists.linuxfoundation.org
	<linux-kernel-mentees@lists.linuxfoundation.org>,
	linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [v10 3/4] media: vidtv: add a bridge driver
Date: Wed, 16 Sep 2020 09:09:30 +0200	[thread overview]
Message-ID: <CAMuHMdW8kzcugxoDosT_50mfAbaHks72_CLWs5yJH5UQ6rz_Zw@mail.gmail.com> (raw)
In-Reply-To: <20200916090111.7dcfa0fe@coco.lan>

Hi Mauro,

On Wed, Sep 16, 2020 at 9:01 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
> Em Tue, 15 Sep 2020 15:35:00 +0200
> Geert Uytterhoeven <geert@linux-m68k.org> escreveu:
> > On Tue, Sep 15, 2020 at 3:26 PM Daniel W. S. Almeida
> > <dwlsalmeida@gmail.com> wrote:
> > > >> +    u32 nbytes = 0;  /* the number of bytes written by this function */
> > > >> +
> > > >> +    u64 nbytes_expected; /* the number of bytes we should have written */
> > > >> +    u64 nbytes_streamed; /* the number of bytes we actually wrote */
> > > >> +    u32 num_null_pkts; /* number of null packets to bridge the gap */
> > > >> +
> > > >> +    u64 elapsed_time_msecs = jiffies_to_usecs(m->timing.current_jiffies -
> > > >> +                                              m->timing.past_jiffies);
> > > >> +
> > > >> +    elapsed_time_msecs = min(elapsed_time_msecs,
> > > >> (u64)VIDTV_MAX_SLEEP_USECS / 1000);
> > > >> +    nbytes_expected = div64_u64(m->mux_rate_kbytes_sec * 1000, MSEC_PER_SEC);
> > > >
> > > > Seriously?!?
> > > >
> > > > You multiply by 1000 first, followed by a division by 1000 using an
> > > > expensive 64-by-64 division?
> > >
> > > This entire function is broken and needs a do-over :)
> > >
> > > > using an expensive 64-by-64 division?
> > >
> > > I am new to kernel development. I wasn't even aware that this was
> > > expensive, to be honest.
> >
> > All divisions involving 64-bit data are expensive, especially on 32-bit
> > platforms.  That's why we have the helpers in <linux/math.h>.  Most
> > of them implement simplified variants, which are less expensive.
>
> I agree that 64-bit math is something that should be used with some
> care. However, it is almost unavoidable do to 64-bit divisions for
> digital TV.

Sure. If 64-bit math is needed, it should be used.

The macros (and the link failure on 32-bit) exist to (a) make sure
people think twice before using 64-math, and (b) let people pick a
less-expensive variant if that is sufficient.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

  reply	other threads:[~2020-09-16  7:09 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-21 12:58 [v10 0/4] media: vidtv: Implement a virtual DVB driver Daniel W. S. Almeida
2020-08-21 12:58 ` [Linux-kernel-mentees] " Daniel W. S. Almeida
2020-08-21 12:58 ` [v10 1/4] media: vidtv: implement a tuner driver Daniel W. S. Almeida
2020-08-21 12:58   ` [Linux-kernel-mentees] " Daniel W. S. Almeida
2020-08-21 12:58 ` [v10 2/4] media: vidtv: implement a demodulator driver Daniel W. S. Almeida
2020-08-21 12:58   ` [Linux-kernel-mentees] " Daniel W. S. Almeida
2020-08-21 12:58 ` [v10 3/4] media: vidtv: add a bridge driver Daniel W. S. Almeida
2020-08-21 12:58   ` [Linux-kernel-mentees] " Daniel W. S. Almeida
2020-09-15 11:53   ` Geert Uytterhoeven
2020-09-15 11:53     ` [Linux-kernel-mentees] " Geert Uytterhoeven
2020-09-15 13:26     ` Daniel W. S. Almeida
2020-09-15 13:26       ` [Linux-kernel-mentees] " Daniel W. S. Almeida
2020-09-15 13:35       ` Geert Uytterhoeven
2020-09-15 13:35         ` [Linux-kernel-mentees] " Geert Uytterhoeven
2020-09-15 18:13         ` Daniel W. S. Almeida
2020-09-15 18:13           ` [Linux-kernel-mentees] " Daniel W. S. Almeida
2020-09-16  7:01         ` Mauro Carvalho Chehab
2020-09-16  7:09           ` Geert Uytterhoeven [this message]
2020-08-21 12:58 ` [v10 4/4] media: Documentation: vidtv: Add ReST documentation for vidtv Daniel W. S. Almeida
2020-08-21 12:58   ` [Linux-kernel-mentees] " Daniel W. S. Almeida
2020-09-11  8:02 ` [v10 0/4] media: vidtv: Implement a virtual DVB driver Mauro Carvalho Chehab
2020-09-11  8:02   ` [Linux-kernel-mentees] " Mauro Carvalho Chehab
2020-09-11 12:18   ` Daniel W. S. Almeida
2020-09-11 12:18     ` [Linux-kernel-mentees] " Daniel W. S. Almeida
2020-09-11 13:10     ` Mauro Carvalho Chehab
2020-09-11 13:56       ` Mauro Carvalho Chehab
2020-09-12  2:54         ` Daniel W. S. Almeida
2020-09-12  7:38           ` Mauro Carvalho Chehab
2020-09-12  8:21 ` Hans Verkuil
2020-09-12  8:21   ` [Linux-kernel-mentees] " Hans Verkuil
2020-09-12  9:15   ` Mauro Carvalho Chehab
2020-09-12  9:15     ` [Linux-kernel-mentees] " Mauro Carvalho Chehab
2020-09-12 14:49     ` Daniel W. S. Almeida
2020-09-12 14:49       ` [Linux-kernel-mentees] " Daniel W. S. Almeida
2020-09-12 17:57       ` Mauro Carvalho Chehab
2020-09-12 17:57         ` [Linux-kernel-mentees] " Mauro Carvalho Chehab
2020-09-14  8:33         ` Hans Verkuil
2020-09-14  8:33           ` [Linux-kernel-mentees] " Hans Verkuil
2020-09-12  8:35 ` Hans Verkuil
2020-09-12  8:35   ` [Linux-kernel-mentees] " Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMuHMdW8kzcugxoDosT_50mfAbaHks72_CLWs5yJH5UQ6rz_Zw@mail.gmail.com \
    --to=geert@linux-m68k.org \
    --cc=dwlsalmeida@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab+huawei@kernel.org \
    --cc=nicolas@ndufresne.ca \
    --cc=r.verdejo@samsung.com \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.