Hi! On Thu, May 31, 2012 at 1:13 PM, Eduardo Valentin <eduardo.valentin@xxxxxx> wrote: > Hello, > > On Thu, May 31, 2012 at 01:16:37PM +0800, Amit Kachhap wrote: >> On 30 May 2012 18:30, Eduardo Valentin <eduardo.valentin@xxxxxx> wrote: >> > Hello Rui, >> > >> > Now I copied Amit, for real :-) >> > >> > I like your proposal, some comments as follow. >> > >> > On Wed, May 30, 2012 at 04:51:00PM +0800, Zhang Rui wrote: >> >> On 三, 2012-05-30 at 16:49 +0800, Zhang Rui wrote: >> >> > Hi, all, >> >> > >> >> > It is great to see more and more users of the generic thermal layer. >> >> > But as we know, the original design of the generic thermal layer comes >> >> > from ACPI thermal management, and some of its implementation seems to be >> >> > too ACPI specific nowadays. >> > >> > >> > Good. We have also basic OMAP support, on top of Amit's work. I sent >> > recently a very basic support. I will be pushing them, while they evolve. >> > >> >> > >> >> > Recently I'm thinking of enhance the generic thermal layer so that it >> >> > works well for more platforms. >> > >> > As you said, for non-ACPI support, the "generic" layer needs some >> > extension and refactoring to be more generic :-). >> > >> >> > >> >> > Below are some thoughts of mine, after reading the patches from Amit >> >> > Daniel Kachhap, and ACPI 3.0 thermal model. Actually, I have started >> >> > coding some RFC patches. But I do really want to get feedback from you >> >> > before going on. >> > >> > OK. >> > >> >> > >> >> > G1. supporting multiple cooling states for active cooling devices. >> >> > >> >> > The current active cooling device supports two cooling states only, >> >> > please refer to the code below, in driver/thermal/thermal_sys.c >> >> > case THERMAL_TRIP_ACTIVE: >> >> > ... >> >> > if (temp >= trip_temp) >> >> > cdev->ops->set_cur_state(cdev, 1); >> >> > else >> >> > cdev->ops->set_cur_state(cdev, 0); >> >> > break; >> >> > >> >> > This is an ACPI specific thing, as our ACPI FAN used to support >> >> > ON/OFF only. >> >> > I think it is reasonable to support multiple active cooling states >> >> > as they are common on many platforms, and note that this is also >> >> > true for ACPI 3.0 FAN device (_FPS). >> >> > >> >> > G2. introduce cooling states range for a certain trip point >> >> > >> >> > This problem comes with the first one. >> >> > If the cooling devices support multiple cooling states, and surely >> >> > we may want only several cooling states for a certain trip point, >> >> > and other cooling states for other active trip points. >> >> > To do this, we should be able to describe the cooling device >> >> > behavior for a certain trip point, rather than for the entire >> >> > thermal zone. >> > >> > For G1+G2, I agree with your proposal. I had some discussion with Amit >> > regarding this. In his series of patches we increase / decrease the cooling >> > device state linearly and steadily. >> > >> > But if we would have what you are saying, we could bind cooling device >> > set of states with trip points. >> > >> >> > >> >> > G3. kernel thermal passive cooling algorithm >> >> > >> >> > Currently, tc1 and tc2 are hard requirements for kernel passive >> >> > cooling. But non-ACPI platforms do not have this information >> >> > (please correct me if I'm wrong). >> >> > Say, for the patches here >> >> > http://marc.info/?l=linux-acpi&m=133681581305341&w=2 >> >> >> >> Sorry, forgot to cc Amit, the author of this patch set. >> >> >> >> thanks, >> >> rui >> >> > They just want to slow down the processor when current temperature >> >> > is higher than the trip point and speed up the processor when the >> >> > temperature is lower than the trip point. >> >> > >> >> > According to Matthew, the platform drivers are responsible to >> >> > provide proper tc1 and tc2 values to use kernel passive cooling. >> >> > But I'm just wondering if we can use something instead. >> >> > Say, introduce .get_trend() in thermal_zone_device_ops. >> >> > And we set cur_state++ or cur_state-- based on the value returned >> >> > by .get_trend(), instead of using tc1 and tc2. >> > >> > I fully support this option and could cook up something on this. >> > The TC1 and TC2 should go inside the .get_trend() callbacks for ACPI. >> > Should probably go away from the registration function that we have >> > currently. >> > >> > We could have generic trending computation though. Based on timestamping >> > and temperature reads, and make it available for zones that want to used it. >> > >> >> > >> >> > G4. Multiple passive trip points >> >> > >> >> > I get this idea also from the patches at >> >> > http://marc.info/?l=linux-acpi&m=133681581305341&w=2 >> >> > >> >> > IMO, they want to get an acceptable performance at a tolerable >> >> > temperature. >> >> > Say, a platform with four P-states. P3 is really low. >> >> > And I'm okay with the temperature at 60C, but 80C? No. >> >> > With G2 resolved, we can use processor P0~P2 for Passive trip point >> >> > 0 (50C), and P3 for Passive trip point 1 (70C). And then the >> >> > temperature may be jumping at around 60C or even 65C, without >> >> > entering P3. >> > >> > Yeah, I guess we need to solve G1+G2 first to allow this. But I also agree >> > that ideally, there should be possibility to have multiple passive trip points. >> > >> >> > >> >> > Further more, IMO, this also works for ACPI platforms. >> >> > Say, we can easily change p-state to cool the system, but using >> >> > t-state is definitely what we do not want to see. The current >> >> > implementation does not expose this difference to the generic >> >> > thermal layer, but if we can have two passive trip points, and use >> >> > p-state for the first one only... (this works if we start polling >> >> > after entering passive cooling mode, without hardware notification) >> >> > >> >> > G5. unify active cooling and passive cooling code >> >> > >> >> > If G4 and G5 are resolved, a new problem to me is that there is no >> >> > difference between passive cooling and active cooling except the >> >> > cooling policy. >> > >> > OK... >> > >> >> > Then we can share the same code for both active and passive cooling. >> >> > maybe something like: >> >> > >> >> > case THERMAL_TRIP_ACTIVE: >> >> > case THERMAL_TRIP_PASSIVE: >> >> > ... >> >> > tz->ops->get_trend(); >> > >> > Would the get_trend take into account if we are cooling with active or passive >> > cooling device? >> > >> >> > if (trend == HEATING) >> >> > cdev->ops->set_cur_state(cdev, cur_state++); >> >> > else if (trend == COOLING) >> >> > cdev->ops->set_cur_state(cdev, cur_state--); >> >> > break; >> > >> > I believe we should have something for temperature stabilization there as well. >> I also agree that thermal stablization is important. I have observed >> that too much state change is happening around the trip point. But yes >> the trend callback may take care of this and set the COOLING trend >> different from HEATING trend. >> > >> > Besides, if we go with this generic policy, then the zone update would be much >> > simpler no? >> > >> >> > >> >> > Here are the gaps in my point of view, I'd like to get your ideas about >> >> > which are reasonable and which are not. >> > >> > Here are some other thoughts: >> > G6. Another point is, would it make sense to allow for policy extension? Meaning, >> > the zone update would call a callback to request for update from the zone >> > device driver? >> This may a simple work with adding notifiers like done for CRITICAL, >> HOT trip type. >> > >> > G7. How do we solve cooling devices being shared between different thermal zones? >> > Should we have a better cooling device constraint management? >> > >> > G8. On same topic as G7, how are we currently making sure that thermal constraints >> > don't get overwritten by, let's say, userspace APIs? I guess the generic CPU cooling >> > propose by Amit suffers of an issue. If user sets cpufreq governor to userspace >> > and sets the frequency to its maximum, say in a busy loop, the thermal cooling >> > could potentially be ruined. >> Yes I agree that is a problem in my implementation but I guess the >> current cpufreq framework does not have anything to stop this. May be >> with cpufreq_max pmqos patches this will be taken care. > > Just a clarification here. I didn't really mean that this is a problem on your implementation. > But a general problem. And needs to be dealt properly. > > And to be frank, assuming that just because we selected userspace at cpufreq level it is > userland problem, it is a weak solution. Specially considering that this may lead to > potentially harmful situations. > > Not sure this needs to be solved at the generic thermal framework, but at least > the thermal framework should be aware of who is dealing with the constraint management. > > The pmqos patch is a good direction to go, IMO. Agree! There is a real need to have a framework for the following: - per-device PM QoS, - devices throughput, - thermal constraints and cooling devices, - min/max performance constraint, - ... However for good reasons the current PM QoS framework cannot address all those requirements at once. >From the code submissions and discussions on the MLs Mark Gross came with the very good idea of organizing a specific micro-conference around the constraints framework at LPC [1]. The topics starting with 'lpc2012-cf-' are part of this micro-conference. Most of the topics -if not all- are about thermal management integration. [1] https://blueprints.launchpad.net/lpc I have big expectations from the LPC. Meeting face to face and discuss the proposals is a good way to make progress, which is not so efficient on the MLs. Regards, Jean > >> > >> > G9. Is there any possibility to have multiple sensors per thermal zone? >> > >> > G10. Do we want to represent other sensing stimuli other that temperature? Say, >> > current sensing? >> Yes this is good field to look into. >> > >> > G11. Do we want to allow for cross zoning policies? Sometimes a policy may use >> > temperature from different thermal zone in order to better represent what >> > is going on in its thermal zone. >> > >> >> > >> >> > Any comments are appreciated! Thanks! >> > >> > >> > Thanks to you for starting this up! The above are points that come to my mind now. >> > I will keep updating the list if something else come to my mind. >> > >> >> > >> >> > -rui >> >> > >> >> > _______________________________________________ >> >> > linux-pm mailing list >> >> > linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx >> >> > https://lists.linuxfoundation.org/mailman/listinfo/linux-pm >> >> >> >> >> >> _______________________________________________ >> >> linux-pm mailing list >> >> linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx >> >> https://lists.linuxfoundation.org/mailman/listinfo/linux-pm >> > >> > All best, >> > >> > --- >> > Eduardo Valentin > _______________________________________________ > linux-pm mailing list > linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx > https://lists.linuxfoundation.org/mailman/listinfo/linux-pm -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
