Re: [PATCH v2 4/5] drm/tegra: Implement VBLANK support
On Tue, Jan 22, 2013 at 11:20 AM, Mario Kleiner
wrote:
> On 22.01.13 19:39, Lucas Stach wrote:
>>
>> Am Dienstag, den 22.01.2013, 18:37 +0100 schrieb Mario Kleiner:
>>>
>>> On 14.01.13 17:05, Thierry Reding wrote:
Implement support for the VBLANK IOCTL. Note that Tegra is somewhat
special in this case because it doesn't use the generic IRQ support
provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one
interrupt handler for each display controller.
While at it, clean up the way that interrupts are enabled to ensure
that the VBLANK interrupt only gets enabled when required.
Signed-off-by: Thierry Reding
>>>
>>>
>>> ... snip ...
>>>
struct drm_driver tegra_drm_driver = {
.driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET |
DRIVER_GEM,
.load = tegra_drm_load,
@@ -96,6 +136,10 @@ struct drm_driver tegra_drm_driver = {
.open = tegra_drm_open,
.lastclose = tegra_drm_lastclose,
+ .get_vblank_counter = drm_vblank_count,
>>>
>>>
>>> -> .get_vblank_counter = drm_vblank_count is a no-op.
>>>
>>> .get_vblank_counter() is supposed to return some real hardware vblank
>>> counter value to reinitialize the software vblank counter at vbl irq
>>> enable time. That software counter gets queried via drm_vblank_count().
>>> If you hook this up to drm_vblank_count() it essentially returns a
>>> constant, frozen vblank count, it has the same effect as returning zero
>>> or any other constant value -- You lose all vblank counter increments
>>> during vblank irq off time. The same problem is present in nouveau-kms.
>>>
>>> I think it would be better to either implement a real hw counter query,
>>> or some function with a /* TODO: Implement me properly */ comment which
>>> returns zero, so it is clear something is missing here.
>>>
>> I've looked this up in the TRM a while ago as I know we have the same
>> problem in nouveau, but it seems there is no HW vblank counter on Tegra.
>>
>> Mario, you know a fair bit more about this than I do, what is the
>> preferred way of handling this if we are sure we are not able to
>> implement anything meaningful here? Just return 0?
>>
>
> I think 0 would be better, just to make it clear to readers of the source
> code that this function is basically unimplemented. For correctness it
> doesn't matter what you return, drm_vblank_count() is ok as well.
>
> If we wanted, we could probably implement a good enough emulated hw-vblank
> counter, at least on nouveau? If there is some on-chip clock register that
> is driven by the same hardware oscillator as the crtc's so it doesn't drift,
> we could store the clock time in the vblank_disable() hook, and some
> measured duration of a video refresh, wrt. that clock. Then in
> .get_vblank_counter() calculate how many vblanks have elapsed from
> (current_clock_time - vblank_off_clock_time) / frame_duration and increment
> our own private software vblank counter. Something along that line...
>
> -mario
>
Looking through the T30 manuals, I see all the
CLK_RST_CONTROLLER_CLK_SOURCE_xxx entries that support PLLD as a
parent are display related. You won't find any timers or counters that
use the same exact clock. Being out-of-sync is a real possibility, and
something adaptive would have to be implement to hide the desync
between a fake and a real vblank once you make the transition.
That said, I think being inaccurate here doesn't have a very high
cost. Please give me some examples if you disagree though, I'd be
interested in some good use cases to throw into my test plan.
- Jon
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/5] drm/tegra: Implement VBLANK support
On Tue, Jan 22, 2013 at 10:39 AM, Lucas Stach wrote:
> Am Dienstag, den 22.01.2013, 18:37 +0100 schrieb Mario Kleiner:
>> On 14.01.13 17:05, Thierry Reding wrote:
>> > Implement support for the VBLANK IOCTL. Note that Tegra is somewhat
>> > special in this case because it doesn't use the generic IRQ support
>> > provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one
>> > interrupt handler for each display controller.
>> >
>> > While at it, clean up the way that interrupts are enabled to ensure
>> > that the VBLANK interrupt only gets enabled when required.
>> >
>> > Signed-off-by: Thierry Reding
>>
>> ... snip ...
>>
>> > struct drm_driver tegra_drm_driver = {
>> > .driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET | DRIVER_GEM,
>> > .load = tegra_drm_load,
>> > @@ -96,6 +136,10 @@ struct drm_driver tegra_drm_driver = {
>> > .open = tegra_drm_open,
>> > .lastclose = tegra_drm_lastclose,
>> >
>> > + .get_vblank_counter = drm_vblank_count,
>>
>> -> .get_vblank_counter = drm_vblank_count is a no-op.
>>
>> .get_vblank_counter() is supposed to return some real hardware vblank
>> counter value to reinitialize the software vblank counter at vbl irq
>> enable time. That software counter gets queried via drm_vblank_count().
>> If you hook this up to drm_vblank_count() it essentially returns a
>> constant, frozen vblank count, it has the same effect as returning zero
>> or any other constant value -- You lose all vblank counter increments
>> during vblank irq off time. The same problem is present in nouveau-kms.
>>
>> I think it would be better to either implement a real hw counter query,
>> or some function with a /* TODO: Implement me properly */ comment which
>> returns zero, so it is clear something is missing here.
>>
> I've looked this up in the TRM a while ago as I know we have the same
> problem in nouveau, but it seems there is no HW vblank counter on Tegra.
>
> Mario, you know a fair bit more about this than I do, what is the
> preferred way of handling this if we are sure we are not able to
> implement anything meaningful here? Just return 0?
>
> Regards,
> Lucas
>
>
In my branch for the old non-DRM version of the tegra driver, I clock
gate and power gate display when using a one-shot smart panel. So not
only are there no more IRQs, but even if Tegra had a hardware vblank
counter it would also be dead too. (it doesn't have one, but I could
make the case to add one in the next chip if we could actually make
use of it, given my previous statement, I don't think it will help)
How badly do people need this feature? Because I really do think smart
panels are going to been the norm in a few years. A bit off-topic to
Thierry's submission, but I'd really like to discourage apps from
relying on this feature, does anyone else agree?
- Jon
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/tegra: Implement VBLANK support
On Mon, Jan 14, 2013 at 7:55 AM, Thierry Reding wrote: > Implement support for the VBLANK IOCTL. Note that Tegra is somewhat > special in this case because it doesn't use the generic IRQ support > provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one > interrupt handler for each display controller. > > While at it, clean up the way that interrupts are enabled to ensure > that the VBLANK interrupt only gets enabled when required. > > Signed-off-by: Thierry Reding > --- Sorry Thierry, but is this a useful feature? Are applications really making use of it? Because it means that that an IRQ will have to trigger every 16.67ms when it is taken, which means the device can't sleep. (probably OK because it should go back to idle when the app lets go of the vblank). But worse, for one-shot panels there is no continuous vblank. I've been doing weird hacks to run a timer when the smart panel is idle to trick applications into thinking they have a vblank pulse. But really I think the whole feature of a vblank pulse is pretty lame and I wish it would die. Were you going to use it for something specific, or just adding it for completeness? - Jon ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/5] drm/tegra: Implement VBLANK support
On Tue, Jan 22, 2013 at 11:20 AM, Mario Kleiner
wrote:
> On 22.01.13 19:39, Lucas Stach wrote:
>>
>> Am Dienstag, den 22.01.2013, 18:37 +0100 schrieb Mario Kleiner:
>>>
>>> On 14.01.13 17:05, Thierry Reding wrote:
Implement support for the VBLANK IOCTL. Note that Tegra is somewhat
special in this case because it doesn't use the generic IRQ support
provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one
interrupt handler for each display controller.
While at it, clean up the way that interrupts are enabled to ensure
that the VBLANK interrupt only gets enabled when required.
Signed-off-by: Thierry Reding
>>>
>>>
>>> ... snip ...
>>>
struct drm_driver tegra_drm_driver = {
.driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET |
DRIVER_GEM,
.load = tegra_drm_load,
@@ -96,6 +136,10 @@ struct drm_driver tegra_drm_driver = {
.open = tegra_drm_open,
.lastclose = tegra_drm_lastclose,
+ .get_vblank_counter = drm_vblank_count,
>>>
>>>
>>> -> .get_vblank_counter = drm_vblank_count is a no-op.
>>>
>>> .get_vblank_counter() is supposed to return some real hardware vblank
>>> counter value to reinitialize the software vblank counter at vbl irq
>>> enable time. That software counter gets queried via drm_vblank_count().
>>> If you hook this up to drm_vblank_count() it essentially returns a
>>> constant, frozen vblank count, it has the same effect as returning zero
>>> or any other constant value -- You lose all vblank counter increments
>>> during vblank irq off time. The same problem is present in nouveau-kms.
>>>
>>> I think it would be better to either implement a real hw counter query,
>>> or some function with a /* TODO: Implement me properly */ comment which
>>> returns zero, so it is clear something is missing here.
>>>
>> I've looked this up in the TRM a while ago as I know we have the same
>> problem in nouveau, but it seems there is no HW vblank counter on Tegra.
>>
>> Mario, you know a fair bit more about this than I do, what is the
>> preferred way of handling this if we are sure we are not able to
>> implement anything meaningful here? Just return 0?
>>
>
> I think 0 would be better, just to make it clear to readers of the source
> code that this function is basically unimplemented. For correctness it
> doesn't matter what you return, drm_vblank_count() is ok as well.
>
> If we wanted, we could probably implement a good enough emulated hw-vblank
> counter, at least on nouveau? If there is some on-chip clock register that
> is driven by the same hardware oscillator as the crtc's so it doesn't drift,
> we could store the clock time in the vblank_disable() hook, and some
> measured duration of a video refresh, wrt. that clock. Then in
> .get_vblank_counter() calculate how many vblanks have elapsed from
> (current_clock_time - vblank_off_clock_time) / frame_duration and increment
> our own private software vblank counter. Something along that line...
>
> -mario
>
Looking through the T30 manuals, I see all the
CLK_RST_CONTROLLER_CLK_SOURCE_xxx entries that support PLLD as a
parent are display related. You won't find any timers or counters that
use the same exact clock. Being out-of-sync is a real possibility, and
something adaptive would have to be implement to hide the desync
between a fake and a real vblank once you make the transition.
That said, I think being inaccurate here doesn't have a very high
cost. Please give me some examples if you disagree though, I'd be
interested in some good use cases to throw into my test plan.
- Jon
[PATCH v2 4/5] drm/tegra: Implement VBLANK support
On Tue, Jan 22, 2013 at 10:39 AM, Lucas Stach wrote:
> Am Dienstag, den 22.01.2013, 18:37 +0100 schrieb Mario Kleiner:
>> On 14.01.13 17:05, Thierry Reding wrote:
>> > Implement support for the VBLANK IOCTL. Note that Tegra is somewhat
>> > special in this case because it doesn't use the generic IRQ support
>> > provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one
>> > interrupt handler for each display controller.
>> >
>> > While at it, clean up the way that interrupts are enabled to ensure
>> > that the VBLANK interrupt only gets enabled when required.
>> >
>> > Signed-off-by: Thierry Reding
>>
>> ... snip ...
>>
>> > struct drm_driver tegra_drm_driver = {
>> > .driver_features = DRIVER_BUS_PLATFORM | DRIVER_MODESET | DRIVER_GEM,
>> > .load = tegra_drm_load,
>> > @@ -96,6 +136,10 @@ struct drm_driver tegra_drm_driver = {
>> > .open = tegra_drm_open,
>> > .lastclose = tegra_drm_lastclose,
>> >
>> > + .get_vblank_counter = drm_vblank_count,
>>
>> -> .get_vblank_counter = drm_vblank_count is a no-op.
>>
>> .get_vblank_counter() is supposed to return some real hardware vblank
>> counter value to reinitialize the software vblank counter at vbl irq
>> enable time. That software counter gets queried via drm_vblank_count().
>> If you hook this up to drm_vblank_count() it essentially returns a
>> constant, frozen vblank count, it has the same effect as returning zero
>> or any other constant value -- You lose all vblank counter increments
>> during vblank irq off time. The same problem is present in nouveau-kms.
>>
>> I think it would be better to either implement a real hw counter query,
>> or some function with a /* TODO: Implement me properly */ comment which
>> returns zero, so it is clear something is missing here.
>>
> I've looked this up in the TRM a while ago as I know we have the same
> problem in nouveau, but it seems there is no HW vblank counter on Tegra.
>
> Mario, you know a fair bit more about this than I do, what is the
> preferred way of handling this if we are sure we are not able to
> implement anything meaningful here? Just return 0?
>
> Regards,
> Lucas
>
>
In my branch for the old non-DRM version of the tegra driver, I clock
gate and power gate display when using a one-shot smart panel. So not
only are there no more IRQs, but even if Tegra had a hardware vblank
counter it would also be dead too. (it doesn't have one, but I could
make the case to add one in the next chip if we could actually make
use of it, given my previous statement, I don't think it will help)
How badly do people need this feature? Because I really do think smart
panels are going to been the norm in a few years. A bit off-topic to
Thierry's submission, but I'd really like to discourage apps from
relying on this feature, does anyone else agree?
- Jon
[PATCH 4/5] drm/tegra: Implement VBLANK support
On Mon, Jan 14, 2013 at 7:55 AM, Thierry Reding wrote: > Implement support for the VBLANK IOCTL. Note that Tegra is somewhat > special in this case because it doesn't use the generic IRQ support > provided by the DRM core (DRIVER_HAVE_IRQ) but rather registers one > interrupt handler for each display controller. > > While at it, clean up the way that interrupts are enabled to ensure > that the VBLANK interrupt only gets enabled when required. > > Signed-off-by: Thierry Reding > --- Sorry Thierry, but is this a useful feature? Are applications really making use of it? Because it means that that an IRQ will have to trigger every 16.67ms when it is taken, which means the device can't sleep. (probably OK because it should go back to idle when the app lets go of the vblank). But worse, for one-shot panels there is no continuous vblank. I've been doing weird hacks to run a timer when the smart panel is idle to trick applications into thinking they have a vblank pulse. But really I think the whole feature of a vblank pulse is pretty lame and I wish it would die. Were you going to use it for something specific, or just adding it for completeness? - Jon
Re: [PATCH v2 2/2] drm: tegra: check HDMI sink capability
On Wed, Jan 16, 2013 at 7:24 AM, Thierry Reding wrote: > On Wed, Jan 16, 2013 at 03:36:42PM +0100, Lucas Stach wrote: >> Check if sink is HDMI capable when enabling an output. This disables >> HDMI audio/infoframes if we are talking to a plain DVI sink. All things >> except this check are already in place. >> >> Signed-off-by: Lucas Stach >> --- >> drivers/gpu/drm/tegra/hdmi.c | 3 +++ >> 1 file changed, 3 insertions(+) > > Reviewed-by: Thierry Reding Reviewed-by: Jon Mayo ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] drm: tegra: check HDMI sink capability
On Wed, Jan 16, 2013 at 7:24 AM, Thierry Reding wrote: > On Wed, Jan 16, 2013 at 03:36:42PM +0100, Lucas Stach wrote: >> Check if sink is HDMI capable when enabling an output. This disables >> HDMI audio/infoframes if we are talking to a plain DVI sink. All things >> except this check are already in place. >> >> Signed-off-by: Lucas Stach >> --- >> drivers/gpu/drm/tegra/hdmi.c | 3 +++ >> 1 file changed, 3 insertions(+) > > Reviewed-by: Thierry Reding Reviewed-by: Jon Mayo
Re: [RFC v2 5/5] drm: Add NVIDIA Tegra support
On Wed, Apr 25, 2012 at 2:45 AM, Thierry Reding wrote: > This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It > currently has rudimentary GEM support and can run a console on the > framebuffer as well as X using the xf86-video-modesetting driver. Only > the RGB output is supported. > > HDMI support was taken from NVIDIA's Linux kernel tree but it doesn't > quite work. EDID data can be retrieved but the output doesn't properly > activate the connected TV. > > The DSI and TVO outputs and the HOST1X driver are just stubs that setup > the corresponding resources but don't do anything useful yet. > Nice work Thierry, I'm happy with the display programming bits. I think the pll_p enable/disable problem with RGB is better fixed when we have some fancier clock infrastructure. I have some ideas how it could be fixed now, but given that I want to see pll_p or pll_d / pll_d2 selected automatically based on final clock, I'd rather hold off on fixing things that are not really broken that going to be replaced anyways. You can save a little power by leaving D pll's off, and the logic to do it isn't hard and fairly safe to hard code into the driver instead of trying to represent it in DT. (FYI - pll_d2 is a Tegra3 feature, this way you can do things like run DSI and HDMI with low jitter, instead of trying to take over pll_c for HDMI, which deprives other modules of the clock freedom) I have more DSI devices on Tegra3 than on Tegra2, so I'm putting Tegra3 support near the top of my TODO list, because in my mind getting forward progress on DSI support depends on it. HDMI programming is not too tough, it's mostly loading calibrated magic values into some SOR registers. The values are different for T20 and T30 series (same fields, just needed different calibration). -- "If you give someone a program, you will frustrate them for a day; if you teach them how to program, you will frustrate them for a lifetime." — David Leinweber ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC v2 5/5] drm: Add NVIDIA Tegra support
On Wed, Apr 25, 2012 at 2:45 AM, Thierry Reding wrote: > This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It > currently has rudimentary GEM support and can run a console on the > framebuffer as well as X using the xf86-video-modesetting driver. Only > the RGB output is supported. > > HDMI support was taken from NVIDIA's Linux kernel tree but it doesn't > quite work. EDID data can be retrieved but the output doesn't properly > activate the connected TV. > > The DSI and TVO outputs and the HOST1X driver are just stubs that setup > the corresponding resources but don't do anything useful yet. > Nice work Thierry, I'm happy with the display programming bits. I think the pll_p enable/disable problem with RGB is better fixed when we have some fancier clock infrastructure. I have some ideas how it could be fixed now, but given that I want to see pll_p or pll_d / pll_d2 selected automatically based on final clock, I'd rather hold off on fixing things that are not really broken that going to be replaced anyways. You can save a little power by leaving D pll's off, and the logic to do it isn't hard and fairly safe to hard code into the driver instead of trying to represent it in DT. (FYI - pll_d2 is a Tegra3 feature, this way you can do things like run DSI and HDMI with low jitter, instead of trying to take over pll_c for HDMI, which deprives other modules of the clock freedom) I have more DSI devices on Tegra3 than on Tegra2, so I'm putting Tegra3 support near the top of my TODO list, because in my mind getting forward progress on DSI support depends on it. HDMI programming is not too tough, it's mostly loading calibrated magic values into some SOR registers. The values are different for T20 and T30 series (same fields, just needed different calibration). -- "If you give someone a program, you will frustrate them for a day; if you teach them how to program, you will frustrate them for a lifetime." ? David Leinweber
Re: [RFC 0/4] Add NVIDIA Tegra DRM support
On 04/19/2012 11:05 PM, Lucas Stach wrote: Am Freitag, den 20.04.2012, 07:02 +0200 schrieb Thierry Reding: *cut* Yes, I think we should go the route that Jerome Glisse pointed out: get in a basic KMS driver first and hide any accel ioctl under a staging config option. Everyone seems happy with that, I have no objections. *cut* I'm really interested how this turns out in the end. I hope we can get a somewhat stable cooperation between NVIDIA and the community, like AMD has established at the moment. Well, we always strive to be better than AMD ;-) But seriously, I don't think GPU technology would be where it is today if NVIDIA and AMD didn't compete aggressively. But our mobile division is more like TI, Qualcomm, Freescale and other SoC vendors. Upstreaming changes for arch/arm has so far been a different challenge than only updating drivers/gpu for x86. Mostly because there are so many aspects to SoCs compared to a driver for a graphics card or PC chipset. And it doesn't help that arch/arm is a real mess of wildly different SoCs, and it lacks the stability and maturity of the x86 code base. The state of ARM is every vendor's fault, every chip generation can be a complete resign of one or more subsystems. x86 tends to be layers of extensions and enhancements from 1-3 vendors. But even if Mobile doesn't consider AMD or Intel to be among the competition, we certainly watch and listen to good open source contributors and try to learn from their successes and mistakes. (and our own mistakes, we're not perfect!) What I'm trying to say is that Tegra's business needs are different, so you should not be surprised to see different behavior from us. There are a lot of difficult issues with open sourcing the work done by my desktop colleagues. But those barriers don't apply to Tegra. Different product, different market, different rules. (My vote is NVIDIA starts using this DRM in-house and builds new extensions on top of it, sharing patches on LKML when the hardware is released) That sounds like a good plan. Ideally you should even share the patches as soon as they're ready. That'll give viewers some head start and you can fix the code even before the hardware is released. One thing I would like to have is upstream first. We have seen much Tegra development in both the nv-linux and chromium trees, but those things are going upstream extremely slowly. I can understand that this strategy was beneficial for a fast bring up of the new Tegra hardware, but the DRM driver shouldn't be time critical and so everything should be done to meet upstream quality from the start. -- Lucas We have a team of interested engineers assigned to just that problem. Fractured trees are not efficient for us internally either. We want upstream to be our common repository for most changes. And internally we have been stricter about conforming to Linux kernel coding conventions in attempt to give ourselves less work when we get to upstreaming a change. I'll be happy when I can tell my customers they can just grab the latest Tegra updates from git.kernel.org Hopefully our efforts will begin to convince you. - Jon ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 0/4] Add NVIDIA Tegra DRM support
On 04/19/2012 11:05 PM, Lucas Stach wrote: > Am Freitag, den 20.04.2012, 07:02 +0200 schrieb Thierry Reding: *cut* > > Yes, I think we should go the route that Jerome Glisse pointed out: get > in a basic KMS driver first and hide any accel ioctl under a staging > config option. Everyone seems happy with that, I have no objections. *cut* > > I'm really interested how this turns out in the end. I hope we can get a > somewhat stable cooperation between NVIDIA and the community, like AMD > has established at the moment. Well, we always strive to be better than AMD ;-) But seriously, I don't think GPU technology would be where it is today if NVIDIA and AMD didn't compete aggressively. But our mobile division is more like TI, Qualcomm, Freescale and other SoC vendors. Upstreaming changes for arch/arm has so far been a different challenge than only updating drivers/gpu for x86. Mostly because there are so many aspects to SoCs compared to a driver for a graphics card or PC chipset. And it doesn't help that arch/arm is a real mess of wildly different SoCs, and it lacks the stability and maturity of the x86 code base. The state of ARM is every vendor's fault, every chip generation can be a complete resign of one or more subsystems. x86 tends to be layers of extensions and enhancements from 1-3 vendors. But even if Mobile doesn't consider AMD or Intel to be among the competition, we certainly watch and listen to good open source contributors and try to learn from their successes and mistakes. (and our own mistakes, we're not perfect!) What I'm trying to say is that Tegra's business needs are different, so you should not be surprised to see different behavior from us. There are a lot of difficult issues with open sourcing the work done by my desktop colleagues. But those barriers don't apply to Tegra. Different product, different market, different rules. >> >>> (My vote is NVIDIA starts using this DRM in-house and builds new >>> extensions on top of it, sharing patches on LKML when the hardware is >>> released) >> >> That sounds like a good plan. Ideally you should even share the patches as >> soon as they're ready. That'll give viewers some head start and you can fix >> the code even before the hardware is released. > > One thing I would like to have is upstream first. We have seen much > Tegra development in both the nv-linux and chromium trees, but those > things are going upstream extremely slowly. I can understand that this > strategy was beneficial for a fast bring up of the new Tegra hardware, > but the DRM driver shouldn't be time critical and so everything should > be done to meet upstream quality from the start. > > -- Lucas > > We have a team of interested engineers assigned to just that problem. Fractured trees are not efficient for us internally either. We want upstream to be our common repository for most changes. And internally we have been stricter about conforming to Linux kernel coding conventions in attempt to give ourselves less work when we get to upstreaming a change. I'll be happy when I can tell my customers they can just grab the latest Tegra updates from git.kernel.org Hopefully our efforts will begin to convince you. - Jon
Re: [RFC 0/4] Add NVIDIA Tegra DRM support
On 04/19/2012 01:40 PM, Thierry Reding wrote: * PGP Signed by an unknown key * Dave Airlie wrote: On Thu, Apr 19, 2012 at 6:35 PM, Thierry Reding wrote: Before posting the next round of patches I wanted to clarify whether we need to take the Tegra driver through staging. Lucas brought this up referring to previous experience with Nouveau where Linus used to complain that userspace ABI was broken for non-staging drivers. I don't know how relevant that is for Tegra. We could also keep it in drivers/gpu/drm and only add userspace interfaces that we are sure are not going to change. Currently there isn't isn't anything that could be easily broken as only some of the standard DRM interfaces are supported anyway. Alternatively we could keep the driver in a separate tree until it becomes mature enough. Any thoughts? It's probably okay to avoid staging if it doesn't add any userspace ioctls. A KMS driver that just supports the dumb ioctls so -modesetting works, would be the first thing to aim for I suppose, like how the exynos guys did it. Adding userspace interfaces is where you'll get into ABI guarantees etc, and these are normally required only for the accel engines. I think this is what Lucas was concerned about. The plan is to look at how much can be used from the Nouveau code and make it work on the Tegra. So would it be possible to get a basic dumb KMS driver into mainline (non-staging) and phase in acceleration later on, with ABI guarantees? I guess development can go on in separate trees until the ABI is stable and can subsequently be ported to the mainline driver. Is that an acceptable approach? Thierry That certainly seems like the most reasonable approach to me. Get KMS only in first. It's a useful driver as-is, and has the lowest barrier to entry into upstream. Then later we can phase in enhancements. We certainly have plenty of places internally and externally to hash out acceleration interfaces, and come to some consensus at at later date (either on linux-tegra or direct email). We have a lot of concerns here. What is best for X11, what is best for Android, how do we keep healthy open source implementations, and how does NVIDIA move forward with supporting new Tegra on an open source implementation. (My vote is NVIDIA starts using this DRM in-house and builds new extensions on top of it, sharing patches on LKML when the hardware is released) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 0/4] Add NVIDIA Tegra DRM support
On 04/19/2012 01:40 PM, Thierry Reding wrote: > * PGP Signed by an unknown key > > * Dave Airlie wrote: >> On Thu, Apr 19, 2012 at 6:35 PM, Thierry Reding >> wrote: >>> Before posting the next round of patches I wanted to clarify whether we need >>> to take the Tegra driver through staging. Lucas brought this up referring to >>> previous experience with Nouveau where Linus used to complain that userspace >>> ABI was broken for non-staging drivers. I don't know how relevant that is >>> for Tegra. We could also keep it in drivers/gpu/drm and only add userspace >>> interfaces that we are sure are not going to change. Currently there isn't >>> isn't anything that could be easily broken as only some of the standard DRM >>> interfaces are supported anyway. >>> >>> Alternatively we could keep the driver in a separate tree until it becomes >>> mature enough. >>> >>> Any thoughts? >> >> It's probably okay to avoid staging if it doesn't add any userspace ioctls. >> >> A KMS driver that just supports the dumb ioctls so -modesetting works, >> would be the first thing to aim for I suppose, like how the exynos >> guys did it. >> >> Adding userspace interfaces is where you'll get into ABI guarantees >> etc, and these are normally required only for the accel engines. > > I think this is what Lucas was concerned about. The plan is to look at how > much can be used from the Nouveau code and make it work on the Tegra. So > would it be possible to get a basic dumb KMS driver into mainline > (non-staging) and phase in acceleration later on, with ABI guarantees? I > guess development can go on in separate trees until the ABI is stable and can > subsequently be ported to the mainline driver. > > Is that an acceptable approach? > > Thierry > That certainly seems like the most reasonable approach to me. Get KMS only in first. It's a useful driver as-is, and has the lowest barrier to entry into upstream. Then later we can phase in enhancements. We certainly have plenty of places internally and externally to hash out acceleration interfaces, and come to some consensus at at later date (either on linux-tegra or direct email). We have a lot of concerns here. What is best for X11, what is best for Android, how do we keep healthy open source implementations, and how does NVIDIA move forward with supporting new Tegra on an open source implementation. (My vote is NVIDIA starts using this DRM in-house and builds new extensions on top of it, sharing patches on LKML when the hardware is released)
Re: [RFC 4/4] drm: Add NVIDIA Tegra support
On 04/11/2012 05:48 AM, Daniel Vetter wrote: On Wed, Apr 11, 2012 at 02:10:30PM +0200, Thierry Reding wrote: This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It currently has rudimentary GEM support and can run a console on the framebuffer as well as X using the xf86-video-modesetting driver. Only the RGB output is supported. Quite a lot of things still need to be worked out and there is a lot of room for cleanup. Indeed, after a quick look there are tons of functions that are just stubs ;-) One thing I wonder though is why you directly use the iommu api and not wrap it up into dma_map? Is arm infrastructure just not there yet or do you plan to tightly integrate the tegra drm with the iommu (e.g. for process space switching or similarly funky stuff)? Yours, Daniel An abstraction to access iommu is preferable since Tegra2 and Tegra3 are quite a bit different when it comes to iommu. But so far I'm pretty pleased with the driver Thierry has done. I'm hoping we can start using this as a base instead of the fb based driver that never made it upstream. (even if I have had a bit of a debate about GEM v. TTM v. ION v. ...) - Jon ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 4/4] drm: Add NVIDIA Tegra support
On 04/11/2012 05:48 AM, Daniel Vetter wrote: > On Wed, Apr 11, 2012 at 02:10:30PM +0200, Thierry Reding wrote: >> This commit adds a very basic DRM driver for NVIDIA Tegra SoCs. It >> currently has rudimentary GEM support and can run a console on the >> framebuffer as well as X using the xf86-video-modesetting driver. >> Only the RGB output is supported. Quite a lot of things still need >> to be worked out and there is a lot of room for cleanup. > > Indeed, after a quick look there are tons of functions that are just stubs > ;-) One thing I wonder though is why you directly use the iommu api and > not wrap it up into dma_map? Is arm infrastructure just not there yet or > do you plan to tightly integrate the tegra drm with the iommu (e.g. for > process space switching or similarly funky stuff)? > > Yours, Daniel An abstraction to access iommu is preferable since Tegra2 and Tegra3 are quite a bit different when it comes to iommu. But so far I'm pretty pleased with the driver Thierry has done. I'm hoping we can start using this as a base instead of the fb based driver that never made it upstream. (even if I have had a bit of a debate about GEM v. TTM v. ION v. ...) - Jon
