Improve readability by adding/changing inline documentation
Signed-off-by: Arthur Borsboom arthurborsboom@gmail.com --- drivers/gpu/drm/gma500/psb_drv.c | 56 +++++++++++++++++++++++++++++++++------- drivers/gpu/drm/gma500/psb_drv.h | 24 +++++++++++------ 2 files changed, 63 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 1199180..5c6cdd0 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -44,7 +44,20 @@ static int psb_probe(struct pci_dev *pdev, const struct pci_device_id *ent); MODULE_PARM_DESC(trap_pagefaults, "Error and reset on MMU pagefaults"); module_param_named(trap_pagefaults, drm_psb_trap_pagefaults, int, 0600);
- +/* + * The table below contains a mapping of the PCI vendor ID and the PCI Device ID + * to the different groups of PowerVR 5-series chip designs + * + * 0x8086 = Intel Corporation + * + * PowerVR SGX535 - Poulsbo - Intel GMA 500, Intel Atom Z5xx + * PowerVR SGX535 - Moorestown - Intel GMA 600 + * PowerVR SGX535 - Oaktrail - Intel GMA 600, Intel Atom Z6xx, E6xx + * PowerVR SGX540 - Medfield - Intel Atom Z2460 + * PowerVR SGX544MP2 - Medfield - + * PowerVR SGX545 - Cedartrail - Intel GMA 3600, Intel Atom D2500, N2600 + * PowerVR SGX545 - Cedartrail - Intel GMA 3650, Intel Atom D2550, D2700, N2800 + */ static DEFINE_PCI_DEVICE_TABLE(pciidlist) = { { 0x8086, 0x8108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops }, { 0x8086, 0x8109, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops }, @@ -207,8 +220,7 @@ static int psb_driver_unload(struct drm_device *dev) { struct drm_psb_private *dev_priv = dev->dev_private;
- /* Kill vblank etc here */ - + /* TODO: Kill vblank etc here */
if (dev_priv) { if (dev_priv->backlight_device) @@ -268,7 +280,22 @@ static int psb_driver_unload(struct drm_device *dev) return 0; }
- +/* + * psb_driver_load - setup chip and create an initial config + * @dev: DRM device + * @flags: startup flags, containing the driver_data field belonging to + * the PCI device ID + * + * The driver load routine has to do several things: + * - allocating and initializing driver private data + * - performing resource allocation and mapping + * - initialize the memory manager + * - setup the DRM framebuffer with the allocated memory + * - install the IRQ handler + * - setup vertical blanking handling + * - mode setting + * - set inital output configuration + */ static int psb_driver_load(struct drm_device *dev, unsigned long chipset) { struct drm_psb_private *dev_priv; @@ -278,6 +305,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset) struct drm_connector *connector; struct gma_encoder *gma_encoder;
+ /* allocating and initializing driver private data */ dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); if (dev_priv == NULL) return -ENOMEM; @@ -369,6 +397,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset)
acpi_video_register();
+ /* + * Setup vertical blanking handling + */ ret = drm_vblank_init(dev, dev_priv->num_pipe); if (ret) goto out_err; @@ -416,11 +447,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset) return ret; psb_intel_opregion_enable_asle(dev); #if 0 - /*enable runtime pm at last*/ + /* Enable runtime pm at last */ pm_runtime_enable(&dev->pdev->dev); pm_runtime_set_active(&dev->pdev->dev); #endif - /*Intel drm driver load is done, continue doing pvr load*/ + /* Intel drm driver load is done, continue doing pvr load */ return 0; out_err: psb_driver_unload(dev); @@ -561,7 +592,7 @@ static int psb_mode_operation_ioctl(struct drm_device *dev, void *data, arg->data = resp; }
- /*do some clean up work*/ + /* Do some clean up work */ if (mode) drm_mode_destroy(dev, mode); mode_op_out: @@ -614,8 +645,8 @@ static long psb_unlocked_ioctl(struct file *filp, unsigned int cmd, /* FIXME: do we need to wrap the other side of this */ }
- -/* When a client dies: +/* + * When a client dies: * - Check for and clean up flipped page state */ static void psb_driver_preclose(struct drm_device *dev, struct drm_file *priv) @@ -655,6 +686,13 @@ static const struct file_operations psb_gem_fops = { .read = drm_read, };
+/* + * DRM driver structure initialization + * + * The drm_driver structure contains static information that describes + * the driver and features it supports, and pointers to methods that DRM + * core will call to implement DRM API. + */ static struct drm_driver driver = { .driver_features = DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | \ DRIVER_MODESET | DRIVER_GEM , diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h index 5ad6a03..85c560e 100644 --- a/drivers/gpu/drm/gma500/psb_drv.h +++ b/drivers/gpu/drm/gma500/psb_drv.h @@ -34,6 +34,19 @@ #include "opregion.h" #include "oaktrail.h"
+/* + * Driver definitions + */ + +/* + * Driver history (deprecated) + * + * The driver version was intended for ABI changes and it's not used anymore. + * There are better ways to tell userspace what features we expose. + * + * TODO: describe better ways + */ + /* Append new drm mode definition here, align with libdrm definition */ #define DRM_MODE_SCALE_NO_SCALE 2
@@ -88,15 +101,11 @@ enum { #define _PSB_PGETBL_ENABLED 0x00000001 #define PSB_SGX_2D_SLAVE_PORT 0x4000
-/* To get rid of */ +/* TODO: To get rid of */ #define PSB_TT_PRIV0_LIMIT (256*1024*1024) #define PSB_TT_PRIV0_PLIMIT (PSB_TT_PRIV0_LIMIT >> PAGE_SHIFT)
/* - * SGX side MMU definitions (these can probably go) - */ - -/* * Flags for external memory type field. */ #define PSB_MMU_CACHED_MEMORY 0x0001 /* Bind to MMU only */ @@ -519,7 +528,7 @@ struct drm_psb_private { uint32_t num_pipe;
/* - * OSPM info (Power management base) (can go ?) + * OSPM info (Power management base) (TODO: can go ?) */ uint32_t ospm_base;
@@ -766,9 +775,8 @@ extern void psb_mmu_remove_pages(struct psb_mmu_pd *pd, uint32_t desired_tile_stride, uint32_t hw_tile_stride); /* - *psb_irq.c + * psb_irq.c */ - extern irqreturn_t psb_irq_handler(int irq, void *arg); extern int psb_irq_enable_dpst(struct drm_device *dev); extern int psb_irq_disable_dpst(struct drm_device *dev);
Cleanup of code by following i915 constant/variable names and ordering Cleanup of code by following directions from kernel documentation: Codingstyle Cleanup of code by following directions from kernel documentation: DRM
Signed-off-by: Arthur Borsboom arthurborsboom@gmail.com --- drivers/gpu/drm/gma500/psb_drv.c | 132 +++++++++++++++++++-------------------- drivers/gpu/drm/gma500/psb_drv.h | 39 ++++-------- 2 files changed, 77 insertions(+), 94 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 5c6cdd0..ae95e31 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -37,9 +37,11 @@ #include <acpi/video.h> #include <linux/module.h>
+static struct drm_driver driver; + static int drm_psb_trap_pagefaults;
-static int psb_probe(struct pci_dev *pdev, const struct pci_device_id *ent); +static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
MODULE_PARM_DESC(trap_pagefaults, "Error and reset on MMU pagefaults"); module_param_named(trap_pagefaults, drm_psb_trap_pagefaults, int, 0600); @@ -62,44 +64,43 @@ static DEFINE_PCI_DEVICE_TABLE(pciidlist) = { { 0x8086, 0x8108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops }, { 0x8086, 0x8109, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops }, #if defined(CONFIG_DRM_GMA600) - { 0x8086, 0x4100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops}, - { 0x8086, 0x4101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops}, - { 0x8086, 0x4102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops}, - { 0x8086, 0x4103, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops}, - { 0x8086, 0x4104, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops}, - { 0x8086, 0x4105, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops}, - { 0x8086, 0x4106, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops}, - { 0x8086, 0x4107, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops}, - /* Atom E620 */ - { 0x8086, 0x4108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops}, + { 0x8086, 0x4100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops }, + { 0x8086, 0x4101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops }, + { 0x8086, 0x4102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops }, + { 0x8086, 0x4103, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops }, + { 0x8086, 0x4104, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops }, + { 0x8086, 0x4105, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops }, + { 0x8086, 0x4106, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops }, + { 0x8086, 0x4107, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops }, + { 0x8086, 0x4108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops }, #endif #if defined(CONFIG_DRM_MEDFIELD) - {0x8086, 0x0130, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops}, - {0x8086, 0x0131, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops}, - {0x8086, 0x0132, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops}, - {0x8086, 0x0133, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops}, - {0x8086, 0x0134, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops}, - {0x8086, 0x0135, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops}, - {0x8086, 0x0136, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops}, - {0x8086, 0x0137, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops}, + { 0x8086, 0x0130, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops }, + { 0x8086, 0x0131, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops }, + { 0x8086, 0x0132, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops }, + { 0x8086, 0x0133, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops }, + { 0x8086, 0x0134, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops }, + { 0x8086, 0x0135, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops }, + { 0x8086, 0x0136, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops }, + { 0x8086, 0x0137, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops }, #endif #if defined(CONFIG_DRM_GMA3600) - { 0x8086, 0x0be0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0be1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0be2, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0be3, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0be4, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0be5, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0be6, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0be7, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0be8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0be9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0bea, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0beb, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0bec, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0bed, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0bee, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, - { 0x8086, 0x0bef, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops}, + { 0x8086, 0x0be0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0be1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0be2, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0be3, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0be4, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0be5, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0be6, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0be7, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0be8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0be9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0bea, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0beb, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0bec, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0bed, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0bee, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, + { 0x8086, 0x0bef, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops }, #endif { 0, } }; @@ -108,7 +109,6 @@ MODULE_DEVICE_TABLE(pci, pciidlist); /* * Standard IOCTLs. */ - #define DRM_IOCTL_GMA_ADB \ DRM_IOWR(DRM_GMA_ADB + DRM_COMMAND_BASE, uint32_t) #define DRM_IOCTL_GMA_MODE_OPERATION \ @@ -160,7 +160,7 @@ static const struct drm_ioctl_desc psb_ioctls[] = { DRM_UNLOCKED | DRM_AUTH), };
-static void psb_lastclose(struct drm_device *dev) +static void psb_driver_lastclose(struct drm_device *dev) { int ret; struct drm_psb_private *dev_priv = dev->dev_private; @@ -190,11 +190,9 @@ static int psb_do_init(struct drm_device *dev) goto out_err; }
- stolen_gtt = (pg->stolen_size >> PAGE_SHIFT) * 4; stolen_gtt = (stolen_gtt + PAGE_SIZE - 1) >> PAGE_SHIFT; - stolen_gtt = - (stolen_gtt < pg->gtt_pages) ? stolen_gtt : pg->gtt_pages; + stolen_gtt = (stolen_gtt < pg->gtt_pages) ? stolen_gtt : pg->gtt_pages;
dev_priv->gatt_free_offset = pg->mmu_gatt_start + (stolen_gtt << PAGE_SHIFT) * 1024; @@ -296,7 +294,7 @@ static int psb_driver_unload(struct drm_device *dev) * - mode setting * - set inital output configuration */ -static int psb_driver_load(struct drm_device *dev, unsigned long chipset) +static int psb_driver_load(struct drm_device *dev, unsigned long flags) { struct drm_psb_private *dev_priv; unsigned long resource_start, resource_len; @@ -310,7 +308,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset) if (dev_priv == NULL) return -ENOMEM;
- dev_priv->ops = (struct psb_ops *)chipset; + dev_priv->ops = (struct psb_ops *)flags; dev_priv->dev = dev; dev->dev_private = (void *) dev_priv;
@@ -421,9 +419,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset) drm_irq_install(dev);
dev->vblank_disable_allowed = true; - dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */ - dev->driver->get_vblank_counter = psb_get_vblank_counter;
psb_modeset_init(dev); @@ -624,7 +620,7 @@ static int psb_driver_open(struct drm_device *dev, struct drm_file *priv) return 0; }
-static void psb_driver_close(struct drm_device *dev, struct drm_file *priv) +static void psb_driver_postclose(struct drm_device *dev, struct drm_file *priv) { }
@@ -653,7 +649,13 @@ static void psb_driver_preclose(struct drm_device *dev, struct drm_file *priv) { }
-static void psb_remove(struct pci_dev *pdev) +static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) +{ + return drm_get_pci_dev(pdev, ent, &driver); +} + + +static void psb_pci_remove(struct pci_dev *pdev) { struct drm_device *dev = pci_get_drvdata(pdev); drm_put_dev(dev); @@ -695,11 +697,14 @@ static const struct file_operations psb_gem_fops = { */ static struct drm_driver driver = { .driver_features = DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | \ - DRIVER_MODESET | DRIVER_GEM , + DRIVER_MODESET | DRIVER_GEM, .load = psb_driver_load, .unload = psb_driver_unload, + .open = psb_driver_open, + .lastclose = psb_driver_lastclose, + .preclose = psb_driver_preclose, + .postclose = psb_driver_postclose,
- .ioctls = psb_ioctls, .num_ioctls = DRM_ARRAY_SIZE(psb_ioctls), .device_is_agp = psb_driver_device_is_agp, .irq_preinstall = psb_irq_preinstall, @@ -709,40 +714,31 @@ static struct drm_driver driver = { .enable_vblank = psb_enable_vblank, .disable_vblank = psb_disable_vblank, .get_vblank_counter = psb_get_vblank_counter, - .lastclose = psb_lastclose, - .open = psb_driver_open, - .preclose = psb_driver_preclose, - .postclose = psb_driver_close,
.gem_free_object = psb_gem_free_object, .gem_vm_ops = &psb_gem_vm_ops, + .dumb_create = psb_gem_dumb_create, .dumb_map_offset = psb_gem_dumb_map_gtt, .dumb_destroy = drm_gem_dumb_destroy, + .ioctls = psb_ioctls, .fops = &psb_gem_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC, - .date = PSB_DRM_DRIVER_DATE, - .major = PSB_DRM_DRIVER_MAJOR, - .minor = PSB_DRM_DRIVER_MINOR, - .patchlevel = PSB_DRM_DRIVER_PATCHLEVEL + .date = DRIVER_DATE, + .major = DRIVER_MAJOR, + .minor = DRIVER_MINOR, + .patchlevel = DRIVER_PATCHLEVEL };
static struct pci_driver psb_pci_driver = { .name = DRIVER_NAME, .id_table = pciidlist, - .probe = psb_probe, - .remove = psb_remove, - .driver = { - .pm = &psb_pm_ops, - } + .probe = psb_pci_probe, + .remove = psb_pci_remove, + .driver.pm = &psb_pm_ops, };
-static int psb_probe(struct pci_dev *pdev, const struct pci_device_id *ent) -{ - return drm_get_pci_dev(pdev, ent, &driver); -} - static int __init psb_init(void) { return drm_pci_init(&driver, &psb_pci_driver); @@ -756,6 +752,6 @@ static void __exit psb_exit(void) late_initcall(psb_init); module_exit(psb_exit);
-MODULE_AUTHOR("Alan Cox alan@linux.intel.com and others"); +MODULE_AUTHOR(DRIVER_AUTHOR); MODULE_DESCRIPTION(DRIVER_DESC); -MODULE_LICENSE("GPL"); +MODULE_LICENSE(DRIVER_LICENSE); diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h index 85c560e..150d9b5 100644 --- a/drivers/gpu/drm/gma500/psb_drv.h +++ b/drivers/gpu/drm/gma500/psb_drv.h @@ -37,6 +37,12 @@ /* * Driver definitions */ +#define DRIVER_AUTHOR "Alan Cox alan@linux.intel.com and others" +#define DRIVER_LICENSE "GPL" + +#define DRIVER_NAME "gma500" +#define DRIVER_DESC "DRM driver for the Intel GMA500, GMA600, GMA3600, GMA3650" +#define DRIVER_DATE "20140312"
/* * Driver history (deprecated) @@ -46,6 +52,9 @@ * * TODO: describe better ways */ +#define DRIVER_MAJOR 1 +#define DRIVER_MINOR 0 +#define DRIVER_PATCHLEVEL 0
/* Append new drm mode definition here, align with libdrm definition */ #define DRM_MODE_SCALE_NO_SCALE 2 @@ -63,18 +72,6 @@ enum { #define IS_CDV(dev) (((dev)->pdev->device & 0xfff0) == 0x0be0)
/* - * Driver definitions - */ - -#define DRIVER_NAME "gma500" -#define DRIVER_DESC "DRM driver for the Intel GMA500" - -#define PSB_DRM_DRIVER_DATE "2011-06-06" -#define PSB_DRM_DRIVER_MAJOR 1 -#define PSB_DRM_DRIVER_MINOR 0 -#define PSB_DRM_DRIVER_PATCHLEVEL 0 - -/* * Hardware offsets */ #define PSB_VDC_OFFSET 0x00000000 @@ -84,6 +81,7 @@ enum { #define PSB_SGX_SIZE 0x8000 #define PSB_SGX_OFFSET 0x00040000 #define MRST_SGX_OFFSET 0x00080000 + /* * PCI resource identifiers */ @@ -91,6 +89,7 @@ enum { #define PSB_AUX_RESOURCE 0 #define PSB_GATT_RESOURCE 2 #define PSB_GTT_RESOURCE 3 + /* * PCI configuration */ @@ -111,12 +110,14 @@ enum { #define PSB_MMU_CACHED_MEMORY 0x0001 /* Bind to MMU only */ #define PSB_MMU_RO_MEMORY 0x0002 /* MMU RO memory */ #define PSB_MMU_WO_MEMORY 0x0004 /* MMU WO memory */ + /* * PTE's and PDE's */ #define PSB_PDE_MASK 0x003FFFFF #define PSB_PDE_SHIFT 22 #define PSB_PTE_SHIFT 12 + /* * Cache control */ @@ -295,7 +296,6 @@ struct intel_gmbus { /* * Register offset maps */ - struct psb_offset { u32 fp0; u32 fp1; @@ -494,7 +494,6 @@ struct drm_psb_private { /* * Register base */ - uint8_t __iomem *sgx_reg; uint8_t __iomem *vdc_reg; uint8_t __iomem *aux_reg; /* Auxillary vdc pipe regs */ @@ -503,7 +502,6 @@ struct drm_psb_private { /* * Fencing / irq. */ - uint32_t vdc_irq_mask; uint32_t pipestat[PSB_NUM_PIPE];
@@ -512,7 +510,6 @@ struct drm_psb_private { /* * Power */ - bool suspended; bool display_power; int display_count; @@ -535,7 +532,6 @@ struct drm_psb_private { /* * Sizes info */ - u32 fuse_reg_value; u32 video_device_fuse;
@@ -594,7 +590,6 @@ struct drm_psb_private { /* * Register state */ - struct psb_save_area regs;
/* MSI reg save */ @@ -604,7 +599,6 @@ struct drm_psb_private { /* * Hotplug handling */ - struct work_struct hotplug_work;
/* @@ -618,7 +612,6 @@ struct drm_psb_private { /* * Watchdog */ - uint32_t apm_reg; uint16_t apm_base;
@@ -676,7 +669,6 @@ struct drm_psb_private { /* * Operations for each board type */ - struct psb_ops { const char *name; unsigned int accel_2d:1; @@ -735,7 +727,6 @@ static inline struct drm_psb_private *psb_priv(struct drm_device *dev) /* * MMU stuff. */ - extern struct psb_mmu_driver *psb_mmu_driver_init(uint8_t __iomem * registers, int trap_pagefaults, int invalid_type, @@ -763,8 +754,6 @@ extern int psb_mmu_virtual_to_pfn(struct psb_mmu_pd *pd, uint32_t virtual, /* * Enable / disable MMU for different requestors. */ - - extern void psb_mmu_set_pd_context(struct psb_mmu_pd *pd, int hw_context); extern int psb_mmu_insert_pages(struct psb_mmu_pd *pd, struct page **pages, unsigned long address, uint32_t num_pages, @@ -816,7 +805,6 @@ extern void psb_spank(struct drm_psb_private *dev_priv); /* * psb_reset.c */ - extern void psb_lid_timer_init(struct drm_psb_private *dev_priv); extern void psb_lid_timer_takedown(struct drm_psb_private *dev_priv); extern void psb_print_pagefault(struct drm_psb_private *dev_priv); @@ -896,7 +884,6 @@ extern int drm_idle_check_interval; /* * Utilities */ - static inline u32 MRST_MSG_READ32(uint port, uint offset) { int mcr = (0xD0<<24) | (port << 16) | (offset << 8);
On Thu, Mar 13, 2014 at 10:49 PM, Arthur Borsboom arthurborsboom@gmail.com wrote:
Cleanup of code by following i915 constant/variable names and ordering Cleanup of code by following directions from kernel documentation: Codingstyle Cleanup of code by following directions from kernel documentation: DRM
~72 col lines are preferred for commit messages. It makes git log look better. Run "git log" and you'll see when it's ok to make exceptions.
Signed-off-by: Arthur Borsboom arthurborsboom@gmail.com
drivers/gpu/drm/gma500/psb_drv.c | 132 +++++++++++++++++++-------------------- drivers/gpu/drm/gma500/psb_drv.h | 39 ++++-------- 2 files changed, 77 insertions(+), 94 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 5c6cdd0..ae95e31 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -37,9 +37,11 @@ #include <acpi/video.h> #include <linux/module.h>
+static struct drm_driver driver;
static int drm_psb_trap_pagefaults;
-static int psb_probe(struct pci_dev *pdev, const struct pci_device_id *ent); +static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
MODULE_PARM_DESC(trap_pagefaults, "Error and reset on MMU pagefaults"); module_param_named(trap_pagefaults, drm_psb_trap_pagefaults, int, 0600); @@ -62,44 +64,43 @@ static DEFINE_PCI_DEVICE_TABLE(pciidlist) = { { 0x8086, 0x8108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops }, { 0x8086, 0x8109, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops }, #if defined(CONFIG_DRM_GMA600)
{ 0x8086, 0x4100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops},{ 0x8086, 0x4101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops},{ 0x8086, 0x4102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops},{ 0x8086, 0x4103, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops},{ 0x8086, 0x4104, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops},{ 0x8086, 0x4105, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops},{ 0x8086, 0x4106, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops},{ 0x8086, 0x4107, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops},/* Atom E620 */{ 0x8086, 0x4108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops},
{ 0x8086, 0x4100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },{ 0x8086, 0x4101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },{ 0x8086, 0x4102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },{ 0x8086, 0x4103, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },{ 0x8086, 0x4104, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },{ 0x8086, 0x4105, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },{ 0x8086, 0x4106, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },{ 0x8086, 0x4107, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },{ 0x8086, 0x4108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &oaktrail_chip_ops },#endif #if defined(CONFIG_DRM_MEDFIELD)
{0x8086, 0x0130, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops},{0x8086, 0x0131, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops},{0x8086, 0x0132, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops},{0x8086, 0x0133, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops},{0x8086, 0x0134, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops},{0x8086, 0x0135, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops},{0x8086, 0x0136, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops},{0x8086, 0x0137, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops},
{ 0x8086, 0x0130, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops },{ 0x8086, 0x0131, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops },{ 0x8086, 0x0132, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops },{ 0x8086, 0x0133, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops },{ 0x8086, 0x0134, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops },{ 0x8086, 0x0135, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops },{ 0x8086, 0x0136, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops },{ 0x8086, 0x0137, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops },#endif #if defined(CONFIG_DRM_GMA3600)
{ 0x8086, 0x0be0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0be1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0be2, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0be3, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0be4, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0be5, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0be6, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0be7, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0be8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0be9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0bea, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0beb, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0bec, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0bed, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0bee, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},{ 0x8086, 0x0bef, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops},
{ 0x8086, 0x0be0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0be1, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0be2, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0be3, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0be4, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0be5, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0be6, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0be7, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0be8, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0be9, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0bea, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0beb, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0bec, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0bed, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0bee, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },{ 0x8086, 0x0bef, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &cdv_chip_ops },#endif { 0, } }; @@ -108,7 +109,6 @@ MODULE_DEVICE_TABLE(pci, pciidlist); /*
- Standard IOCTLs.
*/
#define DRM_IOCTL_GMA_ADB \ DRM_IOWR(DRM_GMA_ADB + DRM_COMMAND_BASE, uint32_t) #define DRM_IOCTL_GMA_MODE_OPERATION \ @@ -160,7 +160,7 @@ static const struct drm_ioctl_desc psb_ioctls[] = { DRM_UNLOCKED | DRM_AUTH), };
-static void psb_lastclose(struct drm_device *dev) +static void psb_driver_lastclose(struct drm_device *dev) { int ret; struct drm_psb_private *dev_priv = dev->dev_private; @@ -190,11 +190,9 @@ static int psb_do_init(struct drm_device *dev) goto out_err; }
stolen_gtt = (pg->stolen_size >> PAGE_SHIFT) * 4; stolen_gtt = (stolen_gtt + PAGE_SIZE - 1) >> PAGE_SHIFT;stolen_gtt =(stolen_gtt < pg->gtt_pages) ? stolen_gtt : pg->gtt_pages;
stolen_gtt = (stolen_gtt < pg->gtt_pages) ? stolen_gtt : pg->gtt_pages; dev_priv->gatt_free_offset = pg->mmu_gatt_start + (stolen_gtt << PAGE_SHIFT) * 1024;@@ -296,7 +294,7 @@ static int psb_driver_unload(struct drm_device *dev)
- mode setting
- set inital output configuration
*/ -static int psb_driver_load(struct drm_device *dev, unsigned long chipset) +static int psb_driver_load(struct drm_device *dev, unsigned long flags) { struct drm_psb_private *dev_priv; unsigned long resource_start, resource_len; @@ -310,7 +308,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset) if (dev_priv == NULL) return -ENOMEM;
dev_priv->ops = (struct psb_ops *)chipset;
dev_priv->ops = (struct psb_ops *)flags; dev_priv->dev = dev; dev->dev_private = (void *) dev_priv;@@ -421,9 +419,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset) drm_irq_install(dev);
dev->vblank_disable_allowed = true;
dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */dev->driver->get_vblank_counter = psb_get_vblank_counter; psb_modeset_init(dev);@@ -624,7 +620,7 @@ static int psb_driver_open(struct drm_device *dev, struct drm_file *priv) return 0; }
-static void psb_driver_close(struct drm_device *dev, struct drm_file *priv) +static void psb_driver_postclose(struct drm_device *dev, struct drm_file *priv) { }
@@ -653,7 +649,13 @@ static void psb_driver_preclose(struct drm_device *dev, struct drm_file *priv) { }
-static void psb_remove(struct pci_dev *pdev) +static int psb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) +{
return drm_get_pci_dev(pdev, ent, &driver);+}
+static void psb_pci_remove(struct pci_dev *pdev) { struct drm_device *dev = pci_get_drvdata(pdev); drm_put_dev(dev); @@ -695,11 +697,14 @@ static const struct file_operations psb_gem_fops = { */ static struct drm_driver driver = { .driver_features = DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | \
DRIVER_MODESET | DRIVER_GEM ,
DRIVER_MODESET | DRIVER_GEM, .load = psb_driver_load, .unload = psb_driver_unload,.open = psb_driver_open,.lastclose = psb_driver_lastclose,.preclose = psb_driver_preclose,.postclose = psb_driver_postclose,
.ioctls = psb_ioctls, .num_ioctls = DRM_ARRAY_SIZE(psb_ioctls), .device_is_agp = psb_driver_device_is_agp, .irq_preinstall = psb_irq_preinstall,@@ -709,40 +714,31 @@ static struct drm_driver driver = { .enable_vblank = psb_enable_vblank, .disable_vblank = psb_disable_vblank, .get_vblank_counter = psb_get_vblank_counter,
.lastclose = psb_lastclose,.open = psb_driver_open,.preclose = psb_driver_preclose,.postclose = psb_driver_close, .gem_free_object = psb_gem_free_object, .gem_vm_ops = &psb_gem_vm_ops,
.dumb_create = psb_gem_dumb_create, .dumb_map_offset = psb_gem_dumb_map_gtt, .dumb_destroy = drm_gem_dumb_destroy,.ioctls = psb_ioctls, .fops = &psb_gem_fops, .name = DRIVER_NAME, .desc = DRIVER_DESC,
.date = PSB_DRM_DRIVER_DATE,.major = PSB_DRM_DRIVER_MAJOR,.minor = PSB_DRM_DRIVER_MINOR,.patchlevel = PSB_DRM_DRIVER_PATCHLEVEL
.date = DRIVER_DATE,.major = DRIVER_MAJOR,.minor = DRIVER_MINOR,.patchlevel = DRIVER_PATCHLEVEL};
static struct pci_driver psb_pci_driver = { .name = DRIVER_NAME, .id_table = pciidlist,
.probe = psb_probe,.remove = psb_remove,.driver = {.pm = &psb_pm_ops,}
.probe = psb_pci_probe,.remove = psb_pci_remove,.driver.pm = &psb_pm_ops,};
-static int psb_probe(struct pci_dev *pdev, const struct pci_device_id *ent) -{
return drm_get_pci_dev(pdev, ent, &driver);-}
static int __init psb_init(void) { return drm_pci_init(&driver, &psb_pci_driver); @@ -756,6 +752,6 @@ static void __exit psb_exit(void) late_initcall(psb_init); module_exit(psb_exit);
-MODULE_AUTHOR("Alan Cox alan@linux.intel.com and others"); +MODULE_AUTHOR(DRIVER_AUTHOR); MODULE_DESCRIPTION(DRIVER_DESC); -MODULE_LICENSE("GPL"); +MODULE_LICENSE(DRIVER_LICENSE); diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h index 85c560e..150d9b5 100644 --- a/drivers/gpu/drm/gma500/psb_drv.h +++ b/drivers/gpu/drm/gma500/psb_drv.h @@ -37,6 +37,12 @@ /*
- Driver definitions
*/ +#define DRIVER_AUTHOR "Alan Cox alan@linux.intel.com and others" +#define DRIVER_LICENSE "GPL"
+#define DRIVER_NAME "gma500" +#define DRIVER_DESC "DRM driver for the Intel GMA500, GMA600, GMA3600, GMA3650" +#define DRIVER_DATE "20140312"
/*
- Driver history (deprecated)
@@ -46,6 +52,9 @@
- TODO: describe better ways
*/ +#define DRIVER_MAJOR 1 +#define DRIVER_MINOR 0 +#define DRIVER_PATCHLEVEL 0
/* Append new drm mode definition here, align with libdrm definition */ #define DRM_MODE_SCALE_NO_SCALE 2 @@ -63,18 +72,6 @@ enum { #define IS_CDV(dev) (((dev)->pdev->device & 0xfff0) == 0x0be0)
/*
- Driver definitions
- */
-#define DRIVER_NAME "gma500" -#define DRIVER_DESC "DRM driver for the Intel GMA500"
-#define PSB_DRM_DRIVER_DATE "2011-06-06" -#define PSB_DRM_DRIVER_MAJOR 1 -#define PSB_DRM_DRIVER_MINOR 0 -#define PSB_DRM_DRIVER_PATCHLEVEL 0
-/*
Hardware offsets*/ #define PSB_VDC_OFFSET 0x00000000 @@ -84,6 +81,7 @@ enum { #define PSB_SGX_SIZE 0x8000 #define PSB_SGX_OFFSET 0x00040000 #define MRST_SGX_OFFSET 0x00080000
/*
PCI resource identifiers*/ @@ -91,6 +89,7 @@ enum { #define PSB_AUX_RESOURCE 0 #define PSB_GATT_RESOURCE 2 #define PSB_GTT_RESOURCE 3
/*
PCI configuration*/ @@ -111,12 +110,14 @@ enum { #define PSB_MMU_CACHED_MEMORY 0x0001 /* Bind to MMU only */ #define PSB_MMU_RO_MEMORY 0x0002 /* MMU RO memory */ #define PSB_MMU_WO_MEMORY 0x0004 /* MMU WO memory */
/*
PTE's and PDE's*/ #define PSB_PDE_MASK 0x003FFFFF #define PSB_PDE_SHIFT 22 #define PSB_PTE_SHIFT 12
/*
Cache control*/ @@ -295,7 +296,6 @@ struct intel_gmbus { /*
Register offset maps*/
struct psb_offset { u32 fp0; u32 fp1; @@ -494,7 +494,6 @@ struct drm_psb_private { /* * Register base */
uint8_t __iomem *sgx_reg; uint8_t __iomem *vdc_reg; uint8_t __iomem *aux_reg; /* Auxillary vdc pipe regs */@@ -503,7 +502,6 @@ struct drm_psb_private { /* * Fencing / irq. */
uint32_t vdc_irq_mask; uint32_t pipestat[PSB_NUM_PIPE];@@ -512,7 +510,6 @@ struct drm_psb_private { /* * Power */
bool suspended; bool display_power; int display_count;@@ -535,7 +532,6 @@ struct drm_psb_private { /* * Sizes info */
u32 fuse_reg_value; u32 video_device_fuse;@@ -594,7 +590,6 @@ struct drm_psb_private { /* * Register state */
struct psb_save_area regs; /* MSI reg save */@@ -604,7 +599,6 @@ struct drm_psb_private { /* * Hotplug handling */
struct work_struct hotplug_work; /*@@ -618,7 +612,6 @@ struct drm_psb_private { /* * Watchdog */
uint32_t apm_reg; uint16_t apm_base;@@ -676,7 +669,6 @@ struct drm_psb_private { /*
Operations for each board type*/
struct psb_ops { const char *name; unsigned int accel_2d:1; @@ -735,7 +727,6 @@ static inline struct drm_psb_private *psb_priv(struct drm_device *dev) /*
- MMU stuff.
*/
extern struct psb_mmu_driver *psb_mmu_driver_init(uint8_t __iomem * registers, int trap_pagefaults, int invalid_type, @@ -763,8 +754,6 @@ extern int psb_mmu_virtual_to_pfn(struct psb_mmu_pd *pd, uint32_t virtual, /*
- Enable / disable MMU for different requestors.
*/
extern void psb_mmu_set_pd_context(struct psb_mmu_pd *pd, int hw_context); extern int psb_mmu_insert_pages(struct psb_mmu_pd *pd, struct page **pages, unsigned long address, uint32_t num_pages, @@ -816,7 +805,6 @@ extern void psb_spank(struct drm_psb_private *dev_priv); /*
- psb_reset.c
*/
extern void psb_lid_timer_init(struct drm_psb_private *dev_priv); extern void psb_lid_timer_takedown(struct drm_psb_private *dev_priv); extern void psb_print_pagefault(struct drm_psb_private *dev_priv); @@ -896,7 +884,6 @@ extern int drm_idle_check_interval; /*
Utilities*/
static inline u32 MRST_MSG_READ32(uint port, uint offset) { int mcr = (0xD0<<24) | (port << 16) | (offset << 8); -- 1.9.0
Removed centralized exiting of function (goto statement), since it was the only used in one single location with only a return statement.
Signed-off-by: Arthur Borsboom arthurborsboom@gmail.com --- drivers/gpu/drm/gma500/psb_drv.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index ae95e31..8d6ad6c 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -182,12 +182,9 @@ static int psb_do_init(struct drm_device *dev)
uint32_t stolen_gtt;
- int ret = -ENOMEM; - if (pg->mmu_gatt_start & 0x0FFFFFFF) { dev_err(dev->dev, "Gatt must be 256M aligned. This is a bug.\n"); - ret = -EINVAL; - goto out_err; + return -EINVAL; }
stolen_gtt = (pg->stolen_size >> PAGE_SHIFT) * 4; @@ -210,8 +207,6 @@ static int psb_do_init(struct drm_device *dev) /* mmu_gatt ?? */ PSB_WSGX32(pg->gatt_start, PSB_CR_BIF_TWOD_REQ_BASE); return 0; -out_err: - return ret; }
static int psb_driver_unload(struct drm_device *dev)
On Thu, Mar 13, 2014 at 10:49 PM, Arthur Borsboom arthurborsboom@gmail.com wrote:
Removed centralized exiting of function (goto statement), since it was the only used in one single location with only a return statement.
As in previous patch, keep the commit message at around 72 cols.
Signed-off-by: Arthur Borsboom arthurborsboom@gmail.com
drivers/gpu/drm/gma500/psb_drv.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index ae95e31..8d6ad6c 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -182,12 +182,9 @@ static int psb_do_init(struct drm_device *dev)
uint32_t stolen_gtt;
int ret = -ENOMEM;if (pg->mmu_gatt_start & 0x0FFFFFFF) { dev_err(dev->dev, "Gatt must be 256M aligned. This is a bug.\n");ret = -EINVAL;goto out_err;
return -EINVAL; } stolen_gtt = (pg->stolen_size >> PAGE_SHIFT) * 4;@@ -210,8 +207,6 @@ static int psb_do_init(struct drm_device *dev) /* mmu_gatt ?? */ PSB_WSGX32(pg->gatt_start, PSB_CR_BIF_TWOD_REQ_BASE); return 0; -out_err:
return ret;}
static int psb_driver_unload(struct drm_device *dev)
1.9.0
On Thu, Mar 13, 2014 at 10:49 PM, Arthur Borsboom arthurborsboom@gmail.com wrote:
Improve readability by adding/changing inline documentation
Signed-off-by: Arthur Borsboom arthurborsboom@gmail.com
drivers/gpu/drm/gma500/psb_drv.c | 56 +++++++++++++++++++++++++++++++++------- drivers/gpu/drm/gma500/psb_drv.h | 24 +++++++++++------ 2 files changed, 63 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 1199180..5c6cdd0 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -44,7 +44,20 @@ static int psb_probe(struct pci_dev *pdev, const struct pci_device_id *ent); MODULE_PARM_DESC(trap_pagefaults, "Error and reset on MMU pagefaults"); module_param_named(trap_pagefaults, drm_psb_trap_pagefaults, int, 0600);
+/*
- The table below contains a mapping of the PCI vendor ID and the PCI Device ID
- to the different groups of PowerVR 5-series chip designs
- 0x8086 = Intel Corporation
- PowerVR SGX535 - Poulsbo - Intel GMA 500, Intel Atom Z5xx
- PowerVR SGX535 - Moorestown - Intel GMA 600
- PowerVR SGX535 - Oaktrail - Intel GMA 600, Intel Atom Z6xx, E6xx
- PowerVR SGX540 - Medfield - Intel Atom Z2460
- PowerVR SGX544MP2 - Medfield -
- PowerVR SGX545 - Cedartrail - Intel GMA 3600, Intel Atom D2500, N2600
- PowerVR SGX545 - Cedartrail - Intel GMA 3650, Intel Atom D2550, D2700, N2800
There is no reason to break the 80 col line limit here. Here is Linus' motivation if that makes more sense than mine: https://lkml.org/lkml/2012/2/3/394
And sometimes we break it by mistake, but that is a different thing.
- */
static DEFINE_PCI_DEVICE_TABLE(pciidlist) = { { 0x8086, 0x8108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops }, { 0x8086, 0x8109, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &psb_chip_ops }, @@ -207,8 +220,7 @@ static int psb_driver_unload(struct drm_device *dev) { struct drm_psb_private *dev_priv = dev->dev_private;
/* Kill vblank etc here */
/* TODO: Kill vblank etc here */ if (dev_priv) { if (dev_priv->backlight_device)@@ -268,7 +280,22 @@ static int psb_driver_unload(struct drm_device *dev) return 0; }
+/*
- psb_driver_load - setup chip and create an initial config
- @dev: DRM device
- @flags: startup flags, containing the driver_data field belonging to
the PCI device ID
- The driver load routine has to do several things:
- allocating and initializing driver private data
- performing resource allocation and mapping
- initialize the memory manager
- setup the DRM framebuffer with the allocated memory
- install the IRQ handler
- setup vertical blanking handling
- mode setting
- set inital output configuration
- */
This is not a constructive comment. It just repeats the DRM documentation. This might look harmless but it is actually a bad thing. For every comment we add to the driver, we add one more place that we must remember to update when we change our code. If you look at the driver, you'll see plenty of places where comments never got removed, even though the code it talks about is long gone.
And most of the time, the code speaks for itself.
static int psb_driver_load(struct drm_device *dev, unsigned long chipset) { struct drm_psb_private *dev_priv; @@ -278,6 +305,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset) struct drm_connector *connector; struct gma_encoder *gma_encoder;
/* allocating and initializing driver private data */ dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); if (dev_priv == NULL) return -ENOMEM;@@ -369,6 +397,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset)
acpi_video_register();
/** Setup vertical blanking handling*/
Single line comments should be just /* ... comment ... */ See Documentation/CodingStyle Chapter 8.
ret = drm_vblank_init(dev, dev_priv->num_pipe); if (ret) goto out_err;@@ -416,11 +447,11 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset) return ret; psb_intel_opregion_enable_asle(dev); #if 0
/*enable runtime pm at last*/
/* Enable runtime pm at last */ pm_runtime_enable(&dev->pdev->dev); pm_runtime_set_active(&dev->pdev->dev);#endif
/*Intel drm driver load is done, continue doing pvr load*/
/* Intel drm driver load is done, continue doing pvr load */ return 0;out_err: psb_driver_unload(dev); @@ -561,7 +592,7 @@ static int psb_mode_operation_ioctl(struct drm_device *dev, void *data, arg->data = resp; }
/*do some clean up work*/
/* Do some clean up work */ if (mode) drm_mode_destroy(dev, mode);mode_op_out: @@ -614,8 +645,8 @@ static long psb_unlocked_ioctl(struct file *filp, unsigned int cmd, /* FIXME: do we need to wrap the other side of this */ }
-/* When a client dies: +/*
*/
- When a client dies:
- Check for and clean up flipped page state
static void psb_driver_preclose(struct drm_device *dev, struct drm_file *priv) @@ -655,6 +686,13 @@ static const struct file_operations psb_gem_fops = { .read = drm_read, };
+/*
- DRM driver structure initialization
- The drm_driver structure contains static information that describes
- the driver and features it supports, and pointers to methods that DRM
- core will call to implement DRM API.
- */
This also just repeats the DRM documentation. No need for it.
static struct drm_driver driver = { .driver_features = DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | \ DRIVER_MODESET | DRIVER_GEM , diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h index 5ad6a03..85c560e 100644 --- a/drivers/gpu/drm/gma500/psb_drv.h +++ b/drivers/gpu/drm/gma500/psb_drv.h @@ -34,6 +34,19 @@ #include "opregion.h" #include "oaktrail.h"
+/*
- Driver definitions
- */
This is just stating the obvious. The code speaks for itself.
+/*
- Driver history (deprecated)
Why _add_ a comment that it is deprecated when it never existed?
- The driver version was intended for ABI changes and it's not used anymore.
- There are better ways to tell userspace what features we expose.
- TODO: describe better ways
- */
This can also go.
/* Append new drm mode definition here, align with libdrm definition */ #define DRM_MODE_SCALE_NO_SCALE 2
@@ -88,15 +101,11 @@ enum { #define _PSB_PGETBL_ENABLED 0x00000001 #define PSB_SGX_2D_SLAVE_PORT 0x4000
-/* To get rid of */ +/* TODO: To get rid of */ #define PSB_TT_PRIV0_LIMIT (256*1024*1024) #define PSB_TT_PRIV0_PLIMIT (PSB_TT_PRIV0_LIMIT >> PAGE_SHIFT)
/*
SGX side MMU definitions (these can probably go)- */
-/*
This comment is actually useful. It tells us that the MMU bits defined below are for the SGX MMU page table entries and not the Intel GTT PTEs. Though it could be combined with the comment below since they talk about the same thing.
Flags for external memory type field.*/ #define PSB_MMU_CACHED_MEMORY 0x0001 /* Bind to MMU only */ @@ -519,7 +528,7 @@ struct drm_psb_private { uint32_t num_pipe;
/*
* OSPM info (Power management base) (can go ?)
* OSPM info (Power management base) (TODO: can go ?) */ uint32_t ospm_base;@@ -766,9 +775,8 @@ extern void psb_mmu_remove_pages(struct psb_mmu_pd *pd, uint32_t desired_tile_stride, uint32_t hw_tile_stride); /*
- *psb_irq.c
*/
- psb_irq.c
extern irqreturn_t psb_irq_handler(int irq, void *arg); extern int psb_irq_enable_dpst(struct drm_device *dev); extern int psb_irq_disable_dpst(struct drm_device *dev); -- 1.9.0
Hi Patrik,
Thanks for reviewing.
* The 80 character comment slipped with all the 80+ pci definitions. Will fix. * Some single line comments were already before in multiple line comment style. Nevertheless, good moment to get rid of them. Will fix. * Although for a beginner like me, I believe inline DRM documentation makes reading the code easier. However, you are right that maintaining the same documentation twice is a waste of time. Will fix. * Driver definitions and driver history were copied from i915 to get more in line. Nevertheless, you might be right; it might be an overkill. Will fix. * SGU MMX comment will be merged. * The 72 column git commit makes sense. Maybe this explains why my editor was predefined with this number. ;) Will fix.
Be prepared for three new patches.
On 15 March 2014 00:04, Patrik Jakobsson patrik.r.jakobsson@gmail.comwrote:
On Thu, Mar 13, 2014 at 10:49 PM, Arthur Borsboom arthurborsboom@gmail.com wrote:
Improve readability by adding/changing inline documentation
Signed-off-by: Arthur Borsboom arthurborsboom@gmail.com
drivers/gpu/drm/gma500/psb_drv.c | 56
+++++++++++++++++++++++++++++++++-------
drivers/gpu/drm/gma500/psb_drv.h | 24 +++++++++++------ 2 files changed, 63 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_drv.c
b/drivers/gpu/drm/gma500/psb_drv.c
index 1199180..5c6cdd0 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -44,7 +44,20 @@ static int psb_probe(struct pci_dev *pdev, const
struct pci_device_id *ent);
MODULE_PARM_DESC(trap_pagefaults, "Error and reset on MMU pagefaults"); module_param_named(trap_pagefaults, drm_psb_trap_pagefaults, int, 0600);
+/*
- The table below contains a mapping of the PCI vendor ID and the PCI
Device ID
- to the different groups of PowerVR 5-series chip designs
- 0x8086 = Intel Corporation
- PowerVR SGX535 - Poulsbo - Intel GMA 500, Intel Atom Z5xx
- PowerVR SGX535 - Moorestown - Intel GMA 600
- PowerVR SGX535 - Oaktrail - Intel GMA 600, Intel Atom Z6xx, E6xx
- PowerVR SGX540 - Medfield - Intel Atom Z2460
- PowerVR SGX544MP2 - Medfield -
- PowerVR SGX545 - Cedartrail - Intel GMA 3600, Intel Atom D2500,
N2600
- PowerVR SGX545 - Cedartrail - Intel GMA 3650, Intel Atom D2550,
D2700, N2800
There is no reason to break the 80 col line limit here. Here is Linus' motivation if that makes more sense than mine: https://lkml.org/lkml/2012/2/3/394
And sometimes we break it by mistake, but that is a different thing.
- */
static DEFINE_PCI_DEVICE_TABLE(pciidlist) = { { 0x8086, 0x8108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long)
&psb_chip_ops },
{ 0x8086, 0x8109, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long)&psb_chip_ops },
@@ -207,8 +220,7 @@ static int psb_driver_unload(struct drm_device *dev) { struct drm_psb_private *dev_priv = dev->dev_private;
/* Kill vblank etc here */
/* TODO: Kill vblank etc here */ if (dev_priv) { if (dev_priv->backlight_device)@@ -268,7 +280,22 @@ static int psb_driver_unload(struct drm_device *dev) return 0; }
+/*
- psb_driver_load - setup chip and create an initial config
- @dev: DRM device
- @flags: startup flags, containing the driver_data field belonging to
the PCI device ID
- The driver load routine has to do several things:
- allocating and initializing driver private data
- performing resource allocation and mapping
- initialize the memory manager
- setup the DRM framebuffer with the allocated memory
- install the IRQ handler
- setup vertical blanking handling
- mode setting
- set inital output configuration
- */
This is not a constructive comment. It just repeats the DRM documentation. This might look harmless but it is actually a bad thing. For every comment we add to the driver, we add one more place that we must remember to update when we change our code. If you look at the driver, you'll see plenty of places where comments never got removed, even though the code it talks about is long gone.
And most of the time, the code speaks for itself.
static int psb_driver_load(struct drm_device *dev, unsigned long
chipset)
{ struct drm_psb_private *dev_priv; @@ -278,6 +305,7 @@ static int psb_driver_load(struct drm_device *dev,
unsigned long chipset)
struct drm_connector *connector; struct gma_encoder *gma_encoder;
/* allocating and initializing driver private data */ dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL); if (dev_priv == NULL) return -ENOMEM;@@ -369,6 +397,9 @@ static int psb_driver_load(struct drm_device *dev,
unsigned long chipset)
acpi_video_register();
/** Setup vertical blanking handling*/Single line comments should be just /* ... comment ... */ See Documentation/CodingStyle Chapter 8.
ret = drm_vblank_init(dev, dev_priv->num_pipe); if (ret) goto out_err;@@ -416,11 +447,11 @@ static int psb_driver_load(struct drm_device *dev,
unsigned long chipset)
return ret; psb_intel_opregion_enable_asle(dev);#if 0
/*enable runtime pm at last*/
/* Enable runtime pm at last */ pm_runtime_enable(&dev->pdev->dev); pm_runtime_set_active(&dev->pdev->dev);#endif
/*Intel drm driver load is done, continue doing pvr load*/
/* Intel drm driver load is done, continue doing pvr load */ return 0;out_err: psb_driver_unload(dev); @@ -561,7 +592,7 @@ static int psb_mode_operation_ioctl(struct
drm_device *dev, void *data,
arg->data = resp; }
/*do some clean up work*/
/* Do some clean up work */ if (mode) drm_mode_destroy(dev, mode);mode_op_out: @@ -614,8 +645,8 @@ static long psb_unlocked_ioctl(struct file *filp,
unsigned int cmd,
/* FIXME: do we need to wrap the other side of this */}
-/* When a client dies: +/*
*/
- When a client dies:
- Check for and clean up flipped page state
static void psb_driver_preclose(struct drm_device *dev, struct drm_file
*priv)
@@ -655,6 +686,13 @@ static const struct file_operations psb_gem_fops = { .read = drm_read, };
+/*
- DRM driver structure initialization
- The drm_driver structure contains static information that describes
- the driver and features it supports, and pointers to methods that DRM
- core will call to implement DRM API.
- */
This also just repeats the DRM documentation. No need for it.
static struct drm_driver driver = { .driver_features = DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | \ DRIVER_MODESET | DRIVER_GEM , diff --git a/drivers/gpu/drm/gma500/psb_drv.h
b/drivers/gpu/drm/gma500/psb_drv.h
index 5ad6a03..85c560e 100644 --- a/drivers/gpu/drm/gma500/psb_drv.h +++ b/drivers/gpu/drm/gma500/psb_drv.h @@ -34,6 +34,19 @@ #include "opregion.h" #include "oaktrail.h"
+/*
- Driver definitions
- */
This is just stating the obvious. The code speaks for itself.
+/*
- Driver history (deprecated)
Why _add_ a comment that it is deprecated when it never existed?
- The driver version was intended for ABI changes and it's not used
anymore.
- There are better ways to tell userspace what features we expose.
- TODO: describe better ways
- */
This can also go.
/* Append new drm mode definition here, align with libdrm definition */ #define DRM_MODE_SCALE_NO_SCALE 2
@@ -88,15 +101,11 @@ enum { #define _PSB_PGETBL_ENABLED 0x00000001 #define PSB_SGX_2D_SLAVE_PORT 0x4000
-/* To get rid of */ +/* TODO: To get rid of */ #define PSB_TT_PRIV0_LIMIT (256*1024*1024) #define PSB_TT_PRIV0_PLIMIT (PSB_TT_PRIV0_LIMIT >> PAGE_SHIFT)
/*
SGX side MMU definitions (these can probably go)- */
-/*
This comment is actually useful. It tells us that the MMU bits defined below are for the SGX MMU page table entries and not the Intel GTT PTEs. Though it could be combined with the comment below since they talk about the same thing.
Flags for external memory type field.*/ #define PSB_MMU_CACHED_MEMORY 0x0001 /* Bind to MMU only */ @@ -519,7 +528,7 @@ struct drm_psb_private { uint32_t num_pipe;
/*
* OSPM info (Power management base) (can go ?)
* OSPM info (Power management base) (TODO: can go ?) */ uint32_t ospm_base;@@ -766,9 +775,8 @@ extern void psb_mmu_remove_pages(struct psb_mmu_pd
*pd,
uint32_t desired_tile_stride, uint32_t hw_tile_stride);/*
- *psb_irq.c
*/
- psb_irq.c
extern irqreturn_t psb_irq_handler(int irq, void *arg); extern int psb_irq_enable_dpst(struct drm_device *dev); extern int psb_irq_disable_dpst(struct drm_device *dev); -- 1.9.0
dri-devel@lists.freedesktop.org
-
Arthur Borsboom -
Patrik Jakobsson