All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0
@ 2019-02-04 10:31 Daniel Vetter
  2019-02-04 10:31 ` [PATCH 2/2] staging/vboxvideo: Add TODO Daniel Vetter
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Daniel Vetter @ 2019-02-04 10:31 UTC (permalink / raw)
  To: DRI Development
  Cc: LKML, Daniel Vetter, Daniel Vetter, Greg Kroah-Hartman,
	Hans de Goede, Nicholas Mc Guire, Emil Velikov,
	Fabio Rafael da Rosa

The compiler already clears this for us.

More important, someone might look what this is actually used for,
and freak out about the dragon staring back at them.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Nicholas Mc Guire <der.herr@hofr.at>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: Fabio Rafael da Rosa <fdr@pid42.net>
---
 drivers/staging/vboxvideo/vbox_drv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
index b0d73d5fba5d..43c3d0a4fa1a 100644
--- a/drivers/staging/vboxvideo/vbox_drv.c
+++ b/drivers/staging/vboxvideo/vbox_drv.c
@@ -222,7 +222,6 @@ static void vbox_master_drop(struct drm_device *dev, struct drm_file *file_priv)
 static struct drm_driver driver = {
 	.driver_features =
 	    DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
-	.dev_priv_size = 0,
 
 	.lastclose = drm_fb_helper_lastclose,
 	.master_set = vbox_master_set,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] staging/vboxvideo: Add TODO
  2019-02-04 10:31 [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0 Daniel Vetter
@ 2019-02-04 10:31 ` Daniel Vetter
  2019-02-04 11:05     ` Greg Kroah-Hartman
  2019-02-04 18:54     ` Sam Ravnborg
  2019-02-04 11:05 ` [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0 Greg Kroah-Hartman
  2019-02-04 18:49   ` Sam Ravnborg
  2 siblings, 2 replies; 15+ messages in thread
From: Daniel Vetter @ 2019-02-04 10:31 UTC (permalink / raw)
  To: DRI Development
  Cc: LKML, Daniel Vetter, Daniel Vetter, Greg Kroah-Hartman,
	Fabio Rafael da Rosa, Nicholas Mc Guire, Hans de Goede

Noticed why wonder what vboxvideo is using the ->master_set/drop hooks
for.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Fabio Rafael da Rosa <fdr@pid42.net>
Cc: Nicholas Mc Guire <der.herr@hofr.at>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/vboxvideo/TODO | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/vboxvideo/TODO b/drivers/staging/vboxvideo/TODO
index 2e0f99c3f10c..2953e7f794fb 100644
--- a/drivers/staging/vboxvideo/TODO
+++ b/drivers/staging/vboxvideo/TODO
@@ -1,5 +1,7 @@
 TODO:
 -Get a full review from the drm-maintainers on dri-devel done on this driver
+-Drop all the logic around initial_mode_queried/master_set&_drop and everything
+related to this. kms clients can handle hotplugs.
 -Extend this TODO with the results of that review
 
 Please send any patches to Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0
  2019-02-04 10:31 [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0 Daniel Vetter
  2019-02-04 10:31 ` [PATCH 2/2] staging/vboxvideo: Add TODO Daniel Vetter
@ 2019-02-04 11:05 ` Greg Kroah-Hartman
  2019-02-04 18:49   ` Sam Ravnborg
  2 siblings, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-02-04 11:05 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, LKML, Daniel Vetter, Hans de Goede,
	Nicholas Mc Guire, Emil Velikov, Fabio Rafael da Rosa

On Mon, Feb 04, 2019 at 11:31:13AM +0100, Daniel Vetter wrote:
> The compiler already clears this for us.
> 
> More important, someone might look what this is actually used for,
> and freak out about the dragon staring back at them.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Nicholas Mc Guire <der.herr@hofr.at>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: Fabio Rafael da Rosa <fdr@pid42.net>
> ---
>  drivers/staging/vboxvideo/vbox_drv.c | 1 -
>  1 file changed, 1 deletion(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] staging/vboxvideo: Add TODO
  2019-02-04 10:31 ` [PATCH 2/2] staging/vboxvideo: Add TODO Daniel Vetter
@ 2019-02-04 11:05     ` Greg Kroah-Hartman
  2019-02-04 18:54     ` Sam Ravnborg
  1 sibling, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-02-04 11:05 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, LKML, Daniel Vetter, Fabio Rafael da Rosa,
	Nicholas Mc Guire, Hans de Goede

On Mon, Feb 04, 2019 at 11:31:14AM +0100, Daniel Vetter wrote:
> Noticed why wonder what vboxvideo is using the ->master_set/drop hooks
> for.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Fabio Rafael da Rosa <fdr@pid42.net>
> Cc: Nicholas Mc Guire <der.herr@hofr.at>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Hans de Goede <hdegoede@redhat.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] staging/vboxvideo: Add TODO
@ 2019-02-04 11:05     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-02-04 11:05 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: LKML, DRI Development, Hans de Goede, Nicholas Mc Guire,
	Daniel Vetter, Fabio Rafael da Rosa

On Mon, Feb 04, 2019 at 11:31:14AM +0100, Daniel Vetter wrote:
> Noticed why wonder what vboxvideo is using the ->master_set/drop hooks
> for.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Fabio Rafael da Rosa <fdr@pid42.net>
> Cc: Nicholas Mc Guire <der.herr@hofr.at>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Hans de Goede <hdegoede@redhat.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0
  2019-02-04 10:31 [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0 Daniel Vetter
@ 2019-02-04 18:49   ` Sam Ravnborg
  2019-02-04 11:05 ` [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0 Greg Kroah-Hartman
  2019-02-04 18:49   ` Sam Ravnborg
  2 siblings, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2019-02-04 18:49 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, LKML, Hans de Goede, Nicholas Mc Guire,
	Greg Kroah-Hartman, Daniel Vetter, Fabio Rafael da Rosa,
	Emil Velikov

Hi Daniel

On Mon, Feb 04, 2019 at 11:31:13AM +0100, Daniel Vetter wrote:
> The compiler already clears this for us.
> 
> More important, someone might look what this is actually used for,
> and freak out about the dragon staring back at them.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Nicholas Mc Guire <der.herr@hofr.at>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: Fabio Rafael da Rosa <fdr@pid42.net>
> ---
>  drivers/staging/vboxvideo/vbox_drv.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
> index b0d73d5fba5d..43c3d0a4fa1a 100644
> --- a/drivers/staging/vboxvideo/vbox_drv.c
> +++ b/drivers/staging/vboxvideo/vbox_drv.c
> @@ -222,7 +222,6 @@ static void vbox_master_drop(struct drm_device *dev, struct drm_file *file_priv)
>  static struct drm_driver driver = {
>  	.driver_features =
>  	    DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
> -	.dev_priv_size = 0,
>  
>  	.lastclose = drm_fb_helper_lastclose,
>  	.master_set = vbox_master_set,

I have stared at the file for a long time and so far no dragon
was staring back at me. There was a few "#ifdef" that screamed
at me, and a drm_fb_helper_fbdev_setup() that looked
suspicious alas no dragon :-(

As for the change above, dragon or no dragon:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0
@ 2019-02-04 18:49   ` Sam Ravnborg
  0 siblings, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2019-02-04 18:49 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Greg Kroah-Hartman, LKML, DRI Development, Hans de Goede,
	Nicholas Mc Guire, Daniel Vetter, Fabio Rafael da Rosa,
	Emil Velikov

Hi Daniel

On Mon, Feb 04, 2019 at 11:31:13AM +0100, Daniel Vetter wrote:
> The compiler already clears this for us.
> 
> More important, someone might look what this is actually used for,
> and freak out about the dragon staring back at them.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Nicholas Mc Guire <der.herr@hofr.at>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: Fabio Rafael da Rosa <fdr@pid42.net>
> ---
>  drivers/staging/vboxvideo/vbox_drv.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
> index b0d73d5fba5d..43c3d0a4fa1a 100644
> --- a/drivers/staging/vboxvideo/vbox_drv.c
> +++ b/drivers/staging/vboxvideo/vbox_drv.c
> @@ -222,7 +222,6 @@ static void vbox_master_drop(struct drm_device *dev, struct drm_file *file_priv)
>  static struct drm_driver driver = {
>  	.driver_features =
>  	    DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
> -	.dev_priv_size = 0,
>  
>  	.lastclose = drm_fb_helper_lastclose,
>  	.master_set = vbox_master_set,

I have stared at the file for a long time and so far no dragon
was staring back at me. There was a few "#ifdef" that screamed
at me, and a drm_fb_helper_fbdev_setup() that looked
suspicious alas no dragon :-(

As for the change above, dragon or no dragon:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] staging/vboxvideo: Add TODO
  2019-02-04 10:31 ` [PATCH 2/2] staging/vboxvideo: Add TODO Daniel Vetter
@ 2019-02-04 18:54     ` Sam Ravnborg
  2019-02-04 18:54     ` Sam Ravnborg
  1 sibling, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2019-02-04 18:54 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, LKML, Hans de Goede, Nicholas Mc Guire,
	Greg Kroah-Hartman, Daniel Vetter, Fabio Rafael da Rosa

Hi Daniel

On Mon, Feb 04, 2019 at 11:31:14AM +0100, Daniel Vetter wrote:
> Noticed why wonder what vboxvideo is using the ->master_set/drop hooks
> for.
Can you improve the gammar a little, I find it hard to read.

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Fabio Rafael da Rosa <fdr@pid42.net>
> Cc: Nicholas Mc Guire <der.herr@hofr.at>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/staging/vboxvideo/TODO | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/vboxvideo/TODO b/drivers/staging/vboxvideo/TODO
> index 2e0f99c3f10c..2953e7f794fb 100644
> --- a/drivers/staging/vboxvideo/TODO
> +++ b/drivers/staging/vboxvideo/TODO
> @@ -1,5 +1,7 @@
>  TODO:
>  -Get a full review from the drm-maintainers on dri-devel done on this driver
> +-Drop all the logic around initial_mode_queried/master_set&_drop and everything
> +related to this. kms clients can handle hotplugs.
>  -Extend this TODO with the results of that review
>  
>  Please send any patches to Greg Kroah-Hartman <gregkh@linuxfoundation.org>,

The syntax around "master_set&_drop" could be better.
I wondered if the "&" was some rst syntax.

But anyway, despite grammar nit and syntax nit:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Which bring me back to a question asked a week ago or so.
What is missing before we can move vboxvideo out of staging/

Could we carry a few TODO items over in drm/ ar do we need a clean TODO?

	Sam

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] staging/vboxvideo: Add TODO
@ 2019-02-04 18:54     ` Sam Ravnborg
  0 siblings, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2019-02-04 18:54 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Greg Kroah-Hartman, LKML, DRI Development, Hans de Goede,
	Nicholas Mc Guire, Daniel Vetter, Fabio Rafael da Rosa

Hi Daniel

On Mon, Feb 04, 2019 at 11:31:14AM +0100, Daniel Vetter wrote:
> Noticed why wonder what vboxvideo is using the ->master_set/drop hooks
> for.
Can you improve the gammar a little, I find it hard to read.

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Fabio Rafael da Rosa <fdr@pid42.net>
> Cc: Nicholas Mc Guire <der.herr@hofr.at>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/staging/vboxvideo/TODO | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/vboxvideo/TODO b/drivers/staging/vboxvideo/TODO
> index 2e0f99c3f10c..2953e7f794fb 100644
> --- a/drivers/staging/vboxvideo/TODO
> +++ b/drivers/staging/vboxvideo/TODO
> @@ -1,5 +1,7 @@
>  TODO:
>  -Get a full review from the drm-maintainers on dri-devel done on this driver
> +-Drop all the logic around initial_mode_queried/master_set&_drop and everything
> +related to this. kms clients can handle hotplugs.
>  -Extend this TODO with the results of that review
>  
>  Please send any patches to Greg Kroah-Hartman <gregkh@linuxfoundation.org>,

The syntax around "master_set&_drop" could be better.
I wondered if the "&" was some rst syntax.

But anyway, despite grammar nit and syntax nit:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Which bring me back to a question asked a week ago or so.
What is missing before we can move vboxvideo out of staging/

Could we carry a few TODO items over in drm/ ar do we need a clean TODO?

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0
  2019-02-04 18:49   ` Sam Ravnborg
  (?)
@ 2019-02-04 21:49   ` Daniel Vetter
  -1 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2019-02-04 21:49 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: DRI Development, LKML, Hans de Goede, Nicholas Mc Guire,
	Greg Kroah-Hartman, Daniel Vetter, Fabio Rafael da Rosa,
	Emil Velikov

On Mon, Feb 4, 2019 at 7:49 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Daniel
>
> On Mon, Feb 04, 2019 at 11:31:13AM +0100, Daniel Vetter wrote:
> > The compiler already clears this for us.
> >
> > More important, someone might look what this is actually used for,
> > and freak out about the dragon staring back at them.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Nicholas Mc Guire <der.herr@hofr.at>
> > Cc: Emil Velikov <emil.velikov@collabora.com>
> > Cc: Fabio Rafael da Rosa <fdr@pid42.net>
> > ---
> >  drivers/staging/vboxvideo/vbox_drv.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/staging/vboxvideo/vbox_drv.c b/drivers/staging/vboxvideo/vbox_drv.c
> > index b0d73d5fba5d..43c3d0a4fa1a 100644
> > --- a/drivers/staging/vboxvideo/vbox_drv.c
> > +++ b/drivers/staging/vboxvideo/vbox_drv.c
> > @@ -222,7 +222,6 @@ static void vbox_master_drop(struct drm_device *dev, struct drm_file *file_priv)
> >  static struct drm_driver driver = {
> >       .driver_features =
> >           DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
> > -     .dev_priv_size = 0,
> >
> >       .lastclose = drm_fb_helper_lastclose,
> >       .master_set = vbox_master_set,
>
> I have stared at the file for a long time and so far no dragon
> was staring back at me. There was a few "#ifdef" that screamed
> at me, and a drm_fb_helper_fbdev_setup() that looked
> suspicious alas no dragon :-(

dev_priv_size is used by drm_bufs.c aka "you want a root-hole? have
it!" code from dri1 days. It's not running on any modern driver, at
least trinity/syzcaller stopped complaining (and I reviewed all the
entry points and made sure they go nowhere else than an immediate
return -errno). Except nouveau, for reasons (we accidentally made it
uapi there, but it's fixed, just need to wait for all those installs
to die so we can nuke it for good). The dragon was right there
breathing down your neck, and wouldn't have seen it coming  if it
decided to have a snack :-)

btw if you're bored, we should probably have a
CONFIG_FEWER_EXPLOITS_IN_DRM_NOUVEAU or so, default n, for the next
unaware traveller wandering into this dragon den.
-Daniel

> As for the change above, dragon or no dragon:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>
>         Sam



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] staging/vboxvideo: Add TODO
  2019-02-04 18:54     ` Sam Ravnborg
  (?)
@ 2019-02-06 15:58     ` Daniel Vetter
  2019-02-06 16:13       ` Hans de Goede
  -1 siblings, 1 reply; 15+ messages in thread
From: Daniel Vetter @ 2019-02-06 15:58 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Daniel Vetter, DRI Development, LKML, Hans de Goede,
	Nicholas Mc Guire, Greg Kroah-Hartman, Daniel Vetter,
	Fabio Rafael da Rosa

On Mon, Feb 04, 2019 at 07:54:16PM +0100, Sam Ravnborg wrote:
> Hi Daniel
> 
> On Mon, Feb 04, 2019 at 11:31:14AM +0100, Daniel Vetter wrote:
> > Noticed why wonder what vboxvideo is using the ->master_set/drop hooks
> > for.
> Can you improve the gammar a little, I find it hard to read.
> 
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Fabio Rafael da Rosa <fdr@pid42.net>
> > Cc: Nicholas Mc Guire <der.herr@hofr.at>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > ---
> >  drivers/staging/vboxvideo/TODO | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/staging/vboxvideo/TODO b/drivers/staging/vboxvideo/TODO
> > index 2e0f99c3f10c..2953e7f794fb 100644
> > --- a/drivers/staging/vboxvideo/TODO
> > +++ b/drivers/staging/vboxvideo/TODO
> > @@ -1,5 +1,7 @@
> >  TODO:
> >  -Get a full review from the drm-maintainers on dri-devel done on this driver
> > +-Drop all the logic around initial_mode_queried/master_set&_drop and everything
> > +related to this. kms clients can handle hotplugs.
> >  -Extend this TODO with the results of that review
> >  
> >  Please send any patches to Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
> 
> The syntax around "master_set&_drop" could be better.
> I wondered if the "&" was some rst syntax.
> 
> But anyway, despite grammar nit and syntax nit:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Both patches queued for 5.1 in drm-misc, thanks for taking a look.

> Which bring me back to a question asked a week ago or so.
> What is missing before we can move vboxvideo out of staging/

I think it boils down to someone needs to submit it and we'll take a look.

> Could we carry a few TODO items over in drm/ ar do we need a clean TODO?

We carry todos in drm too, it's going to be a judgement call.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] staging/vboxvideo: Add TODO
  2019-02-06 15:58     ` Daniel Vetter
@ 2019-02-06 16:13       ` Hans de Goede
  2019-02-13 18:46           ` Sam Ravnborg
  0 siblings, 1 reply; 15+ messages in thread
From: Hans de Goede @ 2019-02-06 16:13 UTC (permalink / raw)
  To: Sam Ravnborg, DRI Development, LKML, Nicholas Mc Guire,
	Greg Kroah-Hartman, Daniel Vetter, Fabio Rafael da Rosa

Hi,

On 06-02-19 16:58, Daniel Vetter wrote:
> On Mon, Feb 04, 2019 at 07:54:16PM +0100, Sam Ravnborg wrote:
>> Hi Daniel
>>
>> On Mon, Feb 04, 2019 at 11:31:14AM +0100, Daniel Vetter wrote:
>>> Noticed why wonder what vboxvideo is using the ->master_set/drop hooks
>>> for.
>> Can you improve the gammar a little, I find it hard to read.
>>
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: Fabio Rafael da Rosa <fdr@pid42.net>
>>> Cc: Nicholas Mc Guire <der.herr@hofr.at>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>>   drivers/staging/vboxvideo/TODO | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/staging/vboxvideo/TODO b/drivers/staging/vboxvideo/TODO
>>> index 2e0f99c3f10c..2953e7f794fb 100644
>>> --- a/drivers/staging/vboxvideo/TODO
>>> +++ b/drivers/staging/vboxvideo/TODO
>>> @@ -1,5 +1,7 @@
>>>   TODO:
>>>   -Get a full review from the drm-maintainers on dri-devel done on this driver
>>> +-Drop all the logic around initial_mode_queried/master_set&_drop and everything
>>> +related to this. kms clients can handle hotplugs.
>>>   -Extend this TODO with the results of that review
>>>   
>>>   Please send any patches to Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
>>
>> The syntax around "master_set&_drop" could be better.
>> I wondered if the "&" was some rst syntax.
>>
>> But anyway, despite grammar nit and syntax nit:
>> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> Both patches queued for 5.1 in drm-misc, thanks for taking a look.
> 
>> Which bring me back to a question asked a week ago or so.
>> What is missing before we can move vboxvideo out of staging/
> 
> I think it boils down to someone needs to submit it and we'll take a look.

Right, I have this on my TODO, but I did not manage to find time for this
the past few weeks. I expect to have time for this in a couple of weeks
from now, which means the move will miss the 5.1 merge window.

Note you (Sam) are certainly welcome to submit a patch doing the move
yourself, I can certainly use some help with maintaining vboxvideo.

Note I will be available to help answer questions resulting from a
review for moving the driver out of staging. And more in general, if you
want to do this I will try to help where I can.

Regards,

Hans

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] staging/vboxvideo: Add TODO
  2019-02-06 16:13       ` Hans de Goede
@ 2019-02-13 18:46           ` Sam Ravnborg
  0 siblings, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2019-02-13 18:46 UTC (permalink / raw)
  To: Hans de Goede
  Cc: DRI Development, LKML, Nicholas Mc Guire, Greg Kroah-Hartman,
	Daniel Vetter, Fabio Rafael da Rosa

Hi Hans
> >
> >>Which bring me back to a question asked a week ago or so.
> >>What is missing before we can move vboxvideo out of staging/
> >
> >I think it boils down to someone needs to submit it and we'll take a look.
> 
> Right, I have this on my TODO, but I did not manage to find time for this
> the past few weeks. I expect to have time for this in a couple of weeks
> from now, which means the move will miss the 5.1 merge window.
> 
> Note you (Sam) are certainly welcome to submit a patch doing the move
> yourself, I can certainly use some help with maintaining vboxvideo.

I have not tried to submit patches to move vboxvideo out of staging.
Partially due to lack of time, mostly lack of competence to
follow-up on the review comments I hope we will see.
I am perfectly happy to do trivial stuff, but when someone ask
to replace the use of ttm with shmem+gem (example - I do not
know if this is a relevant comment), then it is above
me for the moment.
And on top of this then I lack a good testing environment.

I could do it but then anything difficult would just be added
to a TODO file, and then I dunno how useful it is.
Something to consider when we are past the merge window anyway.

	Sam

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] staging/vboxvideo: Add TODO
@ 2019-02-13 18:46           ` Sam Ravnborg
  0 siblings, 0 replies; 15+ messages in thread
From: Sam Ravnborg @ 2019-02-13 18:46 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Greg Kroah-Hartman, LKML, DRI Development, Nicholas Mc Guire,
	Daniel Vetter, Fabio Rafael da Rosa

Hi Hans
> >
> >>Which bring me back to a question asked a week ago or so.
> >>What is missing before we can move vboxvideo out of staging/
> >
> >I think it boils down to someone needs to submit it and we'll take a look.
> 
> Right, I have this on my TODO, but I did not manage to find time for this
> the past few weeks. I expect to have time for this in a couple of weeks
> from now, which means the move will miss the 5.1 merge window.
> 
> Note you (Sam) are certainly welcome to submit a patch doing the move
> yourself, I can certainly use some help with maintaining vboxvideo.

I have not tried to submit patches to move vboxvideo out of staging.
Partially due to lack of time, mostly lack of competence to
follow-up on the review comments I hope we will see.
I am perfectly happy to do trivial stuff, but when someone ask
to replace the use of ttm with shmem+gem (example - I do not
know if this is a relevant comment), then it is above
me for the moment.
And on top of this then I lack a good testing environment.

I could do it but then anything difficult would just be added
to a TODO file, and then I dunno how useful it is.
Something to consider when we are past the merge window anyway.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/2] staging/vboxvideo: Add TODO
  2019-02-13 18:46           ` Sam Ravnborg
  (?)
@ 2019-02-13 22:18           ` Hans de Goede
  -1 siblings, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2019-02-13 22:18 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: DRI Development, LKML, Nicholas Mc Guire, Greg Kroah-Hartman,
	Daniel Vetter, Fabio Rafael da Rosa

Hi,

On 13-02-19 19:46, Sam Ravnborg wrote:
> Hi Hans
>>>
>>>> Which bring me back to a question asked a week ago or so.
>>>> What is missing before we can move vboxvideo out of staging/
>>>
>>> I think it boils down to someone needs to submit it and we'll take a look.
>>
>> Right, I have this on my TODO, but I did not manage to find time for this
>> the past few weeks. I expect to have time for this in a couple of weeks
>> from now, which means the move will miss the 5.1 merge window.
>>
>> Note you (Sam) are certainly welcome to submit a patch doing the move
>> yourself, I can certainly use some help with maintaining vboxvideo.
> 
> I have not tried to submit patches to move vboxvideo out of staging.
> Partially due to lack of time, mostly lack of competence to
> follow-up on the review comments I hope we will see.
> I am perfectly happy to do trivial stuff, but when someone ask
> to replace the use of ttm with shmem+gem (example - I do not
> know if this is a relevant comment), then it is above
> me for the moment.
> And on top of this then I lack a good testing environment.
> 
> I could do it but then anything difficult would just be added
> to a TODO file, and then I dunno how useful it is.
> Something to consider when we are past the merge window anyway.

Ok, I will try to submit a patch doing the move myself soon.

Regards,

Hans


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-02-13 22:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-04 10:31 [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0 Daniel Vetter
2019-02-04 10:31 ` [PATCH 2/2] staging/vboxvideo: Add TODO Daniel Vetter
2019-02-04 11:05   ` Greg Kroah-Hartman
2019-02-04 11:05     ` Greg Kroah-Hartman
2019-02-04 18:54   ` Sam Ravnborg
2019-02-04 18:54     ` Sam Ravnborg
2019-02-06 15:58     ` Daniel Vetter
2019-02-06 16:13       ` Hans de Goede
2019-02-13 18:46         ` Sam Ravnborg
2019-02-13 18:46           ` Sam Ravnborg
2019-02-13 22:18           ` Hans de Goede
2019-02-04 11:05 ` [PATCH 1/2] staging/vboxvideo: don't set dev_priv_size = 0 Greg Kroah-Hartman
2019-02-04 18:49 ` Sam Ravnborg
2019-02-04 18:49   ` Sam Ravnborg
2019-02-04 21:49   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.