| Message ID | 20240814-google-clarifications-v1-2-3ee76d7d0c28@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vkms: Miscelanious clarifications | expand |
On 8/14/24 05:46, Louis Chauvet wrote: > The meaning of index was not clear. Replace them with crtc_index to > clearly indicate its usage. > > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> IMHO no need for this patch, especially considering that you are going to change those lines anyway in a future series. I'd just drop it. Best Regards, - Maíra > --- > drivers/gpu/drm/vkms/vkms_drv.h | 4 ++-- > drivers/gpu/drm/vkms/vkms_output.c | 13 +++++++------ > drivers/gpu/drm/vkms/vkms_plane.c | 4 ++-- > 3 files changed, 11 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > index 526bf5207524..3028678e4f9b 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.h > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > @@ -222,10 +222,10 @@ struct vkms_device { > int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > struct drm_plane *primary, struct drm_plane *cursor); > > -int vkms_output_init(struct vkms_device *vkmsdev, int index); > +int vkms_output_init(struct vkms_device *vkmsdev, int possible_crtc_index); > > struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, > - enum drm_plane_type type, int index); > + enum drm_plane_type type, int possible_crtc_index); > > /* CRC Support */ > const char *const *vkms_get_crc_sources(struct drm_crtc *crtc, > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > index 5ce70dd946aa..d42ca7d10389 100644 > --- a/drivers/gpu/drm/vkms/vkms_output.c > +++ b/drivers/gpu/drm/vkms/vkms_output.c > @@ -31,12 +31,12 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { > .get_modes = vkms_conn_get_modes, > }; > > -static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index, > +static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int possible_crtc_index, > struct drm_crtc *crtc) > { > struct vkms_plane *overlay; > > - overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, index); > + overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, possible_crtc_index); > if (IS_ERR(overlay)) > return PTR_ERR(overlay); > > @@ -46,7 +46,7 @@ static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index, > return 0; > } > > -int vkms_output_init(struct vkms_device *vkmsdev, int index) > +int vkms_output_init(struct vkms_device *vkmsdev, int possible_crtc_index) > { > struct vkms_output *output = &vkmsdev->output; > struct drm_device *dev = &vkmsdev->drm; > @@ -58,20 +58,21 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) > int writeback; > unsigned int n; > > - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index); > + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, possible_crtc_index); > + > if (IS_ERR(primary)) > return PTR_ERR(primary); > > if (vkmsdev->config->overlay) { > for (n = 0; n < NUM_OVERLAY_PLANES; n++) { > - ret = vkms_add_overlay_plane(vkmsdev, index, crtc); > + ret = vkms_add_overlay_plane(vkmsdev, possible_crtc_index, crtc); > if (ret) > return ret; > } > } > > if (vkmsdev->config->cursor) { > - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); > + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, possible_crtc_index); > if (IS_ERR(cursor)) > return PTR_ERR(cursor); > } > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > index 03716616f819..9d85464ee0e9 100644 > --- a/drivers/gpu/drm/vkms/vkms_plane.c > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > @@ -219,12 +219,12 @@ static const struct drm_plane_helper_funcs vkms_plane_helper_funcs = { > }; > > struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, > - enum drm_plane_type type, int index) > + enum drm_plane_type type, int possible_crtc_index) > { > struct drm_device *dev = &vkmsdev->drm; > struct vkms_plane *plane; > > - plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << index, > + plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << possible_crtc_index, > &vkms_plane_funcs, > vkms_formats, ARRAY_SIZE(vkms_formats), > NULL, type, NULL); >
Le 15/08/24 - 11:07, Maíra Canal a écrit : > On 8/14/24 05:46, Louis Chauvet wrote: > > The meaning of index was not clear. Replace them with crtc_index to > > clearly indicate its usage. > > > > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> > > IMHO no need for this patch, especially considering that you are going > to change those lines anyway in a future series. > > I'd just drop it. True, I will drop it for the next iteration. It was a step for me to understand the initialization process, but as you said, my next series complelty remove it, so I will remove this patch. Thanks for your reviews, Louis Chauvet > Best Regards, > - Maíra > > > --- > > drivers/gpu/drm/vkms/vkms_drv.h | 4 ++-- > > drivers/gpu/drm/vkms/vkms_output.c | 13 +++++++------ > > drivers/gpu/drm/vkms/vkms_plane.c | 4 ++-- > > 3 files changed, 11 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > > index 526bf5207524..3028678e4f9b 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > @@ -222,10 +222,10 @@ struct vkms_device { > > int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, > > struct drm_plane *primary, struct drm_plane *cursor); > > > > -int vkms_output_init(struct vkms_device *vkmsdev, int index); > > +int vkms_output_init(struct vkms_device *vkmsdev, int possible_crtc_index); > > > > struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > - enum drm_plane_type type, int index); > > + enum drm_plane_type type, int possible_crtc_index); > > > > /* CRC Support */ > > const char *const *vkms_get_crc_sources(struct drm_crtc *crtc, > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > > index 5ce70dd946aa..d42ca7d10389 100644 > > --- a/drivers/gpu/drm/vkms/vkms_output.c > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > @@ -31,12 +31,12 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { > > .get_modes = vkms_conn_get_modes, > > }; > > > > -static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index, > > +static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int possible_crtc_index, > > struct drm_crtc *crtc) > > { > > struct vkms_plane *overlay; > > > > - overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, index); > > + overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, possible_crtc_index); > > if (IS_ERR(overlay)) > > return PTR_ERR(overlay); > > > > @@ -46,7 +46,7 @@ static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index, > > return 0; > > } > > > > -int vkms_output_init(struct vkms_device *vkmsdev, int index) > > +int vkms_output_init(struct vkms_device *vkmsdev, int possible_crtc_index) > > { > > struct vkms_output *output = &vkmsdev->output; > > struct drm_device *dev = &vkmsdev->drm; > > @@ -58,20 +58,21 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) > > int writeback; > > unsigned int n; > > > > - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index); > > + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, possible_crtc_index); > > + > > if (IS_ERR(primary)) > > return PTR_ERR(primary); > > > > if (vkmsdev->config->overlay) { > > for (n = 0; n < NUM_OVERLAY_PLANES; n++) { > > - ret = vkms_add_overlay_plane(vkmsdev, index, crtc); > > + ret = vkms_add_overlay_plane(vkmsdev, possible_crtc_index, crtc); > > if (ret) > > return ret; > > } > > } > > > > if (vkmsdev->config->cursor) { > > - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); > > + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, possible_crtc_index); > > if (IS_ERR(cursor)) > > return PTR_ERR(cursor); > > } > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > > index 03716616f819..9d85464ee0e9 100644 > > --- a/drivers/gpu/drm/vkms/vkms_plane.c > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > @@ -219,12 +219,12 @@ static const struct drm_plane_helper_funcs vkms_plane_helper_funcs = { > > }; > > > > struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > - enum drm_plane_type type, int index) > > + enum drm_plane_type type, int possible_crtc_index) > > { > > struct drm_device *dev = &vkmsdev->drm; > > struct vkms_plane *plane; > > > > - plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << index, > > + plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << possible_crtc_index, > > &vkms_plane_funcs, > > vkms_formats, ARRAY_SIZE(vkms_formats), > > NULL, type, NULL); > >
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 526bf5207524..3028678e4f9b 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -222,10 +222,10 @@ struct vkms_device { int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, struct drm_plane *primary, struct drm_plane *cursor); -int vkms_output_init(struct vkms_device *vkmsdev, int index); +int vkms_output_init(struct vkms_device *vkmsdev, int possible_crtc_index); struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, - enum drm_plane_type type, int index); + enum drm_plane_type type, int possible_crtc_index); /* CRC Support */ const char *const *vkms_get_crc_sources(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 5ce70dd946aa..d42ca7d10389 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -31,12 +31,12 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { .get_modes = vkms_conn_get_modes, }; -static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index, +static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int possible_crtc_index, struct drm_crtc *crtc) { struct vkms_plane *overlay; - overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, index); + overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, possible_crtc_index); if (IS_ERR(overlay)) return PTR_ERR(overlay); @@ -46,7 +46,7 @@ static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index, return 0; } -int vkms_output_init(struct vkms_device *vkmsdev, int index) +int vkms_output_init(struct vkms_device *vkmsdev, int possible_crtc_index) { struct vkms_output *output = &vkmsdev->output; struct drm_device *dev = &vkmsdev->drm; @@ -58,20 +58,21 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) int writeback; unsigned int n; - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index); + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, possible_crtc_index); + if (IS_ERR(primary)) return PTR_ERR(primary); if (vkmsdev->config->overlay) { for (n = 0; n < NUM_OVERLAY_PLANES; n++) { - ret = vkms_add_overlay_plane(vkmsdev, index, crtc); + ret = vkms_add_overlay_plane(vkmsdev, possible_crtc_index, crtc); if (ret) return ret; } } if (vkmsdev->config->cursor) { - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, possible_crtc_index); if (IS_ERR(cursor)) return PTR_ERR(cursor); } diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 03716616f819..9d85464ee0e9 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -219,12 +219,12 @@ static const struct drm_plane_helper_funcs vkms_plane_helper_funcs = { }; struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, - enum drm_plane_type type, int index) + enum drm_plane_type type, int possible_crtc_index) { struct drm_device *dev = &vkmsdev->drm; struct vkms_plane *plane; - plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << index, + plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << possible_crtc_index, &vkms_plane_funcs, vkms_formats, ARRAY_SIZE(vkms_formats), NULL, type, NULL);
The meaning of index was not clear. Replace them with crtc_index to clearly indicate its usage. Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> --- drivers/gpu/drm/vkms/vkms_drv.h | 4 ++-- drivers/gpu/drm/vkms/vkms_output.c | 13 +++++++------ drivers/gpu/drm/vkms/vkms_plane.c | 4 ++-- 3 files changed, 11 insertions(+), 10 deletions(-)