2016-10-11 14:26:58

by Brian Starkey

[permalink] [raw]
Subject: [PATCH 1/8] drm: mali-dp: Set the drm->irq_enabled flag to match driver's state.

From: Liviu Dudau <[email protected]>

Mali DP driver does not use drm_irq_{un,}install() function so the
drm->irq_enabled flag does not get set automatically.
drm_wait_vblank() checks the value of the flag among other functions.

Signed-off-by: Liviu Dudau <[email protected]>
---

Hi,

This series is a bunch of small driver-internal fixes and cleanup for
Mali-DP.

-Brian

drivers/gpu/drm/arm/malidp_drv.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 9280358..7987ebd 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -377,6 +377,8 @@ static int malidp_bind(struct device *dev)
if (ret < 0)
goto irq_init_fail;

+ drm->irq_enabled = true;
+
ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
if (ret < 0) {
DRM_ERROR("failed to initialise vblank\n");
@@ -402,6 +404,7 @@ fbdev_fail:
vblank_fail:
malidp_se_irq_fini(drm);
malidp_de_irq_fini(drm);
+ drm->irq_enabled = false;
irq_init_fail:
component_unbind_all(dev, drm);
bind_fail:
--
1.7.9.5


2016-10-11 14:26:57

by Brian Starkey

[permalink] [raw]
Subject: [PATCH 4/8] drm: mali-dp: Add pitch alignment check for planes

Check that the framebuffer pitches are appropriately aligned when
checking planes.

Signed-off-by: Brian Starkey <[email protected]>
---
drivers/gpu/drm/arm/malidp_planes.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 82c193e..f95e02d 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -86,17 +86,30 @@ static int malidp_de_plane_check(struct drm_plane *plane,
{
struct malidp_plane *mp = to_malidp_plane(plane);
struct malidp_plane_state *ms = to_malidp_plane_state(state);
+ struct drm_framebuffer *fb;
+ int n_planes, i;
u8 format_id;
u32 src_w, src_h;

if (!state->crtc || !state->fb)
return 0;

+ fb = state->fb;
+
format_id = malidp_hw_get_format_id(&mp->hwdev->map, mp->layer->id,
- state->fb->pixel_format);
+ fb->pixel_format);
if (format_id == MALIDP_INVALID_FORMAT_ID)
return -EINVAL;

+ n_planes = drm_format_num_planes(fb->pixel_format);
+ for (i = 0; i < n_planes; i++) {
+ if (!malidp_hw_pitch_valid(mp->hwdev, fb->pitches[i])) {
+ DRM_DEBUG_KMS("Invalid pitch %u for plane %d\n",
+ fb->pitches[i], i);
+ return -EINVAL;
+ }
+ }
+
src_w = state->src_w >> 16;
src_h = state->src_h >> 16;

--
1.7.9.5

2016-10-11 14:27:01

by Brian Starkey

[permalink] [raw]
Subject: [PATCH 5/8] arm: mali-dp: Extract mode_config cleanup into malidp_fini

Split out malidp_fini as the opposite of malidp_init. This helps keep
the cleanup paths neat and easier to manage.

Signed-off-by: Brian Starkey <[email protected]>
---
drivers/gpu/drm/arm/malidp_drv.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index f2b1923..62a29f6 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -156,6 +156,12 @@ static int malidp_init(struct drm_device *drm)
return 0;
}

+static void malidp_fini(struct drm_device *drm)
+{
+ malidp_de_planes_destroy(drm);
+ drm_mode_config_cleanup(drm);
+}
+
static int malidp_irq_init(struct platform_device *pdev)
{
int irq_de, irq_se, ret = 0;
@@ -414,8 +420,7 @@ bind_fail:
port_fail:
drm_dev_unregister(drm);
register_fail:
- malidp_de_planes_destroy(drm);
- drm_mode_config_cleanup(drm);
+ malidp_fini(drm);
init_fail:
drm->dev_private = NULL;
dev_set_drvdata(dev, NULL);
@@ -448,8 +453,7 @@ static void malidp_unbind(struct device *dev)
of_node_put(malidp->crtc.port);
malidp->crtc.port = NULL;
drm_dev_unregister(drm);
- malidp_de_planes_destroy(drm);
- drm_mode_config_cleanup(drm);
+ malidp_fini(drm);
drm->dev_private = NULL;
dev_set_drvdata(dev, NULL);
clk_disable_unprepare(hwdev->mclk);
--
1.7.9.5

2016-10-11 14:27:03

by Brian Starkey

[permalink] [raw]
Subject: [PATCH 8/8] drm: mali-dp: Store internal format and n_planes in plane state

Save a search through the format lists at commit-time by storing the
internal format ID and number of planes in our plane state.

Signed-off-by: Brian Starkey <[email protected]>
---
drivers/gpu/drm/arm/malidp_drv.h | 3 +++
drivers/gpu/drm/arm/malidp_planes.c | 27 ++++++++++++---------------
2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index 271d2fb..9fc8a2e 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -39,6 +39,9 @@ struct malidp_plane_state {

/* size of the required rotation memory if plane is rotated */
u32 rotmem_size;
+ /* internal format ID */
+ u8 format;
+ u8 n_planes;
};

#define to_malidp_plane(x) container_of(x, struct malidp_plane, base)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index 667b9ca..80f389b 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -70,6 +70,8 @@ struct drm_plane_state *malidp_duplicate_plane_state(struct drm_plane *plane)
m_state = to_malidp_plane_state(plane->state);
__drm_atomic_helper_plane_duplicate_state(plane, &state->base);
state->rotmem_size = m_state->rotmem_size;
+ state->format = m_state->format;
+ state->n_planes = m_state->n_planes;
}

return &state->base;
@@ -99,8 +101,7 @@ static int malidp_de_plane_check(struct drm_plane *plane,
struct malidp_plane *mp = to_malidp_plane(plane);
struct malidp_plane_state *ms = to_malidp_plane_state(state);
struct drm_framebuffer *fb;
- int n_planes, i;
- u8 format_id;
+ int i;
u32 src_w, src_h;

if (!state->crtc || !state->fb)
@@ -108,13 +109,13 @@ static int malidp_de_plane_check(struct drm_plane *plane,

fb = state->fb;

- format_id = malidp_hw_get_format_id(&mp->hwdev->map, mp->layer->id,
+ ms->format = malidp_hw_get_format_id(&mp->hwdev->map, mp->layer->id,
fb->pixel_format);
- if (format_id == MALIDP_INVALID_FORMAT_ID)
+ if (ms->format == MALIDP_INVALID_FORMAT_ID)
return -EINVAL;

- n_planes = drm_format_num_planes(fb->pixel_format);
- for (i = 0; i < n_planes; i++) {
+ ms->n_planes = drm_format_num_planes(fb->pixel_format);
+ for (i = 0; i < ms->n_planes; i++) {
if (!malidp_hw_pitch_valid(mp->hwdev, fb->pitches[i])) {
DRM_DEBUG_KMS("Invalid pitch %u for plane %d\n",
fb->pitches[i], i);
@@ -160,17 +161,13 @@ static void malidp_de_plane_update(struct drm_plane *plane,
struct drm_gem_cma_object *obj;
struct malidp_plane *mp;
const struct malidp_hw_regmap *map;
- u8 format_id;
+ struct malidp_plane_state *ms = to_malidp_plane_state(plane->state);
u16 ptr;
- u32 format, src_w, src_h, dest_w, dest_h, val;
- int num_planes, i;
+ u32 src_w, src_h, dest_w, dest_h, val;
+ int i;

mp = to_malidp_plane(plane);
-
map = &mp->hwdev->map;
- format = plane->state->fb->pixel_format;
- format_id = malidp_hw_get_format_id(map, mp->layer->id, format);
- num_planes = drm_format_num_planes(format);

/* convert src values from Q16 fixed point to integer */
src_w = plane->state->src_w >> 16;
@@ -183,9 +180,9 @@ static void malidp_de_plane_update(struct drm_plane *plane,
dest_h = plane->state->crtc_h;
}

- malidp_hw_write(mp->hwdev, format_id, mp->layer->base);
+ malidp_hw_write(mp->hwdev, ms->format, mp->layer->base);

- for (i = 0; i < num_planes; i++) {
+ for (i = 0; i < ms->n_planes; i++) {
/* calculate the offset for the layer's plane registers */
ptr = mp->layer->ptr + (i << 4);

--
1.7.9.5

2016-10-11 14:27:08

by Brian Starkey

[permalink] [raw]
Subject: [PATCH 7/8] drm: mali-dp: Enable alpha blending

Always enable pixel-level alpha blending with the background, so that
buffers which include an alpha channel are displayed correctly.

Signed-off-by: Brian Starkey <[email protected]>
---
drivers/gpu/drm/arm/malidp_planes.c | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index a17d24b..667b9ca 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -27,6 +27,10 @@
#define LAYER_H_FLIP (1 << 10)
#define LAYER_V_FLIP (1 << 11)
#define LAYER_ROT_MASK (0xf << 8)
+#define LAYER_COMP_MASK (0x3 << 12)
+#define LAYER_COMP_PIXEL (0x3 << 12)
+#define LAYER_COMP_PLANE (0x2 << 12)
+#define MALIDP_LAYER_COMPOSE 0x008
#define MALIDP_LAYER_SIZE 0x00c
#define LAYER_H_VAL(x) (((x) & 0x1fff) << 0)
#define LAYER_V_VAL(x) (((x) & 0x1fff) << 16)
@@ -34,6 +38,14 @@
#define MALIDP_LAYER_OFFSET 0x014
#define MALIDP_LAYER_STRIDE 0x018

+/*
+ * This 4-entry look-up-table is used to determine the full 8-bit alpha value
+ * for formats with 1- or 2-bit alpha channels.
+ * We set it to give 100%/0% opacity for 1-bit formats and 100%/66%/33%/0%
+ * opacity for 2-bit formats.
+ */
+#define MALIDP_ALPHA_LUT 0xffaa5500
+
static void malidp_de_plane_destroy(struct drm_plane *plane)
{
struct malidp_plane *mp = to_malidp_plane(plane);
@@ -150,7 +162,7 @@ static void malidp_de_plane_update(struct drm_plane *plane,
const struct malidp_hw_regmap *map;
u8 format_id;
u16 ptr;
- u32 format, src_w, src_h, dest_w, dest_h, val = 0;
+ u32 format, src_w, src_h, dest_w, dest_h, val;
int num_planes, i;

mp = to_malidp_plane(plane);
@@ -194,10 +206,9 @@ static void malidp_de_plane_update(struct drm_plane *plane,
LAYER_V_VAL(plane->state->crtc_y),
mp->layer->base + MALIDP_LAYER_OFFSET);

- /* first clear the rotation bits in the register */
- malidp_hw_clearbits(mp->hwdev, LAYER_ROT_MASK,
- mp->layer->base + MALIDP_LAYER_CONTROL);
+ val = malidp_hw_read(mp->hwdev, mp->layer->base + MALIDP_LAYER_CONTROL);

+ val &= ~LAYER_ROT_MASK;
/* setup the rotation and axis flip bits */
if (plane->state->rotation & DRM_ROTATE_MASK)
val = ilog2(plane->state->rotation & DRM_ROTATE_MASK) << LAYER_ROT_OFFSET;
@@ -206,11 +217,18 @@ static void malidp_de_plane_update(struct drm_plane *plane,
if (plane->state->rotation & DRM_REFLECT_Y)
val |= LAYER_H_FLIP;

+ /*
+ * always enable pixel alpha blending until we have a way to change
+ * blend modes
+ */
+ val &= ~LAYER_COMP_MASK;
+ val |= LAYER_COMP_PIXEL;
+
/* set the 'enable layer' bit */
val |= LAYER_ENABLE;

- malidp_hw_setbits(mp->hwdev, val,
- mp->layer->base + MALIDP_LAYER_CONTROL);
+ malidp_hw_write(mp->hwdev, val,
+ mp->layer->base + MALIDP_LAYER_CONTROL);
}

static void malidp_de_plane_disable(struct drm_plane *plane,
@@ -292,6 +310,8 @@ int malidp_de_planes_init(struct drm_device *drm)
drm->mode_config.rotation_property,
DRM_ROTATE_0);

+ malidp_hw_write(malidp->dev, MALIDP_ALPHA_LUT,
+ plane->layer->base + MALIDP_LAYER_COMPOSE);
}

kfree(formats);
--
1.7.9.5

2016-10-11 14:28:24

by Brian Starkey

[permalink] [raw]
Subject: [PATCH 6/8] drm: mali-dp: Refactor plane initialisation

As we add more features, it makes sense to skip all the features not
supported by the smart layer together, instead of checking each one
individually. Achieve this by refactoring the plane init loop.

Signed-off-by: Brian Starkey <[email protected]>
Acked-by: Liviu Dudau <[email protected]>
---
drivers/gpu/drm/arm/malidp_planes.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c
index f95e02d..a17d24b 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -267,6 +267,15 @@ int malidp_de_planes_init(struct drm_device *drm)
if (ret < 0)
goto cleanup;

+ drm_plane_helper_add(&plane->base,
+ &malidp_de_plane_helper_funcs);
+ plane->hwdev = malidp->dev;
+ plane->layer = &map->layers[i];
+
+ /* Skip the features which the SMART layer doesn't have */
+ if (id == DE_SMART)
+ continue;
+
if (!drm->mode_config.rotation_property) {
unsigned long flags = DRM_ROTATE_0 |
DRM_ROTATE_90 |
@@ -277,16 +286,12 @@ int malidp_de_planes_init(struct drm_device *drm)
drm->mode_config.rotation_property =
drm_mode_create_rotation_property(drm, flags);
}
- /* SMART layer can't be rotated */
- if (drm->mode_config.rotation_property && (id != DE_SMART))
+
+ if (drm->mode_config.rotation_property)
drm_object_attach_property(&plane->base.base,
drm->mode_config.rotation_property,
DRM_ROTATE_0);

- drm_plane_helper_add(&plane->base,
- &malidp_de_plane_helper_funcs);
- plane->hwdev = malidp->dev;
- plane->layer = &map->layers[i];
}

kfree(formats);
--
1.7.9.5

2016-10-11 14:28:40

by Brian Starkey

[permalink] [raw]
Subject: [PATCH 3/8] drm: mali-dp: Add pitch alignment check function

Different hardware versions have different requirements when it comes to
pitch alignment. Add a function which can be used to check pitch
alignment for a device.

Signed-off-by: Brian Starkey <[email protected]>
---
drivers/gpu/drm/arm/malidp_hw.c | 3 +++
drivers/gpu/drm/arm/malidp_hw.h | 9 +++++++++
2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index a6132f1..7f4a0bd 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -441,6 +441,7 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
},
.input_formats = malidp500_de_formats,
.n_input_formats = ARRAY_SIZE(malidp500_de_formats),
+ .bus_align_bytes = 8,
},
.query_hw = malidp500_query_hw,
.enter_config_mode = malidp500_enter_config_mode,
@@ -473,6 +474,7 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
},
.input_formats = malidp550_de_formats,
.n_input_formats = ARRAY_SIZE(malidp550_de_formats),
+ .bus_align_bytes = 8,
},
.query_hw = malidp550_query_hw,
.enter_config_mode = malidp550_enter_config_mode,
@@ -506,6 +508,7 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = {
},
.input_formats = malidp550_de_formats,
.n_input_formats = ARRAY_SIZE(malidp550_de_formats),
+ .bus_align_bytes = 16,
},
.query_hw = malidp650_query_hw,
.enter_config_mode = malidp550_enter_config_mode,
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 141743e..087e1202 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -88,6 +88,9 @@ struct malidp_hw_regmap {
/* list of supported input formats for each layer */
const struct malidp_input_format *input_formats;
const u8 n_input_formats;
+
+ /* pitch alignment requirement in bytes */
+ const u8 bus_align_bytes;
};

struct malidp_hw_device {
@@ -229,6 +232,12 @@ void malidp_se_irq_fini(struct drm_device *drm);
u8 malidp_hw_get_format_id(const struct malidp_hw_regmap *map,
u8 layer_id, u32 format);

+static inline bool malidp_hw_pitch_valid(struct malidp_hw_device *hwdev,
+ unsigned int pitch)
+{
+ return !(pitch & (hwdev->map.bus_align_bytes - 1));
+}
+
/*
* background color components are defined as 12bits values,
* they will be shifted right when stored on hardware that
--
1.7.9.5

2016-10-11 14:28:39

by Brian Starkey

[permalink] [raw]
Subject: [PATCH 2/8] drm: mali-dp: Clear the config_valid flag before using it in wait_event.

From: Liviu Dudau <[email protected]>

config_valid variable is used to signal the activation of the CVAL
request when the vsync interrupt has fired. malidp_set_and_wait_config_valid()
uses the variable in wait_event_interruptible_timeout without clearing it
first, so the wait is skipped.

Signed-off-by: Liviu Dudau <[email protected]>
---
drivers/gpu/drm/arm/malidp_drv.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 7987ebd..f2b1923 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -42,6 +42,7 @@ static int malidp_set_and_wait_config_valid(struct drm_device *drm)
struct malidp_hw_device *hwdev = malidp->dev;
int ret;

+ atomic_set(&malidp->config_valid, 0);
hwdev->set_config_valid(hwdev);
/* don't wait for config_valid flag if we are in config mode */
if (hwdev->in_config_mode(hwdev))
--
1.7.9.5

2016-10-11 15:04:26

by Liviu Dudau

[permalink] [raw]
Subject: Re: [PATCH 1/8] drm: mali-dp: Set the drm->irq_enabled flag to match driver's state.

On Tue, Oct 11, 2016 at 03:26:02PM +0100, Brian Starkey wrote:
> From: Liviu Dudau <[email protected]>
>
> Mali DP driver does not use drm_irq_{un,}install() function so the
> drm->irq_enabled flag does not get set automatically.
> drm_wait_vblank() checks the value of the flag among other functions.
>
> Signed-off-by: Liviu Dudau <[email protected]>
> ---
>
> Hi,
>
> This series is a bunch of small driver-internal fixes and cleanup for
> Mali-DP.

For the whole series, on the patches not already Signed-off-by me or acked:

Acked-by: Liviu Dudau <[email protected]>

Many thanks,
Liviu

>
> -Brian
>
> drivers/gpu/drm/arm/malidp_drv.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 9280358..7987ebd 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -377,6 +377,8 @@ static int malidp_bind(struct device *dev)
> if (ret < 0)
> goto irq_init_fail;
>
> + drm->irq_enabled = true;
> +
> ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
> if (ret < 0) {
> DRM_ERROR("failed to initialise vblank\n");
> @@ -402,6 +404,7 @@ fbdev_fail:
> vblank_fail:
> malidp_se_irq_fini(drm);
> malidp_de_irq_fini(drm);
> + drm->irq_enabled = false;
> irq_init_fail:
> component_unbind_all(dev, drm);
> bind_fail:
> --
> 1.7.9.5
>

--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯