Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 11/1/18 1:05 PM, Michel Dänzer wrote: > On 2018-11-01 3:58 p.m., Kazlauskas, Nicholas wrote: >> On 11/1/18 6:58 AM, Michel Dänzer wrote: >>> On 2018-10-31 6:54 p.m., Kazlauskas, Nicholas wrote: On 10/31/18 12:20 PM, Michel Dänzer wrote: > On 2018-10-31 3:41 p.m., Kazlauskas, Nicholas wrote: >> On 10/31/18 10:12 AM, Michel Dänzer wrote: >>> On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: I did some more investigation into when amdgpu gets the scanout position and what values we get back out of it. The timestamp is updated shortly after the crtc irq vblank which is typically within a few lines of vbl_start. This makes sense, since we can provide the prediction timestamp early. Waiting for the vblank to wrap around to 0 doesn't really make sense here since that would mean we already hit timeout or the flip occurred >>> >>> Sounds like you're mixing up the two cases of "actual" vblank events >>> (triggered by the "vblank" interrupt => drm_(crtc_)handle_vblank) and >>> flip completion events (triggered by the PFLIP interrupt with our >>> hardware => drm_crtc_send_vblank_event). >>> >>> Actual vblank events need to be delivered to userspace at the start of >>> vblank, so we indeed can't wait until the timestamp is accurate for >>> them. We just need to document the exact semantics of their timestamp >>> with VRR. >>> >>> For page flip completion events though, the timestamp needs to be >>> accurate and correspond to when the flipped frame starts being scanned >>> out, otherwise we'll surely break at least some userspace relying on >>> this information. >>> >> Yeah, I was. I guess what's sent is the estimated vblank timestamp >> calculated at the start of the interrupt. > > s/interrupt/vblank/, yeah. > > >> And since that's just a guess rather than what's actually going to >> happen it's going to be wrong in a lot of cases. >> >> I could see the wrap-around method working if the vblank timestamp was >> somehow updated in amdgpu or in drm_crtc_send_vblank_event. > > DC already calls drm_crtc_accurate_vblank_count before > drm_crtc_send_vblank_event, we "just" need to make sure that results in > an accurate timestamp. > > >> This would be a relatively simple fix but would break anything in >> userspace that relied on the timestamp for vblank interrupt and the >> flip completion being the same value. > > Hmm, that's a good point. So while VRR is enabled, maybe it is safer to > defer delivery of vblank events until the accurate timestamp is known as > well, at least by default. If there is userspace which needs the events > earlier even with VRR but can live with the guessed timestamp, a flag or > something could be added for that. I was under the impression that the vblank timestamp was reused but it's already going to differ since we call drm_crtc_accurate_vblank_count before drm_crtc_send_vblank_event. Thanks for pointing that out. Since that works for updating timestamp what's left is making sure that it waits for the wrap around if it's above crtc_vtotal. It might make sense to add a new flag for this that's only used within amdgpu_get_crtc_scanout_position so the other call sites aren't affected. There isn't a way to get an accurate timestamp with VRR enabled until after the flip happens. So deferring it kind of defeats the purpose of a client using it to make predictions before the flip for displaying their content. >>> >>> That's generally not a valid use-case. The KMS API is defined such that >>> if userspace receives the vblank event for vblank period N and then >>> submits a page flip, the page flip cannot (normally[0]) take effect >>> before vblank period N+1. >>> >>> >>> There are mainly two valid use-cases for waiting for vblank: >>> >>> 1. Wait for vblank N, then submit page flip for vblank N+1. >>> >>> 2. Wait for vblank N, then try to do something before vblank N ends, >>> e.g. draw directly to the front buffer, as a poor man's way of avoiding >>> tearing or similar artifacts. >>> >>> >>> Use-case 1 is used by Xorg (both for Present and DRI2) and at least >>> Weston, but presumably most if not all Wayland compositors. From Pekka's >>> description of how Weston uses it (thanks!), it's clear that the >>> timestamp of vblank events does need to accurately correspond to the end >>> of the vblank period. >>> >>> Use-case 2 is also used by Xorg, but only when not flipping, which rules >>> out VRR. Also, I'd argue that blocking waits for vblank are more >>> suitable for this use-case than vblank events. >>> >>> Therefore, in summary I propose this is how it could work: >>> >>> * Without VRR, timestamps continue to work the same way they do now. >>> >>> * With VRR,
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 2018-11-01 3:58 p.m., Kazlauskas, Nicholas wrote: > On 11/1/18 6:58 AM, Michel Dänzer wrote: >> On 2018-10-31 6:54 p.m., Kazlauskas, Nicholas wrote: >>> On 10/31/18 12:20 PM, Michel Dänzer wrote: On 2018-10-31 3:41 p.m., Kazlauskas, Nicholas wrote: > On 10/31/18 10:12 AM, Michel Dänzer wrote: >> On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: >>> >>> I did some more investigation into when amdgpu gets the scanout position >>> and what values we get back out of it. The timestamp is updated shortly >>> after the crtc irq vblank which is typically within a few lines of >>> vbl_start. This makes sense, since we can provide the prediction >>> timestamp early. Waiting for the vblank to wrap around to 0 doesn't >>> really make sense here since that would mean we already hit timeout or >>> the flip occurred >> >> Sounds like you're mixing up the two cases of "actual" vblank events >> (triggered by the "vblank" interrupt => drm_(crtc_)handle_vblank) and >> flip completion events (triggered by the PFLIP interrupt with our >> hardware => drm_crtc_send_vblank_event). >> >> Actual vblank events need to be delivered to userspace at the start of >> vblank, so we indeed can't wait until the timestamp is accurate for >> them. We just need to document the exact semantics of their timestamp >> with VRR. >> >> For page flip completion events though, the timestamp needs to be >> accurate and correspond to when the flipped frame starts being scanned >> out, otherwise we'll surely break at least some userspace relying on >> this information. >> > Yeah, I was. I guess what's sent is the estimated vblank timestamp > calculated at the start of the interrupt. s/interrupt/vblank/, yeah. > And since that's just a guess rather than what's actually going to > happen it's going to be wrong in a lot of cases. > > I could see the wrap-around method working if the vblank timestamp was > somehow updated in amdgpu or in drm_crtc_send_vblank_event. DC already calls drm_crtc_accurate_vblank_count before drm_crtc_send_vblank_event, we "just" need to make sure that results in an accurate timestamp. > This would be a relatively simple fix but would break anything in > userspace that relied on the timestamp for vblank interrupt and the > flip completion being the same value. Hmm, that's a good point. So while VRR is enabled, maybe it is safer to defer delivery of vblank events until the accurate timestamp is known as well, at least by default. If there is userspace which needs the events earlier even with VRR but can live with the guessed timestamp, a flag or something could be added for that. >>> >>> I was under the impression that the vblank timestamp was reused but it's >>> already going to differ since we call drm_crtc_accurate_vblank_count >>> before drm_crtc_send_vblank_event. Thanks for pointing that out. >>> >>> Since that works for updating timestamp what's left is making sure that >>> it waits for the wrap around if it's above crtc_vtotal. It might make >>> sense to add a new flag for this that's only used within >>> amdgpu_get_crtc_scanout_position so the other call sites aren't affected. >>> >>> There isn't a way to get an accurate timestamp with VRR enabled until >>> after the flip happens. So deferring it kind of defeats the purpose of a >>> client using it to make predictions before the flip for displaying their >>> content. >> >> That's generally not a valid use-case. The KMS API is defined such that >> if userspace receives the vblank event for vblank period N and then >> submits a page flip, the page flip cannot (normally[0]) take effect >> before vblank period N+1. >> >> >> There are mainly two valid use-cases for waiting for vblank: >> >> 1. Wait for vblank N, then submit page flip for vblank N+1. >> >> 2. Wait for vblank N, then try to do something before vblank N ends, >> e.g. draw directly to the front buffer, as a poor man's way of avoiding >> tearing or similar artifacts. >> >> >> Use-case 1 is used by Xorg (both for Present and DRI2) and at least >> Weston, but presumably most if not all Wayland compositors. From Pekka's >> description of how Weston uses it (thanks!), it's clear that the >> timestamp of vblank events does need to accurately correspond to the end >> of the vblank period. >> >> Use-case 2 is also used by Xorg, but only when not flipping, which rules >> out VRR. Also, I'd argue that blocking waits for vblank are more >> suitable for this use-case than vblank events. >> >> Therefore, in summary I propose this is how it could work: >> >> * Without VRR, timestamps continue to work the same way they do now. >> >> * With VRR, delivery of both vblank and page flip completion events must >> be deferred until the timestamp accurately corresponds to the actual end >> of vblank. For
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 11/1/18 6:58 AM, Michel Dänzer wrote: > On 2018-10-31 6:54 p.m., Kazlauskas, Nicholas wrote: >> On 10/31/18 12:20 PM, Michel Dänzer wrote: >>> On 2018-10-31 3:41 p.m., Kazlauskas, Nicholas wrote: On 10/31/18 10:12 AM, Michel Dänzer wrote: > On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: >> On 10/30/18 11:34 AM, Kazlauskas, Nicholas wrote: >>> >>> I understand the issue you're describing now. The timestamp is supposed >>> to signify the end of the current vblank. The call to get scanout >>> position is supposed to return the number of lines until scanout (a >>> negative value) or the number of lines since the next scanout began (a >>> positive value). >>> >>> The amdgpu driver calculates the number of lines based on a hardware >>> register status position which returns an increasing value from 0 that >>> indicates the current vpos/hpos for the display. >>> >>> For any vpos below vbl_start we know the value is correct since the next >>> vblank hasn't begun yet. But for anythign about vbl_start it's probably >>> wrong since it applies a corrective offset based on the fixed value of >>> crtc_vtotal. It's even worse when the value is above crtc_vtotal since >>> it'll be calculating the number of lines since the last scanout since >>> it'll be a positive value. >>> >>> So the issue becomes determining when the vfront porch will end. >>> >>> When the flip address gets written the vfront porch will end at the >>> start of the next line leaving only the back porch plus part of the >>> line. But we don't know when the flip will occur, if at all. It hasn't >>> occurred yet in this case. >>> >>> Waiting for the wrap around to 0 might be the best choice here since >>> there's no guarantee the flip will occur. >> >> I put some more thought into this and I don't think this is as bad as I >> had originally thought. >> >> I think the vblank timestamp is supposed to be for the first active >> pixel of the next scanout. The usage of which is for clients to time >> their content/animation/etc. >> >> The client likely doesn't care when they issue their flip, just that >> their content is matched for that vblank timestamp. The fixed refresh >> model works really well for that kind of application. >> >> Utilizing variable refresh rate would be a mistake in that case since >> the client would then have to time based on when they flip which is a >> lot harder to "predict" precisely. > > It's only a "mistake" as long as the timestamps are misleading. :) As > discussed before, accurate presentation timestamps are one requirement > for achieving perfectly smooth animation. For most 3D games the game world/rendering can advance by an arbitrary timestep - and faster will be smoother, which is what the native mode would give you. For fixed interval content/animation where you can't do that variable refresh rate could vastly improve the output. But like discussed before that would need a way to specify the interval/presentation timestamp to control that front porch duration. The timestamp will be misleading in any other case. >>> >>> I don't agree that an accurate timestamp can ever be more "misleading" >>> than an inaccurate one. But yeah, accurate timestamps and time-based >>> presentation are two sides of the same coin which can buy the holy grail >>> of perfectly smooth animation. :) >>> >>> >> I did some more investigation into when amdgpu gets the scanout position >> and what values we get back out of it. The timestamp is updated shortly >> after the crtc irq vblank which is typically within a few lines of >> vbl_start. This makes sense, since we can provide the prediction >> timestamp early. Waiting for the vblank to wrap around to 0 doesn't >> really make sense here since that would mean we already hit timeout or >> the flip occurred > > Sounds like you're mixing up the two cases of "actual" vblank events > (triggered by the "vblank" interrupt => drm_(crtc_)handle_vblank) and > flip completion events (triggered by the PFLIP interrupt with our > hardware => drm_crtc_send_vblank_event). > > Actual vblank events need to be delivered to userspace at the start of > vblank, so we indeed can't wait until the timestamp is accurate for > them. We just need to document the exact semantics of their timestamp > with VRR. > > For page flip completion events though, the timestamp needs to be > accurate and correspond to when the flipped frame starts being scanned > out, otherwise we'll surely break at least some userspace relying on > this information. > Yeah, I was. I guess what's sent is the estimated vblank timestamp calculated at the start of the interrupt. >>> >>> s/interrupt/vblank/,
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 2018-10-31 6:54 p.m., Kazlauskas, Nicholas wrote: > On 10/31/18 12:20 PM, Michel Dänzer wrote: >> On 2018-10-31 3:41 p.m., Kazlauskas, Nicholas wrote: >>> On 10/31/18 10:12 AM, Michel Dänzer wrote: On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: > On 10/30/18 11:34 AM, Kazlauskas, Nicholas wrote: >> >> I understand the issue you're describing now. The timestamp is supposed >> to signify the end of the current vblank. The call to get scanout >> position is supposed to return the number of lines until scanout (a >> negative value) or the number of lines since the next scanout began (a >> positive value). >> >> The amdgpu driver calculates the number of lines based on a hardware >> register status position which returns an increasing value from 0 that >> indicates the current vpos/hpos for the display. >> >> For any vpos below vbl_start we know the value is correct since the next >> vblank hasn't begun yet. But for anythign about vbl_start it's probably >> wrong since it applies a corrective offset based on the fixed value of >> crtc_vtotal. It's even worse when the value is above crtc_vtotal since >> it'll be calculating the number of lines since the last scanout since >> it'll be a positive value. >> >> So the issue becomes determining when the vfront porch will end. >> >> When the flip address gets written the vfront porch will end at the >> start of the next line leaving only the back porch plus part of the >> line. But we don't know when the flip will occur, if at all. It hasn't >> occurred yet in this case. >> >> Waiting for the wrap around to 0 might be the best choice here since >> there's no guarantee the flip will occur. > > I put some more thought into this and I don't think this is as bad as I > had originally thought. > > I think the vblank timestamp is supposed to be for the first active > pixel of the next scanout. The usage of which is for clients to time > their content/animation/etc. > > The client likely doesn't care when they issue their flip, just that > their content is matched for that vblank timestamp. The fixed refresh > model works really well for that kind of application. > > Utilizing variable refresh rate would be a mistake in that case since > the client would then have to time based on when they flip which is a > lot harder to "predict" precisely. It's only a "mistake" as long as the timestamps are misleading. :) As discussed before, accurate presentation timestamps are one requirement for achieving perfectly smooth animation. >>> >>> For most 3D games the game world/rendering can advance by an arbitrary >>> timestep - and faster will be smoother, which is what the native mode >>> would give you. >>> >>> For fixed interval content/animation where you can't do that variable >>> refresh rate could vastly improve the output. But like discussed before >>> that would need a way to specify the interval/presentation timestamp to >>> control that front porch duration. The timestamp will be misleading in >>> any other case. >> >> I don't agree that an accurate timestamp can ever be more "misleading" >> than an inaccurate one. But yeah, accurate timestamps and time-based >> presentation are two sides of the same coin which can buy the holy grail >> of perfectly smooth animation. :) >> >> > I did some more investigation into when amdgpu gets the scanout position > and what values we get back out of it. The timestamp is updated shortly > after the crtc irq vblank which is typically within a few lines of > vbl_start. This makes sense, since we can provide the prediction > timestamp early. Waiting for the vblank to wrap around to 0 doesn't > really make sense here since that would mean we already hit timeout or > the flip occurred Sounds like you're mixing up the two cases of "actual" vblank events (triggered by the "vblank" interrupt => drm_(crtc_)handle_vblank) and flip completion events (triggered by the PFLIP interrupt with our hardware => drm_crtc_send_vblank_event). Actual vblank events need to be delivered to userspace at the start of vblank, so we indeed can't wait until the timestamp is accurate for them. We just need to document the exact semantics of their timestamp with VRR. For page flip completion events though, the timestamp needs to be accurate and correspond to when the flipped frame starts being scanned out, otherwise we'll surely break at least some userspace relying on this information. >>> Yeah, I was. I guess what's sent is the estimated vblank timestamp >>> calculated at the start of the interrupt. >> >> s/interrupt/vblank/, yeah. >> >> >>> And since that's just a guess rather than what's actually going to >>> happen it's going to be wrong in a lot of cases. >>>
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Wed, 31 Oct 2018 17:54:34 + "Kazlauskas, Nicholas" wrote: > On 10/31/18 12:20 PM, Michel Dänzer wrote: > > On 2018-10-31 3:41 p.m., Kazlauskas, Nicholas wrote: > >> On 10/31/18 10:12 AM, Michel Dänzer wrote: > >>> On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: > On 10/30/18 11:34 AM, Kazlauskas, Nicholas wrote: > > > > I understand the issue you're describing now. The timestamp is supposed > > to signify the end of the current vblank. The call to get scanout > > position is supposed to return the number of lines until scanout (a > > negative value) or the number of lines since the next scanout began (a > > positive value). > > > > The amdgpu driver calculates the number of lines based on a hardware > > register status position which returns an increasing value from 0 that > > indicates the current vpos/hpos for the display. > > > > For any vpos below vbl_start we know the value is correct since the next > > vblank hasn't begun yet. But for anythign about vbl_start it's probably > > wrong since it applies a corrective offset based on the fixed value of > > crtc_vtotal. It's even worse when the value is above crtc_vtotal since > > it'll be calculating the number of lines since the last scanout since > > it'll be a positive value. > > > > So the issue becomes determining when the vfront porch will end. > > > > When the flip address gets written the vfront porch will end at the > > start of the next line leaving only the back porch plus part of the > > line. But we don't know when the flip will occur, if at all. It hasn't > > occurred yet in this case. > > > > Waiting for the wrap around to 0 might be the best choice here since > > there's no guarantee the flip will occur. > > I put some more thought into this and I don't think this is as bad as I > had originally thought. > > I think the vblank timestamp is supposed to be for the first active > pixel of the next scanout. The usage of which is for clients to time > their content/animation/etc. > > The client likely doesn't care when they issue their flip, just that > their content is matched for that vblank timestamp. The fixed refresh > model works really well for that kind of application. > > Utilizing variable refresh rate would be a mistake in that case since > the client would then have to time based on when they flip which is a > lot harder to "predict" precisely. > >>> > >>> It's only a "mistake" as long as the timestamps are misleading. :) As > >>> discussed before, accurate presentation timestamps are one requirement > >>> for achieving perfectly smooth animation. > >> > >> For most 3D games the game world/rendering can advance by an arbitrary > >> timestep - and faster will be smoother, which is what the native mode > >> would give you. > >> > >> For fixed interval content/animation where you can't do that variable > >> refresh rate could vastly improve the output. But like discussed before > >> that would need a way to specify the interval/presentation timestamp to > >> control that front porch duration. The timestamp will be misleading in > >> any other case. > > > > I don't agree that an accurate timestamp can ever be more "misleading" > > than an inaccurate one. But yeah, accurate timestamps and time-based > > presentation are two sides of the same coin which can buy the holy grail > > of perfectly smooth animation. :) > > > > > I did some more investigation into when amdgpu gets the scanout position > and what values we get back out of it. The timestamp is updated shortly > after the crtc irq vblank which is typically within a few lines of > vbl_start. This makes sense, since we can provide the prediction > timestamp early. Waiting for the vblank to wrap around to 0 doesn't > really make sense here since that would mean we already hit timeout or > the flip occurred > >>> > >>> Sounds like you're mixing up the two cases of "actual" vblank events > >>> (triggered by the "vblank" interrupt => drm_(crtc_)handle_vblank) and > >>> flip completion events (triggered by the PFLIP interrupt with our > >>> hardware => drm_crtc_send_vblank_event). > >>> > >>> Actual vblank events need to be delivered to userspace at the start of > >>> vblank, so we indeed can't wait until the timestamp is accurate for > >>> them. We just need to document the exact semantics of their timestamp > >>> with VRR. > >>> > >>> For page flip completion events though, the timestamp needs to be > >>> accurate and correspond to when the flipped frame starts being scanned > >>> out, otherwise we'll surely break at least some userspace relying on > >>> this information. > >>> > >> Yeah, I was. I guess what's sent is the estimated vblank timestamp > >> calculated at the start of the interrupt. > > > > s/interrupt/vblank/,
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/31/18 12:20 PM, Michel Dänzer wrote: > On 2018-10-31 3:41 p.m., Kazlauskas, Nicholas wrote: >> On 10/31/18 10:12 AM, Michel Dänzer wrote: >>> On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: On 10/30/18 11:34 AM, Kazlauskas, Nicholas wrote: > > I understand the issue you're describing now. The timestamp is supposed > to signify the end of the current vblank. The call to get scanout > position is supposed to return the number of lines until scanout (a > negative value) or the number of lines since the next scanout began (a > positive value). > > The amdgpu driver calculates the number of lines based on a hardware > register status position which returns an increasing value from 0 that > indicates the current vpos/hpos for the display. > > For any vpos below vbl_start we know the value is correct since the next > vblank hasn't begun yet. But for anythign about vbl_start it's probably > wrong since it applies a corrective offset based on the fixed value of > crtc_vtotal. It's even worse when the value is above crtc_vtotal since > it'll be calculating the number of lines since the last scanout since > it'll be a positive value. > > So the issue becomes determining when the vfront porch will end. > > When the flip address gets written the vfront porch will end at the > start of the next line leaving only the back porch plus part of the > line. But we don't know when the flip will occur, if at all. It hasn't > occurred yet in this case. > > Waiting for the wrap around to 0 might be the best choice here since > there's no guarantee the flip will occur. I put some more thought into this and I don't think this is as bad as I had originally thought. I think the vblank timestamp is supposed to be for the first active pixel of the next scanout. The usage of which is for clients to time their content/animation/etc. The client likely doesn't care when they issue their flip, just that their content is matched for that vblank timestamp. The fixed refresh model works really well for that kind of application. Utilizing variable refresh rate would be a mistake in that case since the client would then have to time based on when they flip which is a lot harder to "predict" precisely. >>> >>> It's only a "mistake" as long as the timestamps are misleading. :) As >>> discussed before, accurate presentation timestamps are one requirement >>> for achieving perfectly smooth animation. >> >> For most 3D games the game world/rendering can advance by an arbitrary >> timestep - and faster will be smoother, which is what the native mode >> would give you. >> >> For fixed interval content/animation where you can't do that variable >> refresh rate could vastly improve the output. But like discussed before >> that would need a way to specify the interval/presentation timestamp to >> control that front porch duration. The timestamp will be misleading in >> any other case. > > I don't agree that an accurate timestamp can ever be more "misleading" > than an inaccurate one. But yeah, accurate timestamps and time-based > presentation are two sides of the same coin which can buy the holy grail > of perfectly smooth animation. :) > > I did some more investigation into when amdgpu gets the scanout position and what values we get back out of it. The timestamp is updated shortly after the crtc irq vblank which is typically within a few lines of vbl_start. This makes sense, since we can provide the prediction timestamp early. Waiting for the vblank to wrap around to 0 doesn't really make sense here since that would mean we already hit timeout or the flip occurred >>> >>> Sounds like you're mixing up the two cases of "actual" vblank events >>> (triggered by the "vblank" interrupt => drm_(crtc_)handle_vblank) and >>> flip completion events (triggered by the PFLIP interrupt with our >>> hardware => drm_crtc_send_vblank_event). >>> >>> Actual vblank events need to be delivered to userspace at the start of >>> vblank, so we indeed can't wait until the timestamp is accurate for >>> them. We just need to document the exact semantics of their timestamp >>> with VRR. >>> >>> For page flip completion events though, the timestamp needs to be >>> accurate and correspond to when the flipped frame starts being scanned >>> out, otherwise we'll surely break at least some userspace relying on >>> this information. >>> >> Yeah, I was. I guess what's sent is the estimated vblank timestamp >> calculated at the start of the interrupt. > > s/interrupt/vblank/, yeah. > > >> And since that's just a guess rather than what's actually going to >> happen it's going to be wrong in a lot of cases. >> >> I could see the wrap-around method working if the vblank timestamp was >> somehow updated in amdgpu or in drm_crtc_send_vblank_event. > > DC
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 2018-10-31 3:41 p.m., Kazlauskas, Nicholas wrote: > On 10/31/18 10:12 AM, Michel Dänzer wrote: >> On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: >>> On 10/30/18 11:34 AM, Kazlauskas, Nicholas wrote: I understand the issue you're describing now. The timestamp is supposed to signify the end of the current vblank. The call to get scanout position is supposed to return the number of lines until scanout (a negative value) or the number of lines since the next scanout began (a positive value). The amdgpu driver calculates the number of lines based on a hardware register status position which returns an increasing value from 0 that indicates the current vpos/hpos for the display. For any vpos below vbl_start we know the value is correct since the next vblank hasn't begun yet. But for anythign about vbl_start it's probably wrong since it applies a corrective offset based on the fixed value of crtc_vtotal. It's even worse when the value is above crtc_vtotal since it'll be calculating the number of lines since the last scanout since it'll be a positive value. So the issue becomes determining when the vfront porch will end. When the flip address gets written the vfront porch will end at the start of the next line leaving only the back porch plus part of the line. But we don't know when the flip will occur, if at all. It hasn't occurred yet in this case. Waiting for the wrap around to 0 might be the best choice here since there's no guarantee the flip will occur. >>> >>> I put some more thought into this and I don't think this is as bad as I >>> had originally thought. >>> >>> I think the vblank timestamp is supposed to be for the first active >>> pixel of the next scanout. The usage of which is for clients to time >>> their content/animation/etc. >>> >>> The client likely doesn't care when they issue their flip, just that >>> their content is matched for that vblank timestamp. The fixed refresh >>> model works really well for that kind of application. >>> >>> Utilizing variable refresh rate would be a mistake in that case since >>> the client would then have to time based on when they flip which is a >>> lot harder to "predict" precisely. >> >> It's only a "mistake" as long as the timestamps are misleading. :) As >> discussed before, accurate presentation timestamps are one requirement >> for achieving perfectly smooth animation. > > For most 3D games the game world/rendering can advance by an arbitrary > timestep - and faster will be smoother, which is what the native mode > would give you. > > For fixed interval content/animation where you can't do that variable > refresh rate could vastly improve the output. But like discussed before > that would need a way to specify the interval/presentation timestamp to > control that front porch duration. The timestamp will be misleading in > any other case. I don't agree that an accurate timestamp can ever be more "misleading" than an inaccurate one. But yeah, accurate timestamps and time-based presentation are two sides of the same coin which can buy the holy grail of perfectly smooth animation. :) >>> I did some more investigation into when amdgpu gets the scanout position >>> and what values we get back out of it. The timestamp is updated shortly >>> after the crtc irq vblank which is typically within a few lines of >>> vbl_start. This makes sense, since we can provide the prediction >>> timestamp early. Waiting for the vblank to wrap around to 0 doesn't >>> really make sense here since that would mean we already hit timeout or >>> the flip occurred >> >> Sounds like you're mixing up the two cases of "actual" vblank events >> (triggered by the "vblank" interrupt => drm_(crtc_)handle_vblank) and >> flip completion events (triggered by the PFLIP interrupt with our >> hardware => drm_crtc_send_vblank_event). >> >> Actual vblank events need to be delivered to userspace at the start of >> vblank, so we indeed can't wait until the timestamp is accurate for >> them. We just need to document the exact semantics of their timestamp >> with VRR. >> >> For page flip completion events though, the timestamp needs to be >> accurate and correspond to when the flipped frame starts being scanned >> out, otherwise we'll surely break at least some userspace relying on >> this information. >> > Yeah, I was. I guess what's sent is the estimated vblank timestamp > calculated at the start of the interrupt. s/interrupt/vblank/, yeah. > And since that's just a guess rather than what's actually going to > happen it's going to be wrong in a lot of cases. > > I could see the wrap-around method working if the vblank timestamp was > somehow updated in amdgpu or in drm_crtc_send_vblank_event. DC already calls drm_crtc_accurate_vblank_count before drm_crtc_send_vblank_event, we "just" need to make sure that results in an accurate timestamp. >
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/31/18 10:12 AM, Michel Dänzer wrote: > On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: >> On 10/30/18 11:34 AM, Kazlauskas, Nicholas wrote: >>> On 10/30/18 5:29 AM, Michel Dänzer wrote: On 2018-10-29 7:03 p.m., Ville Syrjälä wrote: > On Mon, Oct 29, 2018 at 05:37:49PM +0100, Michel Dänzer wrote: >> On 2018-10-26 7:59 p.m., Ville Syrjälä wrote: >>> On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: On 10/26/18 10:53 AM, Ville Syrjälä wrote: > > Speaking of timestamps. What is the expected behaviour of vblank > timestamps when vrr is enabled? When vrr is enabled the duration of the vertical front porch will be extended until flip or timeout occurs. The vblank timestamp will vary based on duration of the vertical front porch. The min/max duration for the front porch can be specified by the driver via the min/max range. No changes to vblank timestamping handling should be necessary to accommodate variable refresh rate. >>> >>> The problem is that the timestamp is supposed to correspond to the first >>> active pixel. And since we don't know how long the front porch will be >>> we can't realistically report the true value. So I guess just assuming >>> min front porch length is as good as anything else? >> >> That (and documenting that the timestamp corresponds to the earliest >> possible first active pixel, not necessarily the actual one, with VRR) >> might be good enough for the actual vblank event timestamps. >> >> >> However, I'm not so sure about the timestamps of page flip completion >> events. Those could be very misleading if the flip completes towards the >> timeout, which could result in bad behaviour of applications which use >> them for animation timing. >> >> Maybe the timestamp could be updated appropriately (yes, I'm hand-waving >> :) in drm_crtc_send_vblank_event? > > Hmm. Updated how? Whether it's a page flip event or vblank even we won't > know when the first active pixel will come. Although I suppose if > there is some kind of vrr slew rate limit we could at least account > for that to report a more correct "this is the earliest you migth be > able to see your frame" timestamp. > > Oh, or are you actually saying that shceduling a new flip before the > timeout is actually going to latch that flip immediately? I figured > that the flip would get latched on the next start of vblank regardless, > and the act of scheduling a flip will just kick the hardware to start > scanning the previously latched frame earlier. > > scanout A | ... vblank | scanout A | ... vblank | scanout B | ... vblank > ^^^ ^ > ||flip C latch C > flip B latch B This would kind of defeat the point of VRR, wouldn't it? If a flip was scheduled after the start of vblank, the vblank would always time out, resulting in the minimum refresh rate. > scanout A | ... vblank | scanout B | ... vblank | scanout C | ... vblank > ^ ^ ^ ^ > | latch B| latch C > flip B flip C So this is what happens. Regardless, when the flip is latched, AMD hardware generates a "pflip" interrupt, and its handler calls drm_crtc_send_vblank_event (and in the case of DC drm_crtc_accurate_vblank_count before that). So the time when drm_crtc_send_vblank_event is called might be a good approximation of when scanout of the next frame starts. Another possibility might be to wait for the hardware vline counter to wrap around to 0 before calling drm_crtc_accurate_vblank_count, then the calculations should be based on 0 instead of crtc_vtotal. >>> >>> >>> I understand the issue you're describing now. The timestamp is supposed >>> to signify the end of the current vblank. The call to get scanout >>> position is supposed to return the number of lines until scanout (a >>> negative value) or the number of lines since the next scanout began (a >>> positive value). >>> >>> The amdgpu driver calculates the number of lines based on a hardware >>> register status position which returns an increasing value from 0 that >>> indicates the current vpos/hpos for the display. >>> >>> For any vpos below vbl_start we know the value is correct since the next >>> vblank hasn't begun yet. But for anythign about vbl_start it's probably >>> wrong since it applies a corrective offset based on the fixed value of >>> crtc_vtotal. It's even worse when the value is above crtc_vtotal since >>> it'll be calculating the number of lines since the last
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: > On 10/30/18 11:34 AM, Kazlauskas, Nicholas wrote: >> On 10/30/18 5:29 AM, Michel Dänzer wrote: >>> On 2018-10-29 7:03 p.m., Ville Syrjälä wrote: On Mon, Oct 29, 2018 at 05:37:49PM +0100, Michel Dänzer wrote: > On 2018-10-26 7:59 p.m., Ville Syrjälä wrote: >> On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: >>> On 10/26/18 10:53 AM, Ville Syrjälä wrote: Speaking of timestamps. What is the expected behaviour of vblank timestamps when vrr is enabled? >>> >>> When vrr is enabled the duration of the vertical front porch will be >>> extended until flip or timeout occurs. The vblank timestamp will vary >>> based on duration of the vertical front porch. The min/max duration for >>> the front porch can be specified by the driver via the min/max range. >>> >>> No changes to vblank timestamping handling should be necessary to >>> accommodate variable refresh rate. >> >> The problem is that the timestamp is supposed to correspond to the first >> active pixel. And since we don't know how long the front porch will be >> we can't realistically report the true value. So I guess just assuming >> min front porch length is as good as anything else? > > That (and documenting that the timestamp corresponds to the earliest > possible first active pixel, not necessarily the actual one, with VRR) > might be good enough for the actual vblank event timestamps. > > > However, I'm not so sure about the timestamps of page flip completion > events. Those could be very misleading if the flip completes towards the > timeout, which could result in bad behaviour of applications which use > them for animation timing. > > Maybe the timestamp could be updated appropriately (yes, I'm hand-waving > :) in drm_crtc_send_vblank_event? Hmm. Updated how? Whether it's a page flip event or vblank even we won't know when the first active pixel will come. Although I suppose if there is some kind of vrr slew rate limit we could at least account for that to report a more correct "this is the earliest you migth be able to see your frame" timestamp. Oh, or are you actually saying that shceduling a new flip before the timeout is actually going to latch that flip immediately? I figured that the flip would get latched on the next start of vblank regardless, and the act of scheduling a flip will just kick the hardware to start scanning the previously latched frame earlier. scanout A | ... vblank | scanout A | ... vblank | scanout B | ... vblank ^^^ ^ ||flip C latch C flip B latch B >>> >>> This would kind of defeat the point of VRR, wouldn't it? If a flip was >>> scheduled after the start of vblank, the vblank would always time out, >>> resulting in the minimum refresh rate. >>> >>> scanout A | ... vblank | scanout B | ... vblank | scanout C | ... vblank ^ ^ ^ ^ | latch B| latch C flip B flip C >>> >>> So this is what happens. >>> >>> Regardless, when the flip is latched, AMD hardware generates a "pflip" >>> interrupt, and its handler calls drm_crtc_send_vblank_event (and in the >>> case of DC drm_crtc_accurate_vblank_count before that). So the time when >>> drm_crtc_send_vblank_event is called might be a good approximation of >>> when scanout of the next frame starts. >>> >>> Another possibility might be to wait for the hardware vline counter to >>> wrap around to 0 before calling drm_crtc_accurate_vblank_count, then the >>> calculations should be based on 0 instead of crtc_vtotal. >> >> >> I understand the issue you're describing now. The timestamp is supposed >> to signify the end of the current vblank. The call to get scanout >> position is supposed to return the number of lines until scanout (a >> negative value) or the number of lines since the next scanout began (a >> positive value). >> >> The amdgpu driver calculates the number of lines based on a hardware >> register status position which returns an increasing value from 0 that >> indicates the current vpos/hpos for the display. >> >> For any vpos below vbl_start we know the value is correct since the next >> vblank hasn't begun yet. But for anythign about vbl_start it's probably >> wrong since it applies a corrective offset based on the fixed value of >> crtc_vtotal. It's even worse when the value is above crtc_vtotal since >> it'll be calculating the number of lines since the last scanout since >> it'll be a positive value. >> >> So the issue becomes determining when the vfront porch will end. >> >> When the flip address
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/30/18 11:34 AM, Kazlauskas, Nicholas wrote: > On 10/30/18 5:29 AM, Michel Dänzer wrote: >> On 2018-10-29 7:03 p.m., Ville Syrjälä wrote: >>> On Mon, Oct 29, 2018 at 05:37:49PM +0100, Michel Dänzer wrote: On 2018-10-26 7:59 p.m., Ville Syrjälä wrote: > On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: >> On 10/26/18 10:53 AM, Ville Syrjälä wrote: >>> >>> Speaking of timestamps. What is the expected behaviour of vblank >>> timestamps when vrr is enabled? >> >> When vrr is enabled the duration of the vertical front porch will be >> extended until flip or timeout occurs. The vblank timestamp will vary >> based on duration of the vertical front porch. The min/max duration for >> the front porch can be specified by the driver via the min/max range. >> >> No changes to vblank timestamping handling should be necessary to >> accommodate variable refresh rate. > > The problem is that the timestamp is supposed to correspond to the first > active pixel. And since we don't know how long the front porch will be > we can't realistically report the true value. So I guess just assuming > min front porch length is as good as anything else? That (and documenting that the timestamp corresponds to the earliest possible first active pixel, not necessarily the actual one, with VRR) might be good enough for the actual vblank event timestamps. However, I'm not so sure about the timestamps of page flip completion events. Those could be very misleading if the flip completes towards the timeout, which could result in bad behaviour of applications which use them for animation timing. Maybe the timestamp could be updated appropriately (yes, I'm hand-waving :) in drm_crtc_send_vblank_event? >>> >>> Hmm. Updated how? Whether it's a page flip event or vblank even we won't >>> know when the first active pixel will come. Although I suppose if >>> there is some kind of vrr slew rate limit we could at least account >>> for that to report a more correct "this is the earliest you migth be >>> able to see your frame" timestamp. >>> >>> Oh, or are you actually saying that shceduling a new flip before the >>> timeout is actually going to latch that flip immediately? I figured >>> that the flip would get latched on the next start of vblank regardless, >>> and the act of scheduling a flip will just kick the hardware to start >>> scanning the previously latched frame earlier. >>> >>> scanout A | ... vblank | scanout A | ... vblank | scanout B | ... vblank >>> ^^^ ^ >>> ||flip C latch C >>> flip B latch B >> >> This would kind of defeat the point of VRR, wouldn't it? If a flip was >> scheduled after the start of vblank, the vblank would always time out, >> resulting in the minimum refresh rate. >> >> >>> scanout A | ... vblank | scanout B | ... vblank | scanout C | ... vblank >>> ^ ^ ^ ^ >>> | latch B| latch C >>> flip B flip C >> >> So this is what happens. >> >> Regardless, when the flip is latched, AMD hardware generates a "pflip" >> interrupt, and its handler calls drm_crtc_send_vblank_event (and in the >> case of DC drm_crtc_accurate_vblank_count before that). So the time when >> drm_crtc_send_vblank_event is called might be a good approximation of >> when scanout of the next frame starts. >> >> Another possibility might be to wait for the hardware vline counter to >> wrap around to 0 before calling drm_crtc_accurate_vblank_count, then the >> calculations should be based on 0 instead of crtc_vtotal. >> >> > > > I understand the issue you're describing now. The timestamp is supposed > to signify the end of the current vblank. The call to get scanout > position is supposed to return the number of lines until scanout (a > negative value) or the number of lines since the next scanout began (a > positive value). > > The amdgpu driver calculates the number of lines based on a hardware > register status position which returns an increasing value from 0 that > indicates the current vpos/hpos for the display. > > For any vpos below vbl_start we know the value is correct since the next > vblank hasn't begun yet. But for anythign about vbl_start it's probably > wrong since it applies a corrective offset based on the fixed value of > crtc_vtotal. It's even worse when the value is above crtc_vtotal since > it'll be calculating the number of lines since the last scanout since > it'll be a positive value. > > So the issue becomes determining when the vfront porch will end. > > When the flip address gets written the vfront porch will end at the > start of the next line leaving only the back porch plus part of the > line. But we don't know
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/30/18 5:29 AM, Michel Dänzer wrote: > On 2018-10-29 7:03 p.m., Ville Syrjälä wrote: >> On Mon, Oct 29, 2018 at 05:37:49PM +0100, Michel Dänzer wrote: >>> On 2018-10-26 7:59 p.m., Ville Syrjälä wrote: On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 10:53 AM, Ville Syrjälä wrote: >> >> Speaking of timestamps. What is the expected behaviour of vblank >> timestamps when vrr is enabled? > > When vrr is enabled the duration of the vertical front porch will be > extended until flip or timeout occurs. The vblank timestamp will vary > based on duration of the vertical front porch. The min/max duration for > the front porch can be specified by the driver via the min/max range. > > No changes to vblank timestamping handling should be necessary to > accommodate variable refresh rate. The problem is that the timestamp is supposed to correspond to the first active pixel. And since we don't know how long the front porch will be we can't realistically report the true value. So I guess just assuming min front porch length is as good as anything else? >>> >>> That (and documenting that the timestamp corresponds to the earliest >>> possible first active pixel, not necessarily the actual one, with VRR) >>> might be good enough for the actual vblank event timestamps. >>> >>> >>> However, I'm not so sure about the timestamps of page flip completion >>> events. Those could be very misleading if the flip completes towards the >>> timeout, which could result in bad behaviour of applications which use >>> them for animation timing. >>> >>> Maybe the timestamp could be updated appropriately (yes, I'm hand-waving >>> :) in drm_crtc_send_vblank_event? >> >> Hmm. Updated how? Whether it's a page flip event or vblank even we won't >> know when the first active pixel will come. Although I suppose if >> there is some kind of vrr slew rate limit we could at least account >> for that to report a more correct "this is the earliest you migth be >> able to see your frame" timestamp. >> >> Oh, or are you actually saying that shceduling a new flip before the >> timeout is actually going to latch that flip immediately? I figured >> that the flip would get latched on the next start of vblank regardless, >> and the act of scheduling a flip will just kick the hardware to start >> scanning the previously latched frame earlier. >> >> scanout A | ... vblank | scanout A | ... vblank | scanout B | ... vblank >>^^^ ^ >>||flip C latch C >>flip B latch B > > This would kind of defeat the point of VRR, wouldn't it? If a flip was > scheduled after the start of vblank, the vblank would always time out, > resulting in the minimum refresh rate. > > >> scanout A | ... vblank | scanout B | ... vblank | scanout C | ... vblank >>^ ^ ^ ^ >>| latch B| latch C >>flip B flip C > > So this is what happens. > > Regardless, when the flip is latched, AMD hardware generates a "pflip" > interrupt, and its handler calls drm_crtc_send_vblank_event (and in the > case of DC drm_crtc_accurate_vblank_count before that). So the time when > drm_crtc_send_vblank_event is called might be a good approximation of > when scanout of the next frame starts. > > Another possibility might be to wait for the hardware vline counter to > wrap around to 0 before calling drm_crtc_accurate_vblank_count, then the > calculations should be based on 0 instead of crtc_vtotal. > > I understand the issue you're describing now. The timestamp is supposed to signify the end of the current vblank. The call to get scanout position is supposed to return the number of lines until scanout (a negative value) or the number of lines since the next scanout began (a positive value). The amdgpu driver calculates the number of lines based on a hardware register status position which returns an increasing value from 0 that indicates the current vpos/hpos for the display. For any vpos below vbl_start we know the value is correct since the next vblank hasn't begun yet. But for anythign about vbl_start it's probably wrong since it applies a corrective offset based on the fixed value of crtc_vtotal. It's even worse when the value is above crtc_vtotal since it'll be calculating the number of lines since the last scanout since it'll be a positive value. So the issue becomes determining when the vfront porch will end. When the flip address gets written the vfront porch will end at the start of the next line leaving only the back porch plus part of the line. But we don't know when the flip will occur, if at all. It hasn't occurred yet in this case. Waiting for the wrap around to 0 might be the best choice here since there's no
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 2018-10-29 7:03 p.m., Ville Syrjälä wrote: > On Mon, Oct 29, 2018 at 05:37:49PM +0100, Michel Dänzer wrote: >> On 2018-10-26 7:59 p.m., Ville Syrjälä wrote: >>> On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: On 10/26/18 10:53 AM, Ville Syrjälä wrote: > > Speaking of timestamps. What is the expected behaviour of vblank > timestamps when vrr is enabled? When vrr is enabled the duration of the vertical front porch will be extended until flip or timeout occurs. The vblank timestamp will vary based on duration of the vertical front porch. The min/max duration for the front porch can be specified by the driver via the min/max range. No changes to vblank timestamping handling should be necessary to accommodate variable refresh rate. >>> >>> The problem is that the timestamp is supposed to correspond to the first >>> active pixel. And since we don't know how long the front porch will be >>> we can't realistically report the true value. So I guess just assuming >>> min front porch length is as good as anything else? >> >> That (and documenting that the timestamp corresponds to the earliest >> possible first active pixel, not necessarily the actual one, with VRR) >> might be good enough for the actual vblank event timestamps. >> >> >> However, I'm not so sure about the timestamps of page flip completion >> events. Those could be very misleading if the flip completes towards the >> timeout, which could result in bad behaviour of applications which use >> them for animation timing. >> >> Maybe the timestamp could be updated appropriately (yes, I'm hand-waving >> :) in drm_crtc_send_vblank_event? > > Hmm. Updated how? Whether it's a page flip event or vblank even we won't > know when the first active pixel will come. Although I suppose if > there is some kind of vrr slew rate limit we could at least account > for that to report a more correct "this is the earliest you migth be > able to see your frame" timestamp. > > Oh, or are you actually saying that shceduling a new flip before the > timeout is actually going to latch that flip immediately? I figured > that the flip would get latched on the next start of vblank regardless, > and the act of scheduling a flip will just kick the hardware to start > scanning the previously latched frame earlier. > > scanout A | ... vblank | scanout A | ... vblank | scanout B | ... vblank > ^^^ ^ > ||flip C latch C > flip B latch B This would kind of defeat the point of VRR, wouldn't it? If a flip was scheduled after the start of vblank, the vblank would always time out, resulting in the minimum refresh rate. > scanout A | ... vblank | scanout B | ... vblank | scanout C | ... vblank > ^ ^ ^ ^ > | latch B| latch C > flip B flip C So this is what happens. Regardless, when the flip is latched, AMD hardware generates a "pflip" interrupt, and its handler calls drm_crtc_send_vblank_event (and in the case of DC drm_crtc_accurate_vblank_count before that). So the time when drm_crtc_send_vblank_event is called might be a good approximation of when scanout of the next frame starts. Another possibility might be to wait for the hardware vline counter to wrap around to 0 before calling drm_crtc_accurate_vblank_count, then the calculations should be based on 0 instead of crtc_vtotal. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 2018-10-29 8:11 p.m., Kazlauskas, Nicholas wrote: > On 10/29/18 2:03 PM, Ville Syrjälä wrote: >> On Mon, Oct 29, 2018 at 05:37:49PM +0100, Michel Dänzer wrote: >>> On 2018-10-26 7:59 p.m., Ville Syrjälä wrote: On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 10:53 AM, Ville Syrjälä wrote: >> >> Speaking of timestamps. What is the expected behaviour of vblank >> timestamps when vrr is enabled? > > When vrr is enabled the duration of the vertical front porch will be > extended until flip or timeout occurs. The vblank timestamp will vary > based on duration of the vertical front porch. The min/max duration for > the front porch can be specified by the driver via the min/max range. > > No changes to vblank timestamping handling should be necessary to > accommodate variable refresh rate. The problem is that the timestamp is supposed to correspond to the first active pixel. And since we don't know how long the front porch will be we can't realistically report the true value. So I guess just assuming min front porch length is as good as anything else? >>> >>> That (and documenting that the timestamp corresponds to the earliest >>> possible first active pixel, not necessarily the actual one, with VRR) >>> might be good enough for the actual vblank event timestamps. >>> >>> >>> However, I'm not so sure about the timestamps of page flip completion >>> events. Those could be very misleading if the flip completes towards the >>> timeout, which could result in bad behaviour of applications which use >>> them for animation timing. >>> >>> Maybe the timestamp could be updated appropriately (yes, I'm hand-waving >>> :) in drm_crtc_send_vblank_event? >> >> [...] > > I'm not sure any of this is necessary. > > The vblank timestamp is determined (at least if you're using the > helpers) by using the scanout position. > > The crtc_vtotal won't change when variable refresh rate is enabled. What > does change is the behavior of the scanout position. > > The vpos will continue going will beyond the end of crtc_vtotal up until > whatever the vtotal would be for the minimum refresh rate. There's no > clamping performed for these calculations so it ends up working as expected. I'm afraid not. amdgpu_display_get_crtc_scanoutpos returns how many lines after "the end of vblank" (which is calculated based on crtc_vtotal) the current scanout position is. drm_calc_vbltimestamp_from_scanoutpos converts that to a time delta from "the end of vblank", and subtracts that from the current time (when the scanout position was sampled). The result is the timestamp corresponding to "the end of vblank" based on crtc_vtotal. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/29/18 2:03 PM, Ville Syrjälä wrote: > On Mon, Oct 29, 2018 at 05:37:49PM +0100, Michel Dänzer wrote: >> On 2018-10-26 7:59 p.m., Ville Syrjälä wrote: >>> On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: On 10/26/18 10:53 AM, Ville Syrjälä wrote: > > Speaking of timestamps. What is the expected behaviour of vblank > timestamps when vrr is enabled? When vrr is enabled the duration of the vertical front porch will be extended until flip or timeout occurs. The vblank timestamp will vary based on duration of the vertical front porch. The min/max duration for the front porch can be specified by the driver via the min/max range. No changes to vblank timestamping handling should be necessary to accommodate variable refresh rate. >>> >>> The problem is that the timestamp is supposed to correspond to the first >>> active pixel. And since we don't know how long the front porch will be >>> we can't realistically report the true value. So I guess just assuming >>> min front porch length is as good as anything else? >> >> That (and documenting that the timestamp corresponds to the earliest >> possible first active pixel, not necessarily the actual one, with VRR) >> might be good enough for the actual vblank event timestamps. >> >> >> However, I'm not so sure about the timestamps of page flip completion >> events. Those could be very misleading if the flip completes towards the >> timeout, which could result in bad behaviour of applications which use >> them for animation timing. >> >> Maybe the timestamp could be updated appropriately (yes, I'm hand-waving >> :) in drm_crtc_send_vblank_event? > > Hmm. Updated how? Whether it's a page flip event or vblank even we won't > know when the first active pixel will come. Although I suppose if > there is some kind of vrr slew rate limit we could at least account > for that to report a more correct "this is the earliest you migth be > able to see your frame" timestamp. > > Oh, or are you actually saying that shceduling a new flip before the > timeout is actually going to latch that flip immediately? I figured > that the flip would get latched on the next start of vblank regardless, > and the act of scheduling a flip will just kick the hardware to start > scanning the previously latched frame earlier. > > scanout A | ... vblank | scanout A | ... vblank | scanout B | ... vblank >^^^ ^ >||flip C latch C >flip B latch B > > vs. > > scanout A | ... vblank | scanout B | ... vblank | scanout C | ... vblank >^ ^ ^ ^ >| latch B| latch C >flip B flip C > > The latter would seem more like a tearing flip without the tearing. > I'm not sure any of this is necessary. The vblank timestamp is determined (at least if you're using the helpers) by using the scanout position. The crtc_vtotal won't change when variable refresh rate is enabled. What does change is the behavior of the scanout position. The vpos will continue going will beyond the end of crtc_vtotal up until whatever the vtotal would be for the minimum refresh rate. There's no clamping performed for these calculations so it ends up working as expected. Nicholas Kazlauskas ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Mon, Oct 29, 2018 at 05:37:49PM +0100, Michel Dänzer wrote: > On 2018-10-26 7:59 p.m., Ville Syrjälä wrote: > > On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: > >> On 10/26/18 10:53 AM, Ville Syrjälä wrote: > >>> > >>> Speaking of timestamps. What is the expected behaviour of vblank > >>> timestamps when vrr is enabled? > >> > >> When vrr is enabled the duration of the vertical front porch will be > >> extended until flip or timeout occurs. The vblank timestamp will vary > >> based on duration of the vertical front porch. The min/max duration for > >> the front porch can be specified by the driver via the min/max range. > >> > >> No changes to vblank timestamping handling should be necessary to > >> accommodate variable refresh rate. > > > > The problem is that the timestamp is supposed to correspond to the first > > active pixel. And since we don't know how long the front porch will be > > we can't realistically report the true value. So I guess just assuming > > min front porch length is as good as anything else? > > That (and documenting that the timestamp corresponds to the earliest > possible first active pixel, not necessarily the actual one, with VRR) > might be good enough for the actual vblank event timestamps. > > > However, I'm not so sure about the timestamps of page flip completion > events. Those could be very misleading if the flip completes towards the > timeout, which could result in bad behaviour of applications which use > them for animation timing. > > Maybe the timestamp could be updated appropriately (yes, I'm hand-waving > :) in drm_crtc_send_vblank_event? Hmm. Updated how? Whether it's a page flip event or vblank even we won't know when the first active pixel will come. Although I suppose if there is some kind of vrr slew rate limit we could at least account for that to report a more correct "this is the earliest you migth be able to see your frame" timestamp. Oh, or are you actually saying that shceduling a new flip before the timeout is actually going to latch that flip immediately? I figured that the flip would get latched on the next start of vblank regardless, and the act of scheduling a flip will just kick the hardware to start scanning the previously latched frame earlier. scanout A | ... vblank | scanout A | ... vblank | scanout B | ... vblank ^^^ ^ ||flip C latch C flip B latch B vs. scanout A | ... vblank | scanout B | ... vblank | scanout C | ... vblank ^ ^ ^ ^ | latch B| latch C flip B flip C The latter would seem more like a tearing flip without the tearing. -- Ville Syrjälä Intel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 2018-10-26 7:59 p.m., Ville Syrjälä wrote: > On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: >> On 10/26/18 10:53 AM, Ville Syrjälä wrote: >>> >>> Speaking of timestamps. What is the expected behaviour of vblank >>> timestamps when vrr is enabled? >> >> When vrr is enabled the duration of the vertical front porch will be >> extended until flip or timeout occurs. The vblank timestamp will vary >> based on duration of the vertical front porch. The min/max duration for >> the front porch can be specified by the driver via the min/max range. >> >> No changes to vblank timestamping handling should be necessary to >> accommodate variable refresh rate. > > The problem is that the timestamp is supposed to correspond to the first > active pixel. And since we don't know how long the front porch will be > we can't realistically report the true value. So I guess just assuming > min front porch length is as good as anything else? That (and documenting that the timestamp corresponds to the earliest possible first active pixel, not necessarily the actual one, with VRR) might be good enough for the actual vblank event timestamps. However, I'm not so sure about the timestamps of page flip completion events. Those could be very misleading if the flip completes towards the timeout, which could result in bad behaviour of applications which use them for animation timing. Maybe the timestamp could be updated appropriately (yes, I'm hand-waving :) in drm_crtc_send_vblank_event? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/29/18 4:36 AM, Pekka Paalanen wrote: > On Fri, 26 Oct 2018 17:34:15 + > "Kazlauskas, Nicholas" wrote: > >> On 10/26/18 10:53 AM, Ville Syrjälä wrote: >>> On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: On 10/26/18 7:37 AM, Pekka Paalanen wrote: > > Hi, > > where is the documentation that explains how drivers must implement > "variable refresh rate adjustment"? > > What should and could userspace expect to get if it flicks this switch? > > I also think the kernel documentation should include a description of > what VRR actually is and how it conceptually works as far as userspace > is concerned. > > That is, the kernel documentation should describe what this thing does, > so that we avoid every driver implementing a different thing. For > example, one driver could prevent the luminance flickering itself by > tuning the timings while another driver might not do that, which means > that an application tested on the former driver will look just fine > while it is unbearable to watch on the latter. > > > Thanks, > pq I felt it was best to leave this more on the vague side to not impose restrictions yet on what a driver must do. If you think it's worth defining what the "baseline" expectation is for userspace, I would probably describe it as "utilizing the monitor's variable refresh rate to reduce stuttering or tearing that can occur during flipping". This is currently what the amdgpu driver has enabled for support. The implementation also isn't much more complex than just passing the variable refresh range to the hardware. > > Hi, > > sorry, but that says nothing. > > Also, tearing has nothing to do here. VRR reduces stuttering if > userspace is already tear-free. If userspace was driving the display in > a tearing fashion, VRR will not reduce or remove tearing, it just makes > it happen at different points and times. Tearing happens because > framebuffer flips are executed regardless of the refresh cycle, so > adjusting the refresh timings won't help. > > However... > I wouldn't want every driver to be forced to implement some sort of luminance flickering by default. It's not noticeable on many panels and any tuning would inherently add latency to the output. It would probably be better left as a new property or a driver specific feature. > > The point is to give userspace guaranteed expectations. If the > expectation is that some displays might actually emit bad luminance > flickering, then userspace must always assume the worst and implement > the needed slewing algorithms to avoid it, even if it would be > unnecessary. > > Userspace is farther away from the hardware than the drivers are, and > if userspace is required to implement luminance flickering avoidance, > that implementation must be done in all display servers and KMS apps > that might enable VRR. That would also call for a userspace hardware > database, so that people can set up quirks to enable/disable/adjust the > algorithms for specific hardware with the hopes that other users could > then have a good out-of-the-box experience. Instead, if possible, I > would like to see some guarantees from the kernel drivers that > userspace does not need to worry about luminance flickering. > > Unless you would deem all hardware that can exhibit luminance > flickering as faulty and unsupported? > > We need a baseline default expectation. You can modify that expectation > later with new properties, but I believe something needs to be defined > as the default. Even if the definition is really just "hardware takes > care of not flickering". > In general I would imagine that most future VRR features would end up as new properties. Anything that's purely software could be implemented as a drm helper that every driver can use. I think the target presentation timestamp feature is a good example for that. >>> >>> Speaking of timestamps. What is the expected behaviour of vblank >>> timestamps when vrr is enabled? >>> >> >> When vrr is enabled the duration of the vertical front porch will be >> extended until flip or timeout occurs. The vblank timestamp will vary >> based on duration of the vertical front porch. The min/max duration for >> the front porch can be specified by the driver via the min/max range. > > ...This is actually useful information, it explains things. Do all VRR > hardware implementations fundamentally work like this? > > With that definition, there is only more parameter to be exposed to > userspace in the future: what is the length of the timeout? No need to > expose maximum-refresh-freq because that information is already present > in the programmed video mode. > > Btw. even this definition does not give any hint about problems like > the luminance flickering. If possible flickering is an issue that > cannot
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Fri, 26 Oct 2018 17:34:15 + "Kazlauskas, Nicholas" wrote: > On 10/26/18 10:53 AM, Ville Syrjälä wrote: > > On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: > >> On 10/26/18 7:37 AM, Pekka Paalanen wrote: > >>> Hi, > >>> > >>> where is the documentation that explains how drivers must implement > >>> "variable refresh rate adjustment"? > >>> > >>> What should and could userspace expect to get if it flicks this switch? > >>> > >>> I also think the kernel documentation should include a description of > >>> what VRR actually is and how it conceptually works as far as userspace > >>> is concerned. > >>> > >>> That is, the kernel documentation should describe what this thing does, > >>> so that we avoid every driver implementing a different thing. For > >>> example, one driver could prevent the luminance flickering itself by > >>> tuning the timings while another driver might not do that, which means > >>> that an application tested on the former driver will look just fine > >>> while it is unbearable to watch on the latter. > >>> > >>> > >>> Thanks, > >>> pq > >> > >> I felt it was best to leave this more on the vague side to not impose > >> restrictions yet on what a driver must do. > >> > >> If you think it's worth defining what the "baseline" expectation is for > >> userspace, I would probably describe it as "utilizing the monitor's > >> variable refresh rate to reduce stuttering or tearing that can occur > >> during flipping". This is currently what the amdgpu driver has enabled > >> for support. The implementation also isn't much more complex than just > >> passing the variable refresh range to the hardware. Hi, sorry, but that says nothing. Also, tearing has nothing to do here. VRR reduces stuttering if userspace is already tear-free. If userspace was driving the display in a tearing fashion, VRR will not reduce or remove tearing, it just makes it happen at different points and times. Tearing happens because framebuffer flips are executed regardless of the refresh cycle, so adjusting the refresh timings won't help. However... > >> > >> I wouldn't want every driver to be forced to implement some sort of > >> luminance flickering by default. It's not noticeable on many panels and > >> any tuning would inherently add latency to the output. It would probably > >> be better left as a new property or a driver specific feature. The point is to give userspace guaranteed expectations. If the expectation is that some displays might actually emit bad luminance flickering, then userspace must always assume the worst and implement the needed slewing algorithms to avoid it, even if it would be unnecessary. Userspace is farther away from the hardware than the drivers are, and if userspace is required to implement luminance flickering avoidance, that implementation must be done in all display servers and KMS apps that might enable VRR. That would also call for a userspace hardware database, so that people can set up quirks to enable/disable/adjust the algorithms for specific hardware with the hopes that other users could then have a good out-of-the-box experience. Instead, if possible, I would like to see some guarantees from the kernel drivers that userspace does not need to worry about luminance flickering. Unless you would deem all hardware that can exhibit luminance flickering as faulty and unsupported? We need a baseline default expectation. You can modify that expectation later with new properties, but I believe something needs to be defined as the default. Even if the definition is really just "hardware takes care of not flickering". > >> > >> In general I would imagine that most future VRR features would end up as > >> new properties. Anything that's purely software could be implemented as > >> a drm helper that every driver can use. I think the target presentation > >> timestamp feature is a good example for that. > > > > Speaking of timestamps. What is the expected behaviour of vblank > > timestamps when vrr is enabled? > > > > When vrr is enabled the duration of the vertical front porch will be > extended until flip or timeout occurs. The vblank timestamp will vary > based on duration of the vertical front porch. The min/max duration for > the front porch can be specified by the driver via the min/max range. ...This is actually useful information, it explains things. Do all VRR hardware implementations fundamentally work like this? With that definition, there is only more parameter to be exposed to userspace in the future: what is the length of the timeout? No need to expose maximum-refresh-freq because that information is already present in the programmed video mode. Btw. even this definition does not give any hint about problems like the luminance flickering. If possible flickering is an issue that cannot be simply ignored, then something should be documented about it. Do you know of any other "hidden" issues that could require userspace or drivers to
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Fri, Oct 26, 2018 at 08:06:11PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 3:13 PM, Manasi Navare wrote: > > On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: > >> On 10/26/18 10:53 AM, Ville Syrjälä wrote: > >>> On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 7:37 AM, Pekka Paalanen wrote: > > On Thu, 11 Oct 2018 12:39:41 -0400 > > Nicholas Kazlauskas wrote: > > > >> These include the drm_connector 'vrr_capable' and the drm_crtc > >> 'vrr_enabled' properties. > >> > >> Signed-off-by: Nicholas Kazlauskas > >> --- > >> Documentation/gpu/drm-kms.rst | 7 +++ > >> drivers/gpu/drm/drm_connector.c | 22 ++ > >> 2 files changed, 29 insertions(+) > >> > >> diff --git a/Documentation/gpu/drm-kms.rst > >> b/Documentation/gpu/drm-kms.rst > >> index 4b1501b4835b..8da2a178cf85 100644 > >> --- a/Documentation/gpu/drm-kms.rst > >> +++ b/Documentation/gpu/drm-kms.rst > >> @@ -575,6 +575,13 @@ Explicit Fencing Properties > >> .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > >>:doc: explicit fencing properties > >> > >> + > >> +Variable Refresh Properties > >> +--- > >> + > >> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > >> + :doc: Variable refresh properties > >> + > >> Existing KMS Properties > >> --- > >> > >> diff --git a/drivers/gpu/drm/drm_connector.c > >> b/drivers/gpu/drm/drm_connector.c > >> index f0deeb7298d0..2a12853ca917 100644 > >> --- a/drivers/gpu/drm/drm_connector.c > >> +++ b/drivers/gpu/drm/drm_connector.c > >> @@ -1254,6 +1254,28 @@ int > >> drm_mode_create_scaling_mode_property(struct drm_device *dev) > >> } > >> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > >> > >> +/** > >> + * DOC: Variable refresh properties > >> + * > >> + * Variable refresh rate control is supported via properties on the > >> + * _connector and _crtc objects. > >> + * > >> + * "vrr_capable": > >> + *Optional _connector boolean property that drivers should > >> attach > >> + *with drm_connector_attach_vrr_capable_property() on connectors > >> that > >> + *could support variable refresh rates. Drivers should update the > >> + *property value by calling > >> drm_connector_set_vrr_capable_property(). > >> + * > >> + *Absence of the property should indicate absence of support. > >> + * > >> + * "vrr_enabled": > >> + *Default _crtc boolean property that notifies the driver > >> that the > >> + *variable refresh rate adjustment should be enabled for the CRTC. > > > > Hi, > > > > where is the documentation that explains how drivers must implement > > "variable refresh rate adjustment"? > > > > What should and could userspace expect to get if it flicks this switch? > > > > I also think the kernel documentation should include a description of > > what VRR actually is and how it conceptually works as far as userspace > > is concerned. > > > > That is, the kernel documentation should describe what this thing does, > > so that we avoid every driver implementing a different thing. For > > example, one driver could prevent the luminance flickering itself by > > tuning the timings while another driver might not do that, which means > > that an application tested on the former driver will look just fine > > while it is unbearable to watch on the latter. > > > > > > Thanks, > > pq > > I felt it was best to leave this more on the vague side to not impose > restrictions yet on what a driver must do. > > If you think it's worth defining what the "baseline" expectation is for > userspace, I would probably describe it as "utilizing the monitor's > variable refresh rate to reduce stuttering or tearing that can occur > during flipping". This is currently what the amdgpu driver has enabled > > > > I would also mention that without VRR, the display engine processes the > > flips > > independently of the rendering speed which might result into stuttering or > > tearing. > > > > Might also be worth giving a quick example with numbers in the > > documentation. > > Something like: For Eg if the rendering speed is 40Hz, without VRR, display > > engine > > will flip at constant 60Hz, which means that same frame will be displayed > > twice which > > will be observed as stutter/repetition. > > With VRR enabled, the vertical front porch will be extended and the flip > > will > > be processed when its ready or at max blanking time resulting a smooth > > transition without repetition. > > Good points about mentioning the problems it solves
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/26/18 3:13 PM, Manasi Navare wrote: > On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: >> On 10/26/18 10:53 AM, Ville Syrjälä wrote: >>> On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: On 10/26/18 7:37 AM, Pekka Paalanen wrote: > On Thu, 11 Oct 2018 12:39:41 -0400 > Nicholas Kazlauskas wrote: > >> These include the drm_connector 'vrr_capable' and the drm_crtc >> 'vrr_enabled' properties. >> >> Signed-off-by: Nicholas Kazlauskas >> --- >> Documentation/gpu/drm-kms.rst | 7 +++ >> drivers/gpu/drm/drm_connector.c | 22 ++ >> 2 files changed, 29 insertions(+) >> >> diff --git a/Documentation/gpu/drm-kms.rst >> b/Documentation/gpu/drm-kms.rst >> index 4b1501b4835b..8da2a178cf85 100644 >> --- a/Documentation/gpu/drm-kms.rst >> +++ b/Documentation/gpu/drm-kms.rst >> @@ -575,6 +575,13 @@ Explicit Fencing Properties >> .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c >>:doc: explicit fencing properties >> >> + >> +Variable Refresh Properties >> +--- >> + >> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c >> + :doc: Variable refresh properties >> + >> Existing KMS Properties >> --- >> >> diff --git a/drivers/gpu/drm/drm_connector.c >> b/drivers/gpu/drm/drm_connector.c >> index f0deeb7298d0..2a12853ca917 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct >> drm_device *dev) >> } >> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); >> >> +/** >> + * DOC: Variable refresh properties >> + * >> + * Variable refresh rate control is supported via properties on the >> + * _connector and _crtc objects. >> + * >> + * "vrr_capable": >> + * Optional _connector boolean property that drivers should >> attach >> + * with drm_connector_attach_vrr_capable_property() on connectors >> that >> + * could support variable refresh rates. Drivers should update the >> + * property value by calling >> drm_connector_set_vrr_capable_property(). >> + * >> + * Absence of the property should indicate absence of support. >> + * >> + * "vrr_enabled": >> + * Default _crtc boolean property that notifies the driver >> that the >> + * variable refresh rate adjustment should be enabled for the CRTC. > > Hi, > > where is the documentation that explains how drivers must implement > "variable refresh rate adjustment"? > > What should and could userspace expect to get if it flicks this switch? > > I also think the kernel documentation should include a description of > what VRR actually is and how it conceptually works as far as userspace > is concerned. > > That is, the kernel documentation should describe what this thing does, > so that we avoid every driver implementing a different thing. For > example, one driver could prevent the luminance flickering itself by > tuning the timings while another driver might not do that, which means > that an application tested on the former driver will look just fine > while it is unbearable to watch on the latter. > > > Thanks, > pq I felt it was best to leave this more on the vague side to not impose restrictions yet on what a driver must do. If you think it's worth defining what the "baseline" expectation is for userspace, I would probably describe it as "utilizing the monitor's variable refresh rate to reduce stuttering or tearing that can occur during flipping". This is currently what the amdgpu driver has enabled > > I would also mention that without VRR, the display engine processes the flips > independently of the rendering speed which might result into stuttering or > tearing. > > Might also be worth giving a quick example with numbers in the documentation. > Something like: For Eg if the rendering speed is 40Hz, without VRR, display > engine > will flip at constant 60Hz, which means that same frame will be displayed > twice which > will be observed as stutter/repetition. > With VRR enabled, the vertical front porch will be extended and the flip will > be processed when its ready or at max blanking time resulting a smooth > transition without repetition. Good points about mentioning the problems it solves in the documentation. > for support. The implementation also isn't much more complex than just passing the variable refresh range to the hardware. I wouldn't want every driver to be forced to implement some sort of luminance flickering by default. It's not noticeable on
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 10:53 AM, Ville Syrjälä wrote: > > On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: > >> On 10/26/18 7:37 AM, Pekka Paalanen wrote: > >>> On Thu, 11 Oct 2018 12:39:41 -0400 > >>> Nicholas Kazlauskas wrote: > >>> > These include the drm_connector 'vrr_capable' and the drm_crtc > 'vrr_enabled' properties. > > Signed-off-by: Nicholas Kazlauskas > --- > Documentation/gpu/drm-kms.rst | 7 +++ > drivers/gpu/drm/drm_connector.c | 22 ++ > 2 files changed, 29 insertions(+) > > diff --git a/Documentation/gpu/drm-kms.rst > b/Documentation/gpu/drm-kms.rst > index 4b1501b4835b..8da2a178cf85 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -575,6 +575,13 @@ Explicit Fencing Properties > .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > :doc: explicit fencing properties > > + > +Variable Refresh Properties > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > + :doc: Variable refresh properties > + > Existing KMS Properties > --- > > diff --git a/drivers/gpu/drm/drm_connector.c > b/drivers/gpu/drm/drm_connector.c > index f0deeb7298d0..2a12853ca917 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct > drm_device *dev) > } > EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > > +/** > + * DOC: Variable refresh properties > + * > + * Variable refresh rate control is supported via properties on the > + * _connector and _crtc objects. > + * > + * "vrr_capable": > + * Optional _connector boolean property that drivers should > attach > + * with drm_connector_attach_vrr_capable_property() on connectors > that > + * could support variable refresh rates. Drivers should update the > + * property value by calling > drm_connector_set_vrr_capable_property(). > + * > + * Absence of the property should indicate absence of support. > + * > + * "vrr_enabled": > + * Default _crtc boolean property that notifies the driver > that the > + * variable refresh rate adjustment should be enabled for the CRTC. > >>> > >>> Hi, > >>> > >>> where is the documentation that explains how drivers must implement > >>> "variable refresh rate adjustment"? > >>> > >>> What should and could userspace expect to get if it flicks this switch? > >>> > >>> I also think the kernel documentation should include a description of > >>> what VRR actually is and how it conceptually works as far as userspace > >>> is concerned. > >>> > >>> That is, the kernel documentation should describe what this thing does, > >>> so that we avoid every driver implementing a different thing. For > >>> example, one driver could prevent the luminance flickering itself by > >>> tuning the timings while another driver might not do that, which means > >>> that an application tested on the former driver will look just fine > >>> while it is unbearable to watch on the latter. > >>> > >>> > >>> Thanks, > >>> pq > >> > >> I felt it was best to leave this more on the vague side to not impose > >> restrictions yet on what a driver must do. > >> > >> If you think it's worth defining what the "baseline" expectation is for > >> userspace, I would probably describe it as "utilizing the monitor's > >> variable refresh rate to reduce stuttering or tearing that can occur > >> during flipping". This is currently what the amdgpu driver has enabled I would also mention that without VRR, the display engine processes the flips independently of the rendering speed which might result into stuttering or tearing. Might also be worth giving a quick example with numbers in the documentation. Something like: For Eg if the rendering speed is 40Hz, without VRR, display engine will flip at constant 60Hz, which means that same frame will be displayed twice which will be observed as stutter/repetition. With VRR enabled, the vertical front porch will be extended and the flip will be processed when its ready or at max blanking time resulting a smooth transition without repetition. > >> for support. The implementation also isn't much more complex than just > >> passing the variable refresh range to the hardware. > >> > >> I wouldn't want every driver to be forced to implement some sort of > >> luminance flickering by default. It's not noticeable on many panels and > >> any tuning would inherently add latency to the output. It would probably > >> be better left as a new property or a driver specific feature.
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Fri, Oct 26, 2018 at 05:34:15PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 10:53 AM, Ville Syrjälä wrote: > > On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: > >> On 10/26/18 7:37 AM, Pekka Paalanen wrote: > >>> On Thu, 11 Oct 2018 12:39:41 -0400 > >>> Nicholas Kazlauskas wrote: > >>> > These include the drm_connector 'vrr_capable' and the drm_crtc > 'vrr_enabled' properties. > > Signed-off-by: Nicholas Kazlauskas > --- > Documentation/gpu/drm-kms.rst | 7 +++ > drivers/gpu/drm/drm_connector.c | 22 ++ > 2 files changed, 29 insertions(+) > > diff --git a/Documentation/gpu/drm-kms.rst > b/Documentation/gpu/drm-kms.rst > index 4b1501b4835b..8da2a178cf85 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -575,6 +575,13 @@ Explicit Fencing Properties > .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > :doc: explicit fencing properties > > + > +Variable Refresh Properties > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > + :doc: Variable refresh properties > + > Existing KMS Properties > --- > > diff --git a/drivers/gpu/drm/drm_connector.c > b/drivers/gpu/drm/drm_connector.c > index f0deeb7298d0..2a12853ca917 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct > drm_device *dev) > } > EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > > +/** > + * DOC: Variable refresh properties > + * > + * Variable refresh rate control is supported via properties on the > + * _connector and _crtc objects. > + * > + * "vrr_capable": > + * Optional _connector boolean property that drivers should > attach > + * with drm_connector_attach_vrr_capable_property() on connectors > that > + * could support variable refresh rates. Drivers should update the > + * property value by calling > drm_connector_set_vrr_capable_property(). > + * > + * Absence of the property should indicate absence of support. > + * > + * "vrr_enabled": > + * Default _crtc boolean property that notifies the driver > that the > + * variable refresh rate adjustment should be enabled for the CRTC. > >>> > >>> Hi, > >>> > >>> where is the documentation that explains how drivers must implement > >>> "variable refresh rate adjustment"? > >>> > >>> What should and could userspace expect to get if it flicks this switch? > >>> > >>> I also think the kernel documentation should include a description of > >>> what VRR actually is and how it conceptually works as far as userspace > >>> is concerned. > >>> > >>> That is, the kernel documentation should describe what this thing does, > >>> so that we avoid every driver implementing a different thing. For > >>> example, one driver could prevent the luminance flickering itself by > >>> tuning the timings while another driver might not do that, which means > >>> that an application tested on the former driver will look just fine > >>> while it is unbearable to watch on the latter. > >>> > >>> > >>> Thanks, > >>> pq > >> > >> I felt it was best to leave this more on the vague side to not impose > >> restrictions yet on what a driver must do. > >> > >> If you think it's worth defining what the "baseline" expectation is for > >> userspace, I would probably describe it as "utilizing the monitor's > >> variable refresh rate to reduce stuttering or tearing that can occur > >> during flipping". This is currently what the amdgpu driver has enabled > >> for support. The implementation also isn't much more complex than just > >> passing the variable refresh range to the hardware. > >> > >> I wouldn't want every driver to be forced to implement some sort of > >> luminance flickering by default. It's not noticeable on many panels and > >> any tuning would inherently add latency to the output. It would probably > >> be better left as a new property or a driver specific feature. > >> > >> In general I would imagine that most future VRR features would end up as > >> new properties. Anything that's purely software could be implemented as > >> a drm helper that every driver can use. I think the target presentation > >> timestamp feature is a good example for that. > > > > Speaking of timestamps. What is the expected behaviour of vblank > > timestamps when vrr is enabled? > > > > When vrr is enabled the duration of the vertical front porch will be > extended until flip or timeout occurs. The vblank timestamp will vary > based on duration of the vertical front porch. The min/max duration for > the front
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/26/18 10:53 AM, Ville Syrjälä wrote: > On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: >> On 10/26/18 7:37 AM, Pekka Paalanen wrote: >>> On Thu, 11 Oct 2018 12:39:41 -0400 >>> Nicholas Kazlauskas wrote: >>> These include the drm_connector 'vrr_capable' and the drm_crtc 'vrr_enabled' properties. Signed-off-by: Nicholas Kazlauskas --- Documentation/gpu/drm-kms.rst | 7 +++ drivers/gpu/drm/drm_connector.c | 22 ++ 2 files changed, 29 insertions(+) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 4b1501b4835b..8da2a178cf85 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -575,6 +575,13 @@ Explicit Fencing Properties .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c :doc: explicit fencing properties + +Variable Refresh Properties +--- + +.. kernel-doc:: drivers/gpu/drm/drm_connector.c + :doc: Variable refresh properties + Existing KMS Properties --- diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index f0deeb7298d0..2a12853ca917 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev) } EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); +/** + * DOC: Variable refresh properties + * + * Variable refresh rate control is supported via properties on the + * _connector and _crtc objects. + * + * "vrr_capable": + *Optional _connector boolean property that drivers should attach + *with drm_connector_attach_vrr_capable_property() on connectors that + *could support variable refresh rates. Drivers should update the + *property value by calling drm_connector_set_vrr_capable_property(). + * + *Absence of the property should indicate absence of support. + * + * "vrr_enabled": + *Default _crtc boolean property that notifies the driver that the + *variable refresh rate adjustment should be enabled for the CRTC. >>> >>> Hi, >>> >>> where is the documentation that explains how drivers must implement >>> "variable refresh rate adjustment"? >>> >>> What should and could userspace expect to get if it flicks this switch? >>> >>> I also think the kernel documentation should include a description of >>> what VRR actually is and how it conceptually works as far as userspace >>> is concerned. >>> >>> That is, the kernel documentation should describe what this thing does, >>> so that we avoid every driver implementing a different thing. For >>> example, one driver could prevent the luminance flickering itself by >>> tuning the timings while another driver might not do that, which means >>> that an application tested on the former driver will look just fine >>> while it is unbearable to watch on the latter. >>> >>> >>> Thanks, >>> pq >> >> I felt it was best to leave this more on the vague side to not impose >> restrictions yet on what a driver must do. >> >> If you think it's worth defining what the "baseline" expectation is for >> userspace, I would probably describe it as "utilizing the monitor's >> variable refresh rate to reduce stuttering or tearing that can occur >> during flipping". This is currently what the amdgpu driver has enabled >> for support. The implementation also isn't much more complex than just >> passing the variable refresh range to the hardware. >> >> I wouldn't want every driver to be forced to implement some sort of >> luminance flickering by default. It's not noticeable on many panels and >> any tuning would inherently add latency to the output. It would probably >> be better left as a new property or a driver specific feature. >> >> In general I would imagine that most future VRR features would end up as >> new properties. Anything that's purely software could be implemented as >> a drm helper that every driver can use. I think the target presentation >> timestamp feature is a good example for that. > > Speaking of timestamps. What is the expected behaviour of vblank > timestamps when vrr is enabled? > When vrr is enabled the duration of the vertical front porch will be extended until flip or timeout occurs. The vblank timestamp will vary based on duration of the vertical front porch. The min/max duration for the front porch can be specified by the driver via the min/max range. No changes to vblank timestamping handling should be necessary to accommodate variable refresh rate. I think it's probably best to update the documentation for vrr_enable with some of the specifics I described above. That should help
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Fri, Oct 26, 2018 at 02:49:31PM +, Kazlauskas, Nicholas wrote: > On 10/26/18 7:37 AM, Pekka Paalanen wrote: > > On Thu, 11 Oct 2018 12:39:41 -0400 > > Nicholas Kazlauskas wrote: > > > >> These include the drm_connector 'vrr_capable' and the drm_crtc > >> 'vrr_enabled' properties. > >> > >> Signed-off-by: Nicholas Kazlauskas > >> --- > >> Documentation/gpu/drm-kms.rst | 7 +++ > >> drivers/gpu/drm/drm_connector.c | 22 ++ > >> 2 files changed, 29 insertions(+) > >> > >> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > >> index 4b1501b4835b..8da2a178cf85 100644 > >> --- a/Documentation/gpu/drm-kms.rst > >> +++ b/Documentation/gpu/drm-kms.rst > >> @@ -575,6 +575,13 @@ Explicit Fencing Properties > >> .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > >> :doc: explicit fencing properties > >> > >> + > >> +Variable Refresh Properties > >> +--- > >> + > >> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > >> + :doc: Variable refresh properties > >> + > >> Existing KMS Properties > >> --- > >> > >> diff --git a/drivers/gpu/drm/drm_connector.c > >> b/drivers/gpu/drm/drm_connector.c > >> index f0deeb7298d0..2a12853ca917 100644 > >> --- a/drivers/gpu/drm/drm_connector.c > >> +++ b/drivers/gpu/drm/drm_connector.c > >> @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct > >> drm_device *dev) > >> } > >> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > >> > >> +/** > >> + * DOC: Variable refresh properties > >> + * > >> + * Variable refresh rate control is supported via properties on the > >> + * _connector and _crtc objects. > >> + * > >> + * "vrr_capable": > >> + *Optional _connector boolean property that drivers should > >> attach > >> + *with drm_connector_attach_vrr_capable_property() on connectors > >> that > >> + *could support variable refresh rates. Drivers should update the > >> + *property value by calling > >> drm_connector_set_vrr_capable_property(). > >> + * > >> + *Absence of the property should indicate absence of support. > >> + * > >> + * "vrr_enabled": > >> + *Default _crtc boolean property that notifies the driver > >> that the > >> + *variable refresh rate adjustment should be enabled for the CRTC. > > > > Hi, > > > > where is the documentation that explains how drivers must implement > > "variable refresh rate adjustment"? > > > > What should and could userspace expect to get if it flicks this switch? > > > > I also think the kernel documentation should include a description of > > what VRR actually is and how it conceptually works as far as userspace > > is concerned. > > > > That is, the kernel documentation should describe what this thing does, > > so that we avoid every driver implementing a different thing. For > > example, one driver could prevent the luminance flickering itself by > > tuning the timings while another driver might not do that, which means > > that an application tested on the former driver will look just fine > > while it is unbearable to watch on the latter. > > > > > > Thanks, > > pq > > I felt it was best to leave this more on the vague side to not impose > restrictions yet on what a driver must do. > > If you think it's worth defining what the "baseline" expectation is for > userspace, I would probably describe it as "utilizing the monitor's > variable refresh rate to reduce stuttering or tearing that can occur > during flipping". This is currently what the amdgpu driver has enabled > for support. The implementation also isn't much more complex than just > passing the variable refresh range to the hardware. > > I wouldn't want every driver to be forced to implement some sort of > luminance flickering by default. It's not noticeable on many panels and > any tuning would inherently add latency to the output. It would probably > be better left as a new property or a driver specific feature. > > In general I would imagine that most future VRR features would end up as > new properties. Anything that's purely software could be implemented as > a drm helper that every driver can use. I think the target presentation > timestamp feature is a good example for that. Speaking of timestamps. What is the expected behaviour of vblank timestamps when vrr is enabled? -- Ville Syrjälä Intel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On 10/26/18 7:37 AM, Pekka Paalanen wrote: > On Thu, 11 Oct 2018 12:39:41 -0400 > Nicholas Kazlauskas wrote: > >> These include the drm_connector 'vrr_capable' and the drm_crtc >> 'vrr_enabled' properties. >> >> Signed-off-by: Nicholas Kazlauskas >> --- >> Documentation/gpu/drm-kms.rst | 7 +++ >> drivers/gpu/drm/drm_connector.c | 22 ++ >> 2 files changed, 29 insertions(+) >> >> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst >> index 4b1501b4835b..8da2a178cf85 100644 >> --- a/Documentation/gpu/drm-kms.rst >> +++ b/Documentation/gpu/drm-kms.rst >> @@ -575,6 +575,13 @@ Explicit Fencing Properties >> .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c >> :doc: explicit fencing properties >> >> + >> +Variable Refresh Properties >> +--- >> + >> +.. kernel-doc:: drivers/gpu/drm/drm_connector.c >> + :doc: Variable refresh properties >> + >> Existing KMS Properties >> --- >> >> diff --git a/drivers/gpu/drm/drm_connector.c >> b/drivers/gpu/drm/drm_connector.c >> index f0deeb7298d0..2a12853ca917 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct >> drm_device *dev) >> } >> EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); >> >> +/** >> + * DOC: Variable refresh properties >> + * >> + * Variable refresh rate control is supported via properties on the >> + * _connector and _crtc objects. >> + * >> + * "vrr_capable": >> + * Optional _connector boolean property that drivers should attach >> + * with drm_connector_attach_vrr_capable_property() on connectors that >> + * could support variable refresh rates. Drivers should update the >> + * property value by calling drm_connector_set_vrr_capable_property(). >> + * >> + * Absence of the property should indicate absence of support. >> + * >> + * "vrr_enabled": >> + * Default _crtc boolean property that notifies the driver that the >> + * variable refresh rate adjustment should be enabled for the CRTC. > > Hi, > > where is the documentation that explains how drivers must implement > "variable refresh rate adjustment"? > > What should and could userspace expect to get if it flicks this switch? > > I also think the kernel documentation should include a description of > what VRR actually is and how it conceptually works as far as userspace > is concerned. > > That is, the kernel documentation should describe what this thing does, > so that we avoid every driver implementing a different thing. For > example, one driver could prevent the luminance flickering itself by > tuning the timings while another driver might not do that, which means > that an application tested on the former driver will look just fine > while it is unbearable to watch on the latter. > > > Thanks, > pq I felt it was best to leave this more on the vague side to not impose restrictions yet on what a driver must do. If you think it's worth defining what the "baseline" expectation is for userspace, I would probably describe it as "utilizing the monitor's variable refresh rate to reduce stuttering or tearing that can occur during flipping". This is currently what the amdgpu driver has enabled for support. The implementation also isn't much more complex than just passing the variable refresh range to the hardware. I wouldn't want every driver to be forced to implement some sort of luminance flickering by default. It's not noticeable on many panels and any tuning would inherently add latency to the output. It would probably be better left as a new property or a driver specific feature. In general I would imagine that most future VRR features would end up as new properties. Anything that's purely software could be implemented as a drm helper that every driver can use. I think the target presentation timestamp feature is a good example for that. > >> + * >> + * Support for variable refresh rate will depend on the "vrr_capable" >> + * property exposed on the _connector object. >> + */ >> + >> /** >>* drm_connector_attach_vrr_capable_property - creates the >>* vrr_capable property > Nicholas Kazlauskas ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v4 3/4] drm: Document variable refresh properties
On Thu, 11 Oct 2018 12:39:41 -0400 Nicholas Kazlauskas wrote: > These include the drm_connector 'vrr_capable' and the drm_crtc > 'vrr_enabled' properties. > > Signed-off-by: Nicholas Kazlauskas > --- > Documentation/gpu/drm-kms.rst | 7 +++ > drivers/gpu/drm/drm_connector.c | 22 ++ > 2 files changed, 29 insertions(+) > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > index 4b1501b4835b..8da2a178cf85 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -575,6 +575,13 @@ Explicit Fencing Properties > .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c > :doc: explicit fencing properties > > + > +Variable Refresh Properties > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_connector.c > + :doc: Variable refresh properties > + > Existing KMS Properties > --- > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index f0deeb7298d0..2a12853ca917 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1254,6 +1254,28 @@ int drm_mode_create_scaling_mode_property(struct > drm_device *dev) > } > EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); > > +/** > + * DOC: Variable refresh properties > + * > + * Variable refresh rate control is supported via properties on the > + * _connector and _crtc objects. > + * > + * "vrr_capable": > + * Optional _connector boolean property that drivers should attach > + * with drm_connector_attach_vrr_capable_property() on connectors that > + * could support variable refresh rates. Drivers should update the > + * property value by calling drm_connector_set_vrr_capable_property(). > + * > + * Absence of the property should indicate absence of support. > + * > + * "vrr_enabled": > + * Default _crtc boolean property that notifies the driver that the > + * variable refresh rate adjustment should be enabled for the CRTC. Hi, where is the documentation that explains how drivers must implement "variable refresh rate adjustment"? What should and could userspace expect to get if it flicks this switch? I also think the kernel documentation should include a description of what VRR actually is and how it conceptually works as far as userspace is concerned. That is, the kernel documentation should describe what this thing does, so that we avoid every driver implementing a different thing. For example, one driver could prevent the luminance flickering itself by tuning the timings while another driver might not do that, which means that an application tested on the former driver will look just fine while it is unbearable to watch on the latter. Thanks, pq > + * > + * Support for variable refresh rate will depend on the "vrr_capable" > + * property exposed on the _connector object. > + */ > + > /** > * drm_connector_attach_vrr_capable_property - creates the > * vrr_capable property pgp6GpcQTijeC.pgp Description: OpenPGP digital signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
