Hi Mahesh,
Thank you for the patch.
On Friday, 13 July 2018 16:59:42 EEST Mahesh Kumar wrote:
This patch implements get_crc_sources callback, which returns list of
all the crc sources supported by driver in current platform.
Changes Since V1:
- move sources list per-crtc
- init sources-list only for gen3
Signed-off-by: Mahesh Kumar <mahesh1.ku...@intel.com>
Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
---
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 96 ++++++++++++++++++++++++++++++-
drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 3 ++
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index 6a29055a4ab0..bbe417e93fe3
100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -691,6 +691,79 @@ static const struct drm_crtc_helper_funcs
crtc_helper_funcs = { .atomic_disable = rcar_du_crtc_atomic_disable,
};
+static void rcar_du_crtc_crc_sources_list_init(struct rcar_du_crtc *rcrtc)
+{
+ struct rcar_du_device *rcdu = rcrtc->group->dev;
+ struct drm_device *dev = rcrtc->crtc.dev;
+ struct drm_crtc *crtc = &rcrtc->crtc;
+ struct drm_plane *plane;
+ unsigned int count;
+ const char **sources;
+ u32 plane_mask;
+ int i = 0;
i never takes negative values, it can be an unsigned int.
+ /* CRC available only on Gen3 HW */
Please capitalize sentences and end them with a period in comments to match
the driver's style. This applies to other locations in this patch.
+ if (rcdu->info->gen < 3)
+ goto fail;
You can just return here, sources_count and sources are initialized to 0 when
the rcar_du_crtc structure is allocated.
+ drm_for_each_plane(plane, dev) {
+ if (drm_crtc_mask(crtc) & plane->possible_crtcs) {
+ count++;
+ plane_mask |= drm_plane_mask(plane);
+ }
+ }
You can instead iterate over the planes of the associated VSP (hardware
composer).
/* Reserve 1 for "auto" source. */
count = rcrtc->vsp->num_planes + 1;
and get rid of plane_mask.
+ /* reserve 1 for "auto" source */
+ count += 1;
+ sources = kmalloc_array(count, sizeof(char *), GFP_KERNEL);
s/sizeof(char *)/sizeof(*sources)/
+ if (!sources)
+ goto fail;
+
+ sources[i] = kstrdup("auto", GFP_KERNEL);
+ if (!sources[i])
+ goto fail_no_mem;
+
+ i++;
+ drm_for_each_plane_mask(plane, dev, plane_mask) {
+ char name[16];
+
+ sprintf(name, "plane%d", plane->base.id);
The ID is an unsigned integer, you should use %u.
+ sources[i] = kstrdup(name, GFP_KERNEL);
+ if (!sources[i])
+ goto fail_no_mem;
As there will be a single error label, you can just name it "error".
+ i++;
+ }
You can iterate over the VSP planes here too.
for (i = 0; i < rcrtc->vsp->num_planes; ++i) {
struct drm_plane *plane = &rcrtc->vsp->planes[i].plane;
char name[16];
sprintf(name, "plane%u", plane->base.id);
sources[i+1] = kstrdup(name, GFP_KERNEL);
if (!sources[i+1])
goto error;
}
+ rcrtc->sources = sources;
+ rcrtc->sources_count = count;
+ return;
+
+fail_no_mem:
+ while (i > 0) {
+ i--;
+ kfree(sources[i]);
+ }
You'll have to adapt it based on the code above.
+ kfree(sources);
+fail:
+ rcrtc->sources = NULL;
+ rcrtc->sources_count = 0;
+}
+
+static void rcar_du_crtc_crc_sources_list_uninit(struct rcar_du_crtc
*rcrtc)
+{
+ unsigned int i;
+
+ if (!rcrtc->sources)
+ return;
+
+ for (i = 0; i < rcrtc->sources_count; i++)
+ kfree(rcrtc->sources[i]);
+ kfree(rcrtc->sources);
+
+ rcrtc->sources = NULL;
+ rcrtc->sources_count = 0;
+}
+
static struct drm_crtc_state *
rcar_du_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
{
@@ -717,6 +790,15 @@ static void rcar_du_crtc_atomic_destroy_state(struct
drm_crtc *crtc, kfree(to_rcar_crtc_state(state));
}
+static void rcar_du_crtc_cleanup(struct drm_crtc *crtc)
+{
+ struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
+
+ rcar_du_crtc_crc_sources_list_uninit(rcrtc);
+
+ return drm_crtc_cleanup(crtc);
+}
+
static void rcar_du_crtc_reset(struct drm_crtc *crtc)
{
struct rcar_du_crtc_state *state;
@@ -811,6 +893,15 @@ static int rcar_du_crtc_verify_crc_source(struct
drm_crtc *crtc, return 0;
}
+const char *const *rcar_du_crtc_get_crc_sources(struct drm_crtc *crtc,
+ size_t *count)
+{
+ struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
+
+ *count = rcrtc->sources_count;
+ return (const char * const*)rcrtc->sources;
Shouldn't you declare rcrtc->sources as a const char * const * field instead
of casting it here ?
+}
+
static int rcar_du_crtc_set_crc_source(struct drm_crtc *crtc,
const char *source_name)
{
@@ -881,7 +972,7 @@ static const struct drm_crtc_funcs crtc_funcs_gen2 = {
static const struct drm_crtc_funcs crtc_funcs_gen3 = {
.reset = rcar_du_crtc_reset,
- .destroy = drm_crtc_cleanup,
+ .destroy = rcar_du_crtc_cleanup,
.set_config = drm_atomic_helper_set_config,
.page_flip = drm_atomic_helper_page_flip,
.atomic_duplicate_state = rcar_du_crtc_atomic_duplicate_state,
@@ -890,6 +981,7 @@ static const struct drm_crtc_funcs crtc_funcs_gen3 = {
.disable_vblank = rcar_du_crtc_disable_vblank,
.set_crc_source = rcar_du_crtc_set_crc_source,
.verify_crc_source = rcar_du_crtc_verify_crc_source,
+ .get_crc_sources = rcar_du_crtc_get_crc_sources,
};
/*
---------------------------------------------------------------------------
-- @@ -1028,5 +1120,7 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
unsigned int swindex, return ret;
}
+ rcar_du_crtc_crc_sources_list_init(rcrtc);
+
return 0;
}
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h index 7680cb2636c8..0cd0c1655beb
100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -67,6 +67,9 @@ struct rcar_du_crtc {
struct rcar_du_group *group;
struct rcar_du_vsp *vsp;
unsigned int vsp_pipe;
+
+ const char **sources;
+ unsigned int sources_count;
};
#define to_rcar_crtc(c) container_of(c, struct rcar_du_crtc, crtc)
