Version 2: - Add devm_reset_control_get_exclusive to follow explicit reset API - Add missing commit messages & reviewed-by.
Version 1: - Initial commit
The purpose of this set of patches is to clean up the drm stm driver.
Philippe CORNU (7): drm/stm: drv: Rename platform driver name drm/stm: ltdc: Cleanup signal polarity defines drm/stm: ltdc: Lindent and minor cleanups drm/stm: ltdc: Constify funcs structures drm/stm: ltdc: add devm_reset_control & platform_get_ressource drm/stm: ltdc: Cleanup rename returned value drm/stm: dsi: Constify phy ops structure
drivers/gpu/drm/stm/drv.c | 21 ++-- drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 2 +- drivers/gpu/drm/stm/ltdc.c | 225 ++++++++++++++++------------------ 3 files changed, 116 insertions(+), 132 deletions(-)
Rename the platform driver name from "stm" to "stm32-display" for a better readability in /sys/bus/platform/drivers entries.
Note: We keep "stm" as drm_driver.name because it is better when using "modetest -M stm ..." (even if recent modetest patch avoids using -M).
Signed-off-by: Philippe CORNU philippe.cornu@st.com Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/drm/stm/drv.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c index 83ab48f..095971f 100644 --- a/drivers/gpu/drm/stm/drv.c +++ b/drivers/gpu/drm/stm/drv.c @@ -20,13 +20,6 @@
#include "ltdc.h"
-#define DRIVER_NAME "stm" -#define DRIVER_DESC "STMicroelectronics SoC DRM" -#define DRIVER_DATE "20170330" -#define DRIVER_MAJOR 1 -#define DRIVER_MINOR 0 -#define DRIVER_PATCH_LEVEL 0 - #define STM_MAX_FB_WIDTH 2048 #define STM_MAX_FB_HEIGHT 2048 /* same as width to handle orientation */
@@ -59,12 +52,12 @@ static void drv_lastclose(struct drm_device *ddev) .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC, .lastclose = drv_lastclose, - .name = DRIVER_NAME, - .desc = DRIVER_DESC, - .date = DRIVER_DATE, - .major = DRIVER_MAJOR, - .minor = DRIVER_MINOR, - .patchlevel = DRIVER_PATCH_LEVEL, + .name = "stm", + .desc = "STMicroelectronics SoC DRM", + .date = "20170330", + .major = 1, + .minor = 0, + .patchlevel = 0, .fops = &drv_driver_fops, .dumb_create = drm_gem_cma_dumb_create, .dumb_map_offset = drm_gem_cma_dumb_map_offset, @@ -206,7 +199,7 @@ static int stm_drm_platform_remove(struct platform_device *pdev) .probe = stm_drm_platform_probe, .remove = stm_drm_platform_remove, .driver = { - .name = DRIVER_NAME, + .name = "stm32-display", .of_match_table = drv_dt_ids, }, };
The GCR_PCPOL/DEPOL/VSPOL/HSPOL defines are sufficient to describe the HS, VS, DE & PC signal polarities.
Signed-off-by: Philippe CORNU philippe.cornu@st.com Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/drm/stm/ltdc.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index e46b427..50e8a89 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -104,10 +104,10 @@
#define GCR_LTDCEN BIT(0) /* LTDC ENable */ #define GCR_DEN BIT(16) /* Dither ENable */ -#define GCR_PCPOL BIT(28) /* Pixel Clock POLarity */ -#define GCR_DEPOL BIT(29) /* Data Enable POLarity */ -#define GCR_VSPOL BIT(30) /* Vertical Synchro POLarity */ -#define GCR_HSPOL BIT(31) /* Horizontal Synchro POLarity */ +#define GCR_PCPOL BIT(28) /* Pixel Clock POLarity-Inverted */ +#define GCR_DEPOL BIT(29) /* Data Enable POLarity-High */ +#define GCR_VSPOL BIT(30) /* Vertical Synchro POLarity-High */ +#define GCR_HSPOL BIT(31) /* Horizontal Synchro POLarity-High */
#define GC1R_WBCH GENMASK(3, 0) /* Width of Blue CHannel output */ #define GC1R_WGCH GENMASK(7, 4) /* Width of Green Channel output */ @@ -174,14 +174,6 @@
#define LXCFBLNR_CFBLN GENMASK(10, 0) /* Color Frame Buffer Line Number */
-#define HSPOL_AL 0 /* Horizontal Sync POLarity Active Low */ -#define VSPOL_AL 0 /* Vertical Sync POLarity Active Low */ -#define DEPOL_AL 0 /* Data Enable POLarity Active Low */ -#define PCPOL_IPC 0 /* Input Pixel Clock */ -#define HSPOL_AH GCR_HSPOL /* Horizontal Sync POLarity Active High */ -#define VSPOL_AH GCR_VSPOL /* Vertical Sync POLarity Active High */ -#define DEPOL_AH GCR_DEPOL /* Data Enable POLarity Active High */ -#define PCPOL_IIPC GCR_PCPOL /* Inverted Input Pixel Clock */ #define CONSTA_MAX 0xFF /* CONSTant Alpha MAX= 1.0 */ #define BF1_PAXCA 0x600 /* Pixel Alpha x Constant Alpha */ #define BF1_CA 0x400 /* Constant Alpha */ @@ -459,20 +451,20 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
clk_enable(ldev->pixel_clk);
- /* Configures the HS, VS, DE and PC polarities. */ - val = HSPOL_AL | VSPOL_AL | DEPOL_AL | PCPOL_IPC; + /* Configures the HS, VS, DE and PC polarities. Default Active Low */ + val = 0;
if (vm.flags & DISPLAY_FLAGS_HSYNC_HIGH) - val |= HSPOL_AH; + val |= GCR_HSPOL;
if (vm.flags & DISPLAY_FLAGS_VSYNC_HIGH) - val |= VSPOL_AH; + val |= GCR_VSPOL;
if (vm.flags & DISPLAY_FLAGS_DE_HIGH) - val |= DEPOL_AH; + val |= GCR_DEPOL;
if (vm.flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE) - val |= PCPOL_IIPC; + val |= GCR_PCPOL;
reg_update_bits(ldev->regs, LTDC_GCR, GCR_HSPOL | GCR_VSPOL | GCR_DEPOL | GCR_PCPOL, val);
Lindent then checkpatch --strict cleanups
Signed-off-by: Philippe CORNU philippe.cornu@st.com Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/drm/stm/ltdc.c | 172 ++++++++++++++++++++++----------------------- 1 file changed, 85 insertions(+), 87 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 50e8a89..628825b 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -42,52 +42,52 @@ * an extra offset specified with reg_ofs. */ #define REG_OFS_NONE 0 -#define REG_OFS_4 4 /* Insertion of "Layer Configuration 2" reg */ +#define REG_OFS_4 4 /* Insertion of "Layer Conf. 2" reg */ #define REG_OFS (ldev->caps.reg_ofs) -#define LAY_OFS 0x80 /* Register Offset between 2 layers */ +#define LAY_OFS 0x80 /* Register Offset between 2 layers */
/* Global register offsets */ -#define LTDC_IDR 0x0000 /* IDentification */ -#define LTDC_LCR 0x0004 /* Layer Count */ -#define LTDC_SSCR 0x0008 /* Synchronization Size Configuration */ -#define LTDC_BPCR 0x000C /* Back Porch Configuration */ -#define LTDC_AWCR 0x0010 /* Active Width Configuration */ -#define LTDC_TWCR 0x0014 /* Total Width Configuration */ -#define LTDC_GCR 0x0018 /* Global Control */ -#define LTDC_GC1R 0x001C /* Global Configuration 1 */ -#define LTDC_GC2R 0x0020 /* Global Configuration 2 */ -#define LTDC_SRCR 0x0024 /* Shadow Reload Configuration */ -#define LTDC_GACR 0x0028 /* GAmma Correction */ -#define LTDC_BCCR 0x002C /* Background Color Configuration */ -#define LTDC_IER 0x0034 /* Interrupt Enable */ -#define LTDC_ISR 0x0038 /* Interrupt Status */ -#define LTDC_ICR 0x003C /* Interrupt Clear */ -#define LTDC_LIPCR 0x0040 /* Line Interrupt Position Configuration */ -#define LTDC_CPSR 0x0044 /* Current Position Status */ -#define LTDC_CDSR 0x0048 /* Current Display Status */ +#define LTDC_IDR 0x0000 /* IDentification */ +#define LTDC_LCR 0x0004 /* Layer Count */ +#define LTDC_SSCR 0x0008 /* Synchronization Size Configuration */ +#define LTDC_BPCR 0x000C /* Back Porch Configuration */ +#define LTDC_AWCR 0x0010 /* Active Width Configuration */ +#define LTDC_TWCR 0x0014 /* Total Width Configuration */ +#define LTDC_GCR 0x0018 /* Global Control */ +#define LTDC_GC1R 0x001C /* Global Configuration 1 */ +#define LTDC_GC2R 0x0020 /* Global Configuration 2 */ +#define LTDC_SRCR 0x0024 /* Shadow Reload Configuration */ +#define LTDC_GACR 0x0028 /* GAmma Correction */ +#define LTDC_BCCR 0x002C /* Background Color Configuration */ +#define LTDC_IER 0x0034 /* Interrupt Enable */ +#define LTDC_ISR 0x0038 /* Interrupt Status */ +#define LTDC_ICR 0x003C /* Interrupt Clear */ +#define LTDC_LIPCR 0x0040 /* Line Interrupt Position Conf. */ +#define LTDC_CPSR 0x0044 /* Current Position Status */ +#define LTDC_CDSR 0x0048 /* Current Display Status */
/* Layer register offsets */ -#define LTDC_L1LC1R (0x0080) /* L1 Layer Configuration 1 */ -#define LTDC_L1LC2R (0x0084) /* L1 Layer Configuration 2 */ -#define LTDC_L1CR (0x0084 + REG_OFS) /* L1 Control */ -#define LTDC_L1WHPCR (0x0088 + REG_OFS) /* L1 Window Hor Position Config */ -#define LTDC_L1WVPCR (0x008C + REG_OFS) /* L1 Window Vert Position Config */ -#define LTDC_L1CKCR (0x0090 + REG_OFS) /* L1 Color Keying Configuration */ -#define LTDC_L1PFCR (0x0094 + REG_OFS) /* L1 Pixel Format Configuration */ -#define LTDC_L1CACR (0x0098 + REG_OFS) /* L1 Constant Alpha Config */ -#define LTDC_L1DCCR (0x009C + REG_OFS) /* L1 Default Color Configuration */ -#define LTDC_L1BFCR (0x00A0 + REG_OFS) /* L1 Blend Factors Configuration */ -#define LTDC_L1FBBCR (0x00A4 + REG_OFS) /* L1 FrameBuffer Bus Control */ -#define LTDC_L1AFBCR (0x00A8 + REG_OFS) /* L1 AuxFB Control */ -#define LTDC_L1CFBAR (0x00AC + REG_OFS) /* L1 Color FrameBuffer Address */ -#define LTDC_L1CFBLR (0x00B0 + REG_OFS) /* L1 Color FrameBuffer Length */ -#define LTDC_L1CFBLNR (0x00B4 + REG_OFS) /* L1 Color FrameBuffer Line Nb */ -#define LTDC_L1AFBAR (0x00B8 + REG_OFS) /* L1 AuxFB Address */ -#define LTDC_L1AFBLR (0x00BC + REG_OFS) /* L1 AuxFB Length */ -#define LTDC_L1AFBLNR (0x00C0 + REG_OFS) /* L1 AuxFB Line Number */ -#define LTDC_L1CLUTWR (0x00C4 + REG_OFS) /* L1 CLUT Write */ -#define LTDC_L1YS1R (0x00E0 + REG_OFS) /* L1 YCbCr Scale 1 */ -#define LTDC_L1YS2R (0x00E4 + REG_OFS) /* L1 YCbCr Scale 2 */ +#define LTDC_L1LC1R (0x80) /* L1 Layer Configuration 1 */ +#define LTDC_L1LC2R (0x84) /* L1 Layer Configuration 2 */ +#define LTDC_L1CR (0x84 + REG_OFS)/* L1 Control */ +#define LTDC_L1WHPCR (0x88 + REG_OFS)/* L1 Window Hor Position Config */ +#define LTDC_L1WVPCR (0x8C + REG_OFS)/* L1 Window Vert Position Config */ +#define LTDC_L1CKCR (0x90 + REG_OFS)/* L1 Color Keying Configuration */ +#define LTDC_L1PFCR (0x94 + REG_OFS)/* L1 Pixel Format Configuration */ +#define LTDC_L1CACR (0x98 + REG_OFS)/* L1 Constant Alpha Config */ +#define LTDC_L1DCCR (0x9C + REG_OFS)/* L1 Default Color Configuration */ +#define LTDC_L1BFCR (0xA0 + REG_OFS)/* L1 Blend Factors Configuration */ +#define LTDC_L1FBBCR (0xA4 + REG_OFS)/* L1 FrameBuffer Bus Control */ +#define LTDC_L1AFBCR (0xA8 + REG_OFS)/* L1 AuxFB Control */ +#define LTDC_L1CFBAR (0xAC + REG_OFS)/* L1 Color FrameBuffer Address */ +#define LTDC_L1CFBLR (0xB0 + REG_OFS)/* L1 Color FrameBuffer Length */ +#define LTDC_L1CFBLNR (0xB4 + REG_OFS)/* L1 Color FrameBuffer Line Nb */ +#define LTDC_L1AFBAR (0xB8 + REG_OFS)/* L1 AuxFB Address */ +#define LTDC_L1AFBLR (0xBC + REG_OFS)/* L1 AuxFB Length */ +#define LTDC_L1AFBLNR (0xC0 + REG_OFS)/* L1 AuxFB Line Number */ +#define LTDC_L1CLUTWR (0xC4 + REG_OFS)/* L1 CLUT Write */ +#define LTDC_L1YS1R (0xE0 + REG_OFS)/* L1 YCbCr Scale 1 */ +#define LTDC_L1YS2R (0xE4 + REG_OFS)/* L1 YCbCr Scale 2 */
/* Bit definitions */ #define SSCR_VSH GENMASK(10, 0) /* Vertical Synchronization Height */ @@ -172,52 +172,52 @@ #define LXCFBLR_CFBLL GENMASK(12, 0) /* Color Frame Buffer Line Length */ #define LXCFBLR_CFBP GENMASK(28, 16) /* Color Frame Buffer Pitch in bytes */
-#define LXCFBLNR_CFBLN GENMASK(10, 0) /* Color Frame Buffer Line Number */ +#define LXCFBLNR_CFBLN GENMASK(10, 0) /* Color Frame Buffer Line Number */
-#define CONSTA_MAX 0xFF /* CONSTant Alpha MAX= 1.0 */ -#define BF1_PAXCA 0x600 /* Pixel Alpha x Constant Alpha */ -#define BF1_CA 0x400 /* Constant Alpha */ -#define BF2_1PAXCA 0x007 /* 1 - (Pixel Alpha x Constant Alpha) */ -#define BF2_1CA 0x005 /* 1 - Constant Alpha */ +#define CONSTA_MAX 0xFF /* CONSTant Alpha MAX= 1.0 */ +#define BF1_PAXCA 0x600 /* Pixel Alpha x Constant Alpha */ +#define BF1_CA 0x400 /* Constant Alpha */ +#define BF2_1PAXCA 0x007 /* 1 - (Pixel Alpha x Constant Alpha) */ +#define BF2_1CA 0x005 /* 1 - Constant Alpha */
-#define NB_PF 8 /* Max nb of HW pixel format */ +#define NB_PF 8 /* Max nb of HW pixel format */
enum ltdc_pix_fmt { PF_NONE, /* RGB formats */ - PF_ARGB8888, /* ARGB [32 bits] */ - PF_RGBA8888, /* RGBA [32 bits] */ - PF_RGB888, /* RGB [24 bits] */ - PF_RGB565, /* RGB [16 bits] */ - PF_ARGB1555, /* ARGB A:1 bit RGB:15 bits [16 bits] */ - PF_ARGB4444, /* ARGB A:4 bits R/G/B: 4 bits each [16 bits] */ + PF_ARGB8888, /* ARGB [32 bits] */ + PF_RGBA8888, /* RGBA [32 bits] */ + PF_RGB888, /* RGB [24 bits] */ + PF_RGB565, /* RGB [16 bits] */ + PF_ARGB1555, /* ARGB A:1 bit RGB:15 bits [16 bits] */ + PF_ARGB4444, /* ARGB A:4 bits R/G/B: 4 bits each [16 bits] */ /* Indexed formats */ - PF_L8, /* Indexed 8 bits [8 bits] */ - PF_AL44, /* Alpha:4 bits + indexed 4 bits [8 bits] */ - PF_AL88 /* Alpha:8 bits + indexed 8 bits [16 bits] */ + PF_L8, /* Indexed 8 bits [8 bits] */ + PF_AL44, /* Alpha:4 bits + indexed 4 bits [8 bits] */ + PF_AL88 /* Alpha:8 bits + indexed 8 bits [16 bits] */ };
/* The index gives the encoding of the pixel format for an HW version */ static const enum ltdc_pix_fmt ltdc_pix_fmt_a0[NB_PF] = { - PF_ARGB8888, /* 0x00 */ - PF_RGB888, /* 0x01 */ - PF_RGB565, /* 0x02 */ - PF_ARGB1555, /* 0x03 */ - PF_ARGB4444, /* 0x04 */ - PF_L8, /* 0x05 */ - PF_AL44, /* 0x06 */ - PF_AL88 /* 0x07 */ + PF_ARGB8888, /* 0x00 */ + PF_RGB888, /* 0x01 */ + PF_RGB565, /* 0x02 */ + PF_ARGB1555, /* 0x03 */ + PF_ARGB4444, /* 0x04 */ + PF_L8, /* 0x05 */ + PF_AL44, /* 0x06 */ + PF_AL88 /* 0x07 */ };
static const enum ltdc_pix_fmt ltdc_pix_fmt_a1[NB_PF] = { - PF_ARGB8888, /* 0x00 */ - PF_RGB888, /* 0x01 */ - PF_RGB565, /* 0x02 */ - PF_RGBA8888, /* 0x03 */ - PF_AL44, /* 0x04 */ - PF_L8, /* 0x05 */ - PF_ARGB1555, /* 0x06 */ - PF_ARGB4444 /* 0x07 */ + PF_ARGB8888, /* 0x00 */ + PF_RGB888, /* 0x01 */ + PF_RGB565, /* 0x02 */ + PF_RGBA8888, /* 0x03 */ + PF_AL44, /* 0x04 */ + PF_L8, /* 0x05 */ + PF_ARGB1555, /* 0x06 */ + PF_ARGB4444 /* 0x07 */ };
static inline u32 reg_read(void __iomem *base, u32 reg) @@ -294,7 +294,7 @@ static inline enum ltdc_pix_fmt to_ltdc_pixelformat(u32 drm_fmt) default: pf = PF_NONE; break; - /* Note: There are no DRM_FORMAT for AL44 and AL88 */ + /* Note: There are no DRM_FORMAT for AL44 and AL88 */ }
return pf; @@ -317,8 +317,8 @@ static inline u32 to_drm_pixelformat(enum ltdc_pix_fmt pf) return DRM_FORMAT_ARGB4444; case PF_L8: return DRM_FORMAT_C8; - case PF_AL44: /* No DRM support */ - case PF_AL88: /* No DRM support */ + case PF_AL44: /* No DRM support */ + case PF_AL88: /* No DRM support */ case PF_NONE: default: return 0; @@ -602,11 +602,11 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane, src_w = state->src_w >> 16; src_h = state->src_h >> 16;
- DRM_DEBUG_DRIVER( - "plane:%d fb:%d (%dx%d)@(%d,%d) -> (%dx%d)@(%d,%d)\n", - plane->base.id, fb->base.id, - src_w, src_h, src_x, src_y, - state->crtc_w, state->crtc_h, state->crtc_x, state->crtc_y); + DRM_DEBUG_DRIVER("plane:%d fb:%d (%dx%d)@(%d,%d) -> (%dx%d)@(%d,%d)\n", + plane->base.id, fb->base.id, + src_w, src_h, src_x, src_y, + state->crtc_w, state->crtc_h, + state->crtc_x, state->crtc_y);
bpcr = reg_read(ldev->regs, LTDC_BPCR); ahbp = (bpcr & BPCR_AHBP) >> 16; @@ -631,7 +631,7 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane, if (val == NB_PF) { DRM_ERROR("Pixel format %.4s not supported\n", (char *)&fb->format->format); - val = 0; /* set by default ARGB 32 bits */ + val = 0; /* set by default ARGB 32 bits */ } reg_update_bits(ldev->regs, LTDC_L1PFCR + lofs, LXPFCR_PF, val);
@@ -645,8 +645,7 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
/* Specifies the constant alpha value */ val = CONSTA_MAX; - reg_update_bits(ldev->regs, LTDC_L1CACR + lofs, - LXCACR_CONSTA, val); + reg_update_bits(ldev->regs, LTDC_L1CACR + lofs, LXCACR_CONSTA, val);
/* Specifies the blending factors */ val = BF1_PAXCA | BF2_1PAXCA; @@ -655,8 +654,7 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
/* Configures the frame buffer line number */ val = y1 - y0 + 1; - reg_update_bits(ldev->regs, LTDC_L1CFBLNR + lofs, - LXCFBLNR_CFBLN, val); + reg_update_bits(ldev->regs, LTDC_L1CFBLNR + lofs, LXCFBLNR_CFBLN, val);
/* Sets the FB address */ paddr = (u32)drm_fb_cma_get_gem_addr(fb, state, 0); @@ -817,7 +815,7 @@ static int ltdc_encoder_init(struct drm_device *ddev) return -ENOMEM;
encoder->possible_crtcs = CRTC_MASK; - encoder->possible_clones = 0; /* No cloning support */ + encoder->possible_clones = 0; /* No cloning support */
drm_encoder_init(ddev, encoder, <dc_encoder_funcs, DRM_MODE_ENCODER_DPI, NULL);
Constify drm funcs structures.
Signed-off-by: Philippe CORNU philippe.cornu@st.com Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/drm/stm/ltdc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 628825b..92e58ba 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -511,7 +511,7 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc, } }
-static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = { +static const struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = { .load_lut = ltdc_crtc_load_lut, .mode_set_nofb = ltdc_crtc_mode_set_nofb, .atomic_flush = ltdc_crtc_atomic_flush, @@ -537,7 +537,7 @@ void ltdc_crtc_disable_vblank(struct drm_device *ddev, unsigned int pipe) reg_clear(ldev->regs, LTDC_IER, IER_LIE); }
-static struct drm_crtc_funcs ltdc_crtc_funcs = { +static const struct drm_crtc_funcs ltdc_crtc_funcs = { .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, @@ -693,7 +693,7 @@ static void ltdc_plane_atomic_disable(struct drm_plane *plane, oldstate->crtc->base.id, plane->base.id); }
-static struct drm_plane_funcs ltdc_plane_funcs = { +static const struct drm_plane_funcs ltdc_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, .destroy = drm_plane_cleanup,
Use devm_reset_control_get_exclusive to avoid resource leakage (based on patch "Convert drivers to explicit reset API" from Philipp Zabel).
Also use platform_get_resource, which is more usual and consistent with platform_get_irq called later.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com Signed-off-by: Philippe CORNU philippe.cornu@st.com Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Philipp Zabel p.zabel@pengutronix.de --- drivers/gpu/drm/stm/ltdc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 92e58ba..d826045 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -874,7 +874,7 @@ int ltdc_load(struct drm_device *ddev) struct drm_panel *panel; struct drm_crtc *crtc; struct reset_control *rstc; - struct resource res; + struct resource *res; int irq, ret, i;
DRM_DEBUG_DRIVER("\n"); @@ -883,7 +883,7 @@ int ltdc_load(struct drm_device *ddev) if (ret) return ret;
- rstc = of_reset_control_get(np, NULL); + rstc = devm_reset_control_get_exclusive(dev, NULL);
mutex_init(&ldev->err_lock);
@@ -898,13 +898,14 @@ int ltdc_load(struct drm_device *ddev) return -ENODEV; }
- if (of_address_to_resource(np, 0, &res)) { + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { DRM_ERROR("Unable to get resource\n"); ret = -ENODEV; goto err; }
- ldev->regs = devm_ioremap_resource(dev, &res); + ldev->regs = devm_ioremap_resource(dev, res); if (IS_ERR(ldev->regs)) { DRM_ERROR("Unable to get ltdc registers\n"); ret = PTR_ERR(ldev->regs);
Hi Philippe,
On Thu, 2017-07-20 at 14:05 +0200, Philippe CORNU wrote:
Use devm_reset_control_get_exclusive to avoid resource leakage (based on patch "Convert drivers to explicit reset API" from Philipp Zabel).
Also use platform_get_resource, which is more usual and consistent with platform_get_irq called later.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com Signed-off-by: Philippe CORNU philippe.cornu@st.com Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Philipp Zabel p.zabel@pengutronix.de
Looking at the usage below, this driver only seems to care about the reset deassertion before register use, so this could use the shared API. Further, it seems that this reset is optional.
drivers/gpu/drm/stm/ltdc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 92e58ba..d826045 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -874,7 +874,7 @@ int ltdc_load(struct drm_device *ddev) struct drm_panel *panel; struct drm_crtc *crtc; struct reset_control *rstc;
- struct resource res;
struct resource *res; int irq, ret, i;
DRM_DEBUG_DRIVER("\n");
@@ -883,7 +883,7 @@ int ltdc_load(struct drm_device *ddev) if (ret) return ret;
- rstc = of_reset_control_get(np, NULL);
- rstc = devm_reset_control_get_exclusive(dev, NULL);
I would suggest to change this to
- rstc = of_reset_control_get(np, NULL); + rstc = devm_reset_control_get_optional_shared(dev, NULL); + if (IS_ERR(rstc)) + return PTR_ERR(rstc);
mutex_init(&ldev->err_lock);
@@ -898,13 +898,14 @@ int ltdc_load(struct drm_device *ddev) return -ENODEV; }
- if (of_address_to_resource(np, 0, &res)) {
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) { DRM_ERROR("Unable to get resource\n"); ret = -ENODEV; goto err; }
- ldev->regs = devm_ioremap_resource(dev, &res);
- ldev->regs = devm_ioremap_resource(dev, res); if (IS_ERR(ldev->regs)) { DRM_ERROR("Unable to get ltdc registers\n"); ret = PTR_ERR(ldev->regs);
then below you can change:
- if (!IS_ERR(rstc)) - reset_control_deassert(rstc); + reset_control_deassert(rstc);
regards Philipp
Rename the returned value from "res" to "ret" as it is more "readable".
Signed-off-by: Philippe CORNU philippe.cornu@st.com Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/drm/stm/ltdc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index d826045..04cc66d 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -760,7 +760,7 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc) struct ltdc_device *ldev = ddev->dev_private; struct drm_plane *primary, *overlay; unsigned int i; - int res; + int ret;
primary = ltdc_plane_create(ddev, DRM_PLANE_TYPE_PRIMARY); if (!primary) { @@ -768,9 +768,9 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc) return -EINVAL; }
- res = drm_crtc_init_with_planes(ddev, crtc, primary, NULL, + ret = drm_crtc_init_with_planes(ddev, crtc, primary, NULL, <dc_crtc_funcs, NULL); - if (res) { + if (ret) { DRM_ERROR("Can not initialize CRTC\n"); goto cleanup; } @@ -783,7 +783,7 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc) for (i = 1; i < ldev->caps.nb_layers; i++) { overlay = ltdc_plane_create(ddev, DRM_PLANE_TYPE_OVERLAY); if (!overlay) { - res = -ENOMEM; + ret = -ENOMEM; DRM_ERROR("Can not create overlay plane %d\n", i); goto cleanup; } @@ -793,7 +793,7 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
cleanup: ltdc_plane_destroy_all(ddev); - return res; + return ret; }
/*
Constify dw_mipi_dsi_stm_phy_ops as these ops are not supposed to change at runtime.
Signed-off-by: Philippe CORNU philippe.cornu@st.com Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org --- drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c index 16ae00e..568c5d0 100644 --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c @@ -261,7 +261,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data) return 0; }
-static struct dw_mipi_dsi_phy_ops dw_mipi_dsi_stm_phy_ops = { +static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_stm_phy_ops = { .init = dw_mipi_dsi_phy_init, .get_lane_mbps = dw_mipi_dsi_get_lane_mbps, };
2017-07-20 14:05 GMT+02:00 Philippe CORNU philippe.cornu@st.com:
Version 2:
- Add devm_reset_control_get_exclusive to follow explicit reset API
- Add missing commit messages & reviewed-by.
Version 1:
- Initial commit
The purpose of this set of patches is to clean up the drm stm driver.
Philippe CORNU (7): drm/stm: drv: Rename platform driver name drm/stm: ltdc: Cleanup signal polarity defines drm/stm: ltdc: Lindent and minor cleanups drm/stm: ltdc: Constify funcs structures drm/stm: ltdc: add devm_reset_control & platform_get_ressource drm/stm: ltdc: Cleanup rename returned value drm/stm: dsi: Constify phy ops structure
drivers/gpu/drm/stm/drv.c | 21 ++-- drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 2 +- drivers/gpu/drm/stm/ltdc.c | 225 ++++++++++++++++------------------ 3 files changed, 116 insertions(+), 132 deletions(-)
-- 1.9.1
merged in drm-misc-next
dri-devel@lists.freedesktop.org
-
Benjamin Gaignard -
Philipp Zabel -
Philippe CORNU