On 6/9/20 7:38 AM, Markus Armbruster wrote: > Philippe Mathieu-Daudé <f4...@amsat.org> writes: > >> On 5/28/20 1:04 PM, Markus Armbruster wrote: >>> xlnx_dp_init() creates these two devices, but they're never realized. >>> Affects machine xlnx-zcu102. >>> >>> In theory, a device becomes real only on realize. In practice, the >>> transition from unreal to real is a fuzzy one. The work to make a >>> device real can be spread between realize methods (fine), >>> instance_init methods (wrong), and board code wiring up the device >>> (fine as long as it effectively happens on realize). Depending on >>> what exactly is done where, a device can work even when we neglect to >>> realize it. >>> >>> These two appear to work. Nevertheless, it's a clear misuse of the >>> interface. Even when it works today (more or less by chance), it can >>> break tomorrow. >>> >>> Fix by realizing them in xlnx_dp_realize(). >>> >>> Fixes: 58ac482a66de09a7590f705e53fc6a3fb8a055e8 >>> Cc: KONRAD Frederic <fred.kon...@greensocs.com> >>> Cc: Alistair Francis <alist...@alistair23.me> >>> Cc: "Edgar E. Iglesias" <edgar.igles...@gmail.com> >>> Cc: Peter Maydell <peter.mayd...@linaro.org> >>> Cc: qemu-...@nongnu.org >>> Signed-off-by: Markus Armbruster <arm...@redhat.com> >>> Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com> >>> Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> >>> --- >>> hw/display/xlnx_dp.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c >>> index 3e5fb44e06..bdc229a51e 100644 >>> --- a/hw/display/xlnx_dp.c >>> +++ b/hw/display/xlnx_dp.c >>> @@ -1264,9 +1264,13 @@ static void xlnx_dp_realize(DeviceState *dev, Error >>> **errp) >>> DisplaySurface *surface; >>> struct audsettings as; >>> >>> + qdev_init_nofail(DEVICE(s->aux_bus->bridge)); >> >> Eh??? Why not realize the bridge in aux_init_bus()? > > Because then aux_init_bus() is no longer "init", but "init and realize". > Instead: "[PATCH v2 32/58] auxbus: New aux_bus_realize(), pairing with > aux_bus_init()". Okay?
It would be easier to follow having aux_bus_realize() introduced first, then used it directly here. Anyway it is reviewed so I don't have a problem if this patch goes as it. Thanks. > >>> + >>> qdev_init_nofail(DEVICE(s->dpcd)); >>> aux_map_slave(AUX_SLAVE(s->dpcd), 0x0000); >>> >>> + qdev_init_nofail(DEVICE(s->edid)); >> >> This one is OK. >> >>> + >>> s->console = graphic_console_init(dev, 0, &xlnx_dp_gfx_ops, s); >>> surface = qemu_console_surface(s->console); >>> xlnx_dpdma_set_host_data_location(s->dpdma, DP_GRAPHIC_DMA_CHANNEL, >>> > >