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 <[email protected]> > Cc: Laurent Pinchart <[email protected]> > --- > 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) -- Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
