| Message ID | e3bc263b273d91182e0577ed820b1c4f096834ec.1559860606.git.rodrigosiqueiramelo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vkms: Introduces writeback support | expand |
On Thu, Jun 06, 2019 at 07:40:38PM -0300, Rodrigo Siqueira wrote: > When vkms calls drm_universal_plane_init(), it sets 0 for the > possible_crtcs parameter which works well for a single encoder and > connector; however, this approach is not flexible and does not fit well > for vkms. This commit adds an index parameter for vkms_plane_init() > which makes code flexible and enables vkms to support other DRM features. > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> I think a core patch to WARN_ON if this is NULL would be good. Since that's indeed a bit broken ... We'd need to check all callers to make sure there's not other such bugs anywhere ofc. -Daniel > --- > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > drivers/gpu/drm/vkms/vkms_drv.h | 4 ++-- > drivers/gpu/drm/vkms/vkms_output.c | 6 +++--- > drivers/gpu/drm/vkms/vkms_plane.c | 4 ++-- > 4 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > index 738dd6206d85..92296bd8f623 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.c > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > @@ -92,7 +92,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > dev->mode_config.max_height = YRES_MAX; > dev->mode_config.preferred_depth = 24; > > - return vkms_output_init(vkmsdev); > + return vkms_output_init(vkmsdev, 0); > } > > static int __init vkms_init(void) > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > index 81f1cfbeb936..e81073dea154 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.h > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > @@ -113,10 +113,10 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, > int *max_error, ktime_t *vblank_time, > bool in_vblank_irq); > > -int vkms_output_init(struct vkms_device *vkmsdev); > +int vkms_output_init(struct vkms_device *vkmsdev, int index); > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > - enum drm_plane_type type); > + enum drm_plane_type type, int index); > > /* Gem stuff */ > struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > index 3b162b25312e..1442b447c707 100644 > --- a/drivers/gpu/drm/vkms/vkms_output.c > +++ b/drivers/gpu/drm/vkms/vkms_output.c > @@ -36,7 +36,7 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { > .get_modes = vkms_conn_get_modes, > }; > > -int vkms_output_init(struct vkms_device *vkmsdev) > +int vkms_output_init(struct vkms_device *vkmsdev, int index) > { > struct vkms_output *output = &vkmsdev->output; > struct drm_device *dev = &vkmsdev->drm; > @@ -46,12 +46,12 @@ int vkms_output_init(struct vkms_device *vkmsdev) > struct drm_plane *primary, *cursor = NULL; > int ret; > > - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY); > + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index); > if (IS_ERR(primary)) > return PTR_ERR(primary); > > if (enable_cursor) { > - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR); > + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); > if (IS_ERR(cursor)) { > ret = PTR_ERR(cursor); > goto err_cursor; > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > index 0e67d2d42f0c..20ffc52f9194 100644 > --- a/drivers/gpu/drm/vkms/vkms_plane.c > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > @@ -168,7 +168,7 @@ static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = { > }; > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > - enum drm_plane_type type) > + enum drm_plane_type type, int index) > { > struct drm_device *dev = &vkmsdev->drm; > const struct drm_plane_helper_funcs *funcs; > @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > funcs = &vkms_primary_helper_funcs; > } > > - ret = drm_universal_plane_init(dev, plane, 0, > + ret = drm_universal_plane_init(dev, plane, 1 << index, > &vkms_plane_funcs, > formats, nformats, > NULL, type, NULL); > -- > 2.21.0
On Fri, Jun 7, 2019 at 4:40 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Thu, Jun 06, 2019 at 07:40:38PM -0300, Rodrigo Siqueira wrote: > > When vkms calls drm_universal_plane_init(), it sets 0 for the > > possible_crtcs parameter which works well for a single encoder and > > connector; however, this approach is not flexible and does not fit well > > for vkms. This commit adds an index parameter for vkms_plane_init() > > which makes code flexible and enables vkms to support other DRM features. > > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > I think a core patch to WARN_ON if this is NULL would be good. Since > that's indeed a bit broken ... We'd need to check all callers to make sure > there's not other such bugs anywhere ofc. > -Daniel Do you mean add WARN_ON in `drm_universal_plane_init()` if `possible_crtcs` is equal to zero? > > --- > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > drivers/gpu/drm/vkms/vkms_drv.h | 4 ++-- > > drivers/gpu/drm/vkms/vkms_output.c | 6 +++--- > > drivers/gpu/drm/vkms/vkms_plane.c | 4 ++-- > > 4 files changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > index 738dd6206d85..92296bd8f623 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > @@ -92,7 +92,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > dev->mode_config.max_height = YRES_MAX; > > dev->mode_config.preferred_depth = 24; > > > > - return vkms_output_init(vkmsdev); > > + return vkms_output_init(vkmsdev, 0); > > } > > > > static int __init vkms_init(void) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > > index 81f1cfbeb936..e81073dea154 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > @@ -113,10 +113,10 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, > > int *max_error, ktime_t *vblank_time, > > bool in_vblank_irq); > > > > -int vkms_output_init(struct vkms_device *vkmsdev); > > +int vkms_output_init(struct vkms_device *vkmsdev, int index); > > > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > - enum drm_plane_type type); > > + enum drm_plane_type type, int index); > > > > /* Gem stuff */ > > struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > > index 3b162b25312e..1442b447c707 100644 > > --- a/drivers/gpu/drm/vkms/vkms_output.c > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > @@ -36,7 +36,7 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { > > .get_modes = vkms_conn_get_modes, > > }; > > > > -int vkms_output_init(struct vkms_device *vkmsdev) > > +int vkms_output_init(struct vkms_device *vkmsdev, int index) > > { > > struct vkms_output *output = &vkmsdev->output; > > struct drm_device *dev = &vkmsdev->drm; > > @@ -46,12 +46,12 @@ int vkms_output_init(struct vkms_device *vkmsdev) > > struct drm_plane *primary, *cursor = NULL; > > int ret; > > > > - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY); > > + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index); > > if (IS_ERR(primary)) > > return PTR_ERR(primary); > > > > if (enable_cursor) { > > - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR); > > + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); > > if (IS_ERR(cursor)) { > > ret = PTR_ERR(cursor); > > goto err_cursor; > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > > index 0e67d2d42f0c..20ffc52f9194 100644 > > --- a/drivers/gpu/drm/vkms/vkms_plane.c > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > @@ -168,7 +168,7 @@ static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = { > > }; > > > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > - enum drm_plane_type type) > > + enum drm_plane_type type, int index) > > { > > struct drm_device *dev = &vkmsdev->drm; > > const struct drm_plane_helper_funcs *funcs; > > @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > funcs = &vkms_primary_helper_funcs; > > } > > > > - ret = drm_universal_plane_init(dev, plane, 0, > > + ret = drm_universal_plane_init(dev, plane, 1 << index, > > &vkms_plane_funcs, > > formats, nformats, > > NULL, type, NULL); > > -- > > 2.21.0 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Fri, Jun 07, 2019 at 11:37:55AM -0300, Rodrigo Siqueira wrote: > On Fri, Jun 7, 2019 at 4:40 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Thu, Jun 06, 2019 at 07:40:38PM -0300, Rodrigo Siqueira wrote: > > > When vkms calls drm_universal_plane_init(), it sets 0 for the > > > possible_crtcs parameter which works well for a single encoder and > > > connector; however, this approach is not flexible and does not fit well > > > for vkms. This commit adds an index parameter for vkms_plane_init() > > > which makes code flexible and enables vkms to support other DRM features. > > > > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > > I think a core patch to WARN_ON if this is NULL would be good. Since > > that's indeed a bit broken ... We'd need to check all callers to make sure > > there's not other such bugs anywhere ofc. > > -Daniel > > Do you mean add WARN_ON in `drm_universal_plane_init()` if > `possible_crtcs` is equal to zero? Yeah, and same for endcoders I guess too. Altough with encoders I think there's a ton of broken drivers. -Daniel > > > > --- > > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > > drivers/gpu/drm/vkms/vkms_drv.h | 4 ++-- > > > drivers/gpu/drm/vkms/vkms_output.c | 6 +++--- > > > drivers/gpu/drm/vkms/vkms_plane.c | 4 ++-- > > > 4 files changed, 8 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > > index 738dd6206d85..92296bd8f623 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > @@ -92,7 +92,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > dev->mode_config.max_height = YRES_MAX; > > > dev->mode_config.preferred_depth = 24; > > > > > > - return vkms_output_init(vkmsdev); > > > + return vkms_output_init(vkmsdev, 0); > > > } > > > > > > static int __init vkms_init(void) > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > > > index 81f1cfbeb936..e81073dea154 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > > @@ -113,10 +113,10 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, > > > int *max_error, ktime_t *vblank_time, > > > bool in_vblank_irq); > > > > > > -int vkms_output_init(struct vkms_device *vkmsdev); > > > +int vkms_output_init(struct vkms_device *vkmsdev, int index); > > > > > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > > - enum drm_plane_type type); > > > + enum drm_plane_type type, int index); > > > > > > /* Gem stuff */ > > > struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > > > index 3b162b25312e..1442b447c707 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_output.c > > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > > @@ -36,7 +36,7 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { > > > .get_modes = vkms_conn_get_modes, > > > }; > > > > > > -int vkms_output_init(struct vkms_device *vkmsdev) > > > +int vkms_output_init(struct vkms_device *vkmsdev, int index) > > > { > > > struct vkms_output *output = &vkmsdev->output; > > > struct drm_device *dev = &vkmsdev->drm; > > > @@ -46,12 +46,12 @@ int vkms_output_init(struct vkms_device *vkmsdev) > > > struct drm_plane *primary, *cursor = NULL; > > > int ret; > > > > > > - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY); > > > + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index); > > > if (IS_ERR(primary)) > > > return PTR_ERR(primary); > > > > > > if (enable_cursor) { > > > - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR); > > > + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); > > > if (IS_ERR(cursor)) { > > > ret = PTR_ERR(cursor); > > > goto err_cursor; > > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > > > index 0e67d2d42f0c..20ffc52f9194 100644 > > > --- a/drivers/gpu/drm/vkms/vkms_plane.c > > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > > @@ -168,7 +168,7 @@ static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = { > > > }; > > > > > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > > - enum drm_plane_type type) > > > + enum drm_plane_type type, int index) > > > { > > > struct drm_device *dev = &vkmsdev->drm; > > > const struct drm_plane_helper_funcs *funcs; > > > @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > > funcs = &vkms_primary_helper_funcs; > > > } > > > > > > - ret = drm_universal_plane_init(dev, plane, 0, > > > + ret = drm_universal_plane_init(dev, plane, 1 << index, > > > &vkms_plane_funcs, > > > formats, nformats, > > > NULL, type, NULL); > > > -- > > > 2.21.0 > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > > > -- > > Rodrigo Siqueira > https://siqueira.tech
On 06/07, Daniel Vetter wrote: > On Fri, Jun 07, 2019 at 11:37:55AM -0300, Rodrigo Siqueira wrote: > > On Fri, Jun 7, 2019 at 4:40 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > > On Thu, Jun 06, 2019 at 07:40:38PM -0300, Rodrigo Siqueira wrote: > > > > When vkms calls drm_universal_plane_init(), it sets 0 for the > > > > possible_crtcs parameter which works well for a single encoder and > > > > connector; however, this approach is not flexible and does not fit well > > > > for vkms. This commit adds an index parameter for vkms_plane_init() > > > > which makes code flexible and enables vkms to support other DRM features. > > > > > > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > > > > > I think a core patch to WARN_ON if this is NULL would be good. Since > > > that's indeed a bit broken ... We'd need to check all callers to make sure > > > there's not other such bugs anywhere ofc. > > > -Daniel > > > > Do you mean add WARN_ON in `drm_universal_plane_init()` if > > `possible_crtcs` is equal to zero? > > Yeah, and same for endcoders I guess too. Altough with encoders I think > there's a ton of broken drivers. I made the patch, but when I started to write the commit message, I just realized that I did not understand why possible_crtcs should not be equal zero. Why can we not use zero? > -Daniel > > > > > > > --- > > > > drivers/gpu/drm/vkms/vkms_drv.c | 2 +- > > > > drivers/gpu/drm/vkms/vkms_drv.h | 4 ++-- > > > > drivers/gpu/drm/vkms/vkms_output.c | 6 +++--- > > > > drivers/gpu/drm/vkms/vkms_plane.c | 4 ++-- > > > > 4 files changed, 8 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c > > > > index 738dd6206d85..92296bd8f623 100644 > > > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > > > @@ -92,7 +92,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) > > > > dev->mode_config.max_height = YRES_MAX; > > > > dev->mode_config.preferred_depth = 24; > > > > > > > > - return vkms_output_init(vkmsdev); > > > > + return vkms_output_init(vkmsdev, 0); > > > > } > > > > > > > > static int __init vkms_init(void) > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > > > > index 81f1cfbeb936..e81073dea154 100644 > > > > --- a/drivers/gpu/drm/vkms/vkms_drv.h > > > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > > > > @@ -113,10 +113,10 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, > > > > int *max_error, ktime_t *vblank_time, > > > > bool in_vblank_irq); > > > > > > > > -int vkms_output_init(struct vkms_device *vkmsdev); > > > > +int vkms_output_init(struct vkms_device *vkmsdev, int index); > > > > > > > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > > > - enum drm_plane_type type); > > > > + enum drm_plane_type type, int index); > > > > > > > > /* Gem stuff */ > > > > struct drm_gem_object *vkms_gem_create(struct drm_device *dev, > > > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c > > > > index 3b162b25312e..1442b447c707 100644 > > > > --- a/drivers/gpu/drm/vkms/vkms_output.c > > > > +++ b/drivers/gpu/drm/vkms/vkms_output.c > > > > @@ -36,7 +36,7 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { > > > > .get_modes = vkms_conn_get_modes, > > > > }; > > > > > > > > -int vkms_output_init(struct vkms_device *vkmsdev) > > > > +int vkms_output_init(struct vkms_device *vkmsdev, int index) > > > > { > > > > struct vkms_output *output = &vkmsdev->output; > > > > struct drm_device *dev = &vkmsdev->drm; > > > > @@ -46,12 +46,12 @@ int vkms_output_init(struct vkms_device *vkmsdev) > > > > struct drm_plane *primary, *cursor = NULL; > > > > int ret; > > > > > > > > - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY); > > > > + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index); > > > > if (IS_ERR(primary)) > > > > return PTR_ERR(primary); > > > > > > > > if (enable_cursor) { > > > > - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR); > > > > + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); > > > > if (IS_ERR(cursor)) { > > > > ret = PTR_ERR(cursor); > > > > goto err_cursor; > > > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > > > > index 0e67d2d42f0c..20ffc52f9194 100644 > > > > --- a/drivers/gpu/drm/vkms/vkms_plane.c > > > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > > > > @@ -168,7 +168,7 @@ static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = { > > > > }; > > > > > > > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > > > - enum drm_plane_type type) > > > > + enum drm_plane_type type, int index) > > > > { > > > > struct drm_device *dev = &vkmsdev->drm; > > > > const struct drm_plane_helper_funcs *funcs; > > > > @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, > > > > funcs = &vkms_primary_helper_funcs; > > > > } > > > > > > > > - ret = drm_universal_plane_init(dev, plane, 0, > > > > + ret = drm_universal_plane_init(dev, plane, 1 << index, > > > > &vkms_plane_funcs, > > > > formats, nformats, > > > > NULL, type, NULL); > > > > -- > > > > 2.21.0 > > > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > > > > > -- > > > > Rodrigo Siqueira > > https://siqueira.tech > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Tuesday, June 18, 2019 5:19 AM, Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> wrote: > I made the patch, but when I started to write the commit message, I just > realized that I did not understand why possible_crtcs should not be > equal zero. Why can we not use zero? Hi, possible_crtcs is a bitfield. If it's zero, it means the plane cannot be attached to any CRTC, which makes it rather useless. Thanks, Simon
On Tue, Jun 18, 2019 at 2:18 AM Simon Ser <contact@emersion.fr> wrote: > > On Tuesday, June 18, 2019 5:19 AM, Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> wrote: > > I made the patch, but when I started to write the commit message, I just > > realized that I did not understand why possible_crtcs should not be > > equal zero. Why can we not use zero? > > Hi, > > possible_crtcs is a bitfield. If it's zero, it means the plane cannot > be attached to any CRTC, which makes it rather useless. Hi, Thank you very much for your explanation. I'll try to finish the patch. > Thanks, > > Simon
diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 738dd6206d85..92296bd8f623 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -92,7 +92,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) dev->mode_config.max_height = YRES_MAX; dev->mode_config.preferred_depth = 24; - return vkms_output_init(vkmsdev); + return vkms_output_init(vkmsdev, 0); } static int __init vkms_init(void) diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 81f1cfbeb936..e81073dea154 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -113,10 +113,10 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, int *max_error, ktime_t *vblank_time, bool in_vblank_irq); -int vkms_output_init(struct vkms_device *vkmsdev); +int vkms_output_init(struct vkms_device *vkmsdev, int index); struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, - enum drm_plane_type type); + enum drm_plane_type type, int index); /* Gem stuff */ struct drm_gem_object *vkms_gem_create(struct drm_device *dev, diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index 3b162b25312e..1442b447c707 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -36,7 +36,7 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = { .get_modes = vkms_conn_get_modes, }; -int vkms_output_init(struct vkms_device *vkmsdev) +int vkms_output_init(struct vkms_device *vkmsdev, int index) { struct vkms_output *output = &vkmsdev->output; struct drm_device *dev = &vkmsdev->drm; @@ -46,12 +46,12 @@ int vkms_output_init(struct vkms_device *vkmsdev) struct drm_plane *primary, *cursor = NULL; int ret; - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY); + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index); if (IS_ERR(primary)) return PTR_ERR(primary); if (enable_cursor) { - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR); + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); if (IS_ERR(cursor)) { ret = PTR_ERR(cursor); goto err_cursor; diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 0e67d2d42f0c..20ffc52f9194 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -168,7 +168,7 @@ static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = { }; struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, - enum drm_plane_type type) + enum drm_plane_type type, int index) { struct drm_device *dev = &vkmsdev->drm; const struct drm_plane_helper_funcs *funcs; @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, funcs = &vkms_primary_helper_funcs; } - ret = drm_universal_plane_init(dev, plane, 0, + ret = drm_universal_plane_init(dev, plane, 1 << index, &vkms_plane_funcs, formats, nformats, NULL, type, NULL);
When vkms calls drm_universal_plane_init(), it sets 0 for the possible_crtcs parameter which works well for a single encoder and connector; however, this approach is not flexible and does not fit well for vkms. This commit adds an index parameter for vkms_plane_init() which makes code flexible and enables vkms to support other DRM features. Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> --- drivers/gpu/drm/vkms/vkms_drv.c | 2 +- drivers/gpu/drm/vkms/vkms_drv.h | 4 ++-- drivers/gpu/drm/vkms/vkms_output.c | 6 +++--- drivers/gpu/drm/vkms/vkms_plane.c | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-)