diff mbox series

[1/4] drm/vmwgfx: Make console emulation depend on DRM_FBDEV_EMULATION

Message ID 20210415110040.23525-2-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm: Fix config dependencies for fbdev emulation | expand

Commit Message

Thomas Zimmermann April 15, 2021, 11 a.m. UTC
Respect DRM's kconfig setting for fbdev console emulation. If enabled,
it will select all required config options. So remove them from vmwgfx's
Kconfig file.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/vmwgfx/Kconfig      |  7 +------
 drivers/gpu/drm/vmwgfx/Makefile     |  4 +++-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 19 +++++++++++++++++++
 3 files changed, 23 insertions(+), 7 deletions(-)

Comments

Zack Rusin April 15, 2021, 6:21 p.m. UTC | #1
On 4/15/21 7:00 AM, Thomas Zimmermann wrote:
> Respect DRM's kconfig setting for fbdev console emulation. If enabled,
> it will select all required config options. So remove them from vmwgfx's
> Kconfig file.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/vmwgfx/Kconfig      |  7 +------
>   drivers/gpu/drm/vmwgfx/Makefile     |  4 +++-
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 19 +++++++++++++++++++
>   3 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/Kconfig b/drivers/gpu/drm/vmwgfx/Kconfig
> index 15acdf2a7c0f..b3a34196935b 100644
> --- a/drivers/gpu/drm/vmwgfx/Kconfig
> +++ b/drivers/gpu/drm/vmwgfx/Kconfig
> @@ -2,12 +2,7 @@
>   config DRM_VMWGFX
>   	tristate "DRM driver for VMware Virtual GPU"
>   	depends on DRM && PCI && X86 && MMU
> -	select FB_DEFERRED_IO
> -	select FB_CFB_FILLRECT
> -	select FB_CFB_COPYAREA
> -	select FB_CFB_IMAGEBLIT
>   	select DRM_TTM
> -	select FB
>   	select MAPPING_DIRTY_HELPERS
>   	# Only needed for the transitional use of drm_crtc_init - can be removed
>   	# again once vmwgfx sets up the primary plane itself.
> @@ -20,7 +15,7 @@ config DRM_VMWGFX
>   	  The compiled module will be called "vmwgfx.ko".
>   
>   config DRM_VMWGFX_FBCON
> -	depends on DRM_VMWGFX && FB
> +	depends on DRM_VMWGFX && DRM_FBDEV_EMULATION
>   	bool "Enable framebuffer console under vmwgfx by default"
>   	help
>   	   Choose this option if you are shipping a new vmwgfx
> diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
> index 8c02fa5852e7..9f5743013cbb 100644
> --- a/drivers/gpu/drm/vmwgfx/Makefile
> +++ b/drivers/gpu/drm/vmwgfx/Makefile
> @@ -1,6 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0
>   vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
> -	    vmwgfx_fb.o vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
> +	    vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
>   	    vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
>   	    vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
>   	    vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
> @@ -11,5 +11,7 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
>   	    vmwgfx_validation.o vmwgfx_page_dirty.o vmwgfx_streamoutput.o \
>   	    ttm_object.o ttm_lock.o ttm_memory.o
>   
> +vmwgfx-$(CONFIG_DRM_FBDEV_EMULATION) += vmwgfx_fb.o
>   vmwgfx-$(CONFIG_TRANSPARENT_HUGEPAGE) += vmwgfx_thp.o
> +
>   obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 7e6518709e14..e7836da190c4 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -1116,10 +1116,29 @@ extern void vmw_generic_waiter_remove(struct vmw_private *dev_priv,
>    * Kernel framebuffer - vmwgfx_fb.c
>    */
>   
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>   int vmw_fb_init(struct vmw_private *vmw_priv);
>   int vmw_fb_close(struct vmw_private *dev_priv);
>   int vmw_fb_off(struct vmw_private *vmw_priv);
>   int vmw_fb_on(struct vmw_private *vmw_priv);
> +#else
> +static inline int vmw_fb_init(struct vmw_private *vmw_priv)
> +{
> +	return 0;
> +}
> +static inline int vmw_fb_close(struct vmw_private *dev_priv)
> +{
> +	return 0;
> +}
> +static inline int vmw_fb_off(struct vmw_private *vmw_priv)
> +{
> +	return 0;
> +}
> +static inline int vmw_fb_on(struct vmw_private *vmw_priv)
> +{
> +	return 0;
> +}
> +#endif
>   
>   /**
>    * Kernel modesetting - vmwgfx_kms.c
> 

This changes the behavior a bit, I guess DRM_VMWGFX (or at least DRM_VMWGFX_FBCON) would need to select DRM_FBDEV_EMULATION to preserve the old behavior, but that's largely due to the fact that given how those options were setup we never run without FB set. In general it should be ok and looks more reasonable than the current setup. I'll try it out on Monday just in case, but for now:

Reviewed-by: Zack Rusin <zackr@vmware.com>

z
Daniel Vetter April 15, 2021, 6:49 p.m. UTC | #2
On Thu, Apr 15, 2021 at 8:21 PM Zack Rusin <zackr@vmware.com> wrote:
>
> On 4/15/21 7:00 AM, Thomas Zimmermann wrote:
> > Respect DRM's kconfig setting for fbdev console emulation. If enabled,
> > it will select all required config options. So remove them from vmwgfx's
> > Kconfig file.
> >
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> > ---
> >   drivers/gpu/drm/vmwgfx/Kconfig      |  7 +------
> >   drivers/gpu/drm/vmwgfx/Makefile     |  4 +++-
> >   drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 19 +++++++++++++++++++
> >   3 files changed, 23 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/Kconfig b/drivers/gpu/drm/vmwgfx/Kconfig
> > index 15acdf2a7c0f..b3a34196935b 100644
> > --- a/drivers/gpu/drm/vmwgfx/Kconfig
> > +++ b/drivers/gpu/drm/vmwgfx/Kconfig
> > @@ -2,12 +2,7 @@
> >   config DRM_VMWGFX
> >       tristate "DRM driver for VMware Virtual GPU"
> >       depends on DRM && PCI && X86 && MMU
> > -     select FB_DEFERRED_IO
> > -     select FB_CFB_FILLRECT
> > -     select FB_CFB_COPYAREA
> > -     select FB_CFB_IMAGEBLIT
> >       select DRM_TTM
> > -     select FB
> >       select MAPPING_DIRTY_HELPERS
> >       # Only needed for the transitional use of drm_crtc_init - can be removed
> >       # again once vmwgfx sets up the primary plane itself.
> > @@ -20,7 +15,7 @@ config DRM_VMWGFX
> >         The compiled module will be called "vmwgfx.ko".
> >
> >   config DRM_VMWGFX_FBCON
> > -     depends on DRM_VMWGFX && FB
> > +     depends on DRM_VMWGFX && DRM_FBDEV_EMULATION
> >       bool "Enable framebuffer console under vmwgfx by default"
> >       help
> >          Choose this option if you are shipping a new vmwgfx
> > diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
> > index 8c02fa5852e7..9f5743013cbb 100644
> > --- a/drivers/gpu/drm/vmwgfx/Makefile
> > +++ b/drivers/gpu/drm/vmwgfx/Makefile
> > @@ -1,6 +1,6 @@
> >   # SPDX-License-Identifier: GPL-2.0
> >   vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
> > -         vmwgfx_fb.o vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
> > +         vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
> >           vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
> >           vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
> >           vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
> > @@ -11,5 +11,7 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
> >           vmwgfx_validation.o vmwgfx_page_dirty.o vmwgfx_streamoutput.o \
> >           ttm_object.o ttm_lock.o ttm_memory.o
> >
> > +vmwgfx-$(CONFIG_DRM_FBDEV_EMULATION) += vmwgfx_fb.o
> >   vmwgfx-$(CONFIG_TRANSPARENT_HUGEPAGE) += vmwgfx_thp.o
> > +
> >   obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > index 7e6518709e14..e7836da190c4 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> > @@ -1116,10 +1116,29 @@ extern void vmw_generic_waiter_remove(struct vmw_private *dev_priv,
> >    * Kernel framebuffer - vmwgfx_fb.c
> >    */
> >
> > +#ifdef CONFIG_DRM_FBDEV_EMULATION
> >   int vmw_fb_init(struct vmw_private *vmw_priv);
> >   int vmw_fb_close(struct vmw_private *dev_priv);
> >   int vmw_fb_off(struct vmw_private *vmw_priv);
> >   int vmw_fb_on(struct vmw_private *vmw_priv);
> > +#else
> > +static inline int vmw_fb_init(struct vmw_private *vmw_priv)
> > +{
> > +     return 0;
> > +}
> > +static inline int vmw_fb_close(struct vmw_private *dev_priv)
> > +{
> > +     return 0;
> > +}
> > +static inline int vmw_fb_off(struct vmw_private *vmw_priv)
> > +{
> > +     return 0;
> > +}
> > +static inline int vmw_fb_on(struct vmw_private *vmw_priv)
> > +{
> > +     return 0;
> > +}
> > +#endif
> >
> >   /**
> >    * Kernel modesetting - vmwgfx_kms.c
> >
>
> This changes the behavior a bit, I guess DRM_VMWGFX (or at least DRM_VMWGFX_FBCON) would need to select DRM_FBDEV_EMULATION to preserve the old behavior, but that's largely due to the fact that given how those options were setup we never run without FB set. In general it should be ok and looks more reasonable than the current setup. I'll try it out on Monday just in case, but for now:

The issue is that select in Kconfig is pretty annoying (hard to
disable, and it's not recursive), so there's a bit a push to retire
it. Especially for user-facing config knobs like whether you want
fbdev emulation or not. Hence the change.
-Daniel

> Reviewed-by: Zack Rusin <zackr@vmware.com>
>
> z
Thomas Zimmermann April 15, 2021, 6:50 p.m. UTC | #3
Hi

Am 15.04.21 um 20:21 schrieb Zack Rusin:
> On 4/15/21 7:00 AM, Thomas Zimmermann wrote:
>> Respect DRM's kconfig setting for fbdev console emulation. If enabled,
>> it will select all required config options. So remove them from vmwgfx's
>> Kconfig file.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/vmwgfx/Kconfig      |  7 +------
>>   drivers/gpu/drm/vmwgfx/Makefile     |  
4 +++-
>>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 19 +++++++++++++++++++
>>   3 files changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/Kconfig 
>> b/drivers/gpu/drm/vmwgfx/Kconfig
>> index 15acdf2a7c0f..b3a34196935b 100644
>> --- a/drivers/gpu/drm/vmwgfx/Kconfig
>> +++ b/drivers/gpu/drm/vmwgfx/Kconfig
>> @@ -2,12 +2,7 @@
>>   config DRM_VMWGFX
>>       tristate "DRM driver for VMware Virtual 
GPU"
>>       depends on DRM && PCI && X86 && MMU
>> -    select FB_DEFERRED_IO
>> -    select FB_CFB_FILLRECT
>> -    select FB_CFB_COPYAREA
>> -    select FB_CFB_IMAGEBLIT
>>       select DRM_TTM
>> -    select FB
>>       select MAPPING_DIRTY_HELPERS
>>       # Only needed for the transitional use of drm_crtc_init - can be 
>> removed
>>       # again once vmwgfx sets up the primary 
plane itself.
>> @@ -20,7 +15,7 @@ config DRM_VMWGFX
>>         The compiled module will be 
called "vmwgfx.ko".
>>   config DRM_VMWGFX_FBCON
>> -    depends on DRM_VMWGFX && FB
>> +    depends on DRM_VMWGFX && DRM_FBDEV_EMULATION
>>       bool "Enable framebuffer console under vmwgfx by default"
>>       help
>>          Choose this option if 
you are shipping a new vmwgfx
>> diff --git a/drivers/gpu/drm/vmwgfx/Makefile 
>> b/drivers/gpu/drm/vmwgfx/Makefile
>> index 8c02fa5852e7..9f5743013cbb 100644
>> --- a/drivers/gpu/drm/vmwgfx/Makefile
>> +++ b/drivers/gpu/drm/vmwgfx/Makefile
>> @@ -1,6 +1,6 @@
>>   # SPDX-License-Identifier: GPL-2.0
>>   vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
>> -        vmwgfx_fb.o vmwgfx_ioctl.o 
vmwgfx_resource.o 
>> vmwgfx_ttm_buffer.o \
>> +        vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
>>           vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
>>           vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
>>           vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
>> @@ -11,5 +11,7 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o 
>> vmwgfx_kms.o vmwgfx_drv.o \
>>           vmwgfx_validation.o vmwgfx_page_dirty.o vmwgfx_streamoutput.o \
>>           ttm_object.o ttm_lock.o ttm_memory.o
>> +vmwgfx-$(CONFIG_DRM_FBDEV_EMULATION) += vmwgfx_fb.o
>>   vmwgfx-$(CONFIG_TRANSPARENT_HUGEPAGE) += vmwgfx_thp.o
>> +
>>   obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
>> index 7e6518709e14..e7836da190c4 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
>> @@ -1116,10 +1116,29 @@ extern void vmw_generic_waiter_remove(struct 
>> vmw_private *dev_priv,
>>    * Kernel framebuffer - vmwgfx_fb.c
>>    */
>> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>>   int vmw_fb_init(struct vmw_private *vmw_priv);
>>   int vmw_fb_close(struct vmw_private *dev_priv);
>>   int vmw_fb_off(struct vmw_private *vmw_priv);
>>   int vmw_fb_on(struct vmw_private *vmw_priv);
>> +#else
>> +static inline int vmw_fb_init(struct vmw_private *vmw_priv)
>> +{
>> +    return 0;
>> +}
>> +static inline int vmw_fb_close(struct vmw_private *dev_priv)
>> +{
>> +    return 0;
>> +}
>> +static inline int vmw_fb_off(struct vmw_private *vmw_priv)
>> +{
>> +    return 0;
>> +}
>> +static inline int vmw_fb_on(struct vmw_private *vmw_priv)
>> +{
>> +    return 0;
>> +}
>> +#endif
>>   /**
>>    * Kernel modesetting - vmwgfx_kms.c
>>
> 
> This changes the behavior a bit, I guess DRM_VMWGFX (or at least 
> DRM_VMWGFX_FBCON) would need to select DRM_FBDEV_EMULATION to preserve 
> the old behavior, but that's largely due to the fact that given how 
> those options were setup we never run without FB set. In general it 
> should be ok and looks more reasonable than the current setup. I'll try 

> it out on Monday just in case, but for now:
> 
> Reviewed-by: Zack Rusin <zackr@vmware.com>
> 

All other drivers use DRM_FBDEV_EMULATION, so vmwgfx would follow common 
conventions.

AFAICT DRM_VMWGFX_FBCON is just the default on/off setting. How about 
making DRM_VMWGFX_FBCON depend on DRM_FBDEV_EMULATION? Users would be 
able to enable fbdev emulation in general and, if needed, still pick a 
separate default on/off for vmwgfx.

Best regards
Thomas

> z
Zack Rusin April 15, 2021, 7:14 p.m. UTC | #4
On 4/15/21 2:49 PM, Daniel Vetter wrote:
> On Thu, Apr 15, 2021 at 8:21 PM Zack Rusin <zackr@vmware.com> wrote:
>>
>> On 4/15/21 7:00 AM, Thomas Zimmermann wrote:
>>> Respect DRM's kconfig setting for fbdev console emulation. If enabled,
>>> it will select all required config options. So remove them from vmwgfx's
>>> Kconfig file.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>    drivers/gpu/drm/vmwgfx/Kconfig      |  7 +------
>>>    drivers/gpu/drm/vmwgfx/Makefile     |  4 +++-
>>>    drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 19 +++++++++++++++++++
>>>    3 files changed, 23 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/vmwgfx/Kconfig b/drivers/gpu/drm/vmwgfx/Kconfig
>>> index 15acdf2a7c0f..b3a34196935b 100644
>>> --- a/drivers/gpu/drm/vmwgfx/Kconfig
>>> +++ b/drivers/gpu/drm/vmwgfx/Kconfig
>>> @@ -2,12 +2,7 @@
>>>    config DRM_VMWGFX
>>>        tristate "DRM driver for VMware Virtual GPU"
>>>        depends on DRM && PCI && X86 && MMU
>>> -     select FB_DEFERRED_IO
>>> -     select FB_CFB_FILLRECT
>>> -     select FB_CFB_COPYAREA
>>> -     select FB_CFB_IMAGEBLIT
>>>        select DRM_TTM
>>> -     select FB
>>>        select MAPPING_DIRTY_HELPERS
>>>        # Only needed for the transitional use of drm_crtc_init - can be removed
>>>        # again once vmwgfx sets up the primary plane itself.
>>> @@ -20,7 +15,7 @@ config DRM_VMWGFX
>>>          The compiled module will be called "vmwgfx.ko".
>>>
>>>    config DRM_VMWGFX_FBCON
>>> -     depends on DRM_VMWGFX && FB
>>> +     depends on DRM_VMWGFX && DRM_FBDEV_EMULATION
>>>        bool "Enable framebuffer console under vmwgfx by default"
>>>        help
>>>           Choose this option if you are shipping a new vmwgfx
>>> diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
>>> index 8c02fa5852e7..9f5743013cbb 100644
>>> --- a/drivers/gpu/drm/vmwgfx/Makefile
>>> +++ b/drivers/gpu/drm/vmwgfx/Makefile
>>> @@ -1,6 +1,6 @@
>>>    # SPDX-License-Identifier: GPL-2.0
>>>    vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
>>> -         vmwgfx_fb.o vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
>>> +         vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
>>>            vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
>>>            vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
>>>            vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
>>> @@ -11,5 +11,7 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
>>>            vmwgfx_validation.o vmwgfx_page_dirty.o vmwgfx_streamoutput.o \
>>>            ttm_object.o ttm_lock.o ttm_memory.o
>>>
>>> +vmwgfx-$(CONFIG_DRM_FBDEV_EMULATION) += vmwgfx_fb.o
>>>    vmwgfx-$(CONFIG_TRANSPARENT_HUGEPAGE) += vmwgfx_thp.o
>>> +
>>>    obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
>>> index 7e6518709e14..e7836da190c4 100644
>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
>>> @@ -1116,10 +1116,29 @@ extern void vmw_generic_waiter_remove(struct vmw_private *dev_priv,
>>>     * Kernel framebuffer - vmwgfx_fb.c
>>>     */
>>>
>>> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>>>    int vmw_fb_init(struct vmw_private *vmw_priv);
>>>    int vmw_fb_close(struct vmw_private *dev_priv);
>>>    int vmw_fb_off(struct vmw_private *vmw_priv);
>>>    int vmw_fb_on(struct vmw_private *vmw_priv);
>>> +#else
>>> +static inline int vmw_fb_init(struct vmw_private *vmw_priv)
>>> +{
>>> +     return 0;
>>> +}
>>> +static inline int vmw_fb_close(struct vmw_private *dev_priv)
>>> +{
>>> +     return 0;
>>> +}
>>> +static inline int vmw_fb_off(struct vmw_private *vmw_priv)
>>> +{
>>> +     return 0;
>>> +}
>>> +static inline int vmw_fb_on(struct vmw_private *vmw_priv)
>>> +{
>>> +     return 0;
>>> +}
>>> +#endif
>>>
>>>    /**
>>>     * Kernel modesetting - vmwgfx_kms.c
>>>
>>
>> This changes the behavior a bit, I guess DRM_VMWGFX (or at least DRM_VMWGFX_FBCON) would need to select DRM_FBDEV_EMULATION to preserve the old behavior, but that's largely due to the fact that given how those options were setup we never run without FB set. In general it should be ok and looks more reasonable than the current setup. I'll try it out on Monday just in case, but for now:
> 
> The issue is that select in Kconfig is pretty annoying (hard to
> disable, and it's not recursive), so there's a bit a push to retire
> it. Especially for user-facing config knobs like whether you want
> fbdev emulation or not. Hence the change.

Yes, I understand the change. It makes sense. I'm just saying that given how hard it was to even build vmwgfx without FB, it's likely it's never been used/tested in this configuration so it might be broken without FB config set. But like I said, it should be fine (it's basically only if we had some generic init code for the rest of the codebase tied to fb initialization).

z
Thomas Zimmermann April 16, 2021, 10:48 a.m. UTC | #5
Am 15.04.21 um 20:50 schrieb Thomas Zimmermann:
> Hi
> 
> Am 15.04.21 um 20:21 schrieb Zack Rusin:
>> On 4/15/21 7:00 AM, Thomas Zimmermann wrote:
>>> Respect DRM's kconfig setting for fbdev console emulation. If enabled,
>>> it will select all required config options. So remove them from vmwgfx's
>>> Kconfig file.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>   drivers/gpu/drm/vmwgfx/Kconfig      |  7 +------
>>>   drivers/gpu/drm/vmwgfx/Makefile     | 
> 4 +++-
>>>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 19 +++++++++++++++++++
>>>   3 files changed, 23 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/vmwgfx/Kconfig 
>>> b/drivers/gpu/drm/vmwgfx/Kconfig
>>> index 15acdf2a7c0f..b3a34196935b 100644
>>> --- a/drivers/gpu/drm/vmwgfx/Kconfig
>>> +++ b/drivers/gpu/drm/vmwgfx/Kconfig
>>> @@ -2,12 +2,7 @@
>>>   config DRM_VMWGFX
>>>       tristate "DRM driver for VMware Virtual 
> GPU"
>>>       depends on DRM && PCI && X86 && MMU
>>> -    select FB_DEFERRED_IO
>>> -    select FB_CFB_FILLRECT
>>> -    select FB_CFB_COPYAREA
>>> -    select FB_CFB_IMAGEBLIT
>>>       select DRM_TTM
>>> -    select FB
>>>       select MAPPING_DIRTY_HELPERS
>>>       # Only needed for the transitional use 
of drm_crtc_init - can 
>>> be removed
>>>       # again once vmwgfx sets up the primary 
> plane itself.
>>> @@ -20,7 +15,7 @@ config DRM_VMWGFX
>>>         The compiled module will be 
> called "vmwgfx.ko".
>>>   config DRM_VMWGFX_FBCON
>>> -    depends on DRM_VMWGFX && FB
>>> +    depends on DRM_VMWGFX && DRM_FBDEV_EMULATION
>>>       bool "Enable framebuffer console under 
vmwgfx by default"
>>>       help
>>>          Choose this option if 
> you are shipping a new vmwgfx
>>> diff --git a/drivers/gpu/drm/vmwgfx/Makefile 
>>> b/drivers/gpu/drm/vmwgfx/Makefile
>>> index 8c02fa5852e7..9f5743013cbb 100644
>>> --- a/drivers/gpu/drm/vmwgfx/Makefile
>>> +++ b/drivers/gpu/drm/vmwgfx/Makefile
>>> @@ -1,6 +1,6 @@
>>>   # SPDX-License-Identifier: GPL-2.0
>>>   vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
>>> -        vmwgfx_fb.o vmwgfx_ioctl.o 
> vmwgfx_resource.o
>>> vmwgfx_ttm_buffer.o \
>>> +        vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
>>>           vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
>>>           vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
>>>           vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
>>> @@ -11,5 +11,7 @@ vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o 
>>> vmwgfx_kms.o vmwgfx_drv.o \
>>>           vmwgfx_validation.o vmwgfx_page_dirty.o 
>>> vmwgfx_streamoutput.o \
>>>           ttm_object.o ttm_lock.o ttm_memory.o
>>> +vmwgfx-$(CONFIG_DRM_FBDEV_EMULATION) += vmwgfx_fb.o
>>>   vmwgfx-$(CONFIG_TRANSPARENT_HUGEPAGE) += vmwgfx_thp.o
>>> +
>>>   obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
>>> b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
>>> index 7e6518709e14..e7836da190c4 100644
>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
>>> @@ -1116,10 +1116,29 @@ extern void vmw_generic_waiter_remove(struct 
>>> vmw_private *dev_priv,
>>>    * Kernel framebuffer - vmwgfx_fb.c
>>>    */
>>> +#ifdef CONFIG_DRM_FBDEV_EMULATION
>>>   int vmw_fb_init(struct vmw_private *vmw_priv);
>>>   int vmw_fb_close(struct vmw_private *dev_priv);
>>>   int vmw_fb_off(struct vmw_private *vmw_priv);
>>>   int vmw_fb_on(struct vmw_private *vmw_priv);
>>> +#else
>>> +static inline int vmw_fb_init(struct vmw_private *vmw_priv)
>>> +{
>>> +    return 0;
>>> +}
>>> +static inline int vmw_fb_close(struct vmw_private *dev_priv)
>>> +{
>>> +    return 0;
>>> +}
>>> +static inline int vmw_fb_off(struct vmw_private *vmw_priv)
>>> +{
>>> +    return 0;
>>> +}
>>> +static inline int vmw_fb_on(struct vmw_private *vmw_priv)
>>> +{
>>> +    return 0;
>>> +}
>>> +#endif
>>>   /**
>>>    * Kernel modesetting - vmwgfx_kms.c
>>>
>>
>> This changes the behavior a bit, I guess DRM_VMWGFX (or at least 
>> DRM_VMWGFX_FBCON) would need to select DRM_FBDEV_EMULATION to preserve 

>> the old behavior, but that's largely due to the fact that given how 
>> those options were setup we never run without FB set. In general it 
>> should be ok and looks more reasonable than the current setup. I'll try 
> 
>> it out on Monday just in case, but for now:
>>
>> Reviewed-by: Zack Rusin <zackr@vmware.com>
>>
> 
> All other drivers use DRM_FBDEV_EMULATION, so vmwgfx would follow common 
> conventions.
> 
> AFAICT DRM_VMWGFX_FBCON is just the default on/off setting. How about 
> making DRM_VMWGFX_FBCON depend on DRM_FBDEV_EMULATION? Users would be 
> able to enable fbdev emulation in general and, if needed, still pick a 
> separate default on/off for vmwgfx.

Oh, lol. That's already in the patchset.

> 
> Best regards
> Thomas
> 
>> z
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vmwgfx/Kconfig b/drivers/gpu/drm/vmwgfx/Kconfig
index 15acdf2a7c0f..b3a34196935b 100644
--- a/drivers/gpu/drm/vmwgfx/Kconfig
+++ b/drivers/gpu/drm/vmwgfx/Kconfig
@@ -2,12 +2,7 @@ 
 config DRM_VMWGFX
 	tristate "DRM driver for VMware Virtual GPU"
 	depends on DRM && PCI && X86 && MMU
-	select FB_DEFERRED_IO
-	select FB_CFB_FILLRECT
-	select FB_CFB_COPYAREA
-	select FB_CFB_IMAGEBLIT
 	select DRM_TTM
-	select FB
 	select MAPPING_DIRTY_HELPERS
 	# Only needed for the transitional use of drm_crtc_init - can be removed
 	# again once vmwgfx sets up the primary plane itself.
@@ -20,7 +15,7 @@  config DRM_VMWGFX
 	  The compiled module will be called "vmwgfx.ko".
 
 config DRM_VMWGFX_FBCON
-	depends on DRM_VMWGFX && FB
+	depends on DRM_VMWGFX && DRM_FBDEV_EMULATION
 	bool "Enable framebuffer console under vmwgfx by default"
 	help
 	   Choose this option if you are shipping a new vmwgfx
diff --git a/drivers/gpu/drm/vmwgfx/Makefile b/drivers/gpu/drm/vmwgfx/Makefile
index 8c02fa5852e7..9f5743013cbb 100644
--- a/drivers/gpu/drm/vmwgfx/Makefile
+++ b/drivers/gpu/drm/vmwgfx/Makefile
@@ -1,6 +1,6 @@ 
 # SPDX-License-Identifier: GPL-2.0
 vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
-	    vmwgfx_fb.o vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
+	    vmwgfx_ioctl.o vmwgfx_resource.o vmwgfx_ttm_buffer.o \
 	    vmwgfx_cmd.o vmwgfx_irq.o vmwgfx_ldu.o vmwgfx_ttm_glue.o \
 	    vmwgfx_overlay.o vmwgfx_gmrid_manager.o vmwgfx_fence.o \
 	    vmwgfx_bo.o vmwgfx_scrn.o vmwgfx_context.o \
@@ -11,5 +11,7 @@  vmwgfx-y := vmwgfx_execbuf.o vmwgfx_gmr.o vmwgfx_kms.o vmwgfx_drv.o \
 	    vmwgfx_validation.o vmwgfx_page_dirty.o vmwgfx_streamoutput.o \
 	    ttm_object.o ttm_lock.o ttm_memory.o
 
+vmwgfx-$(CONFIG_DRM_FBDEV_EMULATION) += vmwgfx_fb.o
 vmwgfx-$(CONFIG_TRANSPARENT_HUGEPAGE) += vmwgfx_thp.o
+
 obj-$(CONFIG_DRM_VMWGFX) := vmwgfx.o
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 7e6518709e14..e7836da190c4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1116,10 +1116,29 @@  extern void vmw_generic_waiter_remove(struct vmw_private *dev_priv,
  * Kernel framebuffer - vmwgfx_fb.c
  */
 
+#ifdef CONFIG_DRM_FBDEV_EMULATION
 int vmw_fb_init(struct vmw_private *vmw_priv);
 int vmw_fb_close(struct vmw_private *dev_priv);
 int vmw_fb_off(struct vmw_private *vmw_priv);
 int vmw_fb_on(struct vmw_private *vmw_priv);
+#else
+static inline int vmw_fb_init(struct vmw_private *vmw_priv)
+{
+	return 0;
+}
+static inline int vmw_fb_close(struct vmw_private *dev_priv)
+{
+	return 0;
+}
+static inline int vmw_fb_off(struct vmw_private *vmw_priv)
+{
+	return 0;
+}
+static inline int vmw_fb_on(struct vmw_private *vmw_priv)
+{
+	return 0;
+}
+#endif
 
 /**
  * Kernel modesetting - vmwgfx_kms.c