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

[BALATON Zoltan <balaton@eik.bme.hu>]

[-- Attachment #1: Type: text/plain, Size: 4578 bytes --]

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
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>
>
>
>