Re: [Qemu-devel] [PATCH v1 1/4] m68k: Add NeXTcube framebuffer device emulation

[Thomas Huth]

Am Sat, 30 Jun 2018 17:12:01 +0100
schrieb Peter Maydell <address@hidden>:

> On 30 June 2018 at 09:33, Thomas Huth <address@hidden> wrote:
> > The NeXTcube uses a linear framebuffer with 4 greyscale colors and
> > a fixed resolution of 1120 * 832.
> > This code has been taken from Bryce Lanham's GSoC 2011 NeXT branch
> > at
> >
> >  https://github.com/blanham/qemu-NeXT/blob/next-cube/hw/next-fb.c
> >
> > and altered to fit the latest interface of the current QEMU (e.g.
> > the device has been "qdev"-ified etc.).
> >
> > Signed-off-by: Thomas Huth <address@hidden>  
> 
> > +static void nextfb_realize(DeviceState *dev, Error **errp)
> > +{
> > +    NeXTFbState *s = NEXTFB(dev);
> > +
> > +    memory_region_allocate_system_memory(&s->fb_mr, NULL,
> > "next.video",
> > +                                         0x1CB100);  
> 
> memory_region_allocate_system_memory() is for allocating
> a machine model's main memory, not for things like video
> framebuffer RAM. (See the doc comment in memory.h.)

That comment recommends memory_region_allocate_aux_memory(), but
that function does not exist (anymore)?

> > +    memory_region_add_subregion(get_system_memory(), 0xB000000,
> > &s->fb_mr);  
> 
> Devices shouldn't directly add memory regions into
> system memory. Instead they should expose sysbus
> memory regions which the board model then maps into
> the right place.

Ok, ... so just to make sure that I've got you right: The device
should call memory_region_init_ram() in it's realize function and the
board code should then call sysbus_mmio_map() with that region, right?

 Thomas