This is another reason why we can't just make 'model' be the definitive everything and why I've been pushing for a taxonomy based tuple so that we can include as much data as necessary.

That is go/cros-naming, but it isn't quite there yet. Could you have another quick look at the open comments? e.g. is 'sub-model' a good name?

Also are you absolutely sure we cannot use a different model name for these two variants?

We could but its going to be a mess with the partners. The reason is because of how they view and discuss their offerings in the market. They don't give their devices 2 different names when there's options for their configuration. My example was for powerd, but audio configs are also an issue. We need to know mic numbers and there devices that have different config depending on what sku is sold.

I'm not comfortable with where this is going. We're just going to create a confusing mess v2. Let's discuss it in the morning.

Where what is going? The reality of the devices that are being built? Shoehorning everything into 'model' when we can just as easily provide 'model'-'sku num' or whatever seems silly since it's not hard to provide the amount of data necessary. It's just not worth fighting the human confusion we can solve those decisions with further refinement of the underlying details.

By 'where this is going' I mean where we are heading with the meaning of a model.

I don't see much point in having the concept of a model if we don't use it to distinguish most devices. I worry that the design of unified builds will melt away if we head down that path. I'm fine with small variations, but we were talking about chassis colour and OEM, not audio settings.

The confusion doesn't come from the use of model; it comes from the number of devices we have. Obviously we need to set a division for how much of the 'device space' is defined by 'model' and how much by 'sub-model' (or whatever the term is). I would like to think very hard before giving up and making every package have its own configuration for sub-model.

'model' provides an answer for a large number of questions. However, it is not complete for all questions where you need to get into the details. I think your disappointment is the reality that 'model' cannot and does not answer all questions. I've been saying this same thing for quite a while. There needs to be more refinement for making certain decisions. Instead of thinking of 'model' being solution think of it as model plus another piece of information. That's the taxonomy and where 'model' works today you can ignore the more refinement piece for most questions. But there will always be cases where additional data is required. And if we get everyone in the habit of using the same command (taxonomy) then we should be future proof. Counting on having a unique name for 'model' for every possible step up/option of someone selling a device will just confuse the humans. It's not going to pass muster. I can go through the coral sku table with you to understand those, but there are a lot of cases outside of coral where model isn't enough either.

OK let's go through that together f2f. I really want to understand this before giving up.

Also, let's finish off go/cros-naming and get it out for review. That aims to provide the full taxonomy we need.

Back to the original powerd point: is this just the set_wifi_transmit_power_for_tablet_mode pref (which would probably be a no-op if powerd doesn't find a tablet mode switch), or are there other things that need to vary across a single model?

Dan, we have the trackpad wake problem. Right now we usually turn it off for convertibles until we've sorted out our ability to dynamically turn it of off based on lid angle, etc. That's through flag files. However, coral as a single image will have convertible and clamshell devices. Therefore, we need a way to pass said info on to powerd with the notion of clamshell vs convertible.

I think that we control that through udev rules rather than powerd prefs (see https://chromium.googlesource.com/chromiumos/platform2/+/master/power_manager/udev/, and particularly the 'optional' directory).

I'm not sure if that helps or not, though -- those udev rules still get installed by the power_manager ebuild (https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/chromeos-base/power_manager/power_manager-9999.ebuild#111).

Dan, I was misremembering how we disabled that piece and though it was through a flag file, but it's not as you noted: https://chromium-review.googlesource.com/c/438027 And you cleaned that up here: https://chromium-review.googlesource.com/c/476150

Regardless, though, we need a runtime decision on enacting those rules. Do you have ideas on that. I think the touchpad wake is the only thing I can think of right now that would impact powerd w.r.t. coral.

https://crrev.com/c/476150 was for touchscreen wakeup ( issue 487380 ). The concern here is only around touchpad wakeup ( https://crbug.com/710472#c5 ), right?

I wouldn't object to us just disabling touchpad wakeup for any models that include convertible boards. Benson and Sameer, what do you think?

It'd be nice if we collected metrics about wake sources to help inform decisions like this. I don't know if that's something that's easy for powerd_suspend to deduce right now, though.

Yes, this is concerning wakeups from touchpad. For all the convertibles we've been disabling it. However, on coral we have both clamshells and convertibles so we need a way to dynamically enable touchpad wake ups at runtime until we've concocted a better solution for convertibles.

I guess there's no way to do conditional udev rules at runtime? Or do we send some dbus command to change the setting? But then we'd need a way to cache the setting across powerd restarts... 

I could probably add something gross to powerd here, like a disable_touchpad_wakeup_if_convertible pref that tells powerd to ignore the directives from udev and disable wakeup for devices with internal_touchpad tags if a tablet mode switch was found... but I'd rather not. :-P

If we were to instead try to change the udev rules at runtime, how would we trigger that? In other words, what would we use to determine which rules to install? Would we also key that off of whether there's a tablet mode switch?

It already feels arbitrary to me that convertible devices don't support touchpad wakeup even though (I think) the vast majority of their use is in a laptop configuration, so like I mentioned before, I'd be okay with doing the same thing on clamshells in this case.

Runtime, conditional udev labeling is a widespread practice in Linux distros and it's not particularly complicated. If you'd like one of us to implement that, I'd be happy to do so.

The only alternative outside of runtime currently is to disable touchpad wakeups even on clamshells. As for the implementation using udev rules. I don't have a good solution to be honest aside from doing random hacks with symlinks with bind mounts, etc. That's just not appropriate, though.

I have a few other ideas about populating attributes about a device (like is it box, clamshell, detachable, convertible, etc) one time at startup in a tmps. Then we could use that to query for information. But I haven't done anything beyond coming up with said idea. Maybe that's something we should pursue?

If we can use sub-model to determine whether it is a clamshell or convertible then we can put the touchpad setting in the master configuration.

Then we can read it (e.g. with mosys) at runtime and select the correct config file based on the code we already have.

I haven't been involved in the larger naming discussion, but if it's possible to get udev to use the correct set of rules (so that the tags are already in place when powerd hears about the devices), that sounds great to me.

After discussing this with hychao I think we could use the sub-model idea for this one (go/cros-naming). We can add properties to the master configuration like:

astronaut {
   submodel-34 {
      audio {
         dsp-ini = "stereo/dsp.ini";
         hifi-conf = "stereo/hifi.conf";
         volume-curves = "stereo/bxtda7219max";
         alsa-conf = "stereo/bxtda7219max.conf;
         topology = "astronaut_topology.bin";
         firmware = "dfw_sst.bin";
      };
   };

   submodel-35 {
      audio {
         dsp-ini = "mono/dsp.ini";
         hifi-conf = "mono/hifi.conf";
         volume-curves = "mono/bxtda7219max";
         alsa-conf = "mono/bxtda7219max.conf;
         topology = "astronaut_topology.bin";
         firmware = "dfw_sst.bin";
      };
   };

then when the configuration is requested, it returns the correct item for the current sub-model.

The sub-model names are arbitrary, perhaps just the SKU ID.

My understanding for now is that we will use a different model for convertible and clamshell. Is that right?

yes

OK I'll close this and we can revisit if things change.