On Thu, Aug 02, 2018 at 04:24:44PM +0530, Sravanthi Kollukuduru wrote:
Reserve DMA pipe for cursor plane and attach it to the crtc during the initialization.
Signed-off-by: Sravanthi Kollukuduru skolluku@codeaurora.org
Hi Sravanthi, Thanks for sending this. I have a few comments, mostly based off my own cursor patch [1] I posted last week. It's mostly the same as what you have here, but takes a couple different things into consideration.
[1]- https://gitlab.freedesktop.org/seanpaul/dpu-staging/commit/fde9f11f8f3be3cea...
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 53 +++++++++++--------------- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 35 +++++++++++------ drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 9 +---- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 +- 6 files changed, 55 insertions(+), 55 deletions(-)
/snip
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index 8d4678d..dc647ee 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -531,12 +531,13 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms) { struct drm_device *dev; struct drm_plane *primary_planes[MAX_PLANES], *plane;
struct drm_plane *cursor_planes[MAX_PLANES] = { NULL }; struct drm_crtc *crtc;
struct msm_drm_private *priv; struct dpu_mdss_cfg *catalog;
- int primary_planes_idx = 0, i, ret;
int primary_planes_idx = 0, cursor_planes_idx = 0, i, ret; int max_crtc_count;
if (!dpu_kms || !dpu_kms->dev || !dpu_kms->dev->dev) {
@@ -556,16 +557,24 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
max_crtc_count = min(catalog->mixer_count, priv->num_encoders);
- /* Create the planes */
- /* Create the planes, keeping track of one primary/cursor per crtc */ for (i = 0; i < catalog->sspp_count; i++) {
bool primary = true;if (catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR)|| primary_planes_idx >= max_crtc_count)primary = false;plane = dpu_plane_init(dev, catalog->sspp[i].id, primary,(1UL << max_crtc_count) - 1, 0);
enum drm_plane_type type;if ((catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR))&& cursor_planes_idx < max_crtc_count)
You don't need to check that the index is less than max_crtc_count, it'll fit in the array regardless.
type = DRM_PLANE_TYPE_CURSOR;else if (primary_planes_idx < max_crtc_count)type = DRM_PLANE_TYPE_PRIMARY;elsetype = DRM_PLANE_TYPE_OVERLAY;DPU_DEBUG("Create plane type %d with features %lx (cur %lx)\n",type, catalog->sspp[i].features,catalog->sspp[i].features & BIT(DPU_SSPP_CURSOR));plane = dpu_plane_init(dev, catalog->sspp[i].id, type,(1UL << max_crtc_count) - 1, 0);@@ -573,7 +582,9 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms) } priv->planes[priv->num_planes++] = plane;
if (primary)
if (type == DRM_PLANE_TYPE_CURSOR)cursor_planes[cursor_planes_idx++] = plane;else if (type == DRM_PLANE_TYPE_PRIMARY) primary_planes[primary_planes_idx++] = plane;@@ -581,7 +592,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
/* Create one CRTC per encoder */ for (i = 0; i < max_crtc_count; i++) {
crtc = dpu_crtc_init(dev, primary_planes[i]);
crtc = dpu_crtc_init(dev, primary_planes[i], cursor_planes[i]);
Here you assume that there is at least one cursor per crtc. That might not be the case. Check out how I handle this in my patch with max_cursor_planes, it's a bit more safe than this.
if (IS_ERR(crtc)) { ret = PTR_ERR(crtc); goto fail;diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index b640e39..efdf9b2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1827,7 +1827,7 @@ bool is_dpu_plane_virtual(struct drm_plane *plane)
/* initialize plane */ struct drm_plane *dpu_plane_init(struct drm_device *dev,
uint32_t pipe, bool primary_plane,
uint32_t pipe, enum drm_plane_type type,{ struct drm_plane *plane = NULL, *master_plane = NULL; @@ -1835,7 +1835,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev, struct dpu_plane *pdpu; struct msm_drm_private *priv; struct dpu_kms *kms;
- enum drm_plane_type type; int zpos_max = DPU_ZPOS_MAX; int ret = -EINVAL;
@@ -1916,12 +1915,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev, goto clean_sspp; }
- if (pdpu->features & BIT(DPU_SSPP_CURSOR))
type = DRM_PLANE_TYPE_CURSOR;- else if (primary_plane)
type = DRM_PLANE_TYPE_PRIMARY;- else
type = DRM_PLANE_TYPE_OVERLAY;diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h index f6fe6dd..7fed0b6 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h @@ -122,7 +122,7 @@ void dpu_plane_get_ctl_flush(struct drm_plane *plane, struct dpu_hw_ctl *ctl,
- dpu_plane_init - create new dpu plane for the given pipe
- @dev: Pointer to DRM device
- @pipe: dpu hardware pipe identifier
- @primary_plane: true if this pipe is primary plane for crtc
- @type: Plane type - PRIMARY/OVERLAY/CURSOR
- @possible_crtcs: bitmask of crtc that can be attached to the given pipe
- @master_plane_id: primary plane id of a multirect pipe. 0 value passed for
a regular plane initialization. A non-zero primary plane@@ -130,7 +130,7 @@ void dpu_plane_get_ctl_flush(struct drm_plane *plane, struct dpu_hw_ctl *ctl,
*/ struct drm_plane *dpu_plane_init(struct drm_device *dev,
uint32_t pipe, bool primary_plane,
uint32_t pipe, enum drm_plane_type type,/**
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project