| Message ID | 1394536977-6814-1-git-send-email-arthurborsboom@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 11, 2014 at 12:22 PM, Arthur Borsboom <arthurborsboom@gmail.com> wrote: > drm/gma500: Cleanup of code by following i915 constant/variable names and ordering > drm/gma500: Cleanup of code by following directions from kernel documentation: Codingstyle > drm/gma500: Cleanup of code by following directions from kernel documentation: DRM > drm/gma500: Improve readability by adding inline documentation > drm/gma500: Removed centralized exiting of function (goto statement), since it was the only used in one single location with only a return statement. > > CC: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> > > Signed-off-by: Arthur Borsboom <arthurborsboom@gmail.com> Hi Arthur, Thanks for the patch. Could you split this up in three patches. One for the style fixes, one for documentation and one for the goto fix. Also, see my comments below. > --- > drivers/gpu/drm/gma500/psb_drv.c | 162 +++++++++++++++++++++------------------ > drivers/gpu/drm/gma500/psb_drv.h | 58 ++++++-------- > 2 files changed, 111 insertions(+), 109 deletions(-) > > diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c > index 1199180..0432136 100644 > --- a/drivers/gpu/drm/gma500/psb_drv.c > +++ b/drivers/gpu/drm/gma500/psb_drv.c > @@ -37,28 +37,42 @@ > #include <acpi/video.h> > #include <linux/module.h> > > +static struct drm_driver driver; > + Do we need this? Can't see anything referencing it before we define it. > 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); > > - > +/* > + * 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 Coorperation s/Coorperation/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 }, > + {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}, Why remove that space? Most drivers have it and also a space at the end. Don't think CodingStyle says anything about it anyway. > #endif > #if defined(CONFIG_DRM_MEDFIELD) > {0x8086, 0x0130, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) &mdfld_chip_ops}, > @@ -71,31 +85,30 @@ static DEFINE_PCI_DEVICE_TABLE(pciidlist) = { > {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, } > + {0,} > }; Same here. > 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 \ > @@ -147,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; > @@ -169,19 +182,14 @@ 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; > 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; > @@ -192,23 +200,19 @@ static int psb_do_init(struct drm_device *dev) > PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK0); > PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK1); > PSB_RSGX32(PSB_CR_BIF_BANK1); > - PSB_WSGX32(PSB_RSGX32(PSB_CR_BIF_CTRL) | _PSB_MMU_ER_MASK, > - PSB_CR_BIF_CTRL); > + PSB_WSGX32(PSB_RSGX32(PSB_CR_BIF_CTRL) | _PSB_MMU_ER_MASK, PSB_CR_BIF_CTRL); This breaks the 80 column line rule. See Documentation/CodingStyle > psb_spank(dev_priv); > > /* 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) > { > 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 +272,6 @@ static int psb_driver_unload(struct drm_device *dev) > return 0; > } > > - > static int psb_driver_load(struct drm_device *dev, unsigned long chipset) > { > struct drm_psb_private *dev_priv; > @@ -416,11 +419,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 +564,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: > @@ -593,7 +596,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) > { > } > > @@ -614,15 +617,21 @@ 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) > { > } > > -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); > @@ -655,13 +664,23 @@ 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 , > + 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, > @@ -671,40 +690,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); > @@ -718,6 +728,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 5ad6a03..3535640 100644 > --- a/drivers/gpu/drm/gma500/psb_drv.h > +++ b/drivers/gpu/drm/gma500/psb_drv.h > @@ -34,6 +34,25 @@ > #include "opregion.h" > #include "oaktrail.h" > > +/* > + * 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 "20140311" > + > +/* > + * Driver history > + * > + * 1.0: Original. Frambuffer only, no 2D/3D acceleration, no video acceleration, no power management. > + */ We don't do internal driver versioning anymore. Stuff like this tends to get forgotten and out-dated anyway. > +#define DRIVER_MAJOR 1 > +#define DRIVER_MINOR 0 > +#define DRIVER_PATCHLEVEL 1 > + Why bump the patch level? > /* Append new drm mode definition here, align with libdrm definition */ > #define DRM_MODE_SCALE_NO_SCALE 2 > > @@ -50,18 +69,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 > @@ -71,6 +78,7 @@ enum { > #define PSB_SGX_SIZE 0x8000 > #define PSB_SGX_OFFSET 0x00040000 > #define MRST_SGX_OFFSET 0x00080000 > + > /* > * PCI resource identifiers > */ > @@ -78,6 +86,7 @@ enum { > #define PSB_AUX_RESOURCE 0 > #define PSB_GATT_RESOURCE 2 > #define PSB_GTT_RESOURCE 3 > + > /* > * PCI configuration > */ > @@ -88,26 +97,24 @@ 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 */ > #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 > */ > @@ -286,7 +293,6 @@ struct intel_gmbus { > /* > * Register offset maps > */ > - > struct psb_offset { > u32 fp0; > u32 fp1; > @@ -485,7 +491,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 */ > @@ -494,7 +499,6 @@ struct drm_psb_private { > /* > * Fencing / irq. > */ > - > uint32_t vdc_irq_mask; > uint32_t pipestat[PSB_NUM_PIPE]; > > @@ -503,7 +507,6 @@ struct drm_psb_private { > /* > * Power > */ > - > bool suspended; > bool display_power; > int display_count; > @@ -526,7 +529,6 @@ struct drm_psb_private { > /* > * Sizes info > */ > - > u32 fuse_reg_value; > u32 video_device_fuse; > > @@ -585,7 +587,6 @@ struct drm_psb_private { > /* > * Register state > */ > - > struct psb_save_area regs; > > /* MSI reg save */ > @@ -595,7 +596,6 @@ struct drm_psb_private { > /* > * Hotplug handling > */ > - > struct work_struct hotplug_work; > > /* > @@ -609,7 +609,6 @@ struct drm_psb_private { > /* > * Watchdog > */ > - > uint32_t apm_reg; > uint16_t apm_base; > > @@ -667,7 +666,6 @@ struct drm_psb_private { > /* > * Operations for each board type > */ > - > struct psb_ops { > const char *name; > unsigned int accel_2d:1; > @@ -726,7 +724,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, > @@ -754,8 +751,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, > @@ -766,9 +761,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); > @@ -808,7 +802,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); > @@ -888,7 +881,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 >
Hi Arthur, Please keep the mailing list in CC (It's easily dropped when replying in Gmail). Forgot to tell you, you only need the drm/gma500 tags in the subject line. Not needed in the message itself. On Wed, Mar 12, 2014 at 9:04 AM, Arthur Borsboom <arthurborsboom@gmail.com>wrote: > > Split up patch in 3 pieces > > OK > > "static struct drm_driver driver;" needed? > > Yes it became needed, since I moved up the function 'psb_pci_probe' where > it is used, before the creation of the struct (to get more in line with the > i915). The same line is also in the i915 driver, I guess for the same > reason. > Ok > > Intel Corporation > > OK > > > PCI device table, why remove the spacing? > > I noticed that in our PCI device table the spaces were inconsistent, so I > decided to make them all equal. The CodingStyleGuide says nothing about > brackets, but they do about parenthesis, were in general you don't put > spaces. I don't mind either of them, so I will add the spaces to the > beginning and the end. OK > > > This breaks the 80 column line rule. See Documentation/CodingStyle > > This is correct; I saw the warning too when running the checkpatch.plscript. A lot of our driver code contains lines which are a little bit > longer than 80 characters. I know the CodeGuide states that the code should > be readable on a 80x25 screen. But honestly, which developer has actually > only 80 columns on his screen? Even with a crappy netbook like mine, with a > 10" screen and a resolution of 1024x600, in a graphical editor, nano or > emacs I still have more than 80 columns (117 in the GUI to be exact). So, > the only moment when you have 80 columns is when your videocard driver is > failing and it falls back to VGA 80x25. So, IMHO, if a line is 85 or 90 > columns long, and it improves readability, I would make an exception. I > believe in this case, it improves readability. Nevertheless, you are making > the call as the responsible person for this driver. So, after my aaawesome > speech, do you want to keep this line longer than 80 char or make me cut it > down to less than 80? > > Some people (including me) keep two files open next to each other. So the 80 col line rule is still applicable. > We don't do internal driver versioning anymore. > > I didn't know that, however, if you are not bothered by it, I like to keep > doing it (at least for me). When I go back in my log files, for example > dmesg, I can see the version number and driver date. This might help me > finding a regression by these log files. If you rather have it not changed, > I will put it back. > 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. For your dmesg case, just look at the kernel version and if needed, name your files appropriately. > > Why bump the patch level? > > IMO, every code change bumps the version number somehow. Since the change > is very very small, major and minor seem to be incorrect, so I believe the > smallest change number is the patchlevel. > > Thanks for the review Patrik! > > In the beginning I guess we will have small talks like this, and after a > while we are on the same line, I guess. ;) > Yes, there is a lot of "process" and it takes a while to get used to it. Documentation/SubmittingPatches is a good read if you haven't already looked at it. > Cheers, > Arthur. > > > On 12 March 2014 01:03, Patrik Jakobsson <patrik.r.jakobsson@gmail.com>wrote: > >> On Tue, Mar 11, 2014 at 12:22 PM, Arthur Borsboom >> <arthurborsboom@gmail.com> wrote: >> > drm/gma500: Cleanup of code by following i915 constant/variable names >> and ordering >> > drm/gma500: Cleanup of code by following directions from kernel >> documentation: Codingstyle >> > drm/gma500: Cleanup of code by following directions from kernel >> documentation: DRM >> > drm/gma500: Improve readability by adding inline documentation >> > drm/gma500: Removed centralized exiting of function (goto statement), >> since it was the only used in one single location with only a return >> statement. >> > >> > CC: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> >> > >> > Signed-off-by: Arthur Borsboom <arthurborsboom@gmail.com> >> >> Hi Arthur, >> >> Thanks for the patch. Could you split this up in three patches. >> One for the style fixes, one for documentation and one for the goto fix. >> >> Also, see my comments below. >> >> > --- >> > drivers/gpu/drm/gma500/psb_drv.c | 162 >> +++++++++++++++++++++------------------ >> > drivers/gpu/drm/gma500/psb_drv.h | 58 ++++++-------- >> > 2 files changed, 111 insertions(+), 109 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/gma500/psb_drv.c >> b/drivers/gpu/drm/gma500/psb_drv.c >> > index 1199180..0432136 100644 >> > --- a/drivers/gpu/drm/gma500/psb_drv.c >> > +++ b/drivers/gpu/drm/gma500/psb_drv.c >> > @@ -37,28 +37,42 @@ >> > #include <acpi/video.h> >> > #include <linux/module.h> >> > >> > +static struct drm_driver driver; >> > + >> >> Do we need this? Can't see anything referencing it before we define it. >> >> > 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); >> > >> > - >> > +/* >> > + * 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 Coorperation >> >> s/Coorperation/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 }, >> > + {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}, >> >> Why remove that space? Most drivers have it and also a space at the end. >> Don't think CodingStyle says anything about it anyway. >> >> > #endif >> > #if defined(CONFIG_DRM_MEDFIELD) >> > {0x8086, 0x0130, PCI_ANY_ID, PCI_ANY_ID, 0, 0, (long) >> &mdfld_chip_ops}, >> > @@ -71,31 +85,30 @@ static DEFINE_PCI_DEVICE_TABLE(pciidlist) = { >> > {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, } >> > + {0,} >> > }; >> >> Same here. >> >> > 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 \ >> > @@ -147,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; >> > @@ -169,19 +182,14 @@ 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; >> > 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; >> > @@ -192,23 +200,19 @@ static int psb_do_init(struct drm_device *dev) >> > PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK0); >> > PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK1); >> > PSB_RSGX32(PSB_CR_BIF_BANK1); >> > - PSB_WSGX32(PSB_RSGX32(PSB_CR_BIF_CTRL) | _PSB_MMU_ER_MASK, >> > - >> PSB_CR_BIF_CTRL); >> > + PSB_WSGX32(PSB_RSGX32(PSB_CR_BIF_CTRL) | _PSB_MMU_ER_MASK, >> PSB_CR_BIF_CTRL); >> >> This breaks the 80 column line rule. See Documentation/CodingStyle >> >> > psb_spank(dev_priv); >> > >> > /* 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) >> > { >> > 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 +272,6 @@ static int psb_driver_unload(struct drm_device *dev) >> > return 0; >> > } >> > >> > - >> > static int psb_driver_load(struct drm_device *dev, unsigned long >> chipset) >> > { >> > struct drm_psb_private *dev_priv; >> > @@ -416,11 +419,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 +564,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: >> > @@ -593,7 +596,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) >> > { >> > } >> > >> > @@ -614,15 +617,21 @@ 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) >> > { >> > } >> > >> > -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); >> > @@ -655,13 +664,23 @@ 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 , >> > + 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, >> > @@ -671,40 +690,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); >> > @@ -718,6 +728,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 5ad6a03..3535640 100644 >> > --- a/drivers/gpu/drm/gma500/psb_drv.h >> > +++ b/drivers/gpu/drm/gma500/psb_drv.h >> > @@ -34,6 +34,25 @@ >> > #include "opregion.h" >> > #include "oaktrail.h" >> > >> > +/* >> > + * 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 "20140311" >> > + >> > +/* >> > + * Driver history >> > + * >> > + * 1.0: Original. Frambuffer only, no 2D/3D acceleration, no video >> acceleration, no power management. >> > + */ >> >> We don't do internal driver versioning anymore. Stuff like this tends to >> get >> forgotten and out-dated anyway. >> >> > +#define DRIVER_MAJOR 1 >> > +#define DRIVER_MINOR 0 >> > +#define DRIVER_PATCHLEVEL 1 >> > + >> >> Why bump the patch level? >> >> > /* Append new drm mode definition here, align with libdrm definition */ >> > #define DRM_MODE_SCALE_NO_SCALE 2 >> > >> > @@ -50,18 +69,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 >> > @@ -71,6 +78,7 @@ enum { >> > #define PSB_SGX_SIZE 0x8000 >> > #define PSB_SGX_OFFSET 0x00040000 >> > #define MRST_SGX_OFFSET 0x00080000 >> > + >> > /* >> > * PCI resource identifiers >> > */ >> > @@ -78,6 +86,7 @@ enum { >> > #define PSB_AUX_RESOURCE 0 >> > #define PSB_GATT_RESOURCE 2 >> > #define PSB_GTT_RESOURCE 3 >> > + >> > /* >> > * PCI configuration >> > */ >> > @@ -88,26 +97,24 @@ 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 */ >> > #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 >> > */ >> > @@ -286,7 +293,6 @@ struct intel_gmbus { >> > /* >> > * Register offset maps >> > */ >> > - >> > struct psb_offset { >> > u32 fp0; >> > u32 fp1; >> > @@ -485,7 +491,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 */ >> > @@ -494,7 +499,6 @@ struct drm_psb_private { >> > /* >> > * Fencing / irq. >> > */ >> > - >> > uint32_t vdc_irq_mask; >> > uint32_t pipestat[PSB_NUM_PIPE]; >> > >> > @@ -503,7 +507,6 @@ struct drm_psb_private { >> > /* >> > * Power >> > */ >> > - >> > bool suspended; >> > bool display_power; >> > int display_count; >> > @@ -526,7 +529,6 @@ struct drm_psb_private { >> > /* >> > * Sizes info >> > */ >> > - >> > u32 fuse_reg_value; >> > u32 video_device_fuse; >> > >> > @@ -585,7 +587,6 @@ struct drm_psb_private { >> > /* >> > * Register state >> > */ >> > - >> > struct psb_save_area regs; >> > >> > /* MSI reg save */ >> > @@ -595,7 +596,6 @@ struct drm_psb_private { >> > /* >> > * Hotplug handling >> > */ >> > - >> > struct work_struct hotplug_work; >> > >> > /* >> > @@ -609,7 +609,6 @@ struct drm_psb_private { >> > /* >> > * Watchdog >> > */ >> > - >> > uint32_t apm_reg; >> > uint16_t apm_base; >> > >> > @@ -667,7 +666,6 @@ struct drm_psb_private { >> > /* >> > * Operations for each board type >> > */ >> > - >> > struct psb_ops { >> > const char *name; >> > unsigned int accel_2d:1; >> > @@ -726,7 +724,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, >> > @@ -754,8 +751,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, >> > @@ -766,9 +761,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); >> > @@ -808,7 +802,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); >> > @@ -888,7 +881,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 >> > >> > > > > -- > Arthur Borsboom > 11 Rue du Manerick > 44740 Batz Sur Mer, France > Mob: +33785927118 > Email: arthurborsboom@gmail.com > Skype: Arthur Borsboom, The Hague, The Netherlands > > [image: View Arthur's LinkedIn profile]<http://uk.linkedin.com/in/arthurborsboom> >
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 1199180..0432136 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -37,28 +37,42 @@ #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); - +/* + * 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 Coorperation + * + * 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 }, + {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}, @@ -71,31 +85,30 @@ static DEFINE_PCI_DEVICE_TABLE(pciidlist) = { {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, } + {0,} }; 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 \ @@ -147,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; @@ -169,19 +182,14 @@ 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; 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; @@ -192,23 +200,19 @@ static int psb_do_init(struct drm_device *dev) PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK0); PSB_WSGX32(0x00000000, PSB_CR_BIF_BANK1); PSB_RSGX32(PSB_CR_BIF_BANK1); - PSB_WSGX32(PSB_RSGX32(PSB_CR_BIF_CTRL) | _PSB_MMU_ER_MASK, - PSB_CR_BIF_CTRL); + PSB_WSGX32(PSB_RSGX32(PSB_CR_BIF_CTRL) | _PSB_MMU_ER_MASK, PSB_CR_BIF_CTRL); psb_spank(dev_priv); /* 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) { 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 +272,6 @@ static int psb_driver_unload(struct drm_device *dev) return 0; } - static int psb_driver_load(struct drm_device *dev, unsigned long chipset) { struct drm_psb_private *dev_priv; @@ -416,11 +419,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 +564,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: @@ -593,7 +596,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) { } @@ -614,15 +617,21 @@ 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) { } -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); @@ -655,13 +664,23 @@ 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 , + 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, @@ -671,40 +690,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); @@ -718,6 +728,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 5ad6a03..3535640 100644 --- a/drivers/gpu/drm/gma500/psb_drv.h +++ b/drivers/gpu/drm/gma500/psb_drv.h @@ -34,6 +34,25 @@ #include "opregion.h" #include "oaktrail.h" +/* + * 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 "20140311" + +/* + * Driver history + * + * 1.0: Original. Frambuffer only, no 2D/3D acceleration, no video acceleration, no power management. + */ +#define DRIVER_MAJOR 1 +#define DRIVER_MINOR 0 +#define DRIVER_PATCHLEVEL 1 + /* Append new drm mode definition here, align with libdrm definition */ #define DRM_MODE_SCALE_NO_SCALE 2 @@ -50,18 +69,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 @@ -71,6 +78,7 @@ enum { #define PSB_SGX_SIZE 0x8000 #define PSB_SGX_OFFSET 0x00040000 #define MRST_SGX_OFFSET 0x00080000 + /* * PCI resource identifiers */ @@ -78,6 +86,7 @@ enum { #define PSB_AUX_RESOURCE 0 #define PSB_GATT_RESOURCE 2 #define PSB_GTT_RESOURCE 3 + /* * PCI configuration */ @@ -88,26 +97,24 @@ 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 */ #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 */ @@ -286,7 +293,6 @@ struct intel_gmbus { /* * Register offset maps */ - struct psb_offset { u32 fp0; u32 fp1; @@ -485,7 +491,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 */ @@ -494,7 +499,6 @@ struct drm_psb_private { /* * Fencing / irq. */ - uint32_t vdc_irq_mask; uint32_t pipestat[PSB_NUM_PIPE]; @@ -503,7 +507,6 @@ struct drm_psb_private { /* * Power */ - bool suspended; bool display_power; int display_count; @@ -526,7 +529,6 @@ struct drm_psb_private { /* * Sizes info */ - u32 fuse_reg_value; u32 video_device_fuse; @@ -585,7 +587,6 @@ struct drm_psb_private { /* * Register state */ - struct psb_save_area regs; /* MSI reg save */ @@ -595,7 +596,6 @@ struct drm_psb_private { /* * Hotplug handling */ - struct work_struct hotplug_work; /* @@ -609,7 +609,6 @@ struct drm_psb_private { /* * Watchdog */ - uint32_t apm_reg; uint16_t apm_base; @@ -667,7 +666,6 @@ struct drm_psb_private { /* * Operations for each board type */ - struct psb_ops { const char *name; unsigned int accel_2d:1; @@ -726,7 +724,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, @@ -754,8 +751,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, @@ -766,9 +761,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); @@ -808,7 +802,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); @@ -888,7 +881,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);
drm/gma500: Cleanup of code by following i915 constant/variable names and ordering drm/gma500: Cleanup of code by following directions from kernel documentation: Codingstyle drm/gma500: Cleanup of code by following directions from kernel documentation: DRM drm/gma500: Improve readability by adding inline documentation drm/gma500: Removed centralized exiting of function (goto statement), since it was the only used in one single location with only a return statement. CC: Patrik Jakobsson <patrik.r.jakobsson@gmail.com> Signed-off-by: Arthur Borsboom <arthurborsboom@gmail.com> --- drivers/gpu/drm/gma500/psb_drv.c | 162 +++++++++++++++++++++------------------ drivers/gpu/drm/gma500/psb_drv.h | 58 ++++++-------- 2 files changed, 111 insertions(+), 109 deletions(-)