Re: [PATCH v4 01/11] dt-bindings: video-interfaces: Document 'location' property
Hi! > > I'm skeptical about adding now a property for a device that we don't > > support, because we -now- think it's a good idea. I might be wrong, > > but my assumption is that when someone will want to support an > > 'advanced' device, it's easy to add "movable" or whatever else to the > > list of accepted properties values. Am I wrong in assuming this? As > > long as "front" "back" and "external" will stay supported for backward > > DTB compatibility it should be fine, right ? > > The basic rule is that you should not define things unless you KNOW that > they will be needed. So when we actually see new devices for which > "front", "back" or "external" does not fit, then new names can be > created. > It's impossible to cover all situations since we can't predict the future. > The best we can do is to allow for future extensions. Those devices are already being sold, and yes, they are running linux (with some patches probably). I believe it would be better to specify "this camera is selfie -- takes pictures of the user" vs. "this is main camera -- takes pictures of what user is looking at". Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: PGP signature
Re: [PATCH v4 01/11] dt-bindings: video-interfaces: Document 'location' property
On 10/8/19 10:20 AM, Jacopo Mondi wrote: > On Tue, Oct 08, 2019 at 10:06:34AM +0200, Pavel Machek wrote: >> On Tue 2019-10-08 09:58:28, Jacopo Mondi wrote: >>> Pavel, >>> >>> On Tue, Oct 08, 2019 at 09:40:52AM +0200, Pavel Machek wrote: On Mon 2019-10-07 18:29:03, Jacopo Mondi wrote: > Add the 'location' device property, used to specify a device mounting > position. The property is particularly meaningful for mobile devices > with a well defined usage orientation. > > Signed-off-by: Jacopo Mondi > --- > .../devicetree/bindings/media/video-interfaces.txt| 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > b/Documentation/devicetree/bindings/media/video-interfaces.txt > index f884ada0bffc..1211bdf80722 100644 > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > @@ -89,6 +89,17 @@ Optional properties >but a number of degrees counter clockwise. Typical values are 0 and 180 >(upside down). > > +- location: The mount location of a device (typically an image sensor or > a flash > + LED) expressed as a position relative to the usage orientation of the > system > + where the device is installed on. > + Possible values are: > + 0 - Front. The device is mounted on the front facing side of the > system. > + For mobile devices such as smartphones, tablets and laptops the front > side is > + the user facing side. > + 1 - Back. The device is mounted on the back side of the system, which > is > + defined as the opposite side of the front facing one. > + 2 - External. The device is not attached directly to the system but is > + attached in a way that allows it to move freely. I explained why this is not enough, and it did not change. >>> >>> I replied to your email and you did not answered back. >>> >>> I appreciate constructive inputs but just NAK, or throwing a proposal >>> without following up as you did, doesn't help much in improving the end >>> result. >>> >>> I'll paste here my previous reply, so you get a chance to continue the >>> discussion. Please follow up if you're interested in contributing. >> >> Yes, you are basically saying someone can solve those problems in >> future :-(. > > Not really, what I'm saying is that the property definition itself > leaves space for future extensions. As of now the accepted property > values cover the most trivial use case, but they can be expanded to > accommodate more complex setups, possibly with an extended parameters > list (in example something like "movable" with an associated rotation > matrix). In any case, I don't think we're tying anyone's hands by > adding this new property with the most basic locations that covers > 99% of devices. > >> >> I'd add note that for camera-style devices, main sensor would be the >> "back" one, and that for phones, selfie sensor should be marked as a >> "front" one. > > I'm not sure if 'main' or 'selfie' as concepts belongs here at all. I > just want to report where the camera is installed, not the intended > usage. > > The most common use case is to make simple for application to pick one > of the cameras based on the position. The front camera in a phone will > likely be for selfies, but in a laptop will mostly be for > 'videoconference' or whatever. This is a definition that totally > belongs to userspace, and kernel should just report the mounting > location and that's it. > >> >> Plus, I believe you need to add a value for moving sensors, as there >> are already devices that use same sensor for "front" and "back". > > I'm skeptical about adding now a property for a device that we don't > support, because we -now- think it's a good idea. I might be wrong, > but my assumption is that when someone will want to support an > 'advanced' device, it's easy to add "movable" or whatever else to the > list of accepted properties values. Am I wrong in assuming this? As > long as "front" "back" and "external" will stay supported for backward > DTB compatibility it should be fine, right ? The basic rule is that you should not define things unless you KNOW that they will be needed. So when we actually see new devices for which "front", "back" or "external" does not fit, then new names can be created. It's impossible to cover all situations since we can't predict the future. The best we can do is to allow for future extensions. I think this looks good. Regards, Hans > > Thanks >j > >> >> Pavel >> >> >>> I don't think this is nearly enough of description. We have phones with displays and cameras at both sides, where both sides can be used >
Re: [PATCH v4 01/11] dt-bindings: video-interfaces: Document 'location' property
Hi Jacopo, (Cc'ing the devicetree list.) On Mon, Oct 07, 2019 at 06:29:03PM +0200, Jacopo Mondi wrote: > Add the 'location' device property, used to specify a device mounting > position. The property is particularly meaningful for mobile devices > with a well defined usage orientation. > > Signed-off-by: Jacopo Mondi > --- > .../devicetree/bindings/media/video-interfaces.txt| 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > b/Documentation/devicetree/bindings/media/video-interfaces.txt > index f884ada0bffc..1211bdf80722 100644 > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > @@ -89,6 +89,17 @@ Optional properties >but a number of degrees counter clockwise. Typical values are 0 and 180 >(upside down). > > +- location: The mount location of a device (typically an image sensor or a > flash > + LED) expressed as a position relative to the usage orientation of the > system > + where the device is installed on. > + Possible values are: > + 0 - Front. The device is mounted on the front facing side of the system. How about starting from 1? Then 0 can remain "undefined" value, i.e. the caller can initialise the value to zero without the need to figure out whether reading a given property was successful or not. > + For mobile devices such as smartphones, tablets and laptops the front side > is > + the user facing side. > + 1 - Back. The device is mounted on the back side of the system, which is > + defined as the opposite side of the front facing one. > + 2 - External. The device is not attached directly to the system but is > + attached in a way that allows it to move freely. > > Optional endpoint properties > -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v4 01/11] dt-bindings: video-interfaces: Document 'location' property
On Tue, Oct 08, 2019 at 10:06:34AM +0200, Pavel Machek wrote: > On Tue 2019-10-08 09:58:28, Jacopo Mondi wrote: > > Pavel, > > > > On Tue, Oct 08, 2019 at 09:40:52AM +0200, Pavel Machek wrote: > > > On Mon 2019-10-07 18:29:03, Jacopo Mondi wrote: > > > > Add the 'location' device property, used to specify a device mounting > > > > position. The property is particularly meaningful for mobile devices > > > > with a well defined usage orientation. > > > > > > > > Signed-off-by: Jacopo Mondi > > > > --- > > > > .../devicetree/bindings/media/video-interfaces.txt| 11 +++ > > > > 1 file changed, 11 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/media/video-interfaces.txt > > > > b/Documentation/devicetree/bindings/media/video-interfaces.txt > > > > index f884ada0bffc..1211bdf80722 100644 > > > > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > > > > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > > > > @@ -89,6 +89,17 @@ Optional properties > > > >but a number of degrees counter clockwise. Typical values are 0 and > > > > 180 > > > >(upside down). > > > > > > > > +- location: The mount location of a device (typically an image sensor > > > > or a flash > > > > + LED) expressed as a position relative to the usage orientation of > > > > the system > > > > + where the device is installed on. > > > > + Possible values are: > > > > + 0 - Front. The device is mounted on the front facing side of the > > > > system. > > > > + For mobile devices such as smartphones, tablets and laptops the > > > > front side is > > > > + the user facing side. > > > > + 1 - Back. The device is mounted on the back side of the system, > > > > which is > > > > + defined as the opposite side of the front facing one. > > > > + 2 - External. The device is not attached directly to the system but > > > > is > > > > + attached in a way that allows it to move freely. > > > > > > I explained why this is not enough, and it did not change. > > > > I replied to your email and you did not answered back. > > > > I appreciate constructive inputs but just NAK, or throwing a proposal > > without following up as you did, doesn't help much in improving the end > > result. > > > > I'll paste here my previous reply, so you get a chance to continue the > > discussion. Please follow up if you're interested in contributing. > > Yes, you are basically saying someone can solve those problems in > future :-(. Not really, what I'm saying is that the property definition itself leaves space for future extensions. As of now the accepted property values cover the most trivial use case, but they can be expanded to accommodate more complex setups, possibly with an extended parameters list (in example something like "movable" with an associated rotation matrix). In any case, I don't think we're tying anyone's hands by adding this new property with the most basic locations that covers 99% of devices. > > I'd add note that for camera-style devices, main sensor would be the > "back" one, and that for phones, selfie sensor should be marked as a > "front" one. I'm not sure if 'main' or 'selfie' as concepts belongs here at all. I just want to report where the camera is installed, not the intended usage. The most common use case is to make simple for application to pick one of the cameras based on the position. The front camera in a phone will likely be for selfies, but in a laptop will mostly be for 'videoconference' or whatever. This is a definition that totally belongs to userspace, and kernel should just report the mounting location and that's it. > > Plus, I believe you need to add a value for moving sensors, as there > are already devices that use same sensor for "front" and "back". I'm skeptical about adding now a property for a device that we don't support, because we -now- think it's a good idea. I might be wrong, but my assumption is that when someone will want to support an 'advanced' device, it's easy to add "movable" or whatever else to the list of accepted properties values. Am I wrong in assuming this? As long as "front" "back" and "external" will stay supported for backward DTB compatibility it should be fine, right ? Thanks j > > Pavel > > > > > > > I don't think this is nearly enough of description. We have phones > > > with displays and cameras at both sides, where both sides can be used > > > to operate the system. > > > > > > We have phone with display spanning both sides -- Mi Max. > > > > > > https://www.idnes.cz/mobil/telefony/xiaomi-mi-mix-alpha-predstaveni.A190924_105858_telefony_oma > > > > > > We have Galaxy Fold. > > > > > > https://www.samsung.com/global/galaxy/galaxy-fold/ > > > > > > What is front side when device can be used in different > > > configurations? > > > > > > Could we instead s
Re: [PATCH v4 01/11] dt-bindings: video-interfaces: Document 'location' property
On Tue 2019-10-08 09:58:28, Jacopo Mondi wrote: > Pavel, > > On Tue, Oct 08, 2019 at 09:40:52AM +0200, Pavel Machek wrote: > > On Mon 2019-10-07 18:29:03, Jacopo Mondi wrote: > > > Add the 'location' device property, used to specify a device mounting > > > position. The property is particularly meaningful for mobile devices > > > with a well defined usage orientation. > > > > > > Signed-off-by: Jacopo Mondi > > > --- > > > .../devicetree/bindings/media/video-interfaces.txt| 11 +++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > > > b/Documentation/devicetree/bindings/media/video-interfaces.txt > > > index f884ada0bffc..1211bdf80722 100644 > > > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > > > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > > > @@ -89,6 +89,17 @@ Optional properties > > >but a number of degrees counter clockwise. Typical values are 0 and 180 > > >(upside down). > > > > > > +- location: The mount location of a device (typically an image sensor or > > > a flash > > > + LED) expressed as a position relative to the usage orientation of the > > > system > > > + where the device is installed on. > > > + Possible values are: > > > + 0 - Front. The device is mounted on the front facing side of the > > > system. > > > + For mobile devices such as smartphones, tablets and laptops the front > > > side is > > > + the user facing side. > > > + 1 - Back. The device is mounted on the back side of the system, which > > > is > > > + defined as the opposite side of the front facing one. > > > + 2 - External. The device is not attached directly to the system but is > > > + attached in a way that allows it to move freely. > > > > I explained why this is not enough, and it did not change. > > I replied to your email and you did not answered back. > > I appreciate constructive inputs but just NAK, or throwing a proposal > without following up as you did, doesn't help much in improving the end > result. > > I'll paste here my previous reply, so you get a chance to continue the > discussion. Please follow up if you're interested in contributing. Yes, you are basically saying someone can solve those problems in future :-(. I'd add note that for camera-style devices, main sensor would be the "back" one, and that for phones, selfie sensor should be marked as a "front" one. Plus, I believe you need to add a value for moving sensors, as there are already devices that use same sensor for "front" and "back". Pavel > > > I don't think this is nearly enough of description. We have phones > > with displays and cameras at both sides, where both sides can be used > > to operate the system. > > > > We have phone with display spanning both sides -- Mi Max. > > > > https://www.idnes.cz/mobil/telefony/xiaomi-mi-mix-alpha-predstaveni.A190924_105858_telefony_oma > > > > We have Galaxy Fold. > > > > https://www.samsung.com/global/galaxy/galaxy-fold/ > > > > What is front side when device can be used in different > > configurations? > > > > Could we instead say that it is "main" vs "selfie" camera? > > I'm not sure the intended usage is something that belongs to DT. And > 'selfie' implies you have a device side facing you, most like the > 'front' one I have defined here. > > Not to mention again this devices are all but supported by mainline, > which is just a partial justification as they might be an indication > of a trend. > > There is no usable reference place, reference side, reference usage > mode that applies to -all- devices in the world, not one I can think > of. > > I still think defining a location property is not blocking any new > extension that accommodate more advanced use cases. It's not like we're > adding a "front-camera" property, it's a "location" and you can expand > its accepted values with "front-when-device-folded" or whatever you > need for future devices. > > In the description I mentioned the "usage orientation" to leave room > for possible device-specific details in the definition of the values > accepted by the property. > > > > + location expressed as a position relative to the usage orientation of > > > the > > > + system where the device is installed on. > > 99% of devices in the world have a front and a back, as well as they > have a top and a bottom. I still don't see why if a device does not > simply has a front it cannot use something different. The property > definition allows you to do so. > > > > > > > NAK. > > Pavel > > > > -- > > (english) http://www.livejournal.com/~pavelmachek > > (cesky, pictures) > > http://atrey.karlin.mff.cuni.cz/~p
Re: [PATCH v4 01/11] dt-bindings: video-interfaces: Document 'location' property
Pavel, On Tue, Oct 08, 2019 at 09:40:52AM +0200, Pavel Machek wrote: > On Mon 2019-10-07 18:29:03, Jacopo Mondi wrote: > > Add the 'location' device property, used to specify a device mounting > > position. The property is particularly meaningful for mobile devices > > with a well defined usage orientation. > > > > Signed-off-by: Jacopo Mondi > > --- > > .../devicetree/bindings/media/video-interfaces.txt| 11 +++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > > b/Documentation/devicetree/bindings/media/video-interfaces.txt > > index f884ada0bffc..1211bdf80722 100644 > > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > > @@ -89,6 +89,17 @@ Optional properties > >but a number of degrees counter clockwise. Typical values are 0 and 180 > >(upside down). > > > > +- location: The mount location of a device (typically an image sensor or a > > flash > > + LED) expressed as a position relative to the usage orientation of the > > system > > + where the device is installed on. > > + Possible values are: > > + 0 - Front. The device is mounted on the front facing side of the system. > > + For mobile devices such as smartphones, tablets and laptops the front > > side is > > + the user facing side. > > + 1 - Back. The device is mounted on the back side of the system, which is > > + defined as the opposite side of the front facing one. > > + 2 - External. The device is not attached directly to the system but is > > + attached in a way that allows it to move freely. > > I explained why this is not enough, and it did not change. I replied to your email and you did not answered back. I appreciate constructive inputs but just NAK, or throwing a proposal without following up as you did, doesn't help much in improving the end result. I'll paste here my previous reply, so you get a chance to continue the discussion. Please follow up if you're interested in contributing. > I don't think this is nearly enough of description. We have phones > with displays and cameras at both sides, where both sides can be used > to operate the system. > > We have phone with display spanning both sides -- Mi Max. > > https://www.idnes.cz/mobil/telefony/xiaomi-mi-mix-alpha-predstaveni.A190924_105858_telefony_oma > > We have Galaxy Fold. > > https://www.samsung.com/global/galaxy/galaxy-fold/ > > What is front side when device can be used in different > configurations? > > Could we instead say that it is "main" vs "selfie" camera? I'm not sure the intended usage is something that belongs to DT. And 'selfie' implies you have a device side facing you, most like the 'front' one I have defined here. Not to mention again this devices are all but supported by mainline, which is just a partial justification as they might be an indication of a trend. There is no usable reference place, reference side, reference usage mode that applies to -all- devices in the world, not one I can think of. I still think defining a location property is not blocking any new extension that accommodate more advanced use cases. It's not like we're adding a "front-camera" property, it's a "location" and you can expand its accepted values with "front-when-device-folded" or whatever you need for future devices. In the description I mentioned the "usage orientation" to leave room for possible device-specific details in the definition of the values accepted by the property. > > + location expressed as a position relative to the usage orientation of the > > + system where the device is installed on. 99% of devices in the world have a front and a back, as well as they have a top and a bottom. I still don't see why if a device does not simply has a front it cannot use something different. The property definition allows you to do so. > > NAK. > Pavel > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: PGP signature
Re: [PATCH v4 01/11] dt-bindings: video-interfaces: Document 'location' property
On Mon 2019-10-07 18:29:03, Jacopo Mondi wrote: > Add the 'location' device property, used to specify a device mounting > position. The property is particularly meaningful for mobile devices > with a well defined usage orientation. > > Signed-off-by: Jacopo Mondi > --- > .../devicetree/bindings/media/video-interfaces.txt| 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt > b/Documentation/devicetree/bindings/media/video-interfaces.txt > index f884ada0bffc..1211bdf80722 100644 > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > @@ -89,6 +89,17 @@ Optional properties >but a number of degrees counter clockwise. Typical values are 0 and 180 >(upside down). > > +- location: The mount location of a device (typically an image sensor or a > flash > + LED) expressed as a position relative to the usage orientation of the > system > + where the device is installed on. > + Possible values are: > + 0 - Front. The device is mounted on the front facing side of the system. > + For mobile devices such as smartphones, tablets and laptops the front side > is > + the user facing side. > + 1 - Back. The device is mounted on the back side of the system, which is > + defined as the opposite side of the front facing one. > + 2 - External. The device is not attached directly to the system but is > + attached in a way that allows it to move freely. I explained why this is not enough, and it did not change. NAK. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
