* [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver @ 2020-02-25 15:58 ` Thomas Zimmermann 0 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 15:58 UTC (permalink / raw) To: maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA, mripard-DgEjT+Ai2ygdnm+yROfE0A, airlied-cv59FeDIM0c, daniel-/w4YWyX8dFk, bskeggs-H+wXaHxf7aLQT0dZR+AlfA, emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ, alexander.deucher-5C7GfCeVMHo, sam-uyr5N9Q2VtJg9hUCZPvPmw Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW This patchset moves legacy, non-KMS driver state from struct drm_driver into struct drm_legacy_state. Only non-KMS drivers provide an instance of the latter structure. One special case is nouveau, which supports legacy interfaces. It also provides an instance of the legacy state if the legacy interfaces have been enabled (i.e., defines the config option CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) I reviewed all call sites of legacy state and functions to verify that DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that DRIVER_MODESET is not set. With the mutable KMS state removed, instances of struct drm_driver can be declared as constant. The patchset modifies the DRM core accordingly. Individual drivers can follow later on. Thomas Zimmermann (3): drm: Add separate state structure for legacy, non-KMS drivers drm: Move non-kms driver state into struct drm_legacy_state drm: Constify struct drm_driver in DRM core drivers/gpu/drm/drm_bufs.c | 10 +++++----- drivers/gpu/drm/drm_context.c | 9 +++++---- drivers/gpu/drm/drm_drv.c | 12 ++++++++---- drivers/gpu/drm/drm_file.c | 4 ++-- drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- drivers/gpu/drm/drm_lock.c | 7 ++++--- drivers/gpu/drm/drm_pci.c | 16 ++++++++++------ drivers/gpu/drm/drm_vblank.c | 11 ++++++----- drivers/gpu/drm/i810/i810_drv.c | 10 +++++++--- drivers/gpu/drm/mga/mga_drv.c | 16 ++++++++++------ drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ drivers/gpu/drm/r128/r128_drv.c | 16 ++++++++++------ drivers/gpu/drm/savage/savage_drv.c | 12 ++++++++---- drivers/gpu/drm/sis/sis_drv.c | 8 ++++++-- drivers/gpu/drm/tdfx/tdfx_drv.c | 6 +++++- drivers/gpu/drm/via/via_drv.c | 16 ++++++++++------ include/drm/drm_device.h | 2 +- include/drm/drm_drv.h | 21 +++++---------------- include/drm/drm_legacy.h | 27 +++++++++++++++++++++++---- include/drm/drm_pci.h | 4 ++-- 20 files changed, 138 insertions(+), 83 deletions(-) -- 2.25.0 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver @ 2020-02-25 15:58 ` Thomas Zimmermann 0 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 15:58 UTC (permalink / raw) To: maarten.lankhorst, mripard, airlied, daniel, bskeggs, emil.velikov, alexander.deucher, sam Cc: nouveau, Thomas Zimmermann, dri-devel This patchset moves legacy, non-KMS driver state from struct drm_driver into struct drm_legacy_state. Only non-KMS drivers provide an instance of the latter structure. One special case is nouveau, which supports legacy interfaces. It also provides an instance of the legacy state if the legacy interfaces have been enabled (i.e., defines the config option CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) I reviewed all call sites of legacy state and functions to verify that DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that DRIVER_MODESET is not set. With the mutable KMS state removed, instances of struct drm_driver can be declared as constant. The patchset modifies the DRM core accordingly. Individual drivers can follow later on. Thomas Zimmermann (3): drm: Add separate state structure for legacy, non-KMS drivers drm: Move non-kms driver state into struct drm_legacy_state drm: Constify struct drm_driver in DRM core drivers/gpu/drm/drm_bufs.c | 10 +++++----- drivers/gpu/drm/drm_context.c | 9 +++++---- drivers/gpu/drm/drm_drv.c | 12 ++++++++---- drivers/gpu/drm/drm_file.c | 4 ++-- drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- drivers/gpu/drm/drm_lock.c | 7 ++++--- drivers/gpu/drm/drm_pci.c | 16 ++++++++++------ drivers/gpu/drm/drm_vblank.c | 11 ++++++----- drivers/gpu/drm/i810/i810_drv.c | 10 +++++++--- drivers/gpu/drm/mga/mga_drv.c | 16 ++++++++++------ drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ drivers/gpu/drm/r128/r128_drv.c | 16 ++++++++++------ drivers/gpu/drm/savage/savage_drv.c | 12 ++++++++---- drivers/gpu/drm/sis/sis_drv.c | 8 ++++++-- drivers/gpu/drm/tdfx/tdfx_drv.c | 6 +++++- drivers/gpu/drm/via/via_drv.c | 16 ++++++++++------ include/drm/drm_device.h | 2 +- include/drm/drm_drv.h | 21 +++++---------------- include/drm/drm_legacy.h | 27 +++++++++++++++++++++++---- include/drm/drm_pci.h | 4 ++-- 20 files changed, 138 insertions(+), 83 deletions(-) -- 2.25.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20200225155902.9751-1-tzimmermann-l3A5Bk7waGM@public.gmane.org>]
* [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers 2020-02-25 15:58 ` Thomas Zimmermann @ 2020-02-25 15:59 ` Thomas Zimmermann -1 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 15:59 UTC (permalink / raw) To: maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA, mripard-DgEjT+Ai2ygdnm+yROfE0A, airlied-cv59FeDIM0c, daniel-/w4YWyX8dFk, bskeggs-H+wXaHxf7aLQT0dZR+AlfA, emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ, alexander.deucher-5C7GfCeVMHo, sam-uyr5N9Q2VtJg9hUCZPvPmw Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Non-KMS drivers store state in struct drm_driver. This bloats the structure for KMS drivers and prevents it from being declared with 'static const' qualifiers. Moving the non-KMS state into a separate data structure resolves this. Signed-off-by: Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org> --- drivers/gpu/drm/drm_drv.c | 4 ++++ drivers/gpu/drm/i810/i810_drv.c | 4 ++++ drivers/gpu/drm/mga/mga_drv.c | 4 ++++ drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ drivers/gpu/drm/r128/r128_drv.c | 4 ++++ drivers/gpu/drm/savage/savage_drv.c | 4 ++++ drivers/gpu/drm/sis/sis_drv.c | 4 ++++ drivers/gpu/drm/tdfx/tdfx_drv.c | 4 ++++ drivers/gpu/drm/via/via_drv.c | 4 ++++ include/drm/drm_drv.h | 3 +++ include/drm/drm_legacy.h | 6 ++++++ 11 files changed, 49 insertions(+) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 7b1a628d1f6e..4ba0df097602 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -625,6 +625,10 @@ int drm_dev_init(struct drm_device *dev, if (WARN_ON(!parent)) return -EINVAL; + if (drm_core_check_feature(dev, DRIVER_LEGACY) && + WARN_ON(!driver->legacy)) + return -EINVAL; + kref_init(&dev->ref); dev->dev = get_device(parent); dev->driver = driver; diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c index 0e53a066d4db..55f17f00bae9 100644 --- a/drivers/gpu/drm/i810/i810_drv.c +++ b/drivers/gpu/drm/i810/i810_drv.c @@ -56,6 +56,9 @@ static const struct file_operations i810_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = i810_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY, .dev_priv_size = sizeof(drm_i810_buf_priv_t), @@ -71,6 +74,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &i810_legacy_state, }; static struct pci_driver i810_pci_driver = { diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c index 71128e6f6ae9..4865982d949c 100644 --- a/drivers/gpu/drm/mga/mga_drv.c +++ b/drivers/gpu/drm/mga/mga_drv.c @@ -53,6 +53,9 @@ static const struct file_operations mga_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = mga_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY | @@ -78,6 +81,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &mga_legacy_state, }; static struct pci_driver mga_pci_driver = { diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 6b1629c14dd7..c88cf32e521c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -1103,6 +1103,11 @@ nouveau_driver_fops = { .llseek = noop_llseek, }; +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) +static struct drm_legacy_state nouveau_legacy_state{ +}; +#endif + static struct drm_driver driver_stub = { .driver_features = @@ -1150,6 +1155,9 @@ driver_stub = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) + .legacy = &nouveau_legacy_state, +#endif }; static struct pci_device_id diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c index b7a5f162ebae..ecece3a48d93 100644 --- a/drivers/gpu/drm/r128/r128_drv.c +++ b/drivers/gpu/drm/r128/r128_drv.c @@ -57,6 +57,9 @@ static const struct file_operations r128_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = r128_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY | @@ -81,6 +84,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &r128_legacy_state, }; int r128_driver_load(struct drm_device *dev, unsigned long flags) diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c index 799bd11adb9c..c0a7146fbde1 100644 --- a/drivers/gpu/drm/savage/savage_drv.c +++ b/drivers/gpu/drm/savage/savage_drv.c @@ -47,6 +47,9 @@ static const struct file_operations savage_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = savage_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA | DRIVER_LEGACY, @@ -65,6 +68,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &savage_legacy_state, }; static struct pci_driver savage_pci_driver = { diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c index 2c54b33abb54..1c794a82c8e6 100644 --- a/drivers/gpu/drm/sis/sis_drv.c +++ b/drivers/gpu/drm/sis/sis_drv.c @@ -99,6 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev, struct drm_file *file) kfree(file_priv); } +static struct drm_legacy_state = sis_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_LEGACY, .load = sis_driver_load, @@ -116,6 +119,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &sis_legacy_state, }; static struct pci_driver sis_pci_driver = { diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index ab699bf0ac5c..17aa93d82d6b 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -56,6 +56,9 @@ static const struct file_operations tdfx_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = tdfx_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_LEGACY, .fops = &tdfx_driver_fops, @@ -65,6 +68,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &tdfx_legacy_state, }; static struct pci_driver tdfx_pci_driver = { diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c index 5da38082821f..22113d93ecdd 100644 --- a/drivers/gpu/drm/via/via_drv.c +++ b/drivers/gpu/drm/via/via_drv.c @@ -71,6 +71,9 @@ static const struct file_operations via_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = via_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_IRQ | DRIVER_LEGACY, @@ -97,6 +100,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &via_legacy_state, }; static struct pci_driver via_pci_driver = { diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 97109df5beac..94078af0cc28 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -40,6 +40,7 @@ struct dma_buf_attachment; struct drm_display_mode; struct drm_mode_create_dumb; struct drm_printer; +struct drm_legacy_state; /** * enum drm_driver_feature - feature flags @@ -599,6 +600,8 @@ struct drm_driver { const struct file_operations *fops; /* Everything below here is for legacy driver, never use! */ + struct drm_legacy_state *legacy; + /* private: */ /* List of devices hanging off this driver with stealth attach. */ diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index dcef3598f49e..c98a29e1c2b3 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -49,6 +49,12 @@ struct pci_driver; * you're doing it terribly wrong. */ +/** + * State for non-KMS drivers. + */ +struct drm_legacy_state { +}; + /** * DMA buffer. */ -- 2.25.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers @ 2020-02-25 15:59 ` Thomas Zimmermann 0 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 15:59 UTC (permalink / raw) To: maarten.lankhorst, mripard, airlied, daniel, bskeggs, emil.velikov, alexander.deucher, sam Cc: nouveau, Thomas Zimmermann, dri-devel Non-KMS drivers store state in struct drm_driver. This bloats the structure for KMS drivers and prevents it from being declared with 'static const' qualifiers. Moving the non-KMS state into a separate data structure resolves this. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_drv.c | 4 ++++ drivers/gpu/drm/i810/i810_drv.c | 4 ++++ drivers/gpu/drm/mga/mga_drv.c | 4 ++++ drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ drivers/gpu/drm/r128/r128_drv.c | 4 ++++ drivers/gpu/drm/savage/savage_drv.c | 4 ++++ drivers/gpu/drm/sis/sis_drv.c | 4 ++++ drivers/gpu/drm/tdfx/tdfx_drv.c | 4 ++++ drivers/gpu/drm/via/via_drv.c | 4 ++++ include/drm/drm_drv.h | 3 +++ include/drm/drm_legacy.h | 6 ++++++ 11 files changed, 49 insertions(+) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 7b1a628d1f6e..4ba0df097602 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -625,6 +625,10 @@ int drm_dev_init(struct drm_device *dev, if (WARN_ON(!parent)) return -EINVAL; + if (drm_core_check_feature(dev, DRIVER_LEGACY) && + WARN_ON(!driver->legacy)) + return -EINVAL; + kref_init(&dev->ref); dev->dev = get_device(parent); dev->driver = driver; diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c index 0e53a066d4db..55f17f00bae9 100644 --- a/drivers/gpu/drm/i810/i810_drv.c +++ b/drivers/gpu/drm/i810/i810_drv.c @@ -56,6 +56,9 @@ static const struct file_operations i810_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = i810_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY, .dev_priv_size = sizeof(drm_i810_buf_priv_t), @@ -71,6 +74,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &i810_legacy_state, }; static struct pci_driver i810_pci_driver = { diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c index 71128e6f6ae9..4865982d949c 100644 --- a/drivers/gpu/drm/mga/mga_drv.c +++ b/drivers/gpu/drm/mga/mga_drv.c @@ -53,6 +53,9 @@ static const struct file_operations mga_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = mga_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY | @@ -78,6 +81,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &mga_legacy_state, }; static struct pci_driver mga_pci_driver = { diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 6b1629c14dd7..c88cf32e521c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -1103,6 +1103,11 @@ nouveau_driver_fops = { .llseek = noop_llseek, }; +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) +static struct drm_legacy_state nouveau_legacy_state{ +}; +#endif + static struct drm_driver driver_stub = { .driver_features = @@ -1150,6 +1155,9 @@ driver_stub = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) + .legacy = &nouveau_legacy_state, +#endif }; static struct pci_device_id diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c index b7a5f162ebae..ecece3a48d93 100644 --- a/drivers/gpu/drm/r128/r128_drv.c +++ b/drivers/gpu/drm/r128/r128_drv.c @@ -57,6 +57,9 @@ static const struct file_operations r128_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = r128_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY | @@ -81,6 +84,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &r128_legacy_state, }; int r128_driver_load(struct drm_device *dev, unsigned long flags) diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c index 799bd11adb9c..c0a7146fbde1 100644 --- a/drivers/gpu/drm/savage/savage_drv.c +++ b/drivers/gpu/drm/savage/savage_drv.c @@ -47,6 +47,9 @@ static const struct file_operations savage_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = savage_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA | DRIVER_LEGACY, @@ -65,6 +68,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &savage_legacy_state, }; static struct pci_driver savage_pci_driver = { diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c index 2c54b33abb54..1c794a82c8e6 100644 --- a/drivers/gpu/drm/sis/sis_drv.c +++ b/drivers/gpu/drm/sis/sis_drv.c @@ -99,6 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev, struct drm_file *file) kfree(file_priv); } +static struct drm_legacy_state = sis_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_LEGACY, .load = sis_driver_load, @@ -116,6 +119,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &sis_legacy_state, }; static struct pci_driver sis_pci_driver = { diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index ab699bf0ac5c..17aa93d82d6b 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -56,6 +56,9 @@ static const struct file_operations tdfx_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = tdfx_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_LEGACY, .fops = &tdfx_driver_fops, @@ -65,6 +68,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &tdfx_legacy_state, }; static struct pci_driver tdfx_pci_driver = { diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c index 5da38082821f..22113d93ecdd 100644 --- a/drivers/gpu/drm/via/via_drv.c +++ b/drivers/gpu/drm/via/via_drv.c @@ -71,6 +71,9 @@ static const struct file_operations via_driver_fops = { .llseek = noop_llseek, }; +static struct drm_legacy_state = via_legacy_state { +}; + static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_IRQ | DRIVER_LEGACY, @@ -97,6 +100,7 @@ static struct drm_driver driver = { .major = DRIVER_MAJOR, .minor = DRIVER_MINOR, .patchlevel = DRIVER_PATCHLEVEL, + .legacy = &via_legacy_state, }; static struct pci_driver via_pci_driver = { diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 97109df5beac..94078af0cc28 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -40,6 +40,7 @@ struct dma_buf_attachment; struct drm_display_mode; struct drm_mode_create_dumb; struct drm_printer; +struct drm_legacy_state; /** * enum drm_driver_feature - feature flags @@ -599,6 +600,8 @@ struct drm_driver { const struct file_operations *fops; /* Everything below here is for legacy driver, never use! */ + struct drm_legacy_state *legacy; + /* private: */ /* List of devices hanging off this driver with stealth attach. */ diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index dcef3598f49e..c98a29e1c2b3 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -49,6 +49,12 @@ struct pci_driver; * you're doing it terribly wrong. */ +/** + * State for non-KMS drivers. + */ +struct drm_legacy_state { +}; + /** * DMA buffer. */ -- 2.25.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 22+ messages in thread
[parent not found: <20200225155902.9751-2-tzimmermann-l3A5Bk7waGM@public.gmane.org>]
* Re: [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers 2020-02-25 15:59 ` Thomas Zimmermann @ 2020-02-25 16:06 ` Ilia Mirkin -1 siblings, 0 replies; 22+ messages in thread From: Ilia Mirkin @ 2020-02-25 16:06 UTC (permalink / raw) To: Thomas Zimmermann Cc: David Airlie, nouveau, dri-devel, Maarten Lankhorst, mripard-DgEjT+Ai2ygdnm+yROfE0A, Ben Skeggs, Daniel Vetter, Alex Deucher, Sam Ravnborg, Emil Velikov On Tue, Feb 25, 2020 at 10:59 AM Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org> wrote: > > Non-KMS drivers store state in struct drm_driver. This bloats the > structure for KMS drivers and prevents it from being declared with > 'static const' qualifiers. Moving the non-KMS state into a separate > data structure resolves this. > > Signed-off-by: Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org> > --- > drivers/gpu/drm/drm_drv.c | 4 ++++ > drivers/gpu/drm/i810/i810_drv.c | 4 ++++ > drivers/gpu/drm/mga/mga_drv.c | 4 ++++ > drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ > drivers/gpu/drm/r128/r128_drv.c | 4 ++++ > drivers/gpu/drm/savage/savage_drv.c | 4 ++++ > drivers/gpu/drm/sis/sis_drv.c | 4 ++++ > drivers/gpu/drm/tdfx/tdfx_drv.c | 4 ++++ > drivers/gpu/drm/via/via_drv.c | 4 ++++ > include/drm/drm_drv.h | 3 +++ > include/drm/drm_legacy.h | 6 ++++++ > 11 files changed, 49 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 7b1a628d1f6e..4ba0df097602 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -625,6 +625,10 @@ int drm_dev_init(struct drm_device *dev, > if (WARN_ON(!parent)) > return -EINVAL; > > + if (drm_core_check_feature(dev, DRIVER_LEGACY) && > + WARN_ON(!driver->legacy)) > + return -EINVAL; > + > kref_init(&dev->ref); > dev->dev = get_device(parent); > dev->driver = driver; > diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c > index 0e53a066d4db..55f17f00bae9 100644 > --- a/drivers/gpu/drm/i810/i810_drv.c > +++ b/drivers/gpu/drm/i810/i810_drv.c > @@ -56,6 +56,9 @@ static const struct file_operations i810_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = i810_legacy_state { Does this compile? I might have assumed this would need to be static struct drm_legacy_state i810_legacy_state = { > +}; > + > static struct drm_driver driver = { > .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY, > .dev_priv_size = sizeof(drm_i810_buf_priv_t), > @@ -71,6 +74,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &i810_legacy_state, > }; > > static struct pci_driver i810_pci_driver = { > diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c > index 71128e6f6ae9..4865982d949c 100644 > --- a/drivers/gpu/drm/mga/mga_drv.c > +++ b/drivers/gpu/drm/mga/mga_drv.c > @@ -53,6 +53,9 @@ static const struct file_operations mga_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = mga_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = > DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY | > @@ -78,6 +81,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &mga_legacy_state, > }; > > static struct pci_driver mga_pci_driver = { > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c > index 6b1629c14dd7..c88cf32e521c 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -1103,6 +1103,11 @@ nouveau_driver_fops = { > .llseek = noop_llseek, > }; > > +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) > +static struct drm_legacy_state nouveau_legacy_state{ > +}; > +#endif > + > static struct drm_driver > driver_stub = { > .driver_features = > @@ -1150,6 +1155,9 @@ driver_stub = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) > + .legacy = &nouveau_legacy_state, > +#endif > }; > > static struct pci_device_id > diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c > index b7a5f162ebae..ecece3a48d93 100644 > --- a/drivers/gpu/drm/r128/r128_drv.c > +++ b/drivers/gpu/drm/r128/r128_drv.c > @@ -57,6 +57,9 @@ static const struct file_operations r128_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = r128_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = > DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY | > @@ -81,6 +84,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &r128_legacy_state, > }; > > int r128_driver_load(struct drm_device *dev, unsigned long flags) > diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c > index 799bd11adb9c..c0a7146fbde1 100644 > --- a/drivers/gpu/drm/savage/savage_drv.c > +++ b/drivers/gpu/drm/savage/savage_drv.c > @@ -47,6 +47,9 @@ static const struct file_operations savage_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = savage_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = > DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA | DRIVER_LEGACY, > @@ -65,6 +68,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &savage_legacy_state, > }; > > static struct pci_driver savage_pci_driver = { > diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c > index 2c54b33abb54..1c794a82c8e6 100644 > --- a/drivers/gpu/drm/sis/sis_drv.c > +++ b/drivers/gpu/drm/sis/sis_drv.c > @@ -99,6 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev, struct drm_file *file) > kfree(file_priv); > } > > +static struct drm_legacy_state = sis_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = DRIVER_USE_AGP | DRIVER_LEGACY, > .load = sis_driver_load, > @@ -116,6 +119,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &sis_legacy_state, > }; > > static struct pci_driver sis_pci_driver = { > diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c > index ab699bf0ac5c..17aa93d82d6b 100644 > --- a/drivers/gpu/drm/tdfx/tdfx_drv.c > +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c > @@ -56,6 +56,9 @@ static const struct file_operations tdfx_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = tdfx_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = DRIVER_LEGACY, > .fops = &tdfx_driver_fops, > @@ -65,6 +68,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &tdfx_legacy_state, > }; > > static struct pci_driver tdfx_pci_driver = { > diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c > index 5da38082821f..22113d93ecdd 100644 > --- a/drivers/gpu/drm/via/via_drv.c > +++ b/drivers/gpu/drm/via/via_drv.c > @@ -71,6 +71,9 @@ static const struct file_operations via_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = via_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = > DRIVER_USE_AGP | DRIVER_HAVE_IRQ | DRIVER_LEGACY, > @@ -97,6 +100,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &via_legacy_state, > }; > > static struct pci_driver via_pci_driver = { > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 97109df5beac..94078af0cc28 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -40,6 +40,7 @@ struct dma_buf_attachment; > struct drm_display_mode; > struct drm_mode_create_dumb; > struct drm_printer; > +struct drm_legacy_state; > > /** > * enum drm_driver_feature - feature flags > @@ -599,6 +600,8 @@ struct drm_driver { > const struct file_operations *fops; > > /* Everything below here is for legacy driver, never use! */ > + struct drm_legacy_state *legacy; > + > /* private: */ > > /* List of devices hanging off this driver with stealth attach. */ > diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h > index dcef3598f49e..c98a29e1c2b3 100644 > --- a/include/drm/drm_legacy.h > +++ b/include/drm/drm_legacy.h > @@ -49,6 +49,12 @@ struct pci_driver; > * you're doing it terribly wrong. > */ > > +/** > + * State for non-KMS drivers. > + */ > +struct drm_legacy_state { > +}; > + > /** > * DMA buffer. > */ > -- > 2.25.0 > > _______________________________________________ > Nouveau mailing list > Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers @ 2020-02-25 16:06 ` Ilia Mirkin 0 siblings, 0 replies; 22+ messages in thread From: Ilia Mirkin @ 2020-02-25 16:06 UTC (permalink / raw) To: Thomas Zimmermann Cc: David Airlie, nouveau, dri-devel, Ben Skeggs, Alex Deucher, Sam Ravnborg, Emil Velikov On Tue, Feb 25, 2020 at 10:59 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: > > Non-KMS drivers store state in struct drm_driver. This bloats the > structure for KMS drivers and prevents it from being declared with > 'static const' qualifiers. Moving the non-KMS state into a separate > data structure resolves this. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_drv.c | 4 ++++ > drivers/gpu/drm/i810/i810_drv.c | 4 ++++ > drivers/gpu/drm/mga/mga_drv.c | 4 ++++ > drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ > drivers/gpu/drm/r128/r128_drv.c | 4 ++++ > drivers/gpu/drm/savage/savage_drv.c | 4 ++++ > drivers/gpu/drm/sis/sis_drv.c | 4 ++++ > drivers/gpu/drm/tdfx/tdfx_drv.c | 4 ++++ > drivers/gpu/drm/via/via_drv.c | 4 ++++ > include/drm/drm_drv.h | 3 +++ > include/drm/drm_legacy.h | 6 ++++++ > 11 files changed, 49 insertions(+) > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 7b1a628d1f6e..4ba0df097602 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -625,6 +625,10 @@ int drm_dev_init(struct drm_device *dev, > if (WARN_ON(!parent)) > return -EINVAL; > > + if (drm_core_check_feature(dev, DRIVER_LEGACY) && > + WARN_ON(!driver->legacy)) > + return -EINVAL; > + > kref_init(&dev->ref); > dev->dev = get_device(parent); > dev->driver = driver; > diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c > index 0e53a066d4db..55f17f00bae9 100644 > --- a/drivers/gpu/drm/i810/i810_drv.c > +++ b/drivers/gpu/drm/i810/i810_drv.c > @@ -56,6 +56,9 @@ static const struct file_operations i810_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = i810_legacy_state { Does this compile? I might have assumed this would need to be static struct drm_legacy_state i810_legacy_state = { > +}; > + > static struct drm_driver driver = { > .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY, > .dev_priv_size = sizeof(drm_i810_buf_priv_t), > @@ -71,6 +74,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &i810_legacy_state, > }; > > static struct pci_driver i810_pci_driver = { > diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c > index 71128e6f6ae9..4865982d949c 100644 > --- a/drivers/gpu/drm/mga/mga_drv.c > +++ b/drivers/gpu/drm/mga/mga_drv.c > @@ -53,6 +53,9 @@ static const struct file_operations mga_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = mga_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = > DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY | > @@ -78,6 +81,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &mga_legacy_state, > }; > > static struct pci_driver mga_pci_driver = { > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c > index 6b1629c14dd7..c88cf32e521c 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -1103,6 +1103,11 @@ nouveau_driver_fops = { > .llseek = noop_llseek, > }; > > +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) > +static struct drm_legacy_state nouveau_legacy_state{ > +}; > +#endif > + > static struct drm_driver > driver_stub = { > .driver_features = > @@ -1150,6 +1155,9 @@ driver_stub = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) > + .legacy = &nouveau_legacy_state, > +#endif > }; > > static struct pci_device_id > diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c > index b7a5f162ebae..ecece3a48d93 100644 > --- a/drivers/gpu/drm/r128/r128_drv.c > +++ b/drivers/gpu/drm/r128/r128_drv.c > @@ -57,6 +57,9 @@ static const struct file_operations r128_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = r128_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = > DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY | > @@ -81,6 +84,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &r128_legacy_state, > }; > > int r128_driver_load(struct drm_device *dev, unsigned long flags) > diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c > index 799bd11adb9c..c0a7146fbde1 100644 > --- a/drivers/gpu/drm/savage/savage_drv.c > +++ b/drivers/gpu/drm/savage/savage_drv.c > @@ -47,6 +47,9 @@ static const struct file_operations savage_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = savage_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = > DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA | DRIVER_LEGACY, > @@ -65,6 +68,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &savage_legacy_state, > }; > > static struct pci_driver savage_pci_driver = { > diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c > index 2c54b33abb54..1c794a82c8e6 100644 > --- a/drivers/gpu/drm/sis/sis_drv.c > +++ b/drivers/gpu/drm/sis/sis_drv.c > @@ -99,6 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev, struct drm_file *file) > kfree(file_priv); > } > > +static struct drm_legacy_state = sis_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = DRIVER_USE_AGP | DRIVER_LEGACY, > .load = sis_driver_load, > @@ -116,6 +119,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &sis_legacy_state, > }; > > static struct pci_driver sis_pci_driver = { > diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c > index ab699bf0ac5c..17aa93d82d6b 100644 > --- a/drivers/gpu/drm/tdfx/tdfx_drv.c > +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c > @@ -56,6 +56,9 @@ static const struct file_operations tdfx_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = tdfx_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = DRIVER_LEGACY, > .fops = &tdfx_driver_fops, > @@ -65,6 +68,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &tdfx_legacy_state, > }; > > static struct pci_driver tdfx_pci_driver = { > diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c > index 5da38082821f..22113d93ecdd 100644 > --- a/drivers/gpu/drm/via/via_drv.c > +++ b/drivers/gpu/drm/via/via_drv.c > @@ -71,6 +71,9 @@ static const struct file_operations via_driver_fops = { > .llseek = noop_llseek, > }; > > +static struct drm_legacy_state = via_legacy_state { > +}; > + > static struct drm_driver driver = { > .driver_features = > DRIVER_USE_AGP | DRIVER_HAVE_IRQ | DRIVER_LEGACY, > @@ -97,6 +100,7 @@ static struct drm_driver driver = { > .major = DRIVER_MAJOR, > .minor = DRIVER_MINOR, > .patchlevel = DRIVER_PATCHLEVEL, > + .legacy = &via_legacy_state, > }; > > static struct pci_driver via_pci_driver = { > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index 97109df5beac..94078af0cc28 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -40,6 +40,7 @@ struct dma_buf_attachment; > struct drm_display_mode; > struct drm_mode_create_dumb; > struct drm_printer; > +struct drm_legacy_state; > > /** > * enum drm_driver_feature - feature flags > @@ -599,6 +600,8 @@ struct drm_driver { > const struct file_operations *fops; > > /* Everything below here is for legacy driver, never use! */ > + struct drm_legacy_state *legacy; > + > /* private: */ > > /* List of devices hanging off this driver with stealth attach. */ > diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h > index dcef3598f49e..c98a29e1c2b3 100644 > --- a/include/drm/drm_legacy.h > +++ b/include/drm/drm_legacy.h > @@ -49,6 +49,12 @@ struct pci_driver; > * you're doing it terribly wrong. > */ > > +/** > + * State for non-KMS drivers. > + */ > +struct drm_legacy_state { > +}; > + > /** > * DMA buffer. > */ > -- > 2.25.0 > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CAKb7UviDs+rqF+vtDnV4CbHmY8PW8_-ahpAExctORYPKKyL7Yw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers 2020-02-25 16:06 ` [Nouveau] " Ilia Mirkin @ 2020-02-25 16:21 ` Thomas Zimmermann -1 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 16:21 UTC (permalink / raw) To: Ilia Mirkin Cc: David Airlie, nouveau, dri-devel, Ben Skeggs, Alex Deucher, Sam Ravnborg, Emil Velikov [-- Attachment #1.1.1: Type: text/plain, Size: 10521 bytes --] Hi Am 25.02.20 um 17:06 schrieb Ilia Mirkin: > On Tue, Feb 25, 2020 at 10:59 AM Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org> wrote: >> >> Non-KMS drivers store state in struct drm_driver. This bloats the >> structure for KMS drivers and prevents it from being declared with >> 'static const' qualifiers. Moving the non-KMS state into a separate >> data structure resolves this. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org> >> --- >> drivers/gpu/drm/drm_drv.c | 4 ++++ >> drivers/gpu/drm/i810/i810_drv.c | 4 ++++ >> drivers/gpu/drm/mga/mga_drv.c | 4 ++++ >> drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ >> drivers/gpu/drm/r128/r128_drv.c | 4 ++++ >> drivers/gpu/drm/savage/savage_drv.c | 4 ++++ >> drivers/gpu/drm/sis/sis_drv.c | 4 ++++ >> drivers/gpu/drm/tdfx/tdfx_drv.c | 4 ++++ >> drivers/gpu/drm/via/via_drv.c | 4 ++++ >> include/drm/drm_drv.h | 3 +++ >> include/drm/drm_legacy.h | 6 ++++++ >> 11 files changed, 49 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >> index 7b1a628d1f6e..4ba0df097602 100644 >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c >> @@ -625,6 +625,10 @@ int drm_dev_init(struct drm_device *dev, >> if (WARN_ON(!parent)) >> return -EINVAL; >> >> + if (drm_core_check_feature(dev, DRIVER_LEGACY) && >> + WARN_ON(!driver->legacy)) >> + return -EINVAL; >> + >> kref_init(&dev->ref); >> dev->dev = get_device(parent); >> dev->driver = driver; >> diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c >> index 0e53a066d4db..55f17f00bae9 100644 >> --- a/drivers/gpu/drm/i810/i810_drv.c >> +++ b/drivers/gpu/drm/i810/i810_drv.c >> @@ -56,6 +56,9 @@ static const struct file_operations i810_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = i810_legacy_state { > > Does this compile? I might have assumed this would need to be I compiled an tested this patchset. But apparently I sent an old patch or something went wrong somewhere. Thanks for pointing this out. I'll resend a fixed version. Best regards Thomas > > static struct drm_legacy_state i810_legacy_state = { > >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY, >> .dev_priv_size = sizeof(drm_i810_buf_priv_t), >> @@ -71,6 +74,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &i810_legacy_state, >> }; >> >> static struct pci_driver i810_pci_driver = { >> diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c >> index 71128e6f6ae9..4865982d949c 100644 >> --- a/drivers/gpu/drm/mga/mga_drv.c >> +++ b/drivers/gpu/drm/mga/mga_drv.c >> @@ -53,6 +53,9 @@ static const struct file_operations mga_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = mga_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = >> DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY | >> @@ -78,6 +81,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &mga_legacy_state, >> }; >> >> static struct pci_driver mga_pci_driver = { >> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c >> index 6b1629c14dd7..c88cf32e521c 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c >> @@ -1103,6 +1103,11 @@ nouveau_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) >> +static struct drm_legacy_state nouveau_legacy_state{ >> +}; >> +#endif >> + >> static struct drm_driver >> driver_stub = { >> .driver_features = >> @@ -1150,6 +1155,9 @@ driver_stub = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) >> + .legacy = &nouveau_legacy_state, >> +#endif >> }; >> >> static struct pci_device_id >> diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c >> index b7a5f162ebae..ecece3a48d93 100644 >> --- a/drivers/gpu/drm/r128/r128_drv.c >> +++ b/drivers/gpu/drm/r128/r128_drv.c >> @@ -57,6 +57,9 @@ static const struct file_operations r128_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = r128_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = >> DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY | >> @@ -81,6 +84,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &r128_legacy_state, >> }; >> >> int r128_driver_load(struct drm_device *dev, unsigned long flags) >> diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c >> index 799bd11adb9c..c0a7146fbde1 100644 >> --- a/drivers/gpu/drm/savage/savage_drv.c >> +++ b/drivers/gpu/drm/savage/savage_drv.c >> @@ -47,6 +47,9 @@ static const struct file_operations savage_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = savage_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = >> DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA | DRIVER_LEGACY, >> @@ -65,6 +68,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &savage_legacy_state, >> }; >> >> static struct pci_driver savage_pci_driver = { >> diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c >> index 2c54b33abb54..1c794a82c8e6 100644 >> --- a/drivers/gpu/drm/sis/sis_drv.c >> +++ b/drivers/gpu/drm/sis/sis_drv.c >> @@ -99,6 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev, struct drm_file *file) >> kfree(file_priv); >> } >> >> +static struct drm_legacy_state = sis_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = DRIVER_USE_AGP | DRIVER_LEGACY, >> .load = sis_driver_load, >> @@ -116,6 +119,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &sis_legacy_state, >> }; >> >> static struct pci_driver sis_pci_driver = { >> diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c >> index ab699bf0ac5c..17aa93d82d6b 100644 >> --- a/drivers/gpu/drm/tdfx/tdfx_drv.c >> +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c >> @@ -56,6 +56,9 @@ static const struct file_operations tdfx_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = tdfx_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = DRIVER_LEGACY, >> .fops = &tdfx_driver_fops, >> @@ -65,6 +68,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &tdfx_legacy_state, >> }; >> >> static struct pci_driver tdfx_pci_driver = { >> diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c >> index 5da38082821f..22113d93ecdd 100644 >> --- a/drivers/gpu/drm/via/via_drv.c >> +++ b/drivers/gpu/drm/via/via_drv.c >> @@ -71,6 +71,9 @@ static const struct file_operations via_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = via_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = >> DRIVER_USE_AGP | DRIVER_HAVE_IRQ | DRIVER_LEGACY, >> @@ -97,6 +100,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &via_legacy_state, >> }; >> >> static struct pci_driver via_pci_driver = { >> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h >> index 97109df5beac..94078af0cc28 100644 >> --- a/include/drm/drm_drv.h >> +++ b/include/drm/drm_drv.h >> @@ -40,6 +40,7 @@ struct dma_buf_attachment; >> struct drm_display_mode; >> struct drm_mode_create_dumb; >> struct drm_printer; >> +struct drm_legacy_state; >> >> /** >> * enum drm_driver_feature - feature flags >> @@ -599,6 +600,8 @@ struct drm_driver { >> const struct file_operations *fops; >> >> /* Everything below here is for legacy driver, never use! */ >> + struct drm_legacy_state *legacy; >> + >> /* private: */ >> >> /* List of devices hanging off this driver with stealth attach. */ >> diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h >> index dcef3598f49e..c98a29e1c2b3 100644 >> --- a/include/drm/drm_legacy.h >> +++ b/include/drm/drm_legacy.h >> @@ -49,6 +49,12 @@ struct pci_driver; >> * you're doing it terribly wrong. >> */ >> >> +/** >> + * State for non-KMS drivers. >> + */ >> +struct drm_legacy_state { >> +}; >> + >> /** >> * DMA buffer. >> */ >> -- >> 2.25.0 >> >> _______________________________________________ >> Nouveau mailing list >> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >> https://lists.freedesktop.org/mailman/listinfo/nouveau > _______________________________________________ > dri-devel mailing list > dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 182 bytes --] _______________________________________________ Nouveau mailing list Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers @ 2020-02-25 16:21 ` Thomas Zimmermann 0 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 16:21 UTC (permalink / raw) To: Ilia Mirkin Cc: David Airlie, nouveau, dri-devel, Ben Skeggs, Alex Deucher, Sam Ravnborg, Emil Velikov [-- Attachment #1.1.1: Type: text/plain, Size: 10423 bytes --] Hi Am 25.02.20 um 17:06 schrieb Ilia Mirkin: > On Tue, Feb 25, 2020 at 10:59 AM Thomas Zimmermann <tzimmermann@suse.de> wrote: >> >> Non-KMS drivers store state in struct drm_driver. This bloats the >> structure for KMS drivers and prevents it from being declared with >> 'static const' qualifiers. Moving the non-KMS state into a separate >> data structure resolves this. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/drm_drv.c | 4 ++++ >> drivers/gpu/drm/i810/i810_drv.c | 4 ++++ >> drivers/gpu/drm/mga/mga_drv.c | 4 ++++ >> drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ >> drivers/gpu/drm/r128/r128_drv.c | 4 ++++ >> drivers/gpu/drm/savage/savage_drv.c | 4 ++++ >> drivers/gpu/drm/sis/sis_drv.c | 4 ++++ >> drivers/gpu/drm/tdfx/tdfx_drv.c | 4 ++++ >> drivers/gpu/drm/via/via_drv.c | 4 ++++ >> include/drm/drm_drv.h | 3 +++ >> include/drm/drm_legacy.h | 6 ++++++ >> 11 files changed, 49 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >> index 7b1a628d1f6e..4ba0df097602 100644 >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c >> @@ -625,6 +625,10 @@ int drm_dev_init(struct drm_device *dev, >> if (WARN_ON(!parent)) >> return -EINVAL; >> >> + if (drm_core_check_feature(dev, DRIVER_LEGACY) && >> + WARN_ON(!driver->legacy)) >> + return -EINVAL; >> + >> kref_init(&dev->ref); >> dev->dev = get_device(parent); >> dev->driver = driver; >> diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c >> index 0e53a066d4db..55f17f00bae9 100644 >> --- a/drivers/gpu/drm/i810/i810_drv.c >> +++ b/drivers/gpu/drm/i810/i810_drv.c >> @@ -56,6 +56,9 @@ static const struct file_operations i810_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = i810_legacy_state { > > Does this compile? I might have assumed this would need to be I compiled an tested this patchset. But apparently I sent an old patch or something went wrong somewhere. Thanks for pointing this out. I'll resend a fixed version. Best regards Thomas > > static struct drm_legacy_state i810_legacy_state = { > >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY, >> .dev_priv_size = sizeof(drm_i810_buf_priv_t), >> @@ -71,6 +74,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &i810_legacy_state, >> }; >> >> static struct pci_driver i810_pci_driver = { >> diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c >> index 71128e6f6ae9..4865982d949c 100644 >> --- a/drivers/gpu/drm/mga/mga_drv.c >> +++ b/drivers/gpu/drm/mga/mga_drv.c >> @@ -53,6 +53,9 @@ static const struct file_operations mga_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = mga_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = >> DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY | >> @@ -78,6 +81,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &mga_legacy_state, >> }; >> >> static struct pci_driver mga_pci_driver = { >> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c >> index 6b1629c14dd7..c88cf32e521c 100644 >> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c >> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c >> @@ -1103,6 +1103,11 @@ nouveau_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) >> +static struct drm_legacy_state nouveau_legacy_state{ >> +}; >> +#endif >> + >> static struct drm_driver >> driver_stub = { >> .driver_features = >> @@ -1150,6 +1155,9 @@ driver_stub = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) >> + .legacy = &nouveau_legacy_state, >> +#endif >> }; >> >> static struct pci_device_id >> diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c >> index b7a5f162ebae..ecece3a48d93 100644 >> --- a/drivers/gpu/drm/r128/r128_drv.c >> +++ b/drivers/gpu/drm/r128/r128_drv.c >> @@ -57,6 +57,9 @@ static const struct file_operations r128_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = r128_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = >> DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY | >> @@ -81,6 +84,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &r128_legacy_state, >> }; >> >> int r128_driver_load(struct drm_device *dev, unsigned long flags) >> diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c >> index 799bd11adb9c..c0a7146fbde1 100644 >> --- a/drivers/gpu/drm/savage/savage_drv.c >> +++ b/drivers/gpu/drm/savage/savage_drv.c >> @@ -47,6 +47,9 @@ static const struct file_operations savage_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = savage_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = >> DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA | DRIVER_LEGACY, >> @@ -65,6 +68,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &savage_legacy_state, >> }; >> >> static struct pci_driver savage_pci_driver = { >> diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c >> index 2c54b33abb54..1c794a82c8e6 100644 >> --- a/drivers/gpu/drm/sis/sis_drv.c >> +++ b/drivers/gpu/drm/sis/sis_drv.c >> @@ -99,6 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev, struct drm_file *file) >> kfree(file_priv); >> } >> >> +static struct drm_legacy_state = sis_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = DRIVER_USE_AGP | DRIVER_LEGACY, >> .load = sis_driver_load, >> @@ -116,6 +119,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &sis_legacy_state, >> }; >> >> static struct pci_driver sis_pci_driver = { >> diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c >> index ab699bf0ac5c..17aa93d82d6b 100644 >> --- a/drivers/gpu/drm/tdfx/tdfx_drv.c >> +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c >> @@ -56,6 +56,9 @@ static const struct file_operations tdfx_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = tdfx_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = DRIVER_LEGACY, >> .fops = &tdfx_driver_fops, >> @@ -65,6 +68,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &tdfx_legacy_state, >> }; >> >> static struct pci_driver tdfx_pci_driver = { >> diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c >> index 5da38082821f..22113d93ecdd 100644 >> --- a/drivers/gpu/drm/via/via_drv.c >> +++ b/drivers/gpu/drm/via/via_drv.c >> @@ -71,6 +71,9 @@ static const struct file_operations via_driver_fops = { >> .llseek = noop_llseek, >> }; >> >> +static struct drm_legacy_state = via_legacy_state { >> +}; >> + >> static struct drm_driver driver = { >> .driver_features = >> DRIVER_USE_AGP | DRIVER_HAVE_IRQ | DRIVER_LEGACY, >> @@ -97,6 +100,7 @@ static struct drm_driver driver = { >> .major = DRIVER_MAJOR, >> .minor = DRIVER_MINOR, >> .patchlevel = DRIVER_PATCHLEVEL, >> + .legacy = &via_legacy_state, >> }; >> >> static struct pci_driver via_pci_driver = { >> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h >> index 97109df5beac..94078af0cc28 100644 >> --- a/include/drm/drm_drv.h >> +++ b/include/drm/drm_drv.h >> @@ -40,6 +40,7 @@ struct dma_buf_attachment; >> struct drm_display_mode; >> struct drm_mode_create_dumb; >> struct drm_printer; >> +struct drm_legacy_state; >> >> /** >> * enum drm_driver_feature - feature flags >> @@ -599,6 +600,8 @@ struct drm_driver { >> const struct file_operations *fops; >> >> /* Everything below here is for legacy driver, never use! */ >> + struct drm_legacy_state *legacy; >> + >> /* private: */ >> >> /* List of devices hanging off this driver with stealth attach. */ >> diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h >> index dcef3598f49e..c98a29e1c2b3 100644 >> --- a/include/drm/drm_legacy.h >> +++ b/include/drm/drm_legacy.h >> @@ -49,6 +49,12 @@ struct pci_driver; >> * you're doing it terribly wrong. >> */ >> >> +/** >> + * State for non-KMS drivers. >> + */ >> +struct drm_legacy_state { >> +}; >> + >> /** >> * DMA buffer. >> */ >> -- >> 2.25.0 >> >> _______________________________________________ >> Nouveau mailing list >> Nouveau@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/nouveau > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers 2020-02-25 15:59 ` Thomas Zimmermann @ 2020-02-25 17:37 ` Emil Velikov -1 siblings, 0 replies; 22+ messages in thread From: Emil Velikov @ 2020-02-25 17:37 UTC (permalink / raw) To: Thomas Zimmermann Cc: airlied-cv59FeDIM0c, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA, mripard-DgEjT+Ai2ygdnm+yROfE0A, bskeggs-H+wXaHxf7aLQT0dZR+AlfA, daniel-/w4YWyX8dFk, alexander.deucher-5C7GfCeVMHo, sam-uyr5N9Q2VtJg9hUCZPvPmw, emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ [-- Attachment #1.1: Type: text/plain, Size: 636 bytes --] Hi Thomas, On Tuesday, 25 February 2020, Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org> wrote: > Non-KMS drivers store state in struct drm_driver. This bloats the > structure for KMS drivers and prevents it from being declared with > 'static const' qualifiers. Moving the non-KMS state into a separate > data structure resolves this. > > Have you considered subclassing the const drm_driver for legacy drivers. This way the legacy (pointer) data does not need to be exposed to non-legacy drivers. Not to mention that const data (drm_driver), which has (const) pointer to mutable data (legacy bits) is a trap IMHO Emil [-- Attachment #1.2: Type: text/html, Size: 940 bytes --] [-- Attachment #2: Type: text/plain, Size: 182 bytes --] _______________________________________________ Nouveau mailing list Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers @ 2020-02-25 17:37 ` Emil Velikov 0 siblings, 0 replies; 22+ messages in thread From: Emil Velikov @ 2020-02-25 17:37 UTC (permalink / raw) To: Thomas Zimmermann Cc: airlied, nouveau, dri-devel, bskeggs, alexander.deucher, sam, emil.velikov [-- Attachment #1.1: Type: text/plain, Size: 615 bytes --] Hi Thomas, On Tuesday, 25 February 2020, Thomas Zimmermann <tzimmermann@suse.de> wrote: > Non-KMS drivers store state in struct drm_driver. This bloats the > structure for KMS drivers and prevents it from being declared with > 'static const' qualifiers. Moving the non-KMS state into a separate > data structure resolves this. > > Have you considered subclassing the const drm_driver for legacy drivers. This way the legacy (pointer) data does not need to be exposed to non-legacy drivers. Not to mention that const data (drm_driver), which has (const) pointer to mutable data (legacy bits) is a trap IMHO Emil [-- Attachment #1.2: Type: text/html, Size: 898 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CACvgo52fCMEiohuOkXyZi_mVrFnhMmmB--W64AvhEgVe3i2T5A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers 2020-02-25 17:37 ` [Nouveau] " Emil Velikov @ 2020-02-26 5:28 ` Thomas Zimmermann -1 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-26 5:28 UTC (permalink / raw) To: Emil Velikov Cc: airlied-cv59FeDIM0c, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA, mripard-DgEjT+Ai2ygdnm+yROfE0A, bskeggs-H+wXaHxf7aLQT0dZR+AlfA, daniel-/w4YWyX8dFk, alexander.deucher-5C7GfCeVMHo, sam-uyr5N9Q2VtJg9hUCZPvPmw, emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ [-- Attachment #1.1.1: Type: text/plain, Size: 1260 bytes --] Hi Emil Am 25.02.20 um 18:37 schrieb Emil Velikov: > Hi Thomas, > > On Tuesday, 25 February 2020, Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org > <mailto:tzimmermann-l3A5Bk7waGM@public.gmane.org>> wrote: > > Non-KMS drivers store state in struct drm_driver. This bloats the > structure for KMS drivers and prevents it from being declared with > 'static const' qualifiers. Moving the non-KMS state into a separate > data structure resolves this. > > > Have you considered subclassing the const drm_driver for legacy drivers. > This way the legacy (pointer) data does not need to be exposed to > non-legacy drivers. I didn't want to do an upcast each time the legacy data is required. I know that subclassing is closer to the style of current DRM, but given the current code base, the additional legacy pointer seemed like the lesser evil. Best regards Thomas > > Not to mention that const data (drm_driver), which has (const) pointer > to mutable data (legacy bits) is a trap IMHO > > Emil -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 182 bytes --] _______________________________________________ Nouveau mailing list Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers @ 2020-02-26 5:28 ` Thomas Zimmermann 0 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-26 5:28 UTC (permalink / raw) To: Emil Velikov Cc: airlied, nouveau, dri-devel, bskeggs, alexander.deucher, sam, emil.velikov [-- Attachment #1.1.1: Type: text/plain, Size: 1218 bytes --] Hi Emil Am 25.02.20 um 18:37 schrieb Emil Velikov: > Hi Thomas, > > On Tuesday, 25 February 2020, Thomas Zimmermann <tzimmermann@suse.de > <mailto:tzimmermann@suse.de>> wrote: > > Non-KMS drivers store state in struct drm_driver. This bloats the > structure for KMS drivers and prevents it from being declared with > 'static const' qualifiers. Moving the non-KMS state into a separate > data structure resolves this. > > > Have you considered subclassing the const drm_driver for legacy drivers. > This way the legacy (pointer) data does not need to be exposed to > non-legacy drivers. I didn't want to do an upcast each time the legacy data is required. I know that subclassing is closer to the style of current DRM, but given the current code base, the additional legacy pointer seemed like the lesser evil. Best regards Thomas > > Not to mention that const data (drm_driver), which has (const) pointer > to mutable data (legacy bits) is a trap IMHO > > Emil -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/3] drm: Move non-kms driver state into struct drm_legacy_state 2020-02-25 15:58 ` Thomas Zimmermann @ 2020-02-25 15:59 ` Thomas Zimmermann -1 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 15:59 UTC (permalink / raw) To: maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA, mripard-DgEjT+Ai2ygdnm+yROfE0A, airlied-cv59FeDIM0c, daniel-/w4YWyX8dFk, bskeggs-H+wXaHxf7aLQT0dZR+AlfA, emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ, alexander.deucher-5C7GfCeVMHo, sam-uyr5N9Q2VtJg9hUCZPvPmw Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW All non-kms driver fields are now located in struct drm_legacy_state. A driver-wide instance is available via struct drm_driver.legacy. The call sites test if the driver is marked with DRIVER_LEGACY before accessing the fields. Signed-off-by: Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org> --- drivers/gpu/drm/drm_bufs.c | 10 +++++----- drivers/gpu/drm/drm_context.c | 9 +++++---- drivers/gpu/drm/drm_drv.c | 8 ++++---- drivers/gpu/drm/drm_file.c | 4 ++-- drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- drivers/gpu/drm/drm_lock.c | 7 ++++--- drivers/gpu/drm/drm_pci.c | 8 +++++--- drivers/gpu/drm/drm_vblank.c | 11 ++++++----- drivers/gpu/drm/i810/i810_drv.c | 8 ++++---- drivers/gpu/drm/mga/mga_drv.c | 14 +++++++------- drivers/gpu/drm/r128/r128_drv.c | 14 +++++++------- drivers/gpu/drm/savage/savage_drv.c | 10 +++++----- drivers/gpu/drm/sis/sis_drv.c | 6 +++--- drivers/gpu/drm/tdfx/tdfx_drv.c | 2 +- drivers/gpu/drm/via/via_drv.c | 14 +++++++------- include/drm/drm_drv.h | 14 -------------- include/drm/drm_legacy.h | 11 +++++++++++ 17 files changed, 79 insertions(+), 77 deletions(-) diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index 19297e58b232..fbbd5f5820a4 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -812,7 +812,7 @@ int drm_legacy_addbufs_agp(struct drm_device *dev, buf->pending = 0; buf->file_priv = NULL; - buf->dev_priv_size = dev->driver->dev_priv_size; + buf->dev_priv_size = dev->driver->legacy->dev_priv_size; buf->dev_private = kzalloc(buf->dev_priv_size, GFP_KERNEL); if (!buf->dev_private) { /* Set count correctly so we free the proper amount. */ @@ -1011,7 +1011,7 @@ int drm_legacy_addbufs_pci(struct drm_device *dev, buf->pending = 0; buf->file_priv = NULL; - buf->dev_priv_size = dev->driver->dev_priv_size; + buf->dev_priv_size = dev->driver->legacy->dev_priv_size; buf->dev_private = kzalloc(buf->dev_priv_size, GFP_KERNEL); if (!buf->dev_private) { @@ -1175,7 +1175,7 @@ static int drm_legacy_addbufs_sg(struct drm_device *dev, buf->pending = 0; buf->file_priv = NULL; - buf->dev_priv_size = dev->driver->dev_priv_size; + buf->dev_priv_size = dev->driver->legacy->dev_priv_size; buf->dev_private = kzalloc(buf->dev_priv_size, GFP_KERNEL); if (!buf->dev_private) { /* Set count correctly so we free the proper amount. */ @@ -1581,8 +1581,8 @@ int drm_legacy_dma_ioctl(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_LEGACY)) return -EOPNOTSUPP; - if (dev->driver->dma_ioctl) - return dev->driver->dma_ioctl(dev, data, file_priv); + if (dev->driver->legacy->dma_ioctl) + return dev->driver->legacy->dma_ioctl(dev, data, file_priv); else return -EINVAL; } diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c index 1f802d8e5681..b433f755d7c2 100644 --- a/drivers/gpu/drm/drm_context.c +++ b/drivers/gpu/drm/drm_context.c @@ -145,8 +145,9 @@ void drm_legacy_ctxbitmap_flush(struct drm_device *dev, struct drm_file *file) list_for_each_entry_safe(pos, tmp, &dev->ctxlist, head) { if (pos->tag == file && pos->handle != DRM_KERNEL_CONTEXT) { - if (dev->driver->context_dtor) - dev->driver->context_dtor(dev, pos->handle); + if (dev->driver->legacy->context_dtor) + dev->driver->legacy->context_dtor(dev, + pos->handle); drm_legacy_ctxbitmap_free(dev, pos->handle); list_del(&pos->head); @@ -500,8 +501,8 @@ int drm_legacy_rmctx(struct drm_device *dev, void *data, DRM_DEBUG("%d\n", ctx->handle); if (ctx->handle != DRM_KERNEL_CONTEXT) { - if (dev->driver->context_dtor) - dev->driver->context_dtor(dev, ctx->handle); + if (dev->driver->legacy->context_dtor) + dev->driver->legacy->context_dtor(dev, ctx->handle); drm_legacy_ctxbitmap_free(dev, ctx->handle); } diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 4ba0df097602..4875b76e7656 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -625,14 +625,14 @@ int drm_dev_init(struct drm_device *dev, if (WARN_ON(!parent)) return -EINVAL; - if (drm_core_check_feature(dev, DRIVER_LEGACY) && - WARN_ON(!driver->legacy)) - return -EINVAL; - kref_init(&dev->ref); dev->dev = get_device(parent); dev->driver = driver; + if (drm_core_check_feature(dev, DRIVER_LEGACY) && + WARN_ON(!driver->legacy)) + return -EINVAL; + /* no per-device feature limits by default */ dev->driver_features = ~0u; diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index c4c704e01961..e9cb7cf2eb48 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -254,8 +254,8 @@ void drm_file_free(struct drm_file *file) atomic_read(&dev->open_count)); if (drm_core_check_feature(dev, DRIVER_LEGACY) && - dev->driver->preclose) - dev->driver->preclose(dev, file); + dev->driver->legacy->preclose) + dev->driver->legacy->preclose(dev, file); if (drm_core_check_feature(dev, DRIVER_LEGACY)) drm_legacy_lock_release(dev, file->filp); diff --git a/drivers/gpu/drm/drm_legacy_misc.c b/drivers/gpu/drm/drm_legacy_misc.c index 8f54e6a78b6f..07172d0a4f6f 100644 --- a/drivers/gpu/drm/drm_legacy_misc.c +++ b/drivers/gpu/drm/drm_legacy_misc.c @@ -60,9 +60,9 @@ int drm_legacy_setup(struct drm_device * dev) { int ret; - if (dev->driver->firstopen && - drm_core_check_feature(dev, DRIVER_LEGACY)) { - ret = dev->driver->firstopen(dev); + if (drm_core_check_feature(dev, DRIVER_LEGACY) && + dev->driver->legacy->firstopen) { + ret = dev->driver->legacy->firstopen(dev); if (ret != 0) return ret; } diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c index 2c79e8199e3c..f07bf74065ec 100644 --- a/drivers/gpu/drm/drm_lock.c +++ b/drivers/gpu/drm/drm_lock.c @@ -223,7 +223,7 @@ int drm_legacy_lock(struct drm_device *dev, void *data, ret ? "interrupted" : "has lock"); if (ret) return ret; - /* don't set the block all signals on the master process for now + /* don't set the block all signals on the master process for now * really probably not the correct answer but lets us debug xkb * xserver for now */ if (!drm_is_current_master(file_priv)) { @@ -231,9 +231,10 @@ int drm_legacy_lock(struct drm_device *dev, void *data, dev->sigdata.lock = master->lock.hw_lock; } - if (dev->driver->dma_quiescent && (lock->flags & _DRM_LOCK_QUIESCENT)) + if (dev->driver->legacy->dma_quiescent && + (lock->flags & _DRM_LOCK_QUIESCENT)) { - if (dev->driver->dma_quiescent(dev)) { + if (dev->driver->legacy->dma_quiescent(dev)) { DRM_DEBUG("%d waiting for DMA quiescent\n", lock->context); return -EBUSY; diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index c6bb98729a26..24ed0e92fd66 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -239,7 +239,8 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, /* No locking needed since shadow-attach is single-threaded since it may * only be called from the per-driver module init hook. */ if (drm_core_check_feature(dev, DRIVER_LEGACY)) - list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list); + list_add_tail(&dev->legacy_dev_list, + &driver->legacy->legacy_dev_list); return 0; @@ -275,7 +276,7 @@ int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) return -EINVAL; /* If not using KMS, fall back to stealth mode manual scanning. */ - INIT_LIST_HEAD(&driver->legacy_dev_list); + INIT_LIST_HEAD(&driver->legacy->legacy_dev_list); for (i = 0; pdriver->id_table[i].vendor != 0; i++) { pid = &pdriver->id_table[i]; @@ -317,7 +318,8 @@ void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) if (!(driver->driver_features & DRIVER_LEGACY)) { WARN_ON(1); } else { - list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list, + list_for_each_entry_safe(dev, tmp, + &driver->legacy->legacy_dev_list, legacy_dev_list) { list_del(&dev->legacy_dev_list); drm_put_dev(dev); diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 47fc4339ec7f..8655cf04712b 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -30,6 +30,7 @@ #include <drm/drm_crtc.h> #include <drm/drm_drv.h> #include <drm/drm_framebuffer.h> +#include <drm/drm_legacy.h> #include <drm/drm_modeset_helper_vtables.h> #include <drm/drm_print.h> #include <drm/drm_vblank.h> @@ -144,8 +145,8 @@ static u32 __get_vblank_counter(struct drm_device *dev, unsigned int pipe) if (crtc->funcs->get_vblank_counter) return crtc->funcs->get_vblank_counter(crtc); - } else if (dev->driver->get_vblank_counter) { - return dev->driver->get_vblank_counter(dev, pipe); + } else if (dev->driver->legacy->get_vblank_counter) { + return dev->driver->legacy->get_vblank_counter(dev, pipe); } return drm_vblank_no_hw_counter(dev, pipe); @@ -364,7 +365,7 @@ static void __disable_vblank(struct drm_device *dev, unsigned int pipe) if (crtc->funcs->disable_vblank) crtc->funcs->disable_vblank(crtc); } else { - dev->driver->disable_vblank(dev, pipe); + dev->driver->legacy->disable_vblank(dev, pipe); } } @@ -1033,8 +1034,8 @@ static int __enable_vblank(struct drm_device *dev, unsigned int pipe) if (crtc->funcs->enable_vblank) return crtc->funcs->enable_vblank(crtc); - } else if (dev->driver->enable_vblank) { - return dev->driver->enable_vblank(dev, pipe); + } else if (dev->driver->legacy->enable_vblank) { + return dev->driver->legacy->enable_vblank(dev, pipe); } return -EINVAL; diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c index 55f17f00bae9..dd036e3ceb3d 100644 --- a/drivers/gpu/drm/i810/i810_drv.c +++ b/drivers/gpu/drm/i810/i810_drv.c @@ -56,16 +56,16 @@ static const struct file_operations i810_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = i810_legacy_state { +static struct drm_legacy_state i810_legacy_state = { + .dev_priv_size = sizeof(drm_i810_buf_priv_t), + .preclose = i810_driver_preclose, + .dma_quiescent = i810_driver_dma_quiescent, }; static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY, - .dev_priv_size = sizeof(drm_i810_buf_priv_t), .load = i810_driver_load, .lastclose = i810_driver_lastclose, - .preclose = i810_driver_preclose, - .dma_quiescent = i810_driver_dma_quiescent, .ioctls = i810_ioctls, .fops = &i810_driver_fops, .name = DRIVER_NAME, diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c index 4865982d949c..2454c57f9370 100644 --- a/drivers/gpu/drm/mga/mga_drv.c +++ b/drivers/gpu/drm/mga/mga_drv.c @@ -53,27 +53,27 @@ static const struct file_operations mga_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = mga_legacy_state { +static struct drm_legacy_state mga_legacy_state = { + .dev_priv_size = sizeof(drm_mga_buf_priv_t), + .dma_quiescent = mga_driver_dma_quiescent, + .get_vblank_counter = mga_get_vblank_counter, + .enable_vblank = mga_enable_vblank, + .disable_vblank = mga_disable_vblank, + .dma_ioctl = mga_dma_buffers, }; static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY | DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ, - .dev_priv_size = sizeof(drm_mga_buf_priv_t), .load = mga_driver_load, .unload = mga_driver_unload, .lastclose = mga_driver_lastclose, - .dma_quiescent = mga_driver_dma_quiescent, - .get_vblank_counter = mga_get_vblank_counter, - .enable_vblank = mga_enable_vblank, - .disable_vblank = mga_disable_vblank, .irq_preinstall = mga_driver_irq_preinstall, .irq_postinstall = mga_driver_irq_postinstall, .irq_uninstall = mga_driver_irq_uninstall, .irq_handler = mga_driver_irq_handler, .ioctls = mga_ioctls, - .dma_ioctl = mga_dma_buffers, .fops = &mga_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c index ecece3a48d93..9cfd86117e80 100644 --- a/drivers/gpu/drm/r128/r128_drv.c +++ b/drivers/gpu/drm/r128/r128_drv.c @@ -57,26 +57,26 @@ static const struct file_operations r128_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = r128_legacy_state { +static struct drm_legacy_state r128_legacy_state = { + .dev_priv_size = sizeof(drm_r128_buf_priv_t), + .preclose = r128_driver_preclose, + .get_vblank_counter = r128_get_vblank_counter, + .enable_vblank = r128_enable_vblank, + .disable_vblank = r128_disable_vblank, + .dma_ioctl = r128_cce_buffers, }; static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY | DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ, - .dev_priv_size = sizeof(drm_r128_buf_priv_t), .load = r128_driver_load, - .preclose = r128_driver_preclose, .lastclose = r128_driver_lastclose, - .get_vblank_counter = r128_get_vblank_counter, - .enable_vblank = r128_enable_vblank, - .disable_vblank = r128_disable_vblank, .irq_preinstall = r128_driver_irq_preinstall, .irq_postinstall = r128_driver_irq_postinstall, .irq_uninstall = r128_driver_irq_uninstall, .irq_handler = r128_driver_irq_handler, .ioctls = r128_ioctls, - .dma_ioctl = r128_cce_buffers, .fops = &r128_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c index c0a7146fbde1..2d2680b47eff 100644 --- a/drivers/gpu/drm/savage/savage_drv.c +++ b/drivers/gpu/drm/savage/savage_drv.c @@ -47,20 +47,20 @@ static const struct file_operations savage_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = savage_legacy_state { +static struct drm_legacy_state savage_legacy_state = { + .dev_priv_size = sizeof(drm_savage_buf_priv_t), + .firstopen = savage_driver_firstopen, + .preclose = savage_reclaim_buffers, + .dma_ioctl = savage_bci_buffers, }; static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA | DRIVER_LEGACY, - .dev_priv_size = sizeof(drm_savage_buf_priv_t), .load = savage_driver_load, - .firstopen = savage_driver_firstopen, - .preclose = savage_reclaim_buffers, .lastclose = savage_driver_lastclose, .unload = savage_driver_unload, .ioctls = savage_ioctls, - .dma_ioctl = savage_bci_buffers, .fops = &savage_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c index 1c794a82c8e6..c465776e3180 100644 --- a/drivers/gpu/drm/sis/sis_drv.c +++ b/drivers/gpu/drm/sis/sis_drv.c @@ -99,7 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev, struct drm_file *file) kfree(file_priv); } -static struct drm_legacy_state = sis_legacy_state { +static struct drm_legacy_state sis_legacy_state = { + .preclose = sis_reclaim_buffers_locked, + .dma_quiescent = sis_idle, }; static struct drm_driver driver = { @@ -107,9 +109,7 @@ static struct drm_driver driver = { .load = sis_driver_load, .unload = sis_driver_unload, .open = sis_driver_open, - .preclose = sis_reclaim_buffers_locked, .postclose = sis_driver_postclose, - .dma_quiescent = sis_idle, .lastclose = sis_lastclose, .ioctls = sis_ioctls, .fops = &sis_driver_fops, diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index 17aa93d82d6b..a8a6bce6afbe 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -56,7 +56,7 @@ static const struct file_operations tdfx_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = tdfx_legacy_state { +static struct drm_legacy_state tdfx_legacy_state = { }; static struct drm_driver driver = { diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c index 22113d93ecdd..62f026da5958 100644 --- a/drivers/gpu/drm/via/via_drv.c +++ b/drivers/gpu/drm/via/via_drv.c @@ -71,7 +71,13 @@ static const struct file_operations via_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = via_legacy_state { +static struct drm_legacy_state via_legacy_state = { + .preclose = via_reclaim_buffers_locked, + .context_dtor = via_final_context, + .get_vblank_counter = via_get_vblank_counter, + .enable_vblank = via_enable_vblank, + .disable_vblank = via_disable_vblank, + .dma_quiescent = via_driver_dma_quiescent, }; static struct drm_driver driver = { @@ -80,17 +86,11 @@ static struct drm_driver driver = { .load = via_driver_load, .unload = via_driver_unload, .open = via_driver_open, - .preclose = via_reclaim_buffers_locked, .postclose = via_driver_postclose, - .context_dtor = via_final_context, - .get_vblank_counter = via_get_vblank_counter, - .enable_vblank = via_enable_vblank, - .disable_vblank = via_disable_vblank, .irq_preinstall = via_driver_irq_preinstall, .irq_postinstall = via_driver_irq_postinstall, .irq_uninstall = via_driver_irq_uninstall, .irq_handler = via_driver_irq_handler, - .dma_quiescent = via_driver_dma_quiescent, .lastclose = via_lastclose, .ioctls = via_ioctls, .fops = &via_driver_fops, diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 94078af0cc28..15dd9a179216 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -601,20 +601,6 @@ struct drm_driver { /* Everything below here is for legacy driver, never use! */ struct drm_legacy_state *legacy; - - /* private: */ - - /* List of devices hanging off this driver with stealth attach. */ - struct list_head legacy_dev_list; - int (*firstopen) (struct drm_device *); - void (*preclose) (struct drm_device *, struct drm_file *file_priv); - int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file *file_priv); - int (*dma_quiescent) (struct drm_device *); - int (*context_dtor) (struct drm_device *dev, int context); - u32 (*get_vblank_counter)(struct drm_device *dev, unsigned int pipe); - int (*enable_vblank)(struct drm_device *dev, unsigned int pipe); - void (*disable_vblank)(struct drm_device *dev, unsigned int pipe); - int dev_priv_size; }; int drm_dev_init(struct drm_device *dev, diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index c98a29e1c2b3..7b5ee6a94a8c 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -53,6 +53,17 @@ struct pci_driver; * State for non-KMS drivers. */ struct drm_legacy_state { + /* List of devices hanging off this driver with stealth attach. */ + struct list_head legacy_dev_list; + int (*firstopen)(struct drm_device *dev); + void (*preclose)(struct drm_device *dev, struct drm_file *file_priv); + int (*dma_ioctl)(struct drm_device *dev, void *data, struct drm_file *file_priv); + int (*dma_quiescent)(struct drm_device *dev); + int (*context_dtor)(struct drm_device *dev, int context); + u32 (*get_vblank_counter)(struct drm_device *dev, unsigned int pipe); + int (*enable_vblank)(struct drm_device *dev, unsigned int pipe); + void (*disable_vblank)(struct drm_device *dev, unsigned int pipe); + int dev_priv_size; }; /** -- 2.25.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/3] drm: Move non-kms driver state into struct drm_legacy_state @ 2020-02-25 15:59 ` Thomas Zimmermann 0 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 15:59 UTC (permalink / raw) To: maarten.lankhorst, mripard, airlied, daniel, bskeggs, emil.velikov, alexander.deucher, sam Cc: nouveau, Thomas Zimmermann, dri-devel All non-kms driver fields are now located in struct drm_legacy_state. A driver-wide instance is available via struct drm_driver.legacy. The call sites test if the driver is marked with DRIVER_LEGACY before accessing the fields. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_bufs.c | 10 +++++----- drivers/gpu/drm/drm_context.c | 9 +++++---- drivers/gpu/drm/drm_drv.c | 8 ++++---- drivers/gpu/drm/drm_file.c | 4 ++-- drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- drivers/gpu/drm/drm_lock.c | 7 ++++--- drivers/gpu/drm/drm_pci.c | 8 +++++--- drivers/gpu/drm/drm_vblank.c | 11 ++++++----- drivers/gpu/drm/i810/i810_drv.c | 8 ++++---- drivers/gpu/drm/mga/mga_drv.c | 14 +++++++------- drivers/gpu/drm/r128/r128_drv.c | 14 +++++++------- drivers/gpu/drm/savage/savage_drv.c | 10 +++++----- drivers/gpu/drm/sis/sis_drv.c | 6 +++--- drivers/gpu/drm/tdfx/tdfx_drv.c | 2 +- drivers/gpu/drm/via/via_drv.c | 14 +++++++------- include/drm/drm_drv.h | 14 -------------- include/drm/drm_legacy.h | 11 +++++++++++ 17 files changed, 79 insertions(+), 77 deletions(-) diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index 19297e58b232..fbbd5f5820a4 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -812,7 +812,7 @@ int drm_legacy_addbufs_agp(struct drm_device *dev, buf->pending = 0; buf->file_priv = NULL; - buf->dev_priv_size = dev->driver->dev_priv_size; + buf->dev_priv_size = dev->driver->legacy->dev_priv_size; buf->dev_private = kzalloc(buf->dev_priv_size, GFP_KERNEL); if (!buf->dev_private) { /* Set count correctly so we free the proper amount. */ @@ -1011,7 +1011,7 @@ int drm_legacy_addbufs_pci(struct drm_device *dev, buf->pending = 0; buf->file_priv = NULL; - buf->dev_priv_size = dev->driver->dev_priv_size; + buf->dev_priv_size = dev->driver->legacy->dev_priv_size; buf->dev_private = kzalloc(buf->dev_priv_size, GFP_KERNEL); if (!buf->dev_private) { @@ -1175,7 +1175,7 @@ static int drm_legacy_addbufs_sg(struct drm_device *dev, buf->pending = 0; buf->file_priv = NULL; - buf->dev_priv_size = dev->driver->dev_priv_size; + buf->dev_priv_size = dev->driver->legacy->dev_priv_size; buf->dev_private = kzalloc(buf->dev_priv_size, GFP_KERNEL); if (!buf->dev_private) { /* Set count correctly so we free the proper amount. */ @@ -1581,8 +1581,8 @@ int drm_legacy_dma_ioctl(struct drm_device *dev, void *data, if (!drm_core_check_feature(dev, DRIVER_LEGACY)) return -EOPNOTSUPP; - if (dev->driver->dma_ioctl) - return dev->driver->dma_ioctl(dev, data, file_priv); + if (dev->driver->legacy->dma_ioctl) + return dev->driver->legacy->dma_ioctl(dev, data, file_priv); else return -EINVAL; } diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c index 1f802d8e5681..b433f755d7c2 100644 --- a/drivers/gpu/drm/drm_context.c +++ b/drivers/gpu/drm/drm_context.c @@ -145,8 +145,9 @@ void drm_legacy_ctxbitmap_flush(struct drm_device *dev, struct drm_file *file) list_for_each_entry_safe(pos, tmp, &dev->ctxlist, head) { if (pos->tag == file && pos->handle != DRM_KERNEL_CONTEXT) { - if (dev->driver->context_dtor) - dev->driver->context_dtor(dev, pos->handle); + if (dev->driver->legacy->context_dtor) + dev->driver->legacy->context_dtor(dev, + pos->handle); drm_legacy_ctxbitmap_free(dev, pos->handle); list_del(&pos->head); @@ -500,8 +501,8 @@ int drm_legacy_rmctx(struct drm_device *dev, void *data, DRM_DEBUG("%d\n", ctx->handle); if (ctx->handle != DRM_KERNEL_CONTEXT) { - if (dev->driver->context_dtor) - dev->driver->context_dtor(dev, ctx->handle); + if (dev->driver->legacy->context_dtor) + dev->driver->legacy->context_dtor(dev, ctx->handle); drm_legacy_ctxbitmap_free(dev, ctx->handle); } diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 4ba0df097602..4875b76e7656 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -625,14 +625,14 @@ int drm_dev_init(struct drm_device *dev, if (WARN_ON(!parent)) return -EINVAL; - if (drm_core_check_feature(dev, DRIVER_LEGACY) && - WARN_ON(!driver->legacy)) - return -EINVAL; - kref_init(&dev->ref); dev->dev = get_device(parent); dev->driver = driver; + if (drm_core_check_feature(dev, DRIVER_LEGACY) && + WARN_ON(!driver->legacy)) + return -EINVAL; + /* no per-device feature limits by default */ dev->driver_features = ~0u; diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index c4c704e01961..e9cb7cf2eb48 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -254,8 +254,8 @@ void drm_file_free(struct drm_file *file) atomic_read(&dev->open_count)); if (drm_core_check_feature(dev, DRIVER_LEGACY) && - dev->driver->preclose) - dev->driver->preclose(dev, file); + dev->driver->legacy->preclose) + dev->driver->legacy->preclose(dev, file); if (drm_core_check_feature(dev, DRIVER_LEGACY)) drm_legacy_lock_release(dev, file->filp); diff --git a/drivers/gpu/drm/drm_legacy_misc.c b/drivers/gpu/drm/drm_legacy_misc.c index 8f54e6a78b6f..07172d0a4f6f 100644 --- a/drivers/gpu/drm/drm_legacy_misc.c +++ b/drivers/gpu/drm/drm_legacy_misc.c @@ -60,9 +60,9 @@ int drm_legacy_setup(struct drm_device * dev) { int ret; - if (dev->driver->firstopen && - drm_core_check_feature(dev, DRIVER_LEGACY)) { - ret = dev->driver->firstopen(dev); + if (drm_core_check_feature(dev, DRIVER_LEGACY) && + dev->driver->legacy->firstopen) { + ret = dev->driver->legacy->firstopen(dev); if (ret != 0) return ret; } diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c index 2c79e8199e3c..f07bf74065ec 100644 --- a/drivers/gpu/drm/drm_lock.c +++ b/drivers/gpu/drm/drm_lock.c @@ -223,7 +223,7 @@ int drm_legacy_lock(struct drm_device *dev, void *data, ret ? "interrupted" : "has lock"); if (ret) return ret; - /* don't set the block all signals on the master process for now + /* don't set the block all signals on the master process for now * really probably not the correct answer but lets us debug xkb * xserver for now */ if (!drm_is_current_master(file_priv)) { @@ -231,9 +231,10 @@ int drm_legacy_lock(struct drm_device *dev, void *data, dev->sigdata.lock = master->lock.hw_lock; } - if (dev->driver->dma_quiescent && (lock->flags & _DRM_LOCK_QUIESCENT)) + if (dev->driver->legacy->dma_quiescent && + (lock->flags & _DRM_LOCK_QUIESCENT)) { - if (dev->driver->dma_quiescent(dev)) { + if (dev->driver->legacy->dma_quiescent(dev)) { DRM_DEBUG("%d waiting for DMA quiescent\n", lock->context); return -EBUSY; diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index c6bb98729a26..24ed0e92fd66 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -239,7 +239,8 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, /* No locking needed since shadow-attach is single-threaded since it may * only be called from the per-driver module init hook. */ if (drm_core_check_feature(dev, DRIVER_LEGACY)) - list_add_tail(&dev->legacy_dev_list, &driver->legacy_dev_list); + list_add_tail(&dev->legacy_dev_list, + &driver->legacy->legacy_dev_list); return 0; @@ -275,7 +276,7 @@ int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) return -EINVAL; /* If not using KMS, fall back to stealth mode manual scanning. */ - INIT_LIST_HEAD(&driver->legacy_dev_list); + INIT_LIST_HEAD(&driver->legacy->legacy_dev_list); for (i = 0; pdriver->id_table[i].vendor != 0; i++) { pid = &pdriver->id_table[i]; @@ -317,7 +318,8 @@ void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) if (!(driver->driver_features & DRIVER_LEGACY)) { WARN_ON(1); } else { - list_for_each_entry_safe(dev, tmp, &driver->legacy_dev_list, + list_for_each_entry_safe(dev, tmp, + &driver->legacy->legacy_dev_list, legacy_dev_list) { list_del(&dev->legacy_dev_list); drm_put_dev(dev); diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 47fc4339ec7f..8655cf04712b 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -30,6 +30,7 @@ #include <drm/drm_crtc.h> #include <drm/drm_drv.h> #include <drm/drm_framebuffer.h> +#include <drm/drm_legacy.h> #include <drm/drm_modeset_helper_vtables.h> #include <drm/drm_print.h> #include <drm/drm_vblank.h> @@ -144,8 +145,8 @@ static u32 __get_vblank_counter(struct drm_device *dev, unsigned int pipe) if (crtc->funcs->get_vblank_counter) return crtc->funcs->get_vblank_counter(crtc); - } else if (dev->driver->get_vblank_counter) { - return dev->driver->get_vblank_counter(dev, pipe); + } else if (dev->driver->legacy->get_vblank_counter) { + return dev->driver->legacy->get_vblank_counter(dev, pipe); } return drm_vblank_no_hw_counter(dev, pipe); @@ -364,7 +365,7 @@ static void __disable_vblank(struct drm_device *dev, unsigned int pipe) if (crtc->funcs->disable_vblank) crtc->funcs->disable_vblank(crtc); } else { - dev->driver->disable_vblank(dev, pipe); + dev->driver->legacy->disable_vblank(dev, pipe); } } @@ -1033,8 +1034,8 @@ static int __enable_vblank(struct drm_device *dev, unsigned int pipe) if (crtc->funcs->enable_vblank) return crtc->funcs->enable_vblank(crtc); - } else if (dev->driver->enable_vblank) { - return dev->driver->enable_vblank(dev, pipe); + } else if (dev->driver->legacy->enable_vblank) { + return dev->driver->legacy->enable_vblank(dev, pipe); } return -EINVAL; diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c index 55f17f00bae9..dd036e3ceb3d 100644 --- a/drivers/gpu/drm/i810/i810_drv.c +++ b/drivers/gpu/drm/i810/i810_drv.c @@ -56,16 +56,16 @@ static const struct file_operations i810_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = i810_legacy_state { +static struct drm_legacy_state i810_legacy_state = { + .dev_priv_size = sizeof(drm_i810_buf_priv_t), + .preclose = i810_driver_preclose, + .dma_quiescent = i810_driver_dma_quiescent, }; static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY, - .dev_priv_size = sizeof(drm_i810_buf_priv_t), .load = i810_driver_load, .lastclose = i810_driver_lastclose, - .preclose = i810_driver_preclose, - .dma_quiescent = i810_driver_dma_quiescent, .ioctls = i810_ioctls, .fops = &i810_driver_fops, .name = DRIVER_NAME, diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c index 4865982d949c..2454c57f9370 100644 --- a/drivers/gpu/drm/mga/mga_drv.c +++ b/drivers/gpu/drm/mga/mga_drv.c @@ -53,27 +53,27 @@ static const struct file_operations mga_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = mga_legacy_state { +static struct drm_legacy_state mga_legacy_state = { + .dev_priv_size = sizeof(drm_mga_buf_priv_t), + .dma_quiescent = mga_driver_dma_quiescent, + .get_vblank_counter = mga_get_vblank_counter, + .enable_vblank = mga_enable_vblank, + .disable_vblank = mga_disable_vblank, + .dma_ioctl = mga_dma_buffers, }; static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY | DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ, - .dev_priv_size = sizeof(drm_mga_buf_priv_t), .load = mga_driver_load, .unload = mga_driver_unload, .lastclose = mga_driver_lastclose, - .dma_quiescent = mga_driver_dma_quiescent, - .get_vblank_counter = mga_get_vblank_counter, - .enable_vblank = mga_enable_vblank, - .disable_vblank = mga_disable_vblank, .irq_preinstall = mga_driver_irq_preinstall, .irq_postinstall = mga_driver_irq_postinstall, .irq_uninstall = mga_driver_irq_uninstall, .irq_handler = mga_driver_irq_handler, .ioctls = mga_ioctls, - .dma_ioctl = mga_dma_buffers, .fops = &mga_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c index ecece3a48d93..9cfd86117e80 100644 --- a/drivers/gpu/drm/r128/r128_drv.c +++ b/drivers/gpu/drm/r128/r128_drv.c @@ -57,26 +57,26 @@ static const struct file_operations r128_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = r128_legacy_state { +static struct drm_legacy_state r128_legacy_state = { + .dev_priv_size = sizeof(drm_r128_buf_priv_t), + .preclose = r128_driver_preclose, + .get_vblank_counter = r128_get_vblank_counter, + .enable_vblank = r128_enable_vblank, + .disable_vblank = r128_disable_vblank, + .dma_ioctl = r128_cce_buffers, }; static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY | DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ, - .dev_priv_size = sizeof(drm_r128_buf_priv_t), .load = r128_driver_load, - .preclose = r128_driver_preclose, .lastclose = r128_driver_lastclose, - .get_vblank_counter = r128_get_vblank_counter, - .enable_vblank = r128_enable_vblank, - .disable_vblank = r128_disable_vblank, .irq_preinstall = r128_driver_irq_preinstall, .irq_postinstall = r128_driver_irq_postinstall, .irq_uninstall = r128_driver_irq_uninstall, .irq_handler = r128_driver_irq_handler, .ioctls = r128_ioctls, - .dma_ioctl = r128_cce_buffers, .fops = &r128_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c index c0a7146fbde1..2d2680b47eff 100644 --- a/drivers/gpu/drm/savage/savage_drv.c +++ b/drivers/gpu/drm/savage/savage_drv.c @@ -47,20 +47,20 @@ static const struct file_operations savage_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = savage_legacy_state { +static struct drm_legacy_state savage_legacy_state = { + .dev_priv_size = sizeof(drm_savage_buf_priv_t), + .firstopen = savage_driver_firstopen, + .preclose = savage_reclaim_buffers, + .dma_ioctl = savage_bci_buffers, }; static struct drm_driver driver = { .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_PCI_DMA | DRIVER_LEGACY, - .dev_priv_size = sizeof(drm_savage_buf_priv_t), .load = savage_driver_load, - .firstopen = savage_driver_firstopen, - .preclose = savage_reclaim_buffers, .lastclose = savage_driver_lastclose, .unload = savage_driver_unload, .ioctls = savage_ioctls, - .dma_ioctl = savage_bci_buffers, .fops = &savage_driver_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c index 1c794a82c8e6..c465776e3180 100644 --- a/drivers/gpu/drm/sis/sis_drv.c +++ b/drivers/gpu/drm/sis/sis_drv.c @@ -99,7 +99,9 @@ static void sis_driver_postclose(struct drm_device *dev, struct drm_file *file) kfree(file_priv); } -static struct drm_legacy_state = sis_legacy_state { +static struct drm_legacy_state sis_legacy_state = { + .preclose = sis_reclaim_buffers_locked, + .dma_quiescent = sis_idle, }; static struct drm_driver driver = { @@ -107,9 +109,7 @@ static struct drm_driver driver = { .load = sis_driver_load, .unload = sis_driver_unload, .open = sis_driver_open, - .preclose = sis_reclaim_buffers_locked, .postclose = sis_driver_postclose, - .dma_quiescent = sis_idle, .lastclose = sis_lastclose, .ioctls = sis_ioctls, .fops = &sis_driver_fops, diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index 17aa93d82d6b..a8a6bce6afbe 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -56,7 +56,7 @@ static const struct file_operations tdfx_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = tdfx_legacy_state { +static struct drm_legacy_state tdfx_legacy_state = { }; static struct drm_driver driver = { diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c index 22113d93ecdd..62f026da5958 100644 --- a/drivers/gpu/drm/via/via_drv.c +++ b/drivers/gpu/drm/via/via_drv.c @@ -71,7 +71,13 @@ static const struct file_operations via_driver_fops = { .llseek = noop_llseek, }; -static struct drm_legacy_state = via_legacy_state { +static struct drm_legacy_state via_legacy_state = { + .preclose = via_reclaim_buffers_locked, + .context_dtor = via_final_context, + .get_vblank_counter = via_get_vblank_counter, + .enable_vblank = via_enable_vblank, + .disable_vblank = via_disable_vblank, + .dma_quiescent = via_driver_dma_quiescent, }; static struct drm_driver driver = { @@ -80,17 +86,11 @@ static struct drm_driver driver = { .load = via_driver_load, .unload = via_driver_unload, .open = via_driver_open, - .preclose = via_reclaim_buffers_locked, .postclose = via_driver_postclose, - .context_dtor = via_final_context, - .get_vblank_counter = via_get_vblank_counter, - .enable_vblank = via_enable_vblank, - .disable_vblank = via_disable_vblank, .irq_preinstall = via_driver_irq_preinstall, .irq_postinstall = via_driver_irq_postinstall, .irq_uninstall = via_driver_irq_uninstall, .irq_handler = via_driver_irq_handler, - .dma_quiescent = via_driver_dma_quiescent, .lastclose = via_lastclose, .ioctls = via_ioctls, .fops = &via_driver_fops, diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 94078af0cc28..15dd9a179216 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -601,20 +601,6 @@ struct drm_driver { /* Everything below here is for legacy driver, never use! */ struct drm_legacy_state *legacy; - - /* private: */ - - /* List of devices hanging off this driver with stealth attach. */ - struct list_head legacy_dev_list; - int (*firstopen) (struct drm_device *); - void (*preclose) (struct drm_device *, struct drm_file *file_priv); - int (*dma_ioctl) (struct drm_device *dev, void *data, struct drm_file *file_priv); - int (*dma_quiescent) (struct drm_device *); - int (*context_dtor) (struct drm_device *dev, int context); - u32 (*get_vblank_counter)(struct drm_device *dev, unsigned int pipe); - int (*enable_vblank)(struct drm_device *dev, unsigned int pipe); - void (*disable_vblank)(struct drm_device *dev, unsigned int pipe); - int dev_priv_size; }; int drm_dev_init(struct drm_device *dev, diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index c98a29e1c2b3..7b5ee6a94a8c 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -53,6 +53,17 @@ struct pci_driver; * State for non-KMS drivers. */ struct drm_legacy_state { + /* List of devices hanging off this driver with stealth attach. */ + struct list_head legacy_dev_list; + int (*firstopen)(struct drm_device *dev); + void (*preclose)(struct drm_device *dev, struct drm_file *file_priv); + int (*dma_ioctl)(struct drm_device *dev, void *data, struct drm_file *file_priv); + int (*dma_quiescent)(struct drm_device *dev); + int (*context_dtor)(struct drm_device *dev, int context); + u32 (*get_vblank_counter)(struct drm_device *dev, unsigned int pipe); + int (*enable_vblank)(struct drm_device *dev, unsigned int pipe); + void (*disable_vblank)(struct drm_device *dev, unsigned int pipe); + int dev_priv_size; }; /** -- 2.25.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/3] drm: Constify struct drm_driver in DRM core 2020-02-25 15:58 ` Thomas Zimmermann @ 2020-02-25 15:59 ` Thomas Zimmermann -1 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 15:59 UTC (permalink / raw) To: maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA, mripard-DgEjT+Ai2ygdnm+yROfE0A, airlied-cv59FeDIM0c, daniel-/w4YWyX8dFk, bskeggs-H+wXaHxf7aLQT0dZR+AlfA, emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ, alexander.deucher-5C7GfCeVMHo, sam-uyr5N9Q2VtJg9hUCZPvPmw Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Once used for creatign a DRM device, instances of struct drm_driver are not modified. So all related variables in the DRM core can be declared 'const'. In addition to the core interfaces, the patch also changes tdfx accordingly. The other non-KMS drivers set.num_ioctls outside of their instance's initialization, so they cannot have the instance declared const yet. Signed-off-by: Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@public.gmane.org> --- drivers/gpu/drm/drm_drv.c | 8 ++++---- drivers/gpu/drm/drm_pci.c | 8 +++++--- drivers/gpu/drm/tdfx/tdfx_drv.c | 2 +- include/drm/drm_device.h | 2 +- include/drm/drm_drv.h | 6 +++--- include/drm/drm_legacy.h | 10 ++++++---- include/drm/drm_pci.h | 4 ++-- 7 files changed, 22 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 4875b76e7656..98d1f4169e6c 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -612,7 +612,7 @@ static void drm_fs_inode_free(struct inode *inode) * 0 on success, or error code on failure. */ int drm_dev_init(struct drm_device *dev, - struct drm_driver *driver, + const struct drm_driver *driver, struct device *parent) { int ret; @@ -726,7 +726,7 @@ static void devm_drm_dev_init_release(void *data) */ int devm_drm_dev_init(struct device *parent, struct drm_device *dev, - struct drm_driver *driver) + const struct drm_driver *driver) { int ret; @@ -804,7 +804,7 @@ EXPORT_SYMBOL(drm_dev_fini); * RETURNS: * Pointer to new DRM device, or ERR_PTR on failure. */ -struct drm_device *drm_dev_alloc(struct drm_driver *driver, +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, struct device *parent) { struct drm_device *dev; @@ -947,7 +947,7 @@ static void remove_compat_control_link(struct drm_device *dev) */ int drm_dev_register(struct drm_device *dev, unsigned long flags) { - struct drm_driver *driver = dev->driver; + const struct drm_driver *driver = dev->driver; int ret; if (drm_dev_needs_global_mutex(dev)) diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 24ed0e92fd66..f6b7b30f7c5e 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -207,7 +207,7 @@ void drm_pci_agp_destroy(struct drm_device *dev) * Return: 0 on success or a negative error code on failure. */ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, - struct drm_driver *driver) + const struct drm_driver *driver) { struct drm_device *dev; int ret; @@ -264,7 +264,8 @@ EXPORT_SYMBOL(drm_get_pci_dev); * * Return: 0 on success or a negative error code on failure. */ -int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) +int drm_legacy_pci_init(const struct drm_driver *driver, + struct pci_driver *pdriver) { struct pci_dev *pdev = NULL; const struct pci_device_id *pid; @@ -310,7 +311,8 @@ EXPORT_SYMBOL(drm_legacy_pci_init); * Unregister a DRM driver shadow-attached through drm_legacy_pci_init(). This * is deprecated and only used by dri1 drivers. */ -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) +void drm_legacy_pci_exit(const struct drm_driver *driver, + struct pci_driver *pdriver) { struct drm_device *dev, *tmp; DRM_DEBUG("\n"); diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index a8a6bce6afbe..637c9352c1af 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -59,7 +59,7 @@ static const struct file_operations tdfx_driver_fops = { static struct drm_legacy_state tdfx_legacy_state = { }; -static struct drm_driver driver = { +static const struct drm_driver driver = { .driver_features = DRIVER_LEGACY, .fops = &tdfx_driver_fops, .name = DRIVER_NAME, diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index bb60a949f416..6b2e2799d83c 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -68,7 +68,7 @@ struct drm_device { struct device *dev; /** @driver: DRM driver managing the device */ - struct drm_driver *driver; + const struct drm_driver *driver; /** * @dev_private: diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 15dd9a179216..887728d28226 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -604,14 +604,14 @@ struct drm_driver { }; int drm_dev_init(struct drm_device *dev, - struct drm_driver *driver, + const struct drm_driver *driver, struct device *parent); int devm_drm_dev_init(struct device *parent, struct drm_device *dev, - struct drm_driver *driver); + const struct drm_driver *driver); void drm_dev_fini(struct drm_device *dev); -struct drm_device *drm_dev_alloc(struct drm_driver *driver, +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, struct device *parent); int drm_dev_register(struct drm_device *dev, unsigned long flags); void drm_dev_unregister(struct drm_device *dev); diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 7b5ee6a94a8c..abc77cf84752 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -211,18 +211,20 @@ void drm_legacy_idlelock_release(struct drm_lock_data *lock); #ifdef CONFIG_PCI -int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver); -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver); +int drm_legacy_pci_init(const struct drm_driver *driver, + struct pci_driver *pdriver); +void drm_legacy_pci_exit(const struct drm_driver *driver, + struct pci_driver *pdriver); #else -static inline int drm_legacy_pci_init(struct drm_driver *driver, +static inline int drm_legacy_pci_init(const struct drm_driver *driver, struct pci_driver *pdriver) { return -EINVAL; } -static inline void drm_legacy_pci_exit(struct drm_driver *driver, +static inline void drm_legacy_pci_exit(const struct drm_driver *driver, struct pci_driver *pdriver) { } diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h index 9031e217b506..8f98ae8384c2 100644 --- a/include/drm/drm_pci.h +++ b/include/drm/drm_pci.h @@ -47,7 +47,7 @@ void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah); int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, - struct drm_driver *driver); + const struct drm_driver *driver); #else @@ -64,7 +64,7 @@ static inline void drm_pci_free(struct drm_device *dev, static inline int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, - struct drm_driver *driver) + const struct drm_driver *driver) { return -ENOSYS; } -- 2.25.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/3] drm: Constify struct drm_driver in DRM core @ 2020-02-25 15:59 ` Thomas Zimmermann 0 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-25 15:59 UTC (permalink / raw) To: maarten.lankhorst, mripard, airlied, daniel, bskeggs, emil.velikov, alexander.deucher, sam Cc: nouveau, Thomas Zimmermann, dri-devel Once used for creatign a DRM device, instances of struct drm_driver are not modified. So all related variables in the DRM core can be declared 'const'. In addition to the core interfaces, the patch also changes tdfx accordingly. The other non-KMS drivers set.num_ioctls outside of their instance's initialization, so they cannot have the instance declared const yet. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_drv.c | 8 ++++---- drivers/gpu/drm/drm_pci.c | 8 +++++--- drivers/gpu/drm/tdfx/tdfx_drv.c | 2 +- include/drm/drm_device.h | 2 +- include/drm/drm_drv.h | 6 +++--- include/drm/drm_legacy.h | 10 ++++++---- include/drm/drm_pci.h | 4 ++-- 7 files changed, 22 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 4875b76e7656..98d1f4169e6c 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -612,7 +612,7 @@ static void drm_fs_inode_free(struct inode *inode) * 0 on success, or error code on failure. */ int drm_dev_init(struct drm_device *dev, - struct drm_driver *driver, + const struct drm_driver *driver, struct device *parent) { int ret; @@ -726,7 +726,7 @@ static void devm_drm_dev_init_release(void *data) */ int devm_drm_dev_init(struct device *parent, struct drm_device *dev, - struct drm_driver *driver) + const struct drm_driver *driver) { int ret; @@ -804,7 +804,7 @@ EXPORT_SYMBOL(drm_dev_fini); * RETURNS: * Pointer to new DRM device, or ERR_PTR on failure. */ -struct drm_device *drm_dev_alloc(struct drm_driver *driver, +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, struct device *parent) { struct drm_device *dev; @@ -947,7 +947,7 @@ static void remove_compat_control_link(struct drm_device *dev) */ int drm_dev_register(struct drm_device *dev, unsigned long flags) { - struct drm_driver *driver = dev->driver; + const struct drm_driver *driver = dev->driver; int ret; if (drm_dev_needs_global_mutex(dev)) diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 24ed0e92fd66..f6b7b30f7c5e 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -207,7 +207,7 @@ void drm_pci_agp_destroy(struct drm_device *dev) * Return: 0 on success or a negative error code on failure. */ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, - struct drm_driver *driver) + const struct drm_driver *driver) { struct drm_device *dev; int ret; @@ -264,7 +264,8 @@ EXPORT_SYMBOL(drm_get_pci_dev); * * Return: 0 on success or a negative error code on failure. */ -int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) +int drm_legacy_pci_init(const struct drm_driver *driver, + struct pci_driver *pdriver) { struct pci_dev *pdev = NULL; const struct pci_device_id *pid; @@ -310,7 +311,8 @@ EXPORT_SYMBOL(drm_legacy_pci_init); * Unregister a DRM driver shadow-attached through drm_legacy_pci_init(). This * is deprecated and only used by dri1 drivers. */ -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) +void drm_legacy_pci_exit(const struct drm_driver *driver, + struct pci_driver *pdriver) { struct drm_device *dev, *tmp; DRM_DEBUG("\n"); diff --git a/drivers/gpu/drm/tdfx/tdfx_drv.c b/drivers/gpu/drm/tdfx/tdfx_drv.c index a8a6bce6afbe..637c9352c1af 100644 --- a/drivers/gpu/drm/tdfx/tdfx_drv.c +++ b/drivers/gpu/drm/tdfx/tdfx_drv.c @@ -59,7 +59,7 @@ static const struct file_operations tdfx_driver_fops = { static struct drm_legacy_state tdfx_legacy_state = { }; -static struct drm_driver driver = { +static const struct drm_driver driver = { .driver_features = DRIVER_LEGACY, .fops = &tdfx_driver_fops, .name = DRIVER_NAME, diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index bb60a949f416..6b2e2799d83c 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -68,7 +68,7 @@ struct drm_device { struct device *dev; /** @driver: DRM driver managing the device */ - struct drm_driver *driver; + const struct drm_driver *driver; /** * @dev_private: diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 15dd9a179216..887728d28226 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -604,14 +604,14 @@ struct drm_driver { }; int drm_dev_init(struct drm_device *dev, - struct drm_driver *driver, + const struct drm_driver *driver, struct device *parent); int devm_drm_dev_init(struct device *parent, struct drm_device *dev, - struct drm_driver *driver); + const struct drm_driver *driver); void drm_dev_fini(struct drm_device *dev); -struct drm_device *drm_dev_alloc(struct drm_driver *driver, +struct drm_device *drm_dev_alloc(const struct drm_driver *driver, struct device *parent); int drm_dev_register(struct drm_device *dev, unsigned long flags); void drm_dev_unregister(struct drm_device *dev); diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 7b5ee6a94a8c..abc77cf84752 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -211,18 +211,20 @@ void drm_legacy_idlelock_release(struct drm_lock_data *lock); #ifdef CONFIG_PCI -int drm_legacy_pci_init(struct drm_driver *driver, struct pci_driver *pdriver); -void drm_legacy_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver); +int drm_legacy_pci_init(const struct drm_driver *driver, + struct pci_driver *pdriver); +void drm_legacy_pci_exit(const struct drm_driver *driver, + struct pci_driver *pdriver); #else -static inline int drm_legacy_pci_init(struct drm_driver *driver, +static inline int drm_legacy_pci_init(const struct drm_driver *driver, struct pci_driver *pdriver) { return -EINVAL; } -static inline void drm_legacy_pci_exit(struct drm_driver *driver, +static inline void drm_legacy_pci_exit(const struct drm_driver *driver, struct pci_driver *pdriver) { } diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h index 9031e217b506..8f98ae8384c2 100644 --- a/include/drm/drm_pci.h +++ b/include/drm/drm_pci.h @@ -47,7 +47,7 @@ void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah); int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, - struct drm_driver *driver); + const struct drm_driver *driver); #else @@ -64,7 +64,7 @@ static inline void drm_pci_free(struct drm_device *dev, static inline int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent, - struct drm_driver *driver) + const struct drm_driver *driver) { return -ENOSYS; } -- 2.25.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver 2020-02-25 15:58 ` Thomas Zimmermann @ 2020-02-25 17:44 ` Daniel Vetter -1 siblings, 0 replies; 22+ messages in thread From: Daniel Vetter @ 2020-02-25 17:44 UTC (permalink / raw) To: Thomas Zimmermann Cc: airlied-cv59FeDIM0c, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA, mripard-DgEjT+Ai2ygdnm+yROfE0A, bskeggs-H+wXaHxf7aLQT0dZR+AlfA, daniel-/w4YWyX8dFk, alexander.deucher-5C7GfCeVMHo, sam-uyr5N9Q2VtJg9hUCZPvPmw, emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ On Tue, Feb 25, 2020 at 04:58:59PM +0100, Thomas Zimmermann wrote: > This patchset moves legacy, non-KMS driver state from struct drm_driver > into struct drm_legacy_state. Only non-KMS drivers provide an instance > of the latter structure. One special case is nouveau, which supports > legacy interfaces. It also provides an instance of the legacy state if > the legacy interfaces have been enabled (i.e., defines the config option > CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) > > I reviewed all call sites of legacy state and functions to verify that > DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that > DRIVER_MODESET is not set. > > With the mutable KMS state removed, instances of struct drm_driver can > be declared as constant. The patchset modifies the DRM core accordingly. > Individual drivers can follow later on. Bikeshed: We very much have modern non-KMS drivers (vgem, etnaviv, v3d, panfrost, ...). non-KMS != legacy, which is what you're talking about here. Other thing, and it's a bit raining on your parade: I don't see the point. Sprinkling a few more #ifdef CONFIG_DRM_LEGACY over the relevant parts sounds like a reasonable idea. But this is a lot of churn for drivers which are all pretty much dead, and just waiting for their eventual removal. And from a compile-testing pov of making sure modern drivers don't use any of the deprecated stuff wrapping it in CONFIG_DRM_LEGACY should be plenty enough. And from a "make stuff const" I think Laurent's much more minimal series also gets us there for all the drivers we care about. -Daniel > > Thomas Zimmermann (3): > drm: Add separate state structure for legacy, non-KMS drivers > drm: Move non-kms driver state into struct drm_legacy_state > drm: Constify struct drm_driver in DRM core > > drivers/gpu/drm/drm_bufs.c | 10 +++++----- > drivers/gpu/drm/drm_context.c | 9 +++++---- > drivers/gpu/drm/drm_drv.c | 12 ++++++++---- > drivers/gpu/drm/drm_file.c | 4 ++-- > drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- > drivers/gpu/drm/drm_lock.c | 7 ++++--- > drivers/gpu/drm/drm_pci.c | 16 ++++++++++------ > drivers/gpu/drm/drm_vblank.c | 11 ++++++----- > drivers/gpu/drm/i810/i810_drv.c | 10 +++++++--- > drivers/gpu/drm/mga/mga_drv.c | 16 ++++++++++------ > drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ > drivers/gpu/drm/r128/r128_drv.c | 16 ++++++++++------ > drivers/gpu/drm/savage/savage_drv.c | 12 ++++++++---- > drivers/gpu/drm/sis/sis_drv.c | 8 ++++++-- > drivers/gpu/drm/tdfx/tdfx_drv.c | 6 +++++- > drivers/gpu/drm/via/via_drv.c | 16 ++++++++++------ > include/drm/drm_device.h | 2 +- > include/drm/drm_drv.h | 21 +++++---------------- > include/drm/drm_legacy.h | 27 +++++++++++++++++++++++---- > include/drm/drm_pci.h | 4 ++-- > 20 files changed, 138 insertions(+), 83 deletions(-) > > -- > 2.25.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver @ 2020-02-25 17:44 ` Daniel Vetter 0 siblings, 0 replies; 22+ messages in thread From: Daniel Vetter @ 2020-02-25 17:44 UTC (permalink / raw) To: Thomas Zimmermann Cc: airlied, nouveau, dri-devel, bskeggs, alexander.deucher, sam, emil.velikov On Tue, Feb 25, 2020 at 04:58:59PM +0100, Thomas Zimmermann wrote: > This patchset moves legacy, non-KMS driver state from struct drm_driver > into struct drm_legacy_state. Only non-KMS drivers provide an instance > of the latter structure. One special case is nouveau, which supports > legacy interfaces. It also provides an instance of the legacy state if > the legacy interfaces have been enabled (i.e., defines the config option > CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) > > I reviewed all call sites of legacy state and functions to verify that > DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that > DRIVER_MODESET is not set. > > With the mutable KMS state removed, instances of struct drm_driver can > be declared as constant. The patchset modifies the DRM core accordingly. > Individual drivers can follow later on. Bikeshed: We very much have modern non-KMS drivers (vgem, etnaviv, v3d, panfrost, ...). non-KMS != legacy, which is what you're talking about here. Other thing, and it's a bit raining on your parade: I don't see the point. Sprinkling a few more #ifdef CONFIG_DRM_LEGACY over the relevant parts sounds like a reasonable idea. But this is a lot of churn for drivers which are all pretty much dead, and just waiting for their eventual removal. And from a compile-testing pov of making sure modern drivers don't use any of the deprecated stuff wrapping it in CONFIG_DRM_LEGACY should be plenty enough. And from a "make stuff const" I think Laurent's much more minimal series also gets us there for all the drivers we care about. -Daniel > > Thomas Zimmermann (3): > drm: Add separate state structure for legacy, non-KMS drivers > drm: Move non-kms driver state into struct drm_legacy_state > drm: Constify struct drm_driver in DRM core > > drivers/gpu/drm/drm_bufs.c | 10 +++++----- > drivers/gpu/drm/drm_context.c | 9 +++++---- > drivers/gpu/drm/drm_drv.c | 12 ++++++++---- > drivers/gpu/drm/drm_file.c | 4 ++-- > drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- > drivers/gpu/drm/drm_lock.c | 7 ++++--- > drivers/gpu/drm/drm_pci.c | 16 ++++++++++------ > drivers/gpu/drm/drm_vblank.c | 11 ++++++----- > drivers/gpu/drm/i810/i810_drv.c | 10 +++++++--- > drivers/gpu/drm/mga/mga_drv.c | 16 ++++++++++------ > drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ > drivers/gpu/drm/r128/r128_drv.c | 16 ++++++++++------ > drivers/gpu/drm/savage/savage_drv.c | 12 ++++++++---- > drivers/gpu/drm/sis/sis_drv.c | 8 ++++++-- > drivers/gpu/drm/tdfx/tdfx_drv.c | 6 +++++- > drivers/gpu/drm/via/via_drv.c | 16 ++++++++++------ > include/drm/drm_device.h | 2 +- > include/drm/drm_drv.h | 21 +++++---------------- > include/drm/drm_legacy.h | 27 +++++++++++++++++++++++---- > include/drm/drm_pci.h | 4 ++-- > 20 files changed, 138 insertions(+), 83 deletions(-) > > -- > 2.25.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20200225174400.GP2363188-dv86pmgwkMBes7Z6vYuT8azUEOm+Xw19@public.gmane.org>]
* Re: [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver 2020-02-25 17:44 ` Daniel Vetter @ 2020-02-26 5:39 ` Thomas Zimmermann -1 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-26 5:39 UTC (permalink / raw) To: Daniel Vetter Cc: airlied-cv59FeDIM0c, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA, mripard-DgEjT+Ai2ygdnm+yROfE0A, bskeggs-H+wXaHxf7aLQT0dZR+AlfA, alexander.deucher-5C7GfCeVMHo, sam-uyr5N9Q2VtJg9hUCZPvPmw, emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ [-- Attachment #1.1.1: Type: text/plain, Size: 4235 bytes --] Hi Am 25.02.20 um 18:44 schrieb Daniel Vetter: > On Tue, Feb 25, 2020 at 04:58:59PM +0100, Thomas Zimmermann wrote: >> This patchset moves legacy, non-KMS driver state from struct drm_driver >> into struct drm_legacy_state. Only non-KMS drivers provide an instance >> of the latter structure. One special case is nouveau, which supports >> legacy interfaces. It also provides an instance of the legacy state if >> the legacy interfaces have been enabled (i.e., defines the config option >> CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) >> >> I reviewed all call sites of legacy state and functions to verify that >> DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that >> DRIVER_MODESET is not set. >> >> With the mutable KMS state removed, instances of struct drm_driver can >> be declared as constant. The patchset modifies the DRM core accordingly. >> Individual drivers can follow later on. > > Bikeshed: We very much have modern non-KMS drivers (vgem, etnaviv, v3d, > panfrost, ...). non-KMS != legacy, which is what you're talking about > here. OK, probably needs to be more precise. > > Other thing, and it's a bit raining on your parade: I don't see the point. > Sprinkling a few more #ifdef CONFIG_DRM_LEGACY over the relevant parts > sounds like a reasonable idea. But this is a lot of churn for drivers > which are all pretty much dead, and just waiting for their eventual > removal. And from a compile-testing pov of making sure modern drivers > don't use any of the deprecated stuff wrapping it in CONFIG_DRM_LEGACY > should be plenty enough. It's not about these old drivers, it's about having constant driver structures and maybe saving a few bytes in the modern drivers. If these old drivers hold back improvements to the drivers we care about, I don't see we they are not to be touched. This cannot be solved with CONFIG_DRM_LEGACY, unless you want to wrap any reference in core code to legacy data. I tried at first and it turned out to be an unreadable mess. And from a complexity POV the patchset is trivial. It adds a data structure to each old driver and moves a few initializers around. The worst thing that can happen is that code tried to dereference the legacy pointer when it's not set. This bug will happen with modern drivers, so we should see it easily. Best regards Thomas > > And from a "make stuff const" I think Laurent's much more minimal series > also gets us there for all the drivers we care about. > -Daniel > >> >> Thomas Zimmermann (3): >> drm: Add separate state structure for legacy, non-KMS drivers >> drm: Move non-kms driver state into struct drm_legacy_state >> drm: Constify struct drm_driver in DRM core >> >> drivers/gpu/drm/drm_bufs.c | 10 +++++----- >> drivers/gpu/drm/drm_context.c | 9 +++++---- >> drivers/gpu/drm/drm_drv.c | 12 ++++++++---- >> drivers/gpu/drm/drm_file.c | 4 ++-- >> drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- >> drivers/gpu/drm/drm_lock.c | 7 ++++--- >> drivers/gpu/drm/drm_pci.c | 16 ++++++++++------ >> drivers/gpu/drm/drm_vblank.c | 11 ++++++----- >> drivers/gpu/drm/i810/i810_drv.c | 10 +++++++--- >> drivers/gpu/drm/mga/mga_drv.c | 16 ++++++++++------ >> drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ >> drivers/gpu/drm/r128/r128_drv.c | 16 ++++++++++------ >> drivers/gpu/drm/savage/savage_drv.c | 12 ++++++++---- >> drivers/gpu/drm/sis/sis_drv.c | 8 ++++++-- >> drivers/gpu/drm/tdfx/tdfx_drv.c | 6 +++++- >> drivers/gpu/drm/via/via_drv.c | 16 ++++++++++------ >> include/drm/drm_device.h | 2 +- >> include/drm/drm_drv.h | 21 +++++---------------- >> include/drm/drm_legacy.h | 27 +++++++++++++++++++++++---- >> include/drm/drm_pci.h | 4 ++-- >> 20 files changed, 138 insertions(+), 83 deletions(-) >> >> -- >> 2.25.0 >> > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 182 bytes --] _______________________________________________ Nouveau mailing list Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org https://lists.freedesktop.org/mailman/listinfo/nouveau ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver @ 2020-02-26 5:39 ` Thomas Zimmermann 0 siblings, 0 replies; 22+ messages in thread From: Thomas Zimmermann @ 2020-02-26 5:39 UTC (permalink / raw) To: Daniel Vetter Cc: airlied, nouveau, dri-devel, bskeggs, alexander.deucher, sam, emil.velikov [-- Attachment #1.1.1: Type: text/plain, Size: 4235 bytes --] Hi Am 25.02.20 um 18:44 schrieb Daniel Vetter: > On Tue, Feb 25, 2020 at 04:58:59PM +0100, Thomas Zimmermann wrote: >> This patchset moves legacy, non-KMS driver state from struct drm_driver >> into struct drm_legacy_state. Only non-KMS drivers provide an instance >> of the latter structure. One special case is nouveau, which supports >> legacy interfaces. It also provides an instance of the legacy state if >> the legacy interfaces have been enabled (i.e., defines the config option >> CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) >> >> I reviewed all call sites of legacy state and functions to verify that >> DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that >> DRIVER_MODESET is not set. >> >> With the mutable KMS state removed, instances of struct drm_driver can >> be declared as constant. The patchset modifies the DRM core accordingly. >> Individual drivers can follow later on. > > Bikeshed: We very much have modern non-KMS drivers (vgem, etnaviv, v3d, > panfrost, ...). non-KMS != legacy, which is what you're talking about > here. OK, probably needs to be more precise. > > Other thing, and it's a bit raining on your parade: I don't see the point. > Sprinkling a few more #ifdef CONFIG_DRM_LEGACY over the relevant parts > sounds like a reasonable idea. But this is a lot of churn for drivers > which are all pretty much dead, and just waiting for their eventual > removal. And from a compile-testing pov of making sure modern drivers > don't use any of the deprecated stuff wrapping it in CONFIG_DRM_LEGACY > should be plenty enough. It's not about these old drivers, it's about having constant driver structures and maybe saving a few bytes in the modern drivers. If these old drivers hold back improvements to the drivers we care about, I don't see we they are not to be touched. This cannot be solved with CONFIG_DRM_LEGACY, unless you want to wrap any reference in core code to legacy data. I tried at first and it turned out to be an unreadable mess. And from a complexity POV the patchset is trivial. It adds a data structure to each old driver and moves a few initializers around. The worst thing that can happen is that code tried to dereference the legacy pointer when it's not set. This bug will happen with modern drivers, so we should see it easily. Best regards Thomas > > And from a "make stuff const" I think Laurent's much more minimal series > also gets us there for all the drivers we care about. > -Daniel > >> >> Thomas Zimmermann (3): >> drm: Add separate state structure for legacy, non-KMS drivers >> drm: Move non-kms driver state into struct drm_legacy_state >> drm: Constify struct drm_driver in DRM core >> >> drivers/gpu/drm/drm_bufs.c | 10 +++++----- >> drivers/gpu/drm/drm_context.c | 9 +++++---- >> drivers/gpu/drm/drm_drv.c | 12 ++++++++---- >> drivers/gpu/drm/drm_file.c | 4 ++-- >> drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- >> drivers/gpu/drm/drm_lock.c | 7 ++++--- >> drivers/gpu/drm/drm_pci.c | 16 ++++++++++------ >> drivers/gpu/drm/drm_vblank.c | 11 ++++++----- >> drivers/gpu/drm/i810/i810_drv.c | 10 +++++++--- >> drivers/gpu/drm/mga/mga_drv.c | 16 ++++++++++------ >> drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ >> drivers/gpu/drm/r128/r128_drv.c | 16 ++++++++++------ >> drivers/gpu/drm/savage/savage_drv.c | 12 ++++++++---- >> drivers/gpu/drm/sis/sis_drv.c | 8 ++++++-- >> drivers/gpu/drm/tdfx/tdfx_drv.c | 6 +++++- >> drivers/gpu/drm/via/via_drv.c | 16 ++++++++++------ >> include/drm/drm_device.h | 2 +- >> include/drm/drm_drv.h | 21 +++++---------------- >> include/drm/drm_legacy.h | 27 +++++++++++++++++++++++---- >> include/drm/drm_pci.h | 4 ++-- >> 20 files changed, 138 insertions(+), 83 deletions(-) >> >> -- >> 2.25.0 >> > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <f042cfae-8f74-0bd5-bd5f-dfb962c0c3b4-l3A5Bk7waGM@public.gmane.org>]
* Re: [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver 2020-02-26 5:39 ` Thomas Zimmermann @ 2020-02-26 10:26 ` Daniel Vetter -1 siblings, 0 replies; 22+ messages in thread From: Daniel Vetter @ 2020-02-26 10:26 UTC (permalink / raw) To: Thomas Zimmermann Cc: airlied-cv59FeDIM0c, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA, mripard-DgEjT+Ai2ygdnm+yROfE0A, bskeggs-H+wXaHxf7aLQT0dZR+AlfA, Daniel Vetter, alexander.deucher-5C7GfCeVMHo, sam-uyr5N9Q2VtJg9hUCZPvPmw, emil.velikov-ZGY8ohtN/8qB+jHODAdFcQ On Wed, Feb 26, 2020 at 06:39:08AM +0100, Thomas Zimmermann wrote: > Hi > > Am 25.02.20 um 18:44 schrieb Daniel Vetter: > > On Tue, Feb 25, 2020 at 04:58:59PM +0100, Thomas Zimmermann wrote: > >> This patchset moves legacy, non-KMS driver state from struct drm_driver > >> into struct drm_legacy_state. Only non-KMS drivers provide an instance > >> of the latter structure. One special case is nouveau, which supports > >> legacy interfaces. It also provides an instance of the legacy state if > >> the legacy interfaces have been enabled (i.e., defines the config option > >> CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) > >> > >> I reviewed all call sites of legacy state and functions to verify that > >> DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that > >> DRIVER_MODESET is not set. > >> > >> With the mutable KMS state removed, instances of struct drm_driver can > >> be declared as constant. The patchset modifies the DRM core accordingly. > >> Individual drivers can follow later on. > > > > Bikeshed: We very much have modern non-KMS drivers (vgem, etnaviv, v3d, > > panfrost, ...). non-KMS != legacy, which is what you're talking about > > here. > > OK, probably needs to be more precise. > > > > > Other thing, and it's a bit raining on your parade: I don't see the point. > > Sprinkling a few more #ifdef CONFIG_DRM_LEGACY over the relevant parts > > sounds like a reasonable idea. But this is a lot of churn for drivers > > which are all pretty much dead, and just waiting for their eventual > > removal. And from a compile-testing pov of making sure modern drivers > > don't use any of the deprecated stuff wrapping it in CONFIG_DRM_LEGACY > > should be plenty enough. > > It's not about these old drivers, it's about having constant driver > structures and maybe saving a few bytes in the modern drivers. If these > old drivers hold back improvements to the drivers we care about, I don't > see we they are not to be touched. > > This cannot be solved with CONFIG_DRM_LEGACY, unless you want to wrap > any reference in core code to legacy data. I tried at first and it > turned out to be an unreadable mess. Laurent has a patch series to constify drm_driver for all !legacy drivers. Without changing the world. So yeah it's possible, we're not hurting ourselves here (aside from the few bytes if we don't do the #ifdev CONFIG_DRM_LEGACY). https://patchwork.freedesktop.org/series/73811/ > And from a complexity POV the patchset is trivial. It adds a data > structure to each old driver and moves a few initializers around. The > worst thing that can happen is that code tried to dereference the legacy > pointer when it's not set. This bug will happen with modern drivers, so > we should see it easily. Laurent's series is even more trivial I think. -Daniel > > Best regards > Thomas > > > > > And from a "make stuff const" I think Laurent's much more minimal series > > also gets us there for all the drivers we care about. > > -Daniel > > > >> > >> Thomas Zimmermann (3): > >> drm: Add separate state structure for legacy, non-KMS drivers > >> drm: Move non-kms driver state into struct drm_legacy_state > >> drm: Constify struct drm_driver in DRM core > >> > >> drivers/gpu/drm/drm_bufs.c | 10 +++++----- > >> drivers/gpu/drm/drm_context.c | 9 +++++---- > >> drivers/gpu/drm/drm_drv.c | 12 ++++++++---- > >> drivers/gpu/drm/drm_file.c | 4 ++-- > >> drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- > >> drivers/gpu/drm/drm_lock.c | 7 ++++--- > >> drivers/gpu/drm/drm_pci.c | 16 ++++++++++------ > >> drivers/gpu/drm/drm_vblank.c | 11 ++++++----- > >> drivers/gpu/drm/i810/i810_drv.c | 10 +++++++--- > >> drivers/gpu/drm/mga/mga_drv.c | 16 ++++++++++------ > >> drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ > >> drivers/gpu/drm/r128/r128_drv.c | 16 ++++++++++------ > >> drivers/gpu/drm/savage/savage_drv.c | 12 ++++++++---- > >> drivers/gpu/drm/sis/sis_drv.c | 8 ++++++-- > >> drivers/gpu/drm/tdfx/tdfx_drv.c | 6 +++++- > >> drivers/gpu/drm/via/via_drv.c | 16 ++++++++++------ > >> include/drm/drm_device.h | 2 +- > >> include/drm/drm_drv.h | 21 +++++---------------- > >> include/drm/drm_legacy.h | 27 +++++++++++++++++++++++---- > >> include/drm/drm_pci.h | 4 ++-- > >> 20 files changed, 138 insertions(+), 83 deletions(-) > >> > >> -- > >> 2.25.0 > >> > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver @ 2020-02-26 10:26 ` Daniel Vetter 0 siblings, 0 replies; 22+ messages in thread From: Daniel Vetter @ 2020-02-26 10:26 UTC (permalink / raw) To: Thomas Zimmermann Cc: airlied, nouveau, dri-devel, bskeggs, alexander.deucher, sam, emil.velikov On Wed, Feb 26, 2020 at 06:39:08AM +0100, Thomas Zimmermann wrote: > Hi > > Am 25.02.20 um 18:44 schrieb Daniel Vetter: > > On Tue, Feb 25, 2020 at 04:58:59PM +0100, Thomas Zimmermann wrote: > >> This patchset moves legacy, non-KMS driver state from struct drm_driver > >> into struct drm_legacy_state. Only non-KMS drivers provide an instance > >> of the latter structure. One special case is nouveau, which supports > >> legacy interfaces. It also provides an instance of the legacy state if > >> the legacy interfaces have been enabled (i.e., defines the config option > >> CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT) > >> > >> I reviewed all call sites of legacy state and functions to verify that > >> DRIVER_LEGACY or DRIVER_KMS_LEGACY_CONTEXT is set on the device; or that > >> DRIVER_MODESET is not set. > >> > >> With the mutable KMS state removed, instances of struct drm_driver can > >> be declared as constant. The patchset modifies the DRM core accordingly. > >> Individual drivers can follow later on. > > > > Bikeshed: We very much have modern non-KMS drivers (vgem, etnaviv, v3d, > > panfrost, ...). non-KMS != legacy, which is what you're talking about > > here. > > OK, probably needs to be more precise. > > > > > Other thing, and it's a bit raining on your parade: I don't see the point. > > Sprinkling a few more #ifdef CONFIG_DRM_LEGACY over the relevant parts > > sounds like a reasonable idea. But this is a lot of churn for drivers > > which are all pretty much dead, and just waiting for their eventual > > removal. And from a compile-testing pov of making sure modern drivers > > don't use any of the deprecated stuff wrapping it in CONFIG_DRM_LEGACY > > should be plenty enough. > > It's not about these old drivers, it's about having constant driver > structures and maybe saving a few bytes in the modern drivers. If these > old drivers hold back improvements to the drivers we care about, I don't > see we they are not to be touched. > > This cannot be solved with CONFIG_DRM_LEGACY, unless you want to wrap > any reference in core code to legacy data. I tried at first and it > turned out to be an unreadable mess. Laurent has a patch series to constify drm_driver for all !legacy drivers. Without changing the world. So yeah it's possible, we're not hurting ourselves here (aside from the few bytes if we don't do the #ifdev CONFIG_DRM_LEGACY). https://patchwork.freedesktop.org/series/73811/ > And from a complexity POV the patchset is trivial. It adds a data > structure to each old driver and moves a few initializers around. The > worst thing that can happen is that code tried to dereference the legacy > pointer when it's not set. This bug will happen with modern drivers, so > we should see it easily. Laurent's series is even more trivial I think. -Daniel > > Best regards > Thomas > > > > > And from a "make stuff const" I think Laurent's much more minimal series > > also gets us there for all the drivers we care about. > > -Daniel > > > >> > >> Thomas Zimmermann (3): > >> drm: Add separate state structure for legacy, non-KMS drivers > >> drm: Move non-kms driver state into struct drm_legacy_state > >> drm: Constify struct drm_driver in DRM core > >> > >> drivers/gpu/drm/drm_bufs.c | 10 +++++----- > >> drivers/gpu/drm/drm_context.c | 9 +++++---- > >> drivers/gpu/drm/drm_drv.c | 12 ++++++++---- > >> drivers/gpu/drm/drm_file.c | 4 ++-- > >> drivers/gpu/drm/drm_legacy_misc.c | 6 +++--- > >> drivers/gpu/drm/drm_lock.c | 7 ++++--- > >> drivers/gpu/drm/drm_pci.c | 16 ++++++++++------ > >> drivers/gpu/drm/drm_vblank.c | 11 ++++++----- > >> drivers/gpu/drm/i810/i810_drv.c | 10 +++++++--- > >> drivers/gpu/drm/mga/mga_drv.c | 16 ++++++++++------ > >> drivers/gpu/drm/nouveau/nouveau_drm.c | 8 ++++++++ > >> drivers/gpu/drm/r128/r128_drv.c | 16 ++++++++++------ > >> drivers/gpu/drm/savage/savage_drv.c | 12 ++++++++---- > >> drivers/gpu/drm/sis/sis_drv.c | 8 ++++++-- > >> drivers/gpu/drm/tdfx/tdfx_drv.c | 6 +++++- > >> drivers/gpu/drm/via/via_drv.c | 16 ++++++++++------ > >> include/drm/drm_device.h | 2 +- > >> include/drm/drm_drv.h | 21 +++++---------------- > >> include/drm/drm_legacy.h | 27 +++++++++++++++++++++++---- > >> include/drm/drm_pci.h | 4 ++-- > >> 20 files changed, 138 insertions(+), 83 deletions(-) > >> > >> -- > >> 2.25.0 > >> > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-02-26 10:26 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25 15:58 [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver Thomas Zimmermann
2020-02-25 15:58 ` Thomas Zimmermann
[not found] ` <20200225155902.9751-1-tzimmermann-l3A5Bk7waGM@public.gmane.org>
2020-02-25 15:59 ` [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers Thomas Zimmermann
2020-02-25 15:59 ` Thomas Zimmermann
[not found] ` <20200225155902.9751-2-tzimmermann-l3A5Bk7waGM@public.gmane.org>
2020-02-25 16:06 ` Ilia Mirkin
2020-02-25 16:06 ` [Nouveau] " Ilia Mirkin
[not found] ` <CAKb7UviDs+rqF+vtDnV4CbHmY8PW8_-ahpAExctORYPKKyL7Yw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-02-25 16:21 ` Thomas Zimmermann
2020-02-25 16:21 ` [Nouveau] " Thomas Zimmermann
2020-02-25 17:37 ` Emil Velikov
2020-02-25 17:37 ` [Nouveau] " Emil Velikov
[not found] ` <CACvgo52fCMEiohuOkXyZi_mVrFnhMmmB--W64AvhEgVe3i2T5A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2020-02-26 5:28 ` Thomas Zimmermann
2020-02-26 5:28 ` [Nouveau] " Thomas Zimmermann
2020-02-25 15:59 ` [PATCH 2/3] drm: Move non-kms driver state into struct drm_legacy_state Thomas Zimmermann
2020-02-25 15:59 ` Thomas Zimmermann
2020-02-25 15:59 ` [PATCH 3/3] drm: Constify struct drm_driver in DRM core Thomas Zimmermann
2020-02-25 15:59 ` Thomas Zimmermann
2020-02-25 17:44 ` [PATCH 0/3] Add separate non-KMS state; constify struct drm_driver Daniel Vetter
2020-02-25 17:44 ` Daniel Vetter
[not found] ` <20200225174400.GP2363188-dv86pmgwkMBes7Z6vYuT8azUEOm+Xw19@public.gmane.org>
2020-02-26 5:39 ` Thomas Zimmermann
2020-02-26 5:39 ` Thomas Zimmermann
[not found] ` <f042cfae-8f74-0bd5-bd5f-dfb962c0c3b4-l3A5Bk7waGM@public.gmane.org>
2020-02-26 10:26 ` Daniel Vetter
2020-02-26 10:26 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.