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 <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Nicholas Mc Guire <[email protected]>
Cc: Emil Velikov <[email protected]>
Cc: Fabio Rafael da Rosa <[email protected]>
---
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
Noticed why wonder what vboxvideo is using the ->master_set/drop hooks
for.
Signed-off-by: Daniel Vetter <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Fabio Rafael da Rosa <[email protected]>
Cc: Nicholas Mc Guire <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Hans de Goede <[email protected]>
---
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 <[email protected]>,
--
2.20.1
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 <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Nicholas Mc Guire <[email protected]>
> Cc: Emil Velikov <[email protected]>
> Cc: Fabio Rafael da Rosa <[email protected]>
> ---
> drivers/staging/vboxvideo/vbox_drv.c | 1 -
> 1 file changed, 1 deletion(-)
Acked-by: Greg Kroah-Hartman <[email protected]>
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 <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Fabio Rafael da Rosa <[email protected]>
> Cc: Nicholas Mc Guire <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Hans de Goede <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
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 <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Nicholas Mc Guire <[email protected]>
> Cc: Emil Velikov <[email protected]>
> Cc: Fabio Rafael da Rosa <[email protected]>
> ---
> 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 <[email protected]>
Sam
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 <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Fabio Rafael da Rosa <[email protected]>
> Cc: Nicholas Mc Guire <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Hans de Goede <[email protected]>
> ---
> 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 <[email protected]>,
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 <[email protected]>
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
On Mon, Feb 4, 2019 at 7:49 PM Sam Ravnborg <[email protected]> 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 <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Hans de Goede <[email protected]>
> > Cc: Daniel Vetter <[email protected]>
> > Cc: Nicholas Mc Guire <[email protected]>
> > Cc: Emil Velikov <[email protected]>
> > Cc: Fabio Rafael da Rosa <[email protected]>
> > ---
> > 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 <[email protected]>
>
> Sam
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
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 <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Fabio Rafael da Rosa <[email protected]>
> > Cc: Nicholas Mc Guire <[email protected]>
> > Cc: Daniel Vetter <[email protected]>
> > Cc: Hans de Goede <[email protected]>
> > ---
> > 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 <[email protected]>,
>
> 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 <[email protected]>
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
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 <[email protected]>
>>> Cc: Greg Kroah-Hartman <[email protected]>
>>> Cc: Fabio Rafael da Rosa <[email protected]>
>>> Cc: Nicholas Mc Guire <[email protected]>
>>> Cc: Daniel Vetter <[email protected]>
>>> Cc: Hans de Goede <[email protected]>
>>> ---
>>> 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 <[email protected]>,
>>
>> 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 <[email protected]>
>
> 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
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
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