diff mbox series

[PULL,v2,22/24] hw/display: make ATI_VGA depend on PIXMAN

Message ID 20231106095542.1852973-23-marcandre.lureau@redhat.com
State New
Headers show
Series [PULL,v2,01/24] build-sys: add a "pixman" feature | expand

Commit Message

Marc-André Lureau Nov. 6, 2023, 9:55 a.m. UTC
From: Marc-André Lureau <marcandre.lureau@redhat.com>

To avoid a kconfig cycle, change "depends on PCI" to "select PCI".

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 configs/devices/mips64el-softmmu/default.mak | 3 +--
 hw/display/Kconfig                           | 3 ++-
 hw/display/meson.build                       | 2 +-
 hw/mips/Kconfig                              | 2 ++
 4 files changed, 6 insertions(+), 4 deletions(-)

Comments

Philippe Mathieu-Daudé Nov. 6, 2023, 10:02 a.m. UTC | #1
Hi Marc-André,

Cc'ing Fuloong maintainers,

On 6/11/23 10:55, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   configs/devices/mips64el-softmmu/default.mak | 3 +--
>   hw/display/Kconfig                           | 3 ++-
>   hw/display/meson.build                       | 2 +-
>   hw/mips/Kconfig                              | 2 ++
>   4 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
> index d5188f7ea5..4d80d60511 100644
> --- a/configs/devices/mips64el-softmmu/default.mak
> +++ b/configs/devices/mips64el-softmmu/default.mak
> @@ -1,9 +1,8 @@
>   # Default configuration for mips64el-softmmu
>   
>   include ../mips-softmmu/common.mak
> -CONFIG_FULOONG=y
> +#CONFIG_FULOONG=y

Why is the justification to disable this board? From the
bare "avoid a kconfig cycle" commit message, it is not
obvious to me.

>   CONFIG_LOONGSON3V=y
> -CONFIG_ATI_VGA=y
>   CONFIG_RTL8139_PCI=y
>   CONFIG_JAZZ=y
>   CONFIG_VT82C686=y
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 1aafe1923d..8219225a1c 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -125,7 +125,8 @@ config DPCD
>   config ATI_VGA
>       bool
>       default y if PCI_DEVICES
> -    depends on PCI
> +    depends on PIXMAN
> +    select PCI
>       select VGA
>       select BITBANG_I2C
>       select DDC
> diff --git a/hw/display/meson.build b/hw/display/meson.build
> index 9c06aaee20..344dfe3d8c 100644
> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -62,7 +62,7 @@ system_ss.add(when: 'CONFIG_XLNX_DISPLAYPORT', if_true: files('xlnx_dp.c'))
>   
>   system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
>   
> -system_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c', 'ati_2d.c', 'ati_dbg.c'))
> +system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 'ati_2d.c', 'ati_dbg.c'), pixman])
>   
>   
>   if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> index ac1eb06a51..5d40795c0b 100644
> --- a/hw/mips/Kconfig
> +++ b/hw/mips/Kconfig
> @@ -32,7 +32,9 @@ config JAZZ
>   
>   config FULOONG
>       bool
> +    default y if MIPS64
>       select PCI_BONITO
> +    depends on ATI_VGA
>   
>   config LOONGSON3V
>       bool
Marc-André Lureau Nov. 6, 2023, 10:11 a.m. UTC | #2
Hi Philippe

On Mon, Nov 6, 2023 at 2:03 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Marc-André,
>
> Cc'ing Fuloong maintainers,
>
> On 6/11/23 10:55, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
> > ---
> >   configs/devices/mips64el-softmmu/default.mak | 3 +--
> >   hw/display/Kconfig                           | 3 ++-
> >   hw/display/meson.build                       | 2 +-
> >   hw/mips/Kconfig                              | 2 ++
> >   4 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
> > index d5188f7ea5..4d80d60511 100644
> > --- a/configs/devices/mips64el-softmmu/default.mak
> > +++ b/configs/devices/mips64el-softmmu/default.mak
> > @@ -1,9 +1,8 @@
> >   # Default configuration for mips64el-softmmu
> >
> >   include ../mips-softmmu/common.mak
> > -CONFIG_FULOONG=y
> > +#CONFIG_FULOONG=y
>
> Why is the justification to disable this board? From the
> bare "avoid a kconfig cycle" commit message, it is not
> obvious to me.

It's not actually disabled, it's enabled by default in kconfig.

>
> >   CONFIG_LOONGSON3V=y
> > -CONFIG_ATI_VGA=y
> >   CONFIG_RTL8139_PCI=y
> >   CONFIG_JAZZ=y
> >   CONFIG_VT82C686=y
> > diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> > index 1aafe1923d..8219225a1c 100644
> > --- a/hw/display/Kconfig
> > +++ b/hw/display/Kconfig
> > @@ -125,7 +125,8 @@ config DPCD
> >   config ATI_VGA
> >       bool
> >       default y if PCI_DEVICES
> > -    depends on PCI
> > +    depends on PIXMAN
> > +    select PCI
> >       select VGA
> >       select BITBANG_I2C
> >       select DDC
> > diff --git a/hw/display/meson.build b/hw/display/meson.build
> > index 9c06aaee20..344dfe3d8c 100644
> > --- a/hw/display/meson.build
> > +++ b/hw/display/meson.build
> > @@ -62,7 +62,7 @@ system_ss.add(when: 'CONFIG_XLNX_DISPLAYPORT', if_true: files('xlnx_dp.c'))
> >
> >   system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
> >
> > -system_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c', 'ati_2d.c', 'ati_dbg.c'))
> > +system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 'ati_2d.c', 'ati_dbg.c'), pixman])
> >
> >
> >   if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
> > diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> > index ac1eb06a51..5d40795c0b 100644
> > --- a/hw/mips/Kconfig
> > +++ b/hw/mips/Kconfig
> > @@ -32,7 +32,9 @@ config JAZZ
> >
> >   config FULOONG
> >       bool
> > +    default y if MIPS64

It's now enabled here by default for mips64

> >       select PCI_BONITO
> > +    depends on ATI_VGA
> >
> >   config LOONGSON3V
> >       bool
>
>
BALATON Zoltan Nov. 6, 2023, 10:46 a.m. UTC | #3
On Mon, 6 Nov 2023, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>

Now I NACK it. I've sent a series that adds fallbacks to ati-vga without 
pixman so it does not need to be disabled. This series should be rebased 
on that then this patch and the fuloong one can be dropped. Could you 
please try to do that?

Regards,
BALATON Zoltan

> ---
> configs/devices/mips64el-softmmu/default.mak | 3 +--
> hw/display/Kconfig                           | 3 ++-
> hw/display/meson.build                       | 2 +-
> hw/mips/Kconfig                              | 2 ++
> 4 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
> index d5188f7ea5..4d80d60511 100644
> --- a/configs/devices/mips64el-softmmu/default.mak
> +++ b/configs/devices/mips64el-softmmu/default.mak
> @@ -1,9 +1,8 @@
> # Default configuration for mips64el-softmmu
>
> include ../mips-softmmu/common.mak
> -CONFIG_FULOONG=y
> +#CONFIG_FULOONG=y
> CONFIG_LOONGSON3V=y
> -CONFIG_ATI_VGA=y
> CONFIG_RTL8139_PCI=y
> CONFIG_JAZZ=y
> CONFIG_VT82C686=y
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 1aafe1923d..8219225a1c 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -125,7 +125,8 @@ config DPCD
> config ATI_VGA
>     bool
>     default y if PCI_DEVICES
> -    depends on PCI
> +    depends on PIXMAN
> +    select PCI
>     select VGA
>     select BITBANG_I2C
>     select DDC
> diff --git a/hw/display/meson.build b/hw/display/meson.build
> index 9c06aaee20..344dfe3d8c 100644
> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -62,7 +62,7 @@ system_ss.add(when: 'CONFIG_XLNX_DISPLAYPORT', if_true: files('xlnx_dp.c'))
>
> system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
>
> -system_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c', 'ati_2d.c', 'ati_dbg.c'))
> +system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 'ati_2d.c', 'ati_dbg.c'), pixman])
>
>
> if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> index ac1eb06a51..5d40795c0b 100644
> --- a/hw/mips/Kconfig
> +++ b/hw/mips/Kconfig
> @@ -32,7 +32,9 @@ config JAZZ
>
> config FULOONG
>     bool
> +    default y if MIPS64
>     select PCI_BONITO
> +    depends on ATI_VGA
>
> config LOONGSON3V
>     bool
>
Philippe Mathieu-Daudé Nov. 6, 2023, 10:47 a.m. UTC | #4
On 6/11/23 11:11, Marc-André Lureau wrote:
> Hi Philippe
> 
> On Mon, Nov 6, 2023 at 2:03 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Hi Marc-André,
>>
>> Cc'ing Fuloong maintainers,
>>
>> On 6/11/23 10:55, marcandre.lureau@redhat.com wrote:
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>    configs/devices/mips64el-softmmu/default.mak | 3 +--
>>>    hw/display/Kconfig                           | 3 ++-
>>>    hw/display/meson.build                       | 2 +-
>>>    hw/mips/Kconfig                              | 2 ++
>>>    4 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
>>> index d5188f7ea5..4d80d60511 100644
>>> --- a/configs/devices/mips64el-softmmu/default.mak
>>> +++ b/configs/devices/mips64el-softmmu/default.mak
>>> @@ -1,9 +1,8 @@
>>>    # Default configuration for mips64el-softmmu
>>>
>>>    include ../mips-softmmu/common.mak
>>> -CONFIG_FULOONG=y
>>> +#CONFIG_FULOONG=y
>>
>> Why is the justification to disable this board? From the
>> bare "avoid a kconfig cycle" commit message, it is not
>> obvious to me.
> 
> It's not actually disabled, it's enabled by default in kconfig.

OK, so let's remove the line entirely, not comment it, please.

With that,

Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> 
>>
>>>    CONFIG_LOONGSON3V=y
>>> -CONFIG_ATI_VGA=y
>>>    CONFIG_RTL8139_PCI=y
>>>    CONFIG_JAZZ=y
>>>    CONFIG_VT82C686=y
>>> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
>>> index 1aafe1923d..8219225a1c 100644
>>> --- a/hw/display/Kconfig
>>> +++ b/hw/display/Kconfig
>>> @@ -125,7 +125,8 @@ config DPCD
>>>    config ATI_VGA
>>>        bool
>>>        default y if PCI_DEVICES
>>> -    depends on PCI
>>> +    depends on PIXMAN
>>> +    select PCI
>>>        select VGA
>>>        select BITBANG_I2C
>>>        select DDC
>>> diff --git a/hw/display/meson.build b/hw/display/meson.build
>>> index 9c06aaee20..344dfe3d8c 100644
>>> --- a/hw/display/meson.build
>>> +++ b/hw/display/meson.build
>>> @@ -62,7 +62,7 @@ system_ss.add(when: 'CONFIG_XLNX_DISPLAYPORT', if_true: files('xlnx_dp.c'))
>>>
>>>    system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
>>>
>>> -system_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c', 'ati_2d.c', 'ati_dbg.c'))
>>> +system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 'ati_2d.c', 'ati_dbg.c'), pixman])
>>>
>>>
>>>    if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
>>> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
>>> index ac1eb06a51..5d40795c0b 100644
>>> --- a/hw/mips/Kconfig
>>> +++ b/hw/mips/Kconfig
>>> @@ -32,7 +32,9 @@ config JAZZ
>>>
>>>    config FULOONG
>>>        bool
>>> +    default y if MIPS64
> 
> It's now enabled here by default for mips64
> 
>>>        select PCI_BONITO
>>> +    depends on ATI_VGA
>>>
>>>    config LOONGSON3V
>>>        bool
>>
>>
> 
>
BALATON Zoltan Nov. 6, 2023, 10:53 a.m. UTC | #5
On Mon, 6 Nov 2023, Philippe Mathieu-Daudé wrote:
> On 6/11/23 11:11, Marc-André Lureau wrote:
>> Hi Philippe
>> 
>> On Mon, Nov 6, 2023 at 2:03 PM Philippe Mathieu-Daudé <philmd@linaro.org> 
>> wrote:
>>> 
>>> Hi Marc-André,
>>> 
>>> Cc'ing Fuloong maintainers,
>>> 
>>> On 6/11/23 10:55, marcandre.lureau@redhat.com wrote:
>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> 
>>>> To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
>>>> 
>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> ---
>>>>    configs/devices/mips64el-softmmu/default.mak | 3 +--
>>>>    hw/display/Kconfig                           | 3 ++-
>>>>    hw/display/meson.build                       | 2 +-
>>>>    hw/mips/Kconfig                              | 2 ++
>>>>    4 files changed, 6 insertions(+), 4 deletions(-)
>>>> 
>>>> diff --git a/configs/devices/mips64el-softmmu/default.mak 
>>>> b/configs/devices/mips64el-softmmu/default.mak
>>>> index d5188f7ea5..4d80d60511 100644
>>>> --- a/configs/devices/mips64el-softmmu/default.mak
>>>> +++ b/configs/devices/mips64el-softmmu/default.mak
>>>> @@ -1,9 +1,8 @@
>>>>    # Default configuration for mips64el-softmmu
>>>>
>>>>    include ../mips-softmmu/common.mak
>>>> -CONFIG_FULOONG=y
>>>> +#CONFIG_FULOONG=y
>>> 
>>> Why is the justification to disable this board? From the
>>> bare "avoid a kconfig cycle" commit message, it is not
>>> obvious to me.
>> 
>> It's not actually disabled, it's enabled by default in kconfig.
>
> OK, so let's remove the line entirely, not comment it, please.

Well it's disabled without pixman... I have a series adding fallbacks to 
ati-vga similar to sm501 (which is also needed on aarch64 macOS where 
pixman does not work) and with that this isn't needed. So I think it's 
better to rebase this series on mine then some of this becomes 
unnecessary.

Regards,
BALATON Zoltan

> With that,
>
> Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>> 
>>>
>>>>    CONFIG_LOONGSON3V=y
>>>> -CONFIG_ATI_VGA=y
>>>>    CONFIG_RTL8139_PCI=y
>>>>    CONFIG_JAZZ=y
>>>>    CONFIG_VT82C686=y
>>>> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
>>>> index 1aafe1923d..8219225a1c 100644
>>>> --- a/hw/display/Kconfig
>>>> +++ b/hw/display/Kconfig
>>>> @@ -125,7 +125,8 @@ config DPCD
>>>>    config ATI_VGA
>>>>        bool
>>>>        default y if PCI_DEVICES
>>>> -    depends on PCI
>>>> +    depends on PIXMAN
>>>> +    select PCI
>>>>        select VGA
>>>>        select BITBANG_I2C
>>>>        select DDC
>>>> diff --git a/hw/display/meson.build b/hw/display/meson.build
>>>> index 9c06aaee20..344dfe3d8c 100644
>>>> --- a/hw/display/meson.build
>>>> +++ b/hw/display/meson.build
>>>> @@ -62,7 +62,7 @@ system_ss.add(when: 'CONFIG_XLNX_DISPLAYPORT', if_true: 
>>>> files('xlnx_dp.c'))
>>>>
>>>>    system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
>>>> 
>>>> -system_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c', 
>>>> 'ati_2d.c', 'ati_dbg.c'))
>>>> +system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 
>>>> 'ati_2d.c', 'ati_dbg.c'), pixman])
>>>> 
>>>>
>>>>    if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
>>>> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
>>>> index ac1eb06a51..5d40795c0b 100644
>>>> --- a/hw/mips/Kconfig
>>>> +++ b/hw/mips/Kconfig
>>>> @@ -32,7 +32,9 @@ config JAZZ
>>>>
>>>>    config FULOONG
>>>>        bool
>>>> +    default y if MIPS64
>> 
>> It's now enabled here by default for mips64
>>
>>>>        select PCI_BONITO
>>>> +    depends on ATI_VGA
>>>>
>>>>    config LOONGSON3V
>>>>        bool
>>> 
>>> 
>> 
>> 
>
>
>
Marc-André Lureau Nov. 6, 2023, 10:57 a.m. UTC | #6
Hi Zoltan

On Mon, Nov 6, 2023 at 2:53 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Mon, 6 Nov 2023, Philippe Mathieu-Daudé wrote:
> > On 6/11/23 11:11, Marc-André Lureau wrote:
> >> Hi Philippe
> >>
> >> On Mon, Nov 6, 2023 at 2:03 PM Philippe Mathieu-Daudé <philmd@linaro.org>
> >> wrote:
> >>>
> >>> Hi Marc-André,
> >>>
> >>> Cc'ing Fuloong maintainers,
> >>>
> >>> On 6/11/23 10:55, marcandre.lureau@redhat.com wrote:
> >>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>>
> >>>> To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
> >>>>
> >>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
> >>>> ---
> >>>>    configs/devices/mips64el-softmmu/default.mak | 3 +--
> >>>>    hw/display/Kconfig                           | 3 ++-
> >>>>    hw/display/meson.build                       | 2 +-
> >>>>    hw/mips/Kconfig                              | 2 ++
> >>>>    4 files changed, 6 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/configs/devices/mips64el-softmmu/default.mak
> >>>> b/configs/devices/mips64el-softmmu/default.mak
> >>>> index d5188f7ea5..4d80d60511 100644
> >>>> --- a/configs/devices/mips64el-softmmu/default.mak
> >>>> +++ b/configs/devices/mips64el-softmmu/default.mak
> >>>> @@ -1,9 +1,8 @@
> >>>>    # Default configuration for mips64el-softmmu
> >>>>
> >>>>    include ../mips-softmmu/common.mak
> >>>> -CONFIG_FULOONG=y
> >>>> +#CONFIG_FULOONG=y
> >>>
> >>> Why is the justification to disable this board? From the
> >>> bare "avoid a kconfig cycle" commit message, it is not
> >>> obvious to me.
> >>
> >> It's not actually disabled, it's enabled by default in kconfig.
> >
> > OK, so let's remove the line entirely, not comment it, please.
>
> Well it's disabled without pixman... I have a series adding fallbacks to
> ati-vga similar to sm501 (which is also needed on aarch64 macOS where
> pixman does not work) and with that this isn't needed. So I think it's
> better to rebase this series on mine then some of this becomes
> unnecessary.
>


It's not ready yet though. We can later revert some of this change
when your fallback version is applied. In the meantime, this doesn't
change the behaviour unless PIXMAN has been disabled, so it should be
acceptable. Agree? thanks

> Regards,
> BALATON Zoltan
>
> > With that,
> >
> > Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >
> >>
> >>>
> >>>>    CONFIG_LOONGSON3V=y
> >>>> -CONFIG_ATI_VGA=y
> >>>>    CONFIG_RTL8139_PCI=y
> >>>>    CONFIG_JAZZ=y
> >>>>    CONFIG_VT82C686=y
> >>>> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> >>>> index 1aafe1923d..8219225a1c 100644
> >>>> --- a/hw/display/Kconfig
> >>>> +++ b/hw/display/Kconfig
> >>>> @@ -125,7 +125,8 @@ config DPCD
> >>>>    config ATI_VGA
> >>>>        bool
> >>>>        default y if PCI_DEVICES
> >>>> -    depends on PCI
> >>>> +    depends on PIXMAN
> >>>> +    select PCI
> >>>>        select VGA
> >>>>        select BITBANG_I2C
> >>>>        select DDC
> >>>> diff --git a/hw/display/meson.build b/hw/display/meson.build
> >>>> index 9c06aaee20..344dfe3d8c 100644
> >>>> --- a/hw/display/meson.build
> >>>> +++ b/hw/display/meson.build
> >>>> @@ -62,7 +62,7 @@ system_ss.add(when: 'CONFIG_XLNX_DISPLAYPORT', if_true:
> >>>> files('xlnx_dp.c'))
> >>>>
> >>>>    system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
> >>>>
> >>>> -system_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c',
> >>>> 'ati_2d.c', 'ati_dbg.c'))
> >>>> +system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c',
> >>>> 'ati_2d.c', 'ati_dbg.c'), pixman])
> >>>>
> >>>>
> >>>>    if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
> >>>> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> >>>> index ac1eb06a51..5d40795c0b 100644
> >>>> --- a/hw/mips/Kconfig
> >>>> +++ b/hw/mips/Kconfig
> >>>> @@ -32,7 +32,9 @@ config JAZZ
> >>>>
> >>>>    config FULOONG
> >>>>        bool
> >>>> +    default y if MIPS64
> >>
> >> It's now enabled here by default for mips64
> >>
> >>>>        select PCI_BONITO
> >>>> +    depends on ATI_VGA
> >>>>
> >>>>    config LOONGSON3V
> >>>>        bool
> >>>
> >>>
> >>
> >>
> >
> >
> >
BALATON Zoltan Nov. 6, 2023, 11:05 a.m. UTC | #7
On Mon, 6 Nov 2023, Marc-André Lureau wrote:
> Hi Zoltan
>
> On Mon, Nov 6, 2023 at 2:53 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>
>> On Mon, 6 Nov 2023, Philippe Mathieu-Daudé wrote:
>>> On 6/11/23 11:11, Marc-André Lureau wrote:
>>>> Hi Philippe
>>>>
>>>> On Mon, Nov 6, 2023 at 2:03 PM Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> wrote:
>>>>>
>>>>> Hi Marc-André,
>>>>>
>>>>> Cc'ing Fuloong maintainers,
>>>>>
>>>>> On 6/11/23 10:55, marcandre.lureau@redhat.com wrote:
>>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>
>>>>>> To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
>>>>>>
>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>>> ---
>>>>>>    configs/devices/mips64el-softmmu/default.mak | 3 +--
>>>>>>    hw/display/Kconfig                           | 3 ++-
>>>>>>    hw/display/meson.build                       | 2 +-
>>>>>>    hw/mips/Kconfig                              | 2 ++
>>>>>>    4 files changed, 6 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/configs/devices/mips64el-softmmu/default.mak
>>>>>> b/configs/devices/mips64el-softmmu/default.mak
>>>>>> index d5188f7ea5..4d80d60511 100644
>>>>>> --- a/configs/devices/mips64el-softmmu/default.mak
>>>>>> +++ b/configs/devices/mips64el-softmmu/default.mak
>>>>>> @@ -1,9 +1,8 @@
>>>>>>    # Default configuration for mips64el-softmmu
>>>>>>
>>>>>>    include ../mips-softmmu/common.mak
>>>>>> -CONFIG_FULOONG=y
>>>>>> +#CONFIG_FULOONG=y
>>>>>
>>>>> Why is the justification to disable this board? From the
>>>>> bare "avoid a kconfig cycle" commit message, it is not
>>>>> obvious to me.
>>>>
>>>> It's not actually disabled, it's enabled by default in kconfig.
>>>
>>> OK, so let's remove the line entirely, not comment it, please.
>>
>> Well it's disabled without pixman... I have a series adding fallbacks to
>> ati-vga similar to sm501 (which is also needed on aarch64 macOS where
>> pixman does not work) and with that this isn't needed. So I think it's
>> better to rebase this series on mine then some of this becomes
>> unnecessary.
>>
>
>
> It's not ready yet though. We can later revert some of this change

What's not ready about it?

> when your fallback version is applied. In the meantime, this doesn't
> change the behaviour unless PIXMAN has been disabled, so it should be
> acceptable. Agree? thanks

I think it would be better to not commit something that should be reverted 
soon. I understand you don't want to rebase yout series but I also don't 
want to revert it later so either do the rebase now or hold off with this 
series until after mine got in please.

Regards,
BALATON Zoltan

>>> With that,
>>>
>>> Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>>>
>>>>>
>>>>>>    CONFIG_LOONGSON3V=y
>>>>>> -CONFIG_ATI_VGA=y
>>>>>>    CONFIG_RTL8139_PCI=y
>>>>>>    CONFIG_JAZZ=y
>>>>>>    CONFIG_VT82C686=y
>>>>>> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
>>>>>> index 1aafe1923d..8219225a1c 100644
>>>>>> --- a/hw/display/Kconfig
>>>>>> +++ b/hw/display/Kconfig
>>>>>> @@ -125,7 +125,8 @@ config DPCD
>>>>>>    config ATI_VGA
>>>>>>        bool
>>>>>>        default y if PCI_DEVICES
>>>>>> -    depends on PCI
>>>>>> +    depends on PIXMAN
>>>>>> +    select PCI
>>>>>>        select VGA
>>>>>>        select BITBANG_I2C
>>>>>>        select DDC
>>>>>> diff --git a/hw/display/meson.build b/hw/display/meson.build
>>>>>> index 9c06aaee20..344dfe3d8c 100644
>>>>>> --- a/hw/display/meson.build
>>>>>> +++ b/hw/display/meson.build
>>>>>> @@ -62,7 +62,7 @@ system_ss.add(when: 'CONFIG_XLNX_DISPLAYPORT', if_true:
>>>>>> files('xlnx_dp.c'))
>>>>>>
>>>>>>    system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
>>>>>>
>>>>>> -system_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c',
>>>>>> 'ati_2d.c', 'ati_dbg.c'))
>>>>>> +system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c',
>>>>>> 'ati_2d.c', 'ati_dbg.c'), pixman])
>>>>>>
>>>>>>
>>>>>>    if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
>>>>>> diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
>>>>>> index ac1eb06a51..5d40795c0b 100644
>>>>>> --- a/hw/mips/Kconfig
>>>>>> +++ b/hw/mips/Kconfig
>>>>>> @@ -32,7 +32,9 @@ config JAZZ
>>>>>>
>>>>>>    config FULOONG
>>>>>>        bool
>>>>>> +    default y if MIPS64
>>>>
>>>> It's now enabled here by default for mips64
>>>>
>>>>>>        select PCI_BONITO
>>>>>> +    depends on ATI_VGA
>>>>>>
>>>>>>    config LOONGSON3V
>>>>>>        bool
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>
>
>
>
Stefan Hajnoczi Nov. 7, 2023, 12:34 a.m. UTC | #8
On Mon, 6 Nov 2023 at 19:04, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Mon, 6 Nov 2023, Marc-André Lureau wrote:
> > Hi Zoltan
> >
> > On Mon, Nov 6, 2023 at 2:53 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
> >>
> >> On Mon, 6 Nov 2023, Philippe Mathieu-Daudé wrote:
> >>> On 6/11/23 11:11, Marc-André Lureau wrote:
> >>>> Hi Philippe
> >>>>
> >>>> On Mon, Nov 6, 2023 at 2:03 PM Philippe Mathieu-Daudé <philmd@linaro.org>
> >>>> wrote:
> >>>>>
> >>>>> Hi Marc-André,
> >>>>>
> >>>>> Cc'ing Fuloong maintainers,
> >>>>>
> >>>>> On 6/11/23 10:55, marcandre.lureau@redhat.com wrote:
> >>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>>>>
> >>>>>> To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
> >>>>>>
> >>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>>>> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
> >>>>>> ---
> >>>>>>    configs/devices/mips64el-softmmu/default.mak | 3 +--
> >>>>>>    hw/display/Kconfig                           | 3 ++-
> >>>>>>    hw/display/meson.build                       | 2 +-
> >>>>>>    hw/mips/Kconfig                              | 2 ++
> >>>>>>    4 files changed, 6 insertions(+), 4 deletions(-)
> >>>>>>
> >>>>>> diff --git a/configs/devices/mips64el-softmmu/default.mak
> >>>>>> b/configs/devices/mips64el-softmmu/default.mak
> >>>>>> index d5188f7ea5..4d80d60511 100644
> >>>>>> --- a/configs/devices/mips64el-softmmu/default.mak
> >>>>>> +++ b/configs/devices/mips64el-softmmu/default.mak
> >>>>>> @@ -1,9 +1,8 @@
> >>>>>>    # Default configuration for mips64el-softmmu
> >>>>>>
> >>>>>>    include ../mips-softmmu/common.mak
> >>>>>> -CONFIG_FULOONG=y
> >>>>>> +#CONFIG_FULOONG=y
> >>>>>
> >>>>> Why is the justification to disable this board? From the
> >>>>> bare "avoid a kconfig cycle" commit message, it is not
> >>>>> obvious to me.
> >>>>
> >>>> It's not actually disabled, it's enabled by default in kconfig.
> >>>
> >>> OK, so let's remove the line entirely, not comment it, please.
> >>
> >> Well it's disabled without pixman... I have a series adding fallbacks to
> >> ati-vga similar to sm501 (which is also needed on aarch64 macOS where
> >> pixman does not work) and with that this isn't needed. So I think it's
> >> better to rebase this series on mine then some of this becomes
> >> unnecessary.
> >>
> >
> >
> > It's not ready yet though. We can later revert some of this change
>
> What's not ready about it?
>
> > when your fallback version is applied. In the meantime, this doesn't
> > change the behaviour unless PIXMAN has been disabled, so it should be
> > acceptable. Agree? thanks
>
> I think it would be better to not commit something that should be reverted
> soon. I understand you don't want to rebase yout series but I also don't
> want to revert it later so either do the rebase now or hold off with this
> series until after mine got in please.

QEMU 8.2 soft freeze is today. If you both can resolve this and send
pull requests today, then the changes will make it into 8.2.
Otherwise, asking for this pull request to be rebased has the effect
of delaying it until 8.3.

I'll keep an eye on this series today.

Stefan
BALATON Zoltan Nov. 7, 2023, 1:10 a.m. UTC | #9
On Tue, 7 Nov 2023, Stefan Hajnoczi wrote:
> On Mon, 6 Nov 2023 at 19:04, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>
>> On Mon, 6 Nov 2023, Marc-André Lureau wrote:
>>> Hi Zoltan
>>>
>>> On Mon, Nov 6, 2023 at 2:53 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>>>
>>>> On Mon, 6 Nov 2023, Philippe Mathieu-Daudé wrote:
>>>>> On 6/11/23 11:11, Marc-André Lureau wrote:
>>>>>> Hi Philippe
>>>>>>
>>>>>> On Mon, Nov 6, 2023 at 2:03 PM Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Marc-André,
>>>>>>>
>>>>>>> Cc'ing Fuloong maintainers,
>>>>>>>
>>>>>>> On 6/11/23 10:55, marcandre.lureau@redhat.com wrote:
>>>>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>>>
>>>>>>>> To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
>>>>>>>>
>>>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>>>>> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>>>>>> ---
>>>>>>>>    configs/devices/mips64el-softmmu/default.mak | 3 +--
>>>>>>>>    hw/display/Kconfig                           | 3 ++-
>>>>>>>>    hw/display/meson.build                       | 2 +-
>>>>>>>>    hw/mips/Kconfig                              | 2 ++
>>>>>>>>    4 files changed, 6 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/configs/devices/mips64el-softmmu/default.mak
>>>>>>>> b/configs/devices/mips64el-softmmu/default.mak
>>>>>>>> index d5188f7ea5..4d80d60511 100644
>>>>>>>> --- a/configs/devices/mips64el-softmmu/default.mak
>>>>>>>> +++ b/configs/devices/mips64el-softmmu/default.mak
>>>>>>>> @@ -1,9 +1,8 @@
>>>>>>>>    # Default configuration for mips64el-softmmu
>>>>>>>>
>>>>>>>>    include ../mips-softmmu/common.mak
>>>>>>>> -CONFIG_FULOONG=y
>>>>>>>> +#CONFIG_FULOONG=y
>>>>>>>
>>>>>>> Why is the justification to disable this board? From the
>>>>>>> bare "avoid a kconfig cycle" commit message, it is not
>>>>>>> obvious to me.
>>>>>>
>>>>>> It's not actually disabled, it's enabled by default in kconfig.
>>>>>
>>>>> OK, so let's remove the line entirely, not comment it, please.
>>>>
>>>> Well it's disabled without pixman... I have a series adding fallbacks to
>>>> ati-vga similar to sm501 (which is also needed on aarch64 macOS where
>>>> pixman does not work) and with that this isn't needed. So I think it's
>>>> better to rebase this series on mine then some of this becomes
>>>> unnecessary.
>>>>
>>>
>>>
>>> It's not ready yet though. We can later revert some of this change
>>
>> What's not ready about it?
>>
>>> when your fallback version is applied. In the meantime, this doesn't
>>> change the behaviour unless PIXMAN has been disabled, so it should be
>>> acceptable. Agree? thanks
>>
>> I think it would be better to not commit something that should be reverted
>> soon. I understand you don't want to rebase yout series but I also don't
>> want to revert it later so either do the rebase now or hold off with this
>> series until after mine got in please.
>
> QEMU 8.2 soft freeze is today. If you both can resolve this and send
> pull requests today, then the changes will make it into 8.2.
> Otherwise, asking for this pull request to be rebased has the effect
> of delaying it until 8.3.
>
> I'll keep an eye on this series today.

Marc-André has sent a pull request that contains my ati-vga series:
https://patchew.org/QEMU/20231106133219.2173660-1-marcandre.lureau@redhat.com/
so I think he intends to rebase this series and send another version today 
or come back to this later. (8.3 will be called 9.0 by the way).

Regards,
BALATON Zoltan
Stefan Hajnoczi Nov. 7, 2023, 1:36 a.m. UTC | #10
On Tue, 7 Nov 2023 at 09:10, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Tue, 7 Nov 2023, Stefan Hajnoczi wrote:
> > On Mon, 6 Nov 2023 at 19:04, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> >>
> >> On Mon, 6 Nov 2023, Marc-André Lureau wrote:
> >>> Hi Zoltan
> >>>
> >>> On Mon, Nov 6, 2023 at 2:53 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
> >>>>
> >>>> On Mon, 6 Nov 2023, Philippe Mathieu-Daudé wrote:
> >>>>> On 6/11/23 11:11, Marc-André Lureau wrote:
> >>>>>> Hi Philippe
> >>>>>>
> >>>>>> On Mon, Nov 6, 2023 at 2:03 PM Philippe Mathieu-Daudé <philmd@linaro.org>
> >>>>>> wrote:
> >>>>>>>
> >>>>>>> Hi Marc-André,
> >>>>>>>
> >>>>>>> Cc'ing Fuloong maintainers,
> >>>>>>>
> >>>>>>> On 6/11/23 10:55, marcandre.lureau@redhat.com wrote:
> >>>>>>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>>>>>>
> >>>>>>>> To avoid a kconfig cycle, change "depends on PCI" to "select PCI".
> >>>>>>>>
> >>>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>>>>>> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
> >>>>>>>> ---
> >>>>>>>>    configs/devices/mips64el-softmmu/default.mak | 3 +--
> >>>>>>>>    hw/display/Kconfig                           | 3 ++-
> >>>>>>>>    hw/display/meson.build                       | 2 +-
> >>>>>>>>    hw/mips/Kconfig                              | 2 ++
> >>>>>>>>    4 files changed, 6 insertions(+), 4 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/configs/devices/mips64el-softmmu/default.mak
> >>>>>>>> b/configs/devices/mips64el-softmmu/default.mak
> >>>>>>>> index d5188f7ea5..4d80d60511 100644
> >>>>>>>> --- a/configs/devices/mips64el-softmmu/default.mak
> >>>>>>>> +++ b/configs/devices/mips64el-softmmu/default.mak
> >>>>>>>> @@ -1,9 +1,8 @@
> >>>>>>>>    # Default configuration for mips64el-softmmu
> >>>>>>>>
> >>>>>>>>    include ../mips-softmmu/common.mak
> >>>>>>>> -CONFIG_FULOONG=y
> >>>>>>>> +#CONFIG_FULOONG=y
> >>>>>>>
> >>>>>>> Why is the justification to disable this board? From the
> >>>>>>> bare "avoid a kconfig cycle" commit message, it is not
> >>>>>>> obvious to me.
> >>>>>>
> >>>>>> It's not actually disabled, it's enabled by default in kconfig.
> >>>>>
> >>>>> OK, so let's remove the line entirely, not comment it, please.
> >>>>
> >>>> Well it's disabled without pixman... I have a series adding fallbacks to
> >>>> ati-vga similar to sm501 (which is also needed on aarch64 macOS where
> >>>> pixman does not work) and with that this isn't needed. So I think it's
> >>>> better to rebase this series on mine then some of this becomes
> >>>> unnecessary.
> >>>>
> >>>
> >>>
> >>> It's not ready yet though. We can later revert some of this change
> >>
> >> What's not ready about it?
> >>
> >>> when your fallback version is applied. In the meantime, this doesn't
> >>> change the behaviour unless PIXMAN has been disabled, so it should be
> >>> acceptable. Agree? thanks
> >>
> >> I think it would be better to not commit something that should be reverted
> >> soon. I understand you don't want to rebase yout series but I also don't
> >> want to revert it later so either do the rebase now or hold off with this
> >> series until after mine got in please.
> >
> > QEMU 8.2 soft freeze is today. If you both can resolve this and send
> > pull requests today, then the changes will make it into 8.2.
> > Otherwise, asking for this pull request to be rebased has the effect
> > of delaying it until 8.3.
> >
> > I'll keep an eye on this series today.
>
> Marc-André has sent a pull request that contains my ati-vga series:
> https://patchew.org/QEMU/20231106133219.2173660-1-marcandre.lureau@redhat.com/
> so I think he intends to rebase this series and send another version today
> or come back to this later. (8.3 will be called 9.0 by the way).

Excellent, thanks!

Stefan
diff mbox series

Patch

diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
index d5188f7ea5..4d80d60511 100644
--- a/configs/devices/mips64el-softmmu/default.mak
+++ b/configs/devices/mips64el-softmmu/default.mak
@@ -1,9 +1,8 @@ 
 # Default configuration for mips64el-softmmu
 
 include ../mips-softmmu/common.mak
-CONFIG_FULOONG=y
+#CONFIG_FULOONG=y
 CONFIG_LOONGSON3V=y
-CONFIG_ATI_VGA=y
 CONFIG_RTL8139_PCI=y
 CONFIG_JAZZ=y
 CONFIG_VT82C686=y
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 1aafe1923d..8219225a1c 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -125,7 +125,8 @@  config DPCD
 config ATI_VGA
     bool
     default y if PCI_DEVICES
-    depends on PCI
+    depends on PIXMAN
+    select PCI
     select VGA
     select BITBANG_I2C
     select DDC
diff --git a/hw/display/meson.build b/hw/display/meson.build
index 9c06aaee20..344dfe3d8c 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -62,7 +62,7 @@  system_ss.add(when: 'CONFIG_XLNX_DISPLAYPORT', if_true: files('xlnx_dp.c'))
 
 system_ss.add(when: 'CONFIG_ARTIST', if_true: files('artist.c'))
 
-system_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c', 'ati_2d.c', 'ati_dbg.c'))
+system_ss.add(when: 'CONFIG_ATI_VGA', if_true: [files('ati.c', 'ati_2d.c', 'ati_dbg.c'), pixman])
 
 
 if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index ac1eb06a51..5d40795c0b 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -32,7 +32,9 @@  config JAZZ
 
 config FULOONG
     bool
+    default y if MIPS64
     select PCI_BONITO
+    depends on ATI_VGA
 
 config LOONGSON3V
     bool