Message-ID: <d5470665-4fee-432a-9cb7-fff9813b3e97@redhat.com>
Date: Thu, 13 Mar 2025 16:13:24 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Niklas Cassel <cassel@...nel.org>
Cc: Eric <eric.4.debian@...batoulnz.fr>,
 Salvatore Bonaccorso <carnil@...ian.org>,
 Mario Limonciello <mario.limonciello@....com>,
 Christoph Hellwig <hch@...radead.org>,
 Mika Westerberg <mika.westerberg@...ux.intel.com>,
 Damien Le Moal <dlemoal@...nel.org>, Jian-Hong Pan <jhp@...lessos.org>,
 regressions@...ts.linux.dev, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org, linux-ide@...r.kernel.org,
 Dieter Mummenschanz <dmummenschanz@....de>
Subject: Re: Regression from 7627a0edef54 ("ata: ahci: Drop low power policy
 board type") on reboot (but not cold boot)

Hi Niklas,

On 13-Mar-25 1:48 PM, Niklas Cassel wrote:
> Hello Hans,
> 
> On Thu, Mar 13, 2025 at 11:04:13AM +0100, Hans de Goede wrote:
>>
>> I do agree with you that it is a question if this is another bad
>> interaction with Samsung SATA SSDs, or if it is a general ATI SATA
>> controller problem, but see below.
> 
> (snip)
> 
>> Right in the mean time Eric has reported back that the above patch fixes
>> this. Thank you for testing this Eric,
>>
>> One reason why ATA_QUIRK_NO_NCQ_ON_ATI was introduced is because
>> disabling NCQ has severe performance impacts for SSDs, so we did not want
>> to do this for all ATI controllers; or for all Samsung drives. Given that
>> until the recent LPM default change we did not use DIPM on ATI chipsets
>> the above fix IMHO is a good fix, which even keeps the rest of the LPM
>> power-savings.
> 
> One slightly interesting thing was that neither the Maxtor or the Samsung
> drive reported support for Host-Initiated Power Management (HIPM).
> 
> Both drives supported Device-Initiated Power Management (DIPM), and we
> could see that DIPM was enabled on both drives.
> 
> We already know that LPM works on the Samsung drive with an Intel AHCI
> controller. (But since the device does not report support for HIPM, even
> on Intel, only DIPM will be used/enabled.)
> 
> 
>>
>> Right I think it is safe to assume that this is not a Samsung drive problem
>> it is an ATI controller problem. The only question is if this only impacts
>> ATI <-> Samsung SSD combinations or if it is a general issue with ATI
>> controllers. But given the combination of DIPM not having been enabled
>> on these controllers by default anyways, combined with the age of these
>> motherboards (*) I believe that the above patch is a good compromise to
>> fix the regression without needing to wait for more data.
>>
>> Regards,
>>
>> Hans
>>
>> *) And there thus being less users making getting more data hard. And
>> alo meaning not having DIPM will impact only the relatively few remaining
>> users
> 
> I'm still not 100% sure with the best way forward.
> 
> The ATI SATA controller reports that it supports ALPM (i.e. also HIPM).
> It also reports support for slumber and partial, which means that it must
> support both host initiated and device initiated requests to these states.
> (See AHCI spec 3.1.1 - Offset 00h: CAP – HBA Capabilities,
> CAP.PSC and CAP.SSC fields.)
> 
> Considering that DIPM seems to work fine on the Maxtor drive, I guess your
> initial suggestion of a Samsung only quirk which only disables LPM on ATI
> is the best way?

I have no objections against going that route, except that I guess this
should then be something like ATA_QUIRK_NO_DIPM_ON_ATI to not loose the
other LPM modes / savings? AFAIK/IIRC there still is quite some powersaving
to be had without DIPM.

> It seems that ATI and Samsung must have interpreted some spec differently
> from each other, otherwise, I don't understand why this combination
> specificially seems to be so extremely bad, ATI + anything other than
> Samsung, or Samsung + anything other than ATI seems to work.

Yes the most severe problems do seem to come from that specific mix,
although the long list of other ATI controller quirks also shows those
controllers are somewhat finicky.

Regards,

Hans

Powered by blists - more mailing lists