| Message ID | 20240519130610.7773-1-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for Panel Power Savings property | expand |
This makes sense to me in general. I like the fact that it's simple and vendor-neutral. Do we want to hardcode "panel" in the name? Are we sure that this will ever only apply to panels? Do we want to use a name which reflects the intent, rather than the mechanism? In other words, something like "color fidelity" = "preferred" maybe? (I don't know, just throwing ideas around.) Would be nice to add documentation for the property in the "standard connector properties" section.
On 5/21/2024 08:43, Simon Ser wrote: > This makes sense to me in general. I like the fact that it's simple and > vendor-neutral. > > Do we want to hardcode "panel" in the name? Are we sure that this will > ever only apply to panels? > > Do we want to use a name which reflects the intent, rather than the > mechanism? In other words, something like "color fidelity" = "preferred" > maybe? (I don't know, just throwing ideas around.) In that vein, how about: "power saving policy" --> "power saving" --> "color fidelity" > > Would be nice to add documentation for the property in the "standard > connector properties" section. Ack.
Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello <mario.limonciello@amd.com>: > > On 5/21/2024 08:43, Simon Ser wrote: > > This makes sense to me in general. I like the fact that it's simple and > > vendor-neutral. > > > > Do we want to hardcode "panel" in the name? Are we sure that this will > > ever only apply to panels? > > > > Do we want to use a name which reflects the intent, rather than the > > mechanism? In other words, something like "color fidelity" = "preferred" > > maybe? (I don't know, just throwing ideas around.) > > In that vein, how about: > > "power saving policy" > --> "power saving" > --> "color fidelity" It's not just about colors though, is it? The compositor might want to disable it to increase the backlight brightness in bright environments, so "color fidelity" doesn't really sound right > > > > Would be nice to add documentation for the property in the "standard > > connector properties" section. > > Ack. >
On 5/21/2024 11:14, Xaver Hugl wrote: > Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello > <mario.limonciello@amd.com>: >> >> On 5/21/2024 08:43, Simon Ser wrote: >>> This makes sense to me in general. I like the fact that it's simple and >>> vendor-neutral. >>> >>> Do we want to hardcode "panel" in the name? Are we sure that this will >>> ever only apply to panels? >>> >>> Do we want to use a name which reflects the intent, rather than the >>> mechanism? In other words, something like "color fidelity" = "preferred" >>> maybe? (I don't know, just throwing ideas around.) >> >> In that vein, how about: >> >> "power saving policy" >> --> "power saving" >> --> "color fidelity" > > It's not just about colors though, is it? The compositor might want to > disable it to increase the backlight brightness in bright > environments, so "color fidelity" doesn't really sound right Either of these better? "power saving policy" --> "power saving" --> "accuracy" "power saving policy" --> "allowed" --> "forbidden" Or any other idea? > >>> >>> Would be nice to add documentation for the property in the "standard >>> connector properties" section. >> >> Ack. >>
On 2024-05-21 12:21, Mario Limonciello wrote: > On 5/21/2024 11:14, Xaver Hugl wrote: >> Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello >> <mario.limonciello@amd.com>: >>> >>> On 5/21/2024 08:43, Simon Ser wrote: >>>> This makes sense to me in general. I like the fact that it's simple and >>>> vendor-neutral. >>>> >>>> Do we want to hardcode "panel" in the name? Are we sure that this will >>>> ever only apply to panels? >>>> >>>> Do we want to use a name which reflects the intent, rather than the >>>> mechanism? In other words, something like "color fidelity" = "preferred" >>>> maybe? (I don't know, just throwing ideas around.) >>> >>> In that vein, how about: >>> >>> "power saving policy" >>> --> "power saving" >>> --> "color fidelity" >> >> It's not just about colors though, is it? The compositor might want to >> disable it to increase the backlight brightness in bright >> environments, so "color fidelity" doesn't really sound right > > Either of these better? > > "power saving policy" > --> "power saving" > --> "accuracy" > > "power saving policy" > --> "allowed" > --> "forbidden" > > Or any other idea? Another consideration in addition to accuracy is latency. I suppose a compositor may want to disable features such as PSR for use-cases requiring low latency. Touch and stylus input are some examples. I wonder if flags would work better than enums? A compositor can set something like `REQUIRE_ACCURACY & REQUIRE_LOW_LATENCY`, for example. - Leo > >> >>>> >>>> Would be nice to add documentation for the property in the "standard >>>> connector properties" section. >>> >>> Ack. >>> >
On 5/21/2024 12:27, Leo Li wrote: > > > On 2024-05-21 12:21, Mario Limonciello wrote: >> On 5/21/2024 11:14, Xaver Hugl wrote: >>> Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello >>> <mario.limonciello@amd.com>: >>>> >>>> On 5/21/2024 08:43, Simon Ser wrote: >>>>> This makes sense to me in general. I like the fact that it's simple >>>>> and >>>>> vendor-neutral. >>>>> >>>>> Do we want to hardcode "panel" in the name? Are we sure that this will >>>>> ever only apply to panels? >>>>> >>>>> Do we want to use a name which reflects the intent, rather than the >>>>> mechanism? In other words, something like "color fidelity" = >>>>> "preferred" >>>>> maybe? (I don't know, just throwing ideas around.) >>>> >>>> In that vein, how about: >>>> >>>> "power saving policy" >>>> --> "power saving" >>>> --> "color fidelity" >>> >>> It's not just about colors though, is it? The compositor might want to >>> disable it to increase the backlight brightness in bright >>> environments, so "color fidelity" doesn't really sound right >> >> Either of these better? >> >> "power saving policy" >> --> "power saving" >> --> "accuracy" >> >> "power saving policy" >> --> "allowed" >> --> "forbidden" >> >> Or any other idea? > > Another consideration in addition to accuracy is latency. > > I suppose a compositor may want to disable features such as PSR for > use-cases requiring low latency. Touch and stylus input are some examples. > > I wonder if flags would work better than enums? A compositor can set > something like `REQUIRE_ACCURACY & REQUIRE_LOW_LATENCY`, for example. > I thought we had said the PSR latency issue discussed at the hackfest was likely just a bug and that nominally it won't matter? If it really will matter enough then yeah I think you're right that flags would be better. More drivers would probably want to opt into the property for the purpose of turning off PSR/PSR2/panel replay as well then. > - Leo > >> >>> >>>>> >>>>> Would be nice to add documentation for the property in the "standard >>>>> connector properties" section. >>>> >>>> Ack. >>>> >>
Am Di., 21. Mai 2024 um 19:28 Uhr schrieb Leo Li <sunpeng.li@amd.com>: > > > > On 2024-05-21 12:21, Mario Limonciello wrote: > > On 5/21/2024 11:14, Xaver Hugl wrote: > >> Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello > >> <mario.limonciello@amd.com>: > >>> > >>> On 5/21/2024 08:43, Simon Ser wrote: > >>>> This makes sense to me in general. I like the fact that it's simple and > >>>> vendor-neutral. > >>>> > >>>> Do we want to hardcode "panel" in the name? Are we sure that this will > >>>> ever only apply to panels? > >>>> > >>>> Do we want to use a name which reflects the intent, rather than the > >>>> mechanism? In other words, something like "color fidelity" = "preferred" > >>>> maybe? (I don't know, just throwing ideas around.) > >>> > >>> In that vein, how about: > >>> > >>> "power saving policy" > >>> --> "power saving" > >>> --> "color fidelity" > >> > >> It's not just about colors though, is it? The compositor might want to > >> disable it to increase the backlight brightness in bright > >> environments, so "color fidelity" doesn't really sound right > > > > Either of these better? > > > > "power saving policy" > > --> "power saving" > > --> "accuracy" > > > > "power saving policy" > > --> "allowed" > > --> "forbidden" > > > > Or any other idea? > > Another consideration in addition to accuracy is latency. > > I suppose a compositor may want to disable features such as PSR for use-cases > requiring low latency. Touch and stylus input are some examples. > > I wonder if flags would work better than enums? A compositor can set something > like `REQUIRE_ACCURACY & REQUIRE_LOW_LATENCY`, for example. I think that's a good idea. With a flag for color accuracy and one for brightness, the compositor's intent would be communicated well. > - Leo > > > > >> > >>>> > >>>> Would be nice to add documentation for the property in the "standard > >>>> connector properties" section. > >>> > >>> Ack. > >>> > >
On Tuesday, May 21st, 2024 at 19:27, Leo Li <sunpeng.li@amd.com> wrote: > I wonder if flags would work better than enums? A compositor can set something > like `REQUIRE_ACCURACY & REQUIRE_LOW_LATENCY`, for example. (FWIW, the KMS uAPI has first-class support for bitfields.)
On 2024-05-21 13:32, Mario Limonciello wrote: > On 5/21/2024 12:27, Leo Li wrote: >> >> >> On 2024-05-21 12:21, Mario Limonciello wrote: >>> On 5/21/2024 11:14, Xaver Hugl wrote: >>>> Am Di., 21. Mai 2024 um 16:00 Uhr schrieb Mario Limonciello >>>> <mario.limonciello@amd.com>: >>>>> >>>>> On 5/21/2024 08:43, Simon Ser wrote: >>>>>> This makes sense to me in general. I like the fact that it's simple and >>>>>> vendor-neutral. >>>>>> >>>>>> Do we want to hardcode "panel" in the name? Are we sure that this will >>>>>> ever only apply to panels? >>>>>> >>>>>> Do we want to use a name which reflects the intent, rather than the >>>>>> mechanism? In other words, something like "color fidelity" = "preferred" >>>>>> maybe? (I don't know, just throwing ideas around.) >>>>> >>>>> In that vein, how about: >>>>> >>>>> "power saving policy" >>>>> --> "power saving" >>>>> --> "color fidelity" >>>> >>>> It's not just about colors though, is it? The compositor might want to >>>> disable it to increase the backlight brightness in bright >>>> environments, so "color fidelity" doesn't really sound right >>> >>> Either of these better? >>> >>> "power saving policy" >>> --> "power saving" >>> --> "accuracy" >>> >>> "power saving policy" >>> --> "allowed" >>> --> "forbidden" >>> >>> Or any other idea? >> >> Another consideration in addition to accuracy is latency. >> >> I suppose a compositor may want to disable features such as PSR for use-cases >> requiring low latency. Touch and stylus input are some examples. >> >> I wonder if flags would work better than enums? A compositor can set something >> like `REQUIRE_ACCURACY & REQUIRE_LOW_LATENCY`, for example. >> > > I thought we had said the PSR latency issue discussed at the hackfest was likely > just a bug and that nominally it won't matter? Ah, either my memory failed, or I failed to clarify. Let me fix that below. > > If it really will matter enough then yeah I think you're right that flags would > be better. More drivers would probably want to opt into the property for the > purpose of turning off PSR/PSR2/panel replay as well then. Latency technically shouldn't be a problem for Panel Replay, but is a problem for PSR/PSR2 due to their design. At least it is a problem for amdgpu, not sure about other drivers. FWIU, in short, when a panel (DPRX) goes into self-refresh, it's clock can drift from the gpu (DPTX). Since there's no resync mechanism in PSR/PSR2, it is possible for the panel to lag behind by 1-frame-time(ms) for a noticeable period of time. Panel replay has a resync mechanism, so it theoretically can resync within 1 frame. Since, fwict, replay panels are still rare, I think it'll be nice for compositors that care about latency to have the option to temporarily disable PSR/PSR2. - Leo > >> - Leo >> >>> >>>> >>>>>> >>>>>> Would be nice to add documentation for the property in the "standard >>>>>> connector properties" section. >>>>> >>>>> Ack. >>>>> >>> >