Re: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver
And another issue... What is drm_crtc_helper_set_mode() passed as the fb argument? Is it the old fb, or the new fb? bool drm_crtc_helper_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, int x, int y, struct drm_framebuffer *old_fb) ... int drm_crtc_helper_set_config(struct drm_mode_set *set) { ... save_set.fb = set->crtc->fb; ... old_fb = set->crtc->fb; set->crtc->fb = set->fb; if (!drm_crtc_helper_set_mode(set->crtc, set->mode, set->x, set->y, old_fb)) { ... /* Try to restore the config */ if (mode_changed && !drm_crtc_helper_set_mode(save_set.crtc, save_set.mode, save_set.x, save_set.y, save_set.fb)) } ... int drm_helper_resume_force_mode(struct drm_device *dev) { ... ret = drm_crtc_helper_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->fb); The function prototype implies it's the old fb, as does the first use. The second and third uses of it clearly show it being the fb which we wish to be displayed. The deeper I look, the more bugs there seem to be in this DRM stuff, and I'm continuing to look because I'm chasing a framebuffer refcount bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver
On Thu, Jun 13, 2013 at 12:19:03PM +0100, Russell King - ARM Linux wrote: > The deeper I look, the more bugs there seem to be in this DRM stuff, > and I'm continuing to look because I'm chasing a framebuffer refcount > bug. So, this refcount bug - I think I've just found it. This is the flow of references to the new fb on mode set: drm_mode_setcrtc(): fb = drm_framebuffer_lookup(dev, crtc_req->fb_id); set.fb = fb; ret = drm_mode_set_config_internal(&set); drm_mode_set_config_internal(): fb = set->fb; ret = crtc->funcs->set_config(set); drm_crtc_helper_set_config(): old_fb = set->crtc->fb; set->crtc->fb = set->fb; if (!drm_crtc_helper_set_mode(set->crtc, set->mode, set->x, set->y, old_fb)) { drm_helper_disable_unused_functions(dev); drm_helper_disable_unused_functions(): list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { crtc->enabled = drm_helper_crtc_in_use(crtc); if (!crtc->enabled) { crtc->fb = NULL; } } back to drm_mode_set_config_internal(): if (ret == 0) { if (fb) drm_framebuffer_reference(fb); back to drm_mode_setcrtc(): if (fb) drm_framebuffer_unreference(fb); Assuming success all the way through, what happens when a CRTC is unused is: 1. We obtain a reference in drm_mode_setcrtc() via the lookup. 2. We set the mode 3. In trying to set the mode, we discover that all connectors for the CRTC are in the disconnected state, and so we disable the CRTC 4. We set crtc->fb to NULL 5. back in drm_mode_set_config_internal(), we take a reference on the framebuffer irrespective of this. 6. back in drm_mode_setcrtc(), we drop the original reference caused by the lookup. We now have a framebuffer with a reference count incremented by one but no actual reference to it - the CRTC's reference is completely lost by the action of drm_helper_disable_unused_functions(). You could argue that it's something the driver should deal with - fine, but what if it only implements the DPMS method? Should it drop a reference to the framebuffer when DPMS instructs it to turn off? Surely not, because that means when DPMS turns stuff back on you're missing a refcount. Are drivers required to implement a disable function and cater for the imbalance in the upper layers of code? If so, this is not a clean design. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control
Hi, On Thursday 13 June 2013 04:51 PM, Inki Dae wrote: -Original Message- From: Sylwester Nawrocki [mailto:s.nawro...@samsung.com] Sent: Thursday, June 13, 2013 5:56 PM To: Rahul Sharma Cc: Rahul Sharma; Inki Dae; linux-samsung-...@vger.kernel.org; devicetree- disc...@lists.ozlabs.org; DRI mailing list; Kukjin Kim; Seung-Woo Kim; Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren; grant.lik...@linaro.org Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control Hi, On 06/13/2013 06:26 AM, Rahul Sharma wrote: Mr. Dae, Thanks for your valuable inputs. I posted it as RFC because, I also have received comments to register hdmiphy as a clock controller. As we always configure it for specific frequency, hdmi-phy looks similar to a PLL. But it really doesn't belong to that class. Secondly prior to exynos5420, it was a i2c device. I am not sure we can register a I2C device as a clock controller. I wanted to discuss and explore this option here. Have you considered using the generic PHY framework for those HDMI PHY devices [1] ? I guess we could add a dedicated group of ops for video PHYs, similarly as is is done with struct v4l2_subdev_ops. For configuring things like the carrier/pixel clock frequency or anything what's common across the video PHYs. Perhaps you could have a look and see if this framework would be useful for HDMI and possibly point out anything what might be missing ? I'm not sure it it really solves the issues specific to the Exynos HDMI but at least with a generic PHY driver the PHY module would be separate from the PHY controller, as often same HDMI DPHY can be used with various types of a HDMI controller. So this would allow to not duplicate the HDMI PHY drivers in the long-term perspective. Yeah, at least, it seems that we could use PHY module to control PMU register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off callbacks. As you may know, HDMIPHY needs i2c interfaces to control HDMIPHY clock. So with PHY module, HDMIPHY driver could enable PMU more generically, but also has to use existing i2c stuff to control HDMIPHY clock. I had a quick review to Generic PHY Framework[v6] but I didn't see that the PHY module could generically support more features such as i2c stuff. I don't think PHY framework needs to provide i2c interfaces to program certain configurations. Instead in one of the callbacks (init/on/off) PHY driver can program whatever it wants using any of the interfaces it needs. IMO PHY framework should work independent of the interfaces. For example, twl4030 phy driver actually uses i2c to program its registers but still it uses the PHY framework [1]. [1] --> http://www.gossamer-threads.com/lists/linux/kernel/1729414 Thanks Kishon ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver
On Thu, Jun 13, 2013 at 12:50:16PM +0100, Russell King - ARM Linux wrote: > On Thu, Jun 13, 2013 at 12:19:03PM +0100, Russell King - ARM Linux wrote: > > The deeper I look, the more bugs there seem to be in this DRM stuff, > > and I'm continuing to look because I'm chasing a framebuffer refcount > > bug. > > So, this refcount bug - I think I've just found it. This is the flow of > references to the new fb on mode set: > > drm_mode_setcrtc(): > fb = drm_framebuffer_lookup(dev, crtc_req->fb_id); > set.fb = fb; > ret = drm_mode_set_config_internal(&set); > drm_mode_set_config_internal(): > fb = set->fb; > ret = crtc->funcs->set_config(set); > drm_crtc_helper_set_config(): > old_fb = set->crtc->fb; > set->crtc->fb = set->fb; > if (!drm_crtc_helper_set_mode(set->crtc, set->mode, > set->x, set->y, > old_fb)) { > drm_helper_disable_unused_functions(dev); > drm_helper_disable_unused_functions(): > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > crtc->enabled = drm_helper_crtc_in_use(crtc); > if (!crtc->enabled) { > crtc->fb = NULL; > } > } > back to drm_mode_set_config_internal(): > if (ret == 0) { > if (fb) > drm_framebuffer_reference(fb); > back to drm_mode_setcrtc(): > if (fb) > drm_framebuffer_unreference(fb); > > Assuming success all the way through, what happens when a CRTC is unused > is: > > 1. We obtain a reference in drm_mode_setcrtc() via the lookup. > 2. We set the mode > 3. In trying to set the mode, we discover that all connectors for the CRTC >are in the disconnected state, and so we disable the CRTC > 4. We set crtc->fb to NULL > 5. back in drm_mode_set_config_internal(), we take a reference on the >framebuffer irrespective of this. > 6. back in drm_mode_setcrtc(), we drop the original reference caused by >the lookup. > > We now have a framebuffer with a reference count incremented by one but > no actual reference to it - the CRTC's reference is completely lost by > the action of drm_helper_disable_unused_functions(). > > You could argue that it's something the driver should deal with - fine, > but what if it only implements the DPMS method? Should it drop a > reference to the framebuffer when DPMS instructs it to turn off? Surely > not, because that means when DPMS turns stuff back on you're missing a > refcount. > > Are drivers required to implement a disable function and cater for the > imbalance in the upper layers of code? If so, this is not a clean > design. There's a bigger issue here - if it's possible for drm_crtc_helper_set_config() to be called with set->fb set but set->mode NULL, then we overwrite set->fb to NULL. Again, that results in a lost reference. For the time being, I'm using this patch, which solves my dropped refcount problem, and marks the other possible dropped reference. Either that check needs to be removed or it needs to properly drop the refcount on the fb before 'losing' the reference to it. diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index dd64a06..774d7a6 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2014,13 +2014,16 @@ int drm_mode_set_config_internal(struct drm_mode_set *set) old_fb = crtc->fb; fb = set->fb; + if (fb) + drm_framebuffer_reference(fb); ret = crtc->funcs->set_config(set); if (ret == 0) { if (old_fb) drm_framebuffer_unreference(old_fb); + } else { if (fb) - drm_framebuffer_reference(fb); + drm_framebuffer_unreference(fb); } return ret; diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 7b2d378..0d18fb2 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -299,6 +299,8 @@ void drm_helper_disable_unused_functions(struct drm_device *dev) (*crtc_funcs->disable)(crtc); else (*crtc_funcs->dpms)(crtc, DRM_MODE_DPMS_OFF); + if (crtc->fb) + drm_framebuffer_unreference(crtc->fb); crtc->fb = NULL; } } @@ -573,6 +575,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) crtc_funcs = set->crtc->helper_private; + /* FIXME: this loses a refcount on the fb */ if (!set->mode) set->fb = NULL; ___ dri-devel mailing list dri-devel@lists.freed
Re: [RFC PATCH] dmabuf-sync: Introduce buffer synchronization framework
On Thu, Jun 13, 2013 at 05:28:08PM +0900, Inki Dae wrote: > This patch adds a buffer synchronization framework based on DMA BUF[1] > and reservation[2] to use dma-buf resource, and based on ww-mutexes[3] > for lock mechanism. > > The purpose of this framework is not only to couple cache operations, > and buffer access control to CPU and DMA but also to provide easy-to-use > interfaces for device drivers and potentially user application > (not implemented for user applications, yet). And this framework can be > used for all dma devices using system memory as dma buffer, especially > for most ARM based SoCs. > > The mechanism of this framework has the following steps, > 1. Register dmabufs to a sync object - A task gets a new sync object and > can add one or more dmabufs that the task wants to access. > This registering should be performed when a device context or an event > context such as a page flip event is created or before CPU accesses a > shared > buffer. > > dma_buf_sync_get(a sync object, a dmabuf); > > 2. Lock a sync object - A task tries to lock all dmabufs added in its own > sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid dead > lock issue and for race condition between CPU and CPU, CPU and DMA, and > DMA > and DMA. Taking a lock means that others cannot access all locked dmabufs > until the task that locked the corresponding dmabufs, unlocks all the > locked > dmabufs. > This locking should be performed before DMA or CPU accesses these dmabufs. > > dma_buf_sync_lock(a sync object); > > 3. Unlock a sync object - The task unlocks all dmabufs added in its own > sync > object. The unlock means that the DMA or CPU accesses to the dmabufs have > been completed so that others may access them. > This unlocking should be performed after DMA or CPU has completed accesses > to the dmabufs. > > dma_buf_sync_unlock(a sync object); > > 4. Unregister one or all dmabufs from a sync object - A task unregisters > the given dmabufs from the sync object. This means that the task dosen't > want to lock the dmabufs. > The unregistering should be performed after DMA or CPU has completed > accesses to the dmabufs or when dma_buf_sync_lock() is failed. > > dma_buf_sync_put(a sync object, a dmabuf); > dma_buf_sync_put_all(a sync object); > > The described steps may be summarized as: > get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put > > This framework includes the following two features. > 1. read (shared) and write (exclusive) locks - A task is required to > declare > the access type when the task tries to register a dmabuf; > READ, WRITE, READ DMA, or WRITE DMA. > > The below is example codes, > struct dmabuf_sync *sync; > > sync = dmabuf_sync_init(NULL, "test sync"); > > dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ); > ... > > And the below can be used as access types: > DMA_BUF_ACCESS_READ, > - CPU will access a buffer for read. > DMA_BUF_ACCESS_WRITE, > - CPU will access a buffer for read or write. > DMA_BUF_ACCESS_READ | DMA_BUF_ACCESS_DMA, > - DMA will access a buffer for read > DMA_BUF_ACCESS_WRITE | DMA_BUF_ACCESS_DMA, > - DMA will access a buffer for read or write. > > 2. Mandatory resource releasing - a task cannot hold a lock indefinitely. > A task may never try to unlock a buffer after taking a lock to the buffer. > In this case, a timer handler to the corresponding sync object is called > in five (default) seconds and then the timed-out buffer is unlocked by > work > queue handler to avoid lockups and to enforce resources of the buffer. > > The below is how to use: > 1. Allocate and Initialize a sync object: > struct dmabuf_sync *sync; > > sync = dmabuf_sync_init(NULL, "test sync"); > ... > > 2. Add a dmabuf to the sync object when setting up dma buffer relevant > registers: > dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ); > ... > > 3. Lock all dmabufs of the sync object before DMA or CPU accesses > the dmabufs: > dmabuf_sync_lock(sync); > ... > > 4. Now CPU or DMA can access all dmabufs locked in step 3. > > 5. Unlock all dmabufs added in a sync object after DMA or CPU access > to these dmabufs is completed: > dmabuf_sync_unlock(sync); > > And call the following functions to release all resources, > dmabuf_sync_put_all(sync); > dmabuf_sync_fini(sync); > > You can refer to actual example codes: > > https://git.kernel.org/cgit/linux/kernel/git/daeinki/drm-exynos.git/ > > commit/?h=dmabuf-sync&id=
Re: [PATCH] radeon: Fix a false positive lockup after 10s of inactivity
On Wed, Jun 12, 2013 at 6:56 AM, Jerome Glisse wrote: > On Wed, Jun 12, 2013 at 6:26 AM, Michel Dänzer wrote: >> On Die, 2013-06-11 at 16:23 -0700, Andy Lutomirski wrote: >>> If the device is idle for over ten seconds, then the next attempt to do >>> anything can race with the lockup detector and cause a bogus lockup >>> to be detected. >>> >>> Oddly, the situation is well-described in the lockup detector's comments >>> and a fix is even described. This patch implements that fix (and corrects >>> some typos in the description). >>> >>> My system has been stable for about a week running this code. Without this, >>> my screen would go blank every now and then and, when it came back, >>> everything >>> would be remarkably slow (the latter is a separate bug). >>> >>> Signed-off-by: Andy Lutomirski >> >> [...] >> >>> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c >>> b/drivers/gpu/drm/radeon/radeon_ring.c >>> index 1ef5eaa..fb7b3ea 100644 >>> --- a/drivers/gpu/drm/radeon/radeon_ring.c >>> +++ b/drivers/gpu/drm/radeon/radeon_ring.c >>> @@ -547,12 +547,12 @@ void radeon_ring_lockup_update(struct radeon_ring >>> *ring) >>> * have CP rptr to a different value of jiffies wrap around which will >>> force >>> * initialization of the lockup tracking informations. >>> * >>> - * A possible false positivie is if we get call after while and >>> last_cp_rptr == >>> - * the current CP rptr, even if it's unlikely it might happen. To avoid >>> this >>> - * if the elapsed time since last call is bigger than 2 second than we >>> return >>> - * false and update the tracking information. Due to this the caller must >>> call >>> - * radeon_ring_test_lockup several time in less than 2sec for lockup to be >>> reported >>> - * the fencing code should be cautious about that. >>> + * A possible false positive is if we get called after a while and >>> + * last_cp_rptr == the current CP rptr, even if it's unlikely it might >>> + * happen. To avoid this if the elapsed time since the last call is bigger >>> + * than 2 second then we return false and update the tracking >>> + * information. Due to this the caller must call radeon_ring_test_lockup >>> + * more frequently than once every 2s when waiting. >> >> Is it guaranteed that radeon_ring_test_lockup will be called more often >> than every 2s when waiting? If not, this change might prevent a real >> lockup from being detected? > > Yes it will if you wait for a fence, because the fence timeout wait is > way smaller than 2sec so radeon_ring_is_lockup get call several time, > which call radeon_ring_force_activity and then > radeon_ring_test_lockup. > > This also means it very very very unlikely (see below for the likely > case) to have a wrap around that give last rptr same as current one. > > The likely case is when you have something like a long compute, then > nothing is lockup but you keep filling ring with > radeon_ring_force_activity but the cp is still stuck on the ib of the > compute stuff so rptr does not progress. > >> Either way, I wonder if there might not be a simpler solution to the >> problem, e.g. by updating last_activity when submitting commands to a >> previously empty ring. > > Maybe but i still don't think it should matter. > > Andy can you test (without your patch) and see if it helps with your issue : > http://people.freedesktop.org/~glisse/0001-drm-radeon-update-lockup-tracking-when-scheduling-in.patch Testing now. I'll report back in a couple of days. I don't think that long computes have anything to do with it. The bogus lockups happen when I look away from my computer for a while and then click something. I thing the graphics are usually completely idle when this happens. AFAIK I've never run an OpenCL or similar application on this system. --Andy ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control
Hello Kishon, On 2013년 06월 13일 21:54, Kishon Vijay Abraham I wrote: > Hi, > > On Thursday 13 June 2013 04:51 PM, Inki Dae wrote: >> >> >>> -Original Message- >>> From: Sylwester Nawrocki [mailto:s.nawro...@samsung.com] >>> Sent: Thursday, June 13, 2013 5:56 PM >>> To: Rahul Sharma >>> Cc: Rahul Sharma; Inki Dae; linux-samsung-...@vger.kernel.org; >>> devicetree- >>> disc...@lists.ozlabs.org; DRI mailing list; Kukjin Kim; Seung-Woo Kim; >>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren; >>> grant.lik...@linaro.org >>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with >>> pmu reg control >>> >>> Hi, >>> >>> On 06/13/2013 06:26 AM, Rahul Sharma wrote: Mr. Dae, Thanks for your valuable inputs. I posted it as RFC because, I also have received comments to register hdmiphy as a clock controller. As we always configure it for specific frequency, hdmi-phy looks similar to a PLL. But it really doesn't belong to that class. Secondly prior to exynos5420, it was a i2c device. I am not sure we can register a I2C device as a clock controller. I wanted to discuss and explore this option here. >>> >>> Have you considered using the generic PHY framework for those HDMI >>> PHY devices [1] ? I guess we could add a dedicated group of ops for >>> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For >>> configuring things like the carrier/pixel clock frequency or anything >>> what's common across the video PHYs. >>> >>> Perhaps you could have a look and see if this framework would be >>> useful for HDMI and possibly point out anything what might be missing ? >>> >>> I'm not sure it it really solves the issues specific to the Exynos >>> HDMI but at least with a generic PHY driver the PHY module would be >>> separate from the PHY controller, as often same HDMI DPHY can be used >>> with various types of a HDMI controller. So this would allow to not >>> duplicate the HDMI PHY drivers in the long-term perspective. >> >> Yeah, at least, it seems that we could use PHY module to control PMU >> register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off >> callbacks. As you may know, HDMIPHY needs i2c interfaces to control >> HDMIPHY >> clock. So with PHY module, HDMIPHY driver could enable PMU more >> generically, >> but also has to use existing i2c stuff to control HDMIPHY clock. I had a >> quick review to Generic PHY Framework[v6] but I didn't see that the PHY >> module could generically support more features such as i2c stuff. > > I don't think PHY framework needs to provide i2c interfaces to program > certain configurations. Instead in one of the callbacks (init/on/off) > PHY driver can program whatever it wants using any of the interfaces it > needs. IMO PHY framework should work independent of the interfaces. In exnoys hdmi case, i2c interface is not the exact issue. In exynos hdmi, hdmiphy should send i2c configuration about video clock information as the video mode information including resolution, bit per pixel, refresh rate passed from drm subsystem. So init/on/off callbacks of phy framework are not enough for exynos hdmiphy and it should have a callback to set video mode. Do you have plan to add driver specific extend callback pointers to phy framework? Currently, hdmi directly calls phy operations, but Rahul's another patch set, mentioned by Inki, divides hdmi and hdmiphy and hdmi and hdmiphy is connected with exynos hdmi own sub driver callback operations. IMHO, if phy framework can support extend callback feature, then this own sub driver callbacks can be replaced with phy framework at long term view. Thanks and Regards, - Seung-Woo Kim > > For example, twl4030 phy driver actually uses i2c to program its > registers but still it uses the PHY framework [1]. > > [1] --> http://www.gossamer-threads.com/lists/linux/kernel/1729414 > > Thanks > Kishon > -- > To unsubscribe from this list: send the line "unsubscribe > linux-samsung-soc" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Seung-Woo Kim Samsung Software R&D Center -- ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/9] drm/exynos: use SoC name to identify hdmi version
Hello Rahul, On 2013년 06월 11일 23:11, Rahul Sharma wrote: > Exynos hdmi IP version is named after hdmi specification version i.e. > 1.3 and 1.4. This versioning mechanism is not sufficient to handle > the diversity in the hdmi/phy IPs which are present across the exynos > SoC family. > > This patch changes the hdmi version to the name of the SoC in which > the IP was introduced for the first time. Same version is applicable > to all subsequent SoCs having the same IP version. > > Exynos4210 has 1.3 HDMI, i2c mapped phy with configuration set. > Exynos5250 has 1.4 HDMI, i2c mapped phy with configuration set. > Exynos5420 has 1.4 HDMI, Platform Bus mapped phy with configuration set. > > Based on the HDMI IP version we cannot decide to pick Exynos5250 phy conf > and use i2c for data transfer or Exynos5420 phy confs and platform bus > calls for communication. Considering your other patch to divide hdmi and hdmiphy, how do you think using hdmiphy version parsed from hdmiphy dt binding from phy code instead of using hdmi version for both hdmi and hdmiphy? If that, this SoC identifying hdmi version is not necessary because there is no change at least in hdmi side. And IMO, it seems easy to merge hdmiphy related patch first before merging patch for exynos5420. > > Signed-off-by: Rahul Sharma > --- > drivers/gpu/drm/exynos/exynos_hdmi.c | 249 > +- > drivers/gpu/drm/exynos/regs-hdmi.h | 78 +-- > 2 files changed, 164 insertions(+), 163 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > b/drivers/gpu/drm/exynos/exynos_hdmi.c > index 75a6bf3..9384ffc 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -73,9 +73,9 @@ enum HDMI_PACKET_TYPE { > HDMI_PACKET_TYPE_AUI = HDMI_PACKET_TYPE_INFOFRAME + 4 > }; > > -enum hdmi_type { > - HDMI_TYPE13, > - HDMI_TYPE14, > +enum hdmi_version { > + HDMI_VER_EXYNOS4210, > + HDMI_VER_EXYNOS4212, > }; -- Seung-Woo Kim Samsung Software R&D Center -- ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/9] drm/exynos: add support for exynos5420 mixer
Hello Rahul, This patch looks good to me just with mixer part of 2nd patch because there is no hdmiphy related things. On 2013년 06월 11일 23:11, Rahul Sharma wrote: > Add support for exynos5420 mixer IP in the drm mixer driver. > > Signed-off-by: Rahul Sharma > --- > drivers/gpu/drm/exynos/exynos_mixer.c | 49 > + > drivers/gpu/drm/exynos/regs-mixer.h |7 + > 2 files changed, 44 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c > b/drivers/gpu/drm/exynos/exynos_mixer.c > index 58dfd3f..101d5bb 100644 > --- a/drivers/gpu/drm/exynos/exynos_mixer.c > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c > @@ -78,6 +78,7 @@ struct mixer_resources { > enum mixer_version_id { > MXR_VER_0_0_0_16, > MXR_VER_16_0_33_0, > + MXR_VER_128_0_0_184, > }; > > struct mixer_context { > @@ -283,17 +284,19 @@ static void mixer_cfg_scan(struct mixer_context *ctx, > unsigned int height) > val = (ctx->interlace ? MXR_CFG_SCAN_INTERLACE : > MXR_CFG_SCAN_PROGRASSIVE); > > - /* choosing between porper HD and SD mode */ > - if (height <= 480) > - val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD; > - else if (height <= 576) > - val |= MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD; > - else if (height <= 720) > - val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD; > - else if (height <= 1080) > - val |= MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD; > - else > - val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD; > + if (ctx->mxr_ver != MXR_VER_128_0_0_184) { > + /* choosing between proper HD and SD mode */ > + if (height <= 480) > + val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD; > + else if (height <= 576) > + val |= MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD; > + else if (height <= 720) > + val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD; > + else if (height <= 1080) > + val |= MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD; > + else > + val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD; > + } > > mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_SCAN_MASK); > } > @@ -557,6 +560,14 @@ static void mixer_graph_buffer(struct mixer_context > *ctx, int win) > /* setup geometry */ > mixer_reg_write(res, MXR_GRAPHIC_SPAN(win), win_data->fb_width); > > + /* setup display size */ > + if (ctx->mxr_ver == MXR_VER_128_0_0_184 && > + win == MIXER_DEFAULT_WIN) { > + val = MXR_MXR_RES_HEIGHT(win_data->fb_height); > + val |= MXR_MXR_RES_WIDTH(win_data->fb_width); > + mixer_reg_write(res, MXR_RESOLUTION, val); > + } > + > val = MXR_GRP_WH_WIDTH(win_data->crtc_width); > val |= MXR_GRP_WH_HEIGHT(win_data->crtc_height); > val |= MXR_GRP_WH_H_SCALE(x_ratio); > @@ -581,7 +592,8 @@ static void mixer_graph_buffer(struct mixer_context *ctx, > int win) > mixer_cfg_layer(ctx, win, true); > > /* layer update mandatory for mixer 16.0.33.0 */ > - if (ctx->mxr_ver == MXR_VER_16_0_33_0) > + if (ctx->mxr_ver == MXR_VER_16_0_33_0 || > + ctx->mxr_ver == MXR_VER_128_0_0_184) > mixer_layer_update(ctx); > > mixer_run(ctx); > @@ -822,6 +834,7 @@ static void mixer_win_disable(void *ctx, int win) > > static int mixer_check_mode(void *ctx, struct drm_display_mode *mode) > { > + struct mixer_context *mixer_ctx = ctx; > u32 w, h; > > w = mode->hdisplay; > @@ -831,6 +844,10 @@ static int mixer_check_mode(void *ctx, struct > drm_display_mode *mode) > mode->hdisplay, mode->vdisplay, mode->vrefresh, > (mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0); > > + if (mixer_ctx->mxr_ver == MXR_VER_0_0_0_16 || > + mixer_ctx->mxr_ver == MXR_VER_128_0_0_184) > + return 0; > + > if ((w >= 464 && w <= 720 && h >= 261 && h <= 576) || > (w >= 1024 && w <= 1280 && h >= 576 && h <= 720) || > (w >= 1664 && w <= 1920 && h >= 936 && h <= 1080)) > @@ -1127,6 +1144,11 @@ static int vp_resources_init(struct > exynos_drm_hdmi_context *ctx, > return 0; > } > > +static struct mixer_drv_data exynos5420_mxr_drv_data = { > + .version = MXR_VER_128_0_0_184, > + .is_vp_enabled = 0, > +}; > + > static struct mixer_drv_data exynos5250_mxr_drv_data = { > .version = MXR_VER_16_0_33_0, > .is_vp_enabled = 0, > @@ -1151,6 +1173,9 @@ static struct platform_device_id mixer_driver_types[] = > { > > static struct of_device_id mixer_match_types[] = { > { > + .compatible = "samsung,exynos5420-mixer", > + .data = &exynos5420_mxr_drv_data, > + }, { > .compatible = "samsung,exynos5250-mixer", > .data = &exynos5250_mxr_drv_data,
Re: [PATCH 7/9] drm/exynos: use of_get_named_gpio to get hdmi hpd gpio
Hello Rahul, this patch is not related with others and it looks good to me. On 2013년 06월 11일 23:11, Rahul Sharma wrote: > Cleanup by removing flags variable from drm_hdmi_dt_parse_pdata > which is not used anywhere. Swtiching to of_get_named_gpio instead > of of_get_named_gpio_flags solved this. > > Signed-off-by: Rahul Sharma Acked-by: Seung-Woo Kim > --- > drivers/gpu/drm/exynos/exynos_hdmi.c |3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > b/drivers/gpu/drm/exynos/exynos_hdmi.c > index 1eb5ffb..fc6a9b0 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -2081,7 +2081,6 @@ static struct s5p_hdmi_platform_data > *drm_hdmi_dt_parse_pdata > { > struct device_node *np = dev->of_node; > struct s5p_hdmi_platform_data *pd; > - enum of_gpio_flags flags; > u32 value; > > pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); > @@ -2095,7 +2094,7 @@ static struct s5p_hdmi_platform_data > *drm_hdmi_dt_parse_pdata > goto err_data; > } > > - pd->hpd_gpio = of_get_named_gpio_flags(np, "hpd-gpio", 0, &flags); > + pd->hpd_gpio = of_get_named_gpio(np, "hpd-gpio", 0); > > return pd; > > -- Seung-Woo Kim Samsung Software R&D Center -- ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 64257] RS880 issues with r600-llvm-compiler
https://bugs.freedesktop.org/show_bug.cgi?id=64257 --- Comment #64 from Marc Dietrich --- patch in comment 60 does not fix the issues (no change). -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65438] GTK apps crash X11 since last update of LLVMM
https://bugs.freedesktop.org/show_bug.cgi?id=65438 Michel Dänzer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #12 from Michel Dänzer --- Tom committed his fix as revision 183937. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65730] no sound via HDMI for Radeon 6850 mesa driver
https://bugs.freedesktop.org/show_bug.cgi?id=65730 --- Comment #5 from Andy Furniss --- (In reply to comment #4) > Before I being, I must make a disclaimer that I am not a Linux genius by any > means, but a moderate user, able and willing to find my way around things. > I do occasionally assist one of the pcsx2 emulator devs with me applying and > testing various patches for the code. Sometimes I need help, though, > finding the correct file to patch, as I am not a coder. Anyways, back to > the topic at hand. > > I'm assuming the file in question should be r600_hdmi.c? > > I have checked using catfish file searcher and also manually for this file. > This includes the > /lib/modules/3.8.0-23-generic/kernel/drivers/gpu/drm/radeon directory. All > I have is radeon.ko in that directory. In other words, I don't see any > trace of where r600_hdmi.c is or should be anywhere in my file system. > > Again I apologize for the inconvenenience. I am not a dev and don't know ubuntu or any distro really. You would need to be building your own kernels and so have the kernel source tree to find hdmi.c. One thing that puzzles me is the use of grub command line to pass module parameters - I thought that would be if radeon was built into kernel, but as you show you are using a module. If you search for how to set module parameters in ubuntu you may have more luck - it will involve putting option radeon audio=1 somewhere and regenerating something ... -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/exynos: make sure to handle an error case to vm_mmap call
vm_mmap function returns unsigned long so addr type should be unsigned long. a pointer or address variable is required to use unsigned long or uint64_t type for 64bits address support. So this patch makes sure that addr has unsigned long type and also exynos_drm_gem_mmap_ioctl returns correct error type. Signed-off-by: Inki Dae Signed-off-by: Kyungmin Park --- drivers/gpu/drm/exynos/exynos_drm_gem.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index 5af1478..c3f15e7 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -420,7 +420,7 @@ int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data, { struct drm_exynos_gem_mmap *args = data; struct drm_gem_object *obj; - unsigned int addr; + unsigned long addr; if (!(dev->driver->driver_features & DRIVER_GEM)) { DRM_ERROR("does not support GEM.\n"); @@ -462,14 +462,14 @@ int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data, drm_gem_object_unreference(obj); - if (IS_ERR((void *)addr)) { + if (IS_ERR_VALUE(addr)) { /* check filp->f_op, filp->private_data are restored */ if (file_priv->filp->f_op == &exynos_drm_gem_fops) { file_priv->filp->f_op = fops_get(dev->driver->fops); file_priv->filp->private_data = file_priv; } mutex_unlock(&dev->struct_mutex); - return PTR_ERR((void *)addr); + return (int)addr; } mutex_unlock(&dev->struct_mutex); -- 1.7.5.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65730] no sound via HDMI for Radeon 6850 mesa driver
https://bugs.freedesktop.org/show_bug.cgi?id=65730 --- Comment #6 from Andy Furniss --- (In reply to comment #5) > option radeon audio=1 Oops, should be options radeon audio=1 -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: shmobile: Use devm_* managed functions
Hi Dave, Could you please take this patch in your tree for v3.11 ? On Thursday 25 April 2013 12:14:54 Laurent Pinchart wrote: > This simplifies cleanup paths and fixes a probe time crash in the error > path when trying to cleanup mode setting before it was initialized. > > Signed-off-by: Laurent Pinchart > --- > drivers/gpu/drm/shmobile/shmob_drm_drv.c | 28 +-- > drivers/gpu/drm/shmobile/shmob_drm_plane.c | 7 +-- > 2 files changed, 10 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_drv.c > b/drivers/gpu/drm/shmobile/shmob_drm_drv.c index f6e0b53..29d15e3 100644 > --- a/drivers/gpu/drm/shmobile/shmob_drm_drv.c > +++ b/drivers/gpu/drm/shmobile/shmob_drm_drv.c > @@ -90,7 +90,7 @@ static int shmob_drm_setup_clocks(struct shmob_drm_device > *sdev, return -EINVAL; > } > > - clk = clk_get(sdev->dev, clkname); > + clk = devm_clk_get(sdev->dev, clkname); > if (IS_ERR(clk)) { > dev_err(sdev->dev, "cannot get dot clock %s\n", clkname); > return PTR_ERR(clk); > @@ -106,21 +106,12 @@ static int shmob_drm_setup_clocks(struct > shmob_drm_device *sdev, > > static int shmob_drm_unload(struct drm_device *dev) > { > - struct shmob_drm_device *sdev = dev->dev_private; > - > drm_kms_helper_poll_fini(dev); > drm_mode_config_cleanup(dev); > drm_vblank_cleanup(dev); > drm_irq_uninstall(dev); > > - if (sdev->clock) > - clk_put(sdev->clock); > - > - if (sdev->mmio) > - iounmap(sdev->mmio); > - > dev->dev_private = NULL; > - kfree(sdev); > > return 0; > } > @@ -139,7 +130,7 @@ static int shmob_drm_load(struct drm_device *dev, > unsigned long flags) return -EINVAL; > } > > - sdev = kzalloc(sizeof(*sdev), GFP_KERNEL); > + sdev = devm_kzalloc(&pdev->dev, sizeof(*sdev), GFP_KERNEL); > if (sdev == NULL) { > dev_err(dev->dev, "failed to allocate private data\n"); > return -ENOMEM; > @@ -156,29 +147,28 @@ static int shmob_drm_load(struct drm_device *dev, > unsigned long flags) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (res == NULL) { > dev_err(&pdev->dev, "failed to get memory resource\n"); > - ret = -EINVAL; > - goto done; > + return -EINVAL; > } > > - sdev->mmio = ioremap_nocache(res->start, resource_size(res)); > + sdev->mmio = devm_ioremap_nocache(&pdev->dev, res->start, > + resource_size(res)); > if (sdev->mmio == NULL) { > dev_err(&pdev->dev, "failed to remap memory resource\n"); > - ret = -ENOMEM; > - goto done; > + return -ENOMEM; > } > > ret = shmob_drm_setup_clocks(sdev, pdata->clk_source); > if (ret < 0) > - goto done; > + return ret; > > ret = shmob_drm_init_interface(sdev); > if (ret < 0) > - goto done; > + return ret; > > ret = shmob_drm_modeset_init(sdev); > if (ret < 0) { > dev_err(&pdev->dev, "failed to initialize mode setting\n"); > - goto done; > + return ret; > } > > for (i = 0; i < 4; ++i) { > diff --git a/drivers/gpu/drm/shmobile/shmob_drm_plane.c > b/drivers/gpu/drm/shmobile/shmob_drm_plane.c index e1eb899..6898f6f 100644 > --- a/drivers/gpu/drm/shmobile/shmob_drm_plane.c > +++ b/drivers/gpu/drm/shmobile/shmob_drm_plane.c > @@ -221,11 +221,8 @@ static int shmob_drm_plane_disable(struct drm_plane > *plane) > > static void shmob_drm_plane_destroy(struct drm_plane *plane) > { > - struct shmob_drm_plane *splane = to_shmob_plane(plane); > - > shmob_drm_plane_disable(plane); > drm_plane_cleanup(plane); > - kfree(splane); > } > > static const struct drm_plane_funcs shmob_drm_plane_funcs = { > @@ -251,7 +248,7 @@ int shmob_drm_plane_create(struct shmob_drm_device > *sdev, unsigned int index) struct shmob_drm_plane *splane; > int ret; > > - splane = kzalloc(sizeof(*splane), GFP_KERNEL); > + splane = devm_kzalloc(sdev->dev, sizeof(*splane), GFP_KERNEL); > if (splane == NULL) > return -ENOMEM; > > @@ -261,8 +258,6 @@ int shmob_drm_plane_create(struct shmob_drm_device > *sdev, unsigned int index) ret = drm_plane_init(sdev->ddev, &splane->plane, > 1, >&shmob_drm_plane_funcs, formats, >ARRAY_SIZE(formats), false); > - if (ret < 0) > - kfree(splane); > > return ret; > } -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] clk/exynos5250: add clocks for hdmi subsystem
Hi, Tested this series on snow board and is working fine. Tested-by: Arun Kumar K Regards Arun On Mon, Jun 10, 2013 at 4:30 PM, Rahul Sharma wrote: > Add clock changes for hdmi subsystem for exynos5250 SoC. These > include addition of new clocks like mout_hdmi and smmu_tv, associating > ID to clk_hdmiphy and some essential corrections. > > This set is based on kukjin's for-next branch at > http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git. > > Arun Kumar K (1): > clk/exynos5250: Fix HDMI clock number in documentation > > Rahul Sharma (4): > clk/exynos5250: add mout_hdmi mux clock for hdmi > clk/exynos5250: add sclk_hdmiphy in the list of special clocks > clk/exynos5250: add clock for tv sysmmu > clk/exynos5250: change parent to aclk200_disp1 for hdmi subsystem > > .../devicetree/bindings/clock/exynos5250-clock.txt | 12 +++- > drivers/clk/samsung/clk-exynos5250.c | 18 > +- > 2 files changed, 24 insertions(+), 6 deletions(-) > > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65730] no sound via HDMI for Radeon 6850 mesa driver
https://bugs.freedesktop.org/show_bug.cgi?id=65730 --- Comment #7 from Michel Dänzer --- The kernel command line works regardless of whether the driver was built into the kernel or as a module. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65730] no sound via HDMI for Radeon 6850 mesa driver
https://bugs.freedesktop.org/show_bug.cgi?id=65730 --- Comment #8 from Andy Furniss --- (In reply to comment #7) > The kernel command line works regardless of whether the driver was built > into the kernel or as a module. Oh OK, sorry for the noise. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65723] Xonotic glsl 1.30 broken due to lack of derivatives support in radeonsi
https://bugs.freedesktop.org/show_bug.cgi?id=65723 Michel Dänzer changed: What|Removed |Added Summary|Xonotic glsl 1.30 broken|Xonotic glsl 1.30 broken ||due to lack of derivatives ||support in radeonsi --- Comment #4 from Michel Dänzer --- Looks like Xonotic uses derivatives with GLSL 1.30, which radeonsi doesn't implement yet, so it falls back to dummy shaders: warning: failed to translate tgsi opcode DDX to LLVM Failed to translate shader from TGSI to LLVM EE ../../../../../src/gallium/drivers/radeonsi/si_state.c:1951 si_shader_select - Failed to build shader variant (type=1) -22 Maybe you can work around this by disabling / lowering some settings in Xonotic, or you could try the WIP patches from http://lists.freedesktop.org/archives/mesa-dev/2013-June/040406.html and http://lists.freedesktop.org/archives/mesa-dev/2013-June/040412.html . -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver
On Thu, Jun 13, 2013 at 12:50:16PM +0100, Russell King - ARM Linux wrote: > On Thu, Jun 13, 2013 at 12:19:03PM +0100, Russell King - ARM Linux wrote: > > The deeper I look, the more bugs there seem to be in this DRM stuff, > > and I'm continuing to look because I'm chasing a framebuffer refcount > > bug. > > So, this refcount bug - I think I've just found it. This is the flow of > references to the new fb on mode set: > > drm_mode_setcrtc(): > fb = drm_framebuffer_lookup(dev, crtc_req->fb_id); > set.fb = fb; > ret = drm_mode_set_config_internal(&set); > drm_mode_set_config_internal(): > fb = set->fb; > ret = crtc->funcs->set_config(set); > drm_crtc_helper_set_config(): > old_fb = set->crtc->fb; > set->crtc->fb = set->fb; > if (!drm_crtc_helper_set_mode(set->crtc, set->mode, > set->x, set->y, > old_fb)) { > drm_helper_disable_unused_functions(dev); > drm_helper_disable_unused_functions(): > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { > crtc->enabled = drm_helper_crtc_in_use(crtc); > if (!crtc->enabled) { > crtc->fb = NULL; > } > } > back to drm_mode_set_config_internal(): > if (ret == 0) { > if (fb) > drm_framebuffer_reference(fb); > back to drm_mode_setcrtc(): > if (fb) > drm_framebuffer_unreference(fb); > > Assuming success all the way through, what happens when a CRTC is unused > is: > > 1. We obtain a reference in drm_mode_setcrtc() via the lookup. > 2. We set the mode > 3. In trying to set the mode, we discover that all connectors for the CRTC >are in the disconnected state, and so we disable the CRTC > 4. We set crtc->fb to NULL > 5. back in drm_mode_set_config_internal(), we take a reference on the >framebuffer irrespective of this. > 6. back in drm_mode_setcrtc(), we drop the original reference caused by >the lookup. > > We now have a framebuffer with a reference count incremented by one but > no actual reference to it - the CRTC's reference is completely lost by > the action of drm_helper_disable_unused_functions(). > > You could argue that it's something the driver should deal with - fine, > but what if it only implements the DPMS method? Should it drop a > reference to the framebuffer when DPMS instructs it to turn off? Surely > not, because that means when DPMS turns stuff back on you're missing a > refcount. > > Are drivers required to implement a disable function and cater for the > imbalance in the upper layers of code? If so, this is not a clean > design. Yep, if your driver grabs additional references (underlying gem object, pinning, whatever) you need to wire up your own ->disable hook to drop those. Note that for truly dumb kms drivers which only ever allocate an fb, the upper layer actually _does_ take care of all the refcounting. Also note the crtc helpers in drm_crtc_helper.c are purely optional. The real drm core -> driver interface is all contained in drm_crtc.c. And crtc helpers do make a few critical design assumptions about how your hw works (and there's a bit room for api cleanup, I agree on that). So if they simply don't work out for you no one will get upset if you roll your own modeset infrastructure. And in drm/i915 we've had to do just that since the impedance mismatch between crtc helper assumptions and what our hw needed grew to big (and in really fundamental ways, not just a bit of interface ugliness like you're seeing here). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm: Renesas R-Car Display Unit DRM driver
On Fri, Jun 14, 2013 at 02:54:04AM +0200, Laurent Pinchart wrote: > Hi Daniel, > > On Friday 07 June 2013 10:50:55 Daniel Vetter wrote: > > On Fri, Jun 07, 2013 at 09:44:45AM +0200, Laurent Pinchart wrote: > > > On Wednesday 05 June 2013 10:55:05 Daniel Vetter wrote: > > > > On Wed, Jun 05, 2013 at 03:51:53AM +0200, Laurent Pinchart wrote: > > > > > On Tuesday 04 June 2013 20:36:20 Daniel Vetter wrote: > > > > > > On Tue, Jun 4, 2013 at 8:03 PM, Laurent Pinchart wrote: > > > > > > > On Tuesday 04 June 2013 16:12:36 Daniel Vetter wrote: > > > > > > >> On Tue, Jun 04, 2013 at 04:53:40AM +0200, Laurent Pinchart wrote: > > > > [snip] > > > > > > > > > > >> > +static int rcar_du_vga_connector_get_modes(struct > > > > > > >> > drm_connector > > > > > > >> > *connector) > > > > > > >> > +{ > > > > > > >> > + return drm_add_modes_noedid(connector, 1280, 768); > > > > > > >> > +} > > > > > > >> > > > > > > >> This (and the dummy detect function below) looks a bit funny, > > > > > > >> since it essentially overrides the default behaviour already > > > > > > >> provided by the crtc helpers. Until rcar has at least proper > > > > > > >> detect support for VGA > > > > > > > > > > > > > > I would add that but the DDC signals are not connected on the > > > > > > > boards I have access to :-/ > > > > > > > > > > > > > >> I'd just kill this and use the connector force support (and > > > > > > >> manually adding the right resolutions). > > > > > > > > > > > > > > Looks like that's a candidate for better documentation... How does > > > > > > > force support work ? > > > > > > > > > > > > Grep for DRM_FORCE_ON, iirc it can be set on the commandline, where > > > > > > you can also force a specific mode. The best I could find wrt docs > > > > > > is the kerneldoc for drm_mode_parse_command_line_for_connector. With > > > > > > a bit more reading it looks like it's intermingled with the fbdev > > > > > > helper code, but should be fairly easy to extract and used by your > > > > > > driver. > > > > > > > > > > It makes sense to force the connector state from command line, but I'm > > > > > not sure if the same mechanism is the best solution here. As the > > > > > driver has no way to know the connector state, the best we can do is > > > > > guess what modes are supported. I can just return 0 in the get_modes > > > > > handler, but then the core will not call drm_add_modes_noedid(), and > > > > > modes will need to be added manually. > > > > > > > > > > Is your point that for a board on which the VGA connector state can't > > > > > be detected, the user should always be responsible for adding all the > > > > > modes supported by the VGA monitor on the command line ? > > > > > > > > My point is that we already have both an established code for connected > > > > outputs without EDID to add fallback modes and means to force connectors > > > > to certain states. Your code here seems to reinvent that wheel, so I > > > > wonder what we should/need to improve in the common code to suit your > > > > needs. > > > > > > The currently available code might suit my needs, it might just be that I > > > fail to see how to use it properly. > > > > > > Regarding the "code for connected outputs without EDID to add fallback > > > modes" you're referring to, is that > > > > > > if (count == 0 && connector->status == connector_status_connected) > > > count = drm_add_modes_noedid(connector, 1024, 768); > > > > > > in drm_helper_probe_single_connector_modes() ? That function will only be > > > called if the connector status is connector_status_connected. There are > > > two ways to enforce that: > > > > > > - returning connector_status_connected from the connector detect() > > > operation, which seems to defeat the purpose of having > > > connector_status_unknown completely. > > > > We might want to add such a default mode also for unknown, I'm not sure. > > Userspace policy is to first try to light up any connected outputs, and if > > there's none try to light up any unknown outputs. Not sure whether userspace > > (i.e. X) will automatically add a default mode. fbcon might also handle this > > less gracefully. > > > > Personally I'm ok with extending this to unknown, it shouldn't really hurt > > (since we already try really hard not to leak unknown anywhere visible). > > Do you mean something like > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > b/drivers/gpu/drm/drm_crtc_helper.c > index f554516..9aae384 100644 > --- a/drivers/gpu/drm/drm_crtc_helper.c > +++ b/drivers/gpu/drm/drm_crtc_helper.c > @@ -160,7 +160,8 @@ int drm_helper_probe_single_connector_modes(struct > drm_connector *connector, > #endif > count = (*connector_funcs->get_modes)(connector); > > - if (count == 0 && connector->status == connector_status_connected) > + if (count == 0 && (connector->status == connector_status_connected || > +connector->status == connector_status_unknown)) >
Re: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver
On Thu, Jun 13, 2013 at 3:03 PM, Russell King - ARM Linux wrote: > On Thu, Jun 13, 2013 at 12:50:16PM +0100, Russell King - ARM Linux wrote: >> On Thu, Jun 13, 2013 at 12:19:03PM +0100, Russell King - ARM Linux wrote: >> > The deeper I look, the more bugs there seem to be in this DRM stuff, >> > and I'm continuing to look because I'm chasing a framebuffer refcount >> > bug. >> >> So, this refcount bug - I think I've just found it. This is the flow of >> references to the new fb on mode set: >> >> drm_mode_setcrtc(): >> fb = drm_framebuffer_lookup(dev, crtc_req->fb_id); >> set.fb = fb; >> ret = drm_mode_set_config_internal(&set); >> drm_mode_set_config_internal(): >> fb = set->fb; >> ret = crtc->funcs->set_config(set); >> drm_crtc_helper_set_config(): >> old_fb = set->crtc->fb; >> set->crtc->fb = set->fb; >> if (!drm_crtc_helper_set_mode(set->crtc, set->mode, >> set->x, set->y, >> old_fb)) { >> drm_helper_disable_unused_functions(dev); >> drm_helper_disable_unused_functions(): >> list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { >> crtc->enabled = drm_helper_crtc_in_use(crtc); >> if (!crtc->enabled) { >> crtc->fb = NULL; >> } >> } >> back to drm_mode_set_config_internal(): >> if (ret == 0) { >> if (fb) >> drm_framebuffer_reference(fb); >> back to drm_mode_setcrtc(): >> if (fb) >> drm_framebuffer_unreference(fb); >> >> Assuming success all the way through, what happens when a CRTC is unused >> is: >> >> 1. We obtain a reference in drm_mode_setcrtc() via the lookup. >> 2. We set the mode >> 3. In trying to set the mode, we discover that all connectors for the CRTC >>are in the disconnected state, and so we disable the CRTC >> 4. We set crtc->fb to NULL >> 5. back in drm_mode_set_config_internal(), we take a reference on the >>framebuffer irrespective of this. >> 6. back in drm_mode_setcrtc(), we drop the original reference caused by >>the lookup. >> >> We now have a framebuffer with a reference count incremented by one but >> no actual reference to it - the CRTC's reference is completely lost by >> the action of drm_helper_disable_unused_functions(). >> >> You could argue that it's something the driver should deal with - fine, >> but what if it only implements the DPMS method? Should it drop a >> reference to the framebuffer when DPMS instructs it to turn off? Surely >> not, because that means when DPMS turns stuff back on you're missing a >> refcount. >> >> Are drivers required to implement a disable function and cater for the >> imbalance in the upper layers of code? If so, this is not a clean >> design. > > There's a bigger issue here - if it's possible for > drm_crtc_helper_set_config() > to be called with set->fb set but set->mode NULL, then we overwrite > set->fb to NULL. Again, that results in a lost reference. > > For the time being, I'm using this patch, which solves my dropped > refcount problem, and marks the other possible dropped reference. > Either that check needs to be removed or it needs to properly drop > the refcount on the fb before 'losing' the reference to it. Scrap my other mail, I see now where the leaking happens. One of them is interface abuse which is now fixed (and i915 has a bunch of BUG_ONs to enforce them). The other one is indeed a real case that eluded me when I've done the refcountification for drm_framebuffers. I'll hack up some patches, since this seems to be a good excuse to port some of the i915 modeset improvements back to the crtc helpers. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm: add mmap function to prime helpers
On 06/12/2013 06:16 AM, Joonyoung Shim wrote: This adds to call low-level mmap() from prime helpers. Signed-off-by: Joonyoung Shim --- drivers/gpu/drm/drm_prime.c | 5 - include/drm/drmP.h | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index d92853e..3a008b2 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -165,7 +165,10 @@ static void drm_gem_dmabuf_kunmap(struct dma_buf *dma_buf, static int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma) { - return -EINVAL; + struct drm_gem_object *obj = dma_buf->priv; + struct drm_device *dev = obj->dev; + + return dev->driver->gem_prime_mmap(obj, vma); Won't this crash if the driver doesn't fill in the new field and userspace tries to map it? } static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 82670ac..12083dc 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -937,6 +937,8 @@ struct drm_driver { struct sg_table *sgt); void *(*gem_prime_vmap)(struct drm_gem_object *obj); void (*gem_prime_vunmap)(struct drm_gem_object *obj, void *vaddr); + int (*gem_prime_mmap)(struct drm_gem_object *obj, + struct vm_area_struct *vma); /* vga arb irq handler */ void (*vgaarb_irq)(struct drm_device *dev, bool state); -- Aaron ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65730] no sound via HDMI for Radeon 6850 mesa driver
https://bugs.freedesktop.org/show_bug.cgi?id=65730 --- Comment #9 from Jason --- Well, I downloaded the latest stable 3.9.6 kernel from www.kernel.org and applied the patch. Apparently my system didn't like the new kernel because it broke my X. I'm just not going to worry about this one for now. Thanks for the assistance. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65723] Xonotic glsl 1.30 broken due to lack of derivatives support in radeonsi
https://bugs.freedesktop.org/show_bug.cgi?id=65723 --- Comment #5 from Rafael Castillo --- many thanks for your answer, i will try those patches and report back -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65438] GTK apps crash X11 since last update of LLVMM
https://bugs.freedesktop.org/show_bug.cgi?id=65438 --- Comment #13 from Rafael Castillo --- many thanks for the help very appreciated -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver
On Fri, Jun 14, 2013 at 4:42 PM, Russell King - ARM Linux wrote: > On Fri, Jun 14, 2013 at 04:23:22PM +0200, Daniel Vetter wrote: >> On Thu, Jun 13, 2013 at 3:03 PM, Russell King - ARM Linux >> wrote: >> > There's a bigger issue here - if it's possible for >> > drm_crtc_helper_set_config() >> > to be called with set->fb set but set->mode NULL, then we overwrite >> > set->fb to NULL. Again, that results in a lost reference. >> > >> > For the time being, I'm using this patch, which solves my dropped >> > refcount problem, and marks the other possible dropped reference. >> > Either that check needs to be removed or it needs to properly drop >> > the refcount on the fb before 'losing' the reference to it. >> >> Scrap my other mail, I see now where the leaking happens. One of them >> is interface abuse which is now fixed (and i915 has a bunch of BUG_ONs >> to enforce them). The other one is indeed a real case that eluded me >> when I've done the refcountification for drm_framebuffers. I'll hack >> up some patches, since this seems to be a good excuse to port some of >> the i915 modeset improvements back to the crtc helpers. > > If you're happy with the patch I supplied, that's probably the minimal fix > which should go to stable kernels (I'm using 3.9 here) - this also counts > as a "user visible bug". It's something I've tripped over which causes > exhausts memory and can prevent the X server from starting up. > > If you want me to package the patch up with a commit message and sign-off.. Your patch doesn't fix drm/i915 (since we don't use the crtc helpers any more). And I don't think it's good to have the refcounting partially in the drm core and partially in drivers. I've also thrown a few more things on top just to port a few of the i915 cleanups to the crtc helper. I'll submit my patches asap (need to test them a bit more first). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [pull] radeon drm-fixes-3.10
Hi Dave, I added one more patch on top if it's not too late. It's a fix for UVD on big endian: drm/radeon: fix UVD on big endian This fixes the kernel side so that the ring should come up and ring and IB tests should work. The userspace UVD drivers will also need big endian fixes. Signed-off-by: Alex Deucher Thanks, Alex On Thu, Jun 13, 2013 at 7:08 PM, wrote: > From: Alex Deucher > > Hi Dave, > > Just two small fixes from Jerome. Remove some harmless but confusing > VM related error messages and fix a regression with suspend and UVD. > > The following changes since commit df63d3ecbca514bad99513b2401448d19a9bb92e: > > Merge tag 'drm-intel-fixes-2013-06-11' of > git://people.freedesktop.org/~danvet/drm-intel into drm-fixes (2013-06-11 > 19:38:27 +1000) > > are available in the git repository at: > > git://people.freedesktop.org/~agd5f/linux drm-fixes-3.10 > > Jerome Glisse (2): > drm/radeon: do not try to uselessly update virtual memory pagetable > drm/radeon: fix write back suspend regression with uvd v2 > > drivers/gpu/drm/radeon/radeon_device.c | 53 ++- > drivers/gpu/drm/radeon/radeon_fence.c | 10 +- > drivers/gpu/drm/radeon/radeon_gart.c |6 ++- > drivers/gpu/drm/radeon/radeon_uvd.c| 14 > 4 files changed, 50 insertions(+), 33 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 00/23] modetest enhancements
Hello, After a (too) long delay here's the fifth version of my modetest enhancements patch set. Beside various cleanups, these patches allow dropping master after mode set, configuring more than two pipes and planes, setting properties from the command line, setting plane positions and configuring pipes with multiple connectors in cloned mode. The code has been tested with the R-Car DU DRM driver. Changes since v4: - Update the usage line with the newly added parameters - Fixed a bad copy&paste in the -w argument handling - Use widthxheight[+-]x[+-]y to specify plane size and position - Fixed two bugs in buffer allocation and pattern generation (22/23 and 23/23) Laurent Pinchart (23): modetest: Fix warnings modetest: Remove extern declarations of opt(arg|ind|err|opt) modetest: Sort command line arguments modetest: Add a command line parameter to select the driver modetest: Add a command line parameter to drop master after mode set modetest: Retrieve all resources in one go modetest: Don't limit mode set and planes to two instances modetest: Add a command line parameter to set properties modetest: Allow specifying plane position modetest: Print the plane ID when setting up a plane modetest: Remove the -m argument modetest: Create a device structure modetest: Compute CRTC pipe number as needed modetest: Remove the struct connector_arg encoder field modetest: Store the crtc in the connector_arg structure modetest: Store the mode in the crtc structure modetest: Give the CRTC ID to the -P option modetest: Split mode setting and plane setup modetest: Rename struct connector_arg to struct pipe_arg modetest: Support pipes with multiple connectors modetest: Try all possible encoders for a connector modetest: Fix line stride in SMPTE YUV packet pattern generator modetest: Allocate NV buffers large enough for the two planes tests/modetest/Makefile.am |4 +- tests/modetest/buffers.c | 44 +- tests/modetest/buffers.h |5 +- tests/modetest/modetest.c | 1195 +++- 4 files changed, 882 insertions(+), 366 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 01/23] modetest: Fix warnings
Enable all standard automake warnings except for -Wpointer-arith (as the test pattern generation code uses void pointer arithmetics) and fix them. Signed-off-by: Laurent Pinchart --- tests/modetest/Makefile.am | 4 +++- tests/modetest/buffers.c | 13 +++-- tests/modetest/buffers.h | 5 +++-- tests/modetest/modetest.c | 44 ++-- 4 files changed, 35 insertions(+), 31 deletions(-) diff --git a/tests/modetest/Makefile.am b/tests/modetest/Makefile.am index 410c632..6e7ff14 100644 --- a/tests/modetest/Makefile.am +++ b/tests/modetest/Makefile.am @@ -1,4 +1,6 @@ -AM_CFLAGS = \ +AM_CFLAGS = $(filter-out -Wpointer-arith, $(WARN_CFLAGS)) + +AM_CFLAGS += \ -I$(top_srcdir)/include/drm \ -I$(top_srcdir)/libkms/ \ -I$(top_srcdir) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index b75cca7..1ca3be5 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -999,8 +999,8 @@ fill_pattern(unsigned int format, enum fill_pattern pattern, void *planes[3], */ static struct kms_bo * -allocate_buffer(struct kms_driver *kms, - int width, int height, int *stride) +allocate_buffer(struct kms_driver *kms, unsigned int width, unsigned int height, + unsigned int *stride) { struct kms_bo *bo; unsigned bo_attribs[] = { @@ -1034,13 +1034,14 @@ allocate_buffer(struct kms_driver *kms, struct kms_bo * create_test_buffer(struct kms_driver *kms, unsigned int format, - int width, int height, int handles[4], - int pitches[4], int offsets[4], enum fill_pattern pattern) + unsigned int width, unsigned int height, + unsigned int handles[4], unsigned int pitches[4], + unsigned int offsets[4], enum fill_pattern pattern) { struct kms_bo *bo; - int ret, stride; - void *planes[3]; + void *planes[3] = { 0, }; void *virtual; + int ret; bo = allocate_buffer(kms, width, height, &pitches[0]); if (!bo) diff --git a/tests/modetest/buffers.h b/tests/modetest/buffers.h index 2b15ce5..e320389 100644 --- a/tests/modetest/buffers.h +++ b/tests/modetest/buffers.h @@ -37,8 +37,9 @@ enum fill_pattern { }; struct kms_bo *create_test_buffer(struct kms_driver *kms, unsigned int format, - int width, int height, int handles[4], int pitches[4], - int offsets[4], enum fill_pattern pattern); + unsigned int width, unsigned int height, + unsigned int handles[4], unsigned int pitches[4], + unsigned int offsets[4], enum fill_pattern pattern); unsigned int format_fourcc(const char *name); diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 8afd2b1..47704db 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -64,11 +64,11 @@ int fd, modes; struct type_name { int type; - char *name; + const char *name; }; #define type_name_fn(res) \ -char * res##_str(int type) { \ +const char * res##_str(int type) { \ unsigned int i; \ for (i = 0; i < ARRAY_SIZE(res##_names); i++) { \ if (res##_names[i].type == type)\ @@ -85,7 +85,7 @@ struct type_name encoder_type_names[] = { { DRM_MODE_ENCODER_TVDAC, "TVDAC" }, }; -type_name_fn(encoder_type) +static type_name_fn(encoder_type) struct type_name connector_status_names[] = { { DRM_MODE_CONNECTED, "connected" }, @@ -93,7 +93,7 @@ struct type_name connector_status_names[] = { { DRM_MODE_UNKNOWNCONNECTION, "unknown" }, }; -type_name_fn(connector_status) +static type_name_fn(connector_status) struct type_name connector_type_names[] = { { DRM_MODE_CONNECTOR_Unknown, "unknown" }, @@ -113,11 +113,11 @@ struct type_name connector_type_names[] = { { DRM_MODE_CONNECTOR_eDP, "eDP" }, }; -type_name_fn(connector_type) +static type_name_fn(connector_type) #define bit_name_fn(res) \ -char * res##_str(int type) { \ - int i; \ +const char * res##_str(int type) { \ + unsigned int i; \ const char *sep = ""; \ for (i = 0; i < ARRAY_SIZE(res##_names); i++) { \ if (type & (1 << i)) { \ @@ -138,7 +138,7 @@ static const char *mode_type_names[] = { "driver", }; -bit_name_fn(mode_type) +static bit_name_fn(mode_type) static const char *mode_flag_names[] = { "phsync", @@ -157,9 +157,9 @@ static const char *mode_flag_names[] = { "clkdiv2" }; -bit_name_fn(mode_flag) +static bit_name_fn(mode_flag) -void dump_encoders(vo
[PATCH v5 02/23] modetest: Remove extern declarations of opt(arg|ind|err|opt)
Those variables are declared in unistd.h, there's no need to redeclare them here. Signed-off-by: Laurent Pinchart Reviewed-by: Jani Nikula --- tests/modetest/modetest.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 47704db..e0d7d72 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p, int plane_count, kms_destroy(&kms); } -extern char *optarg; -extern int optind, opterr, optopt; static char optstr[] = "ecpmfs:P:v"; #define min(a, b) ((a) < (b) ? (a) : (b)) -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 03/23] modetest: Sort command line arguments
The current mostly random sort order hinders code readability. Sort the options alphabetically in the code, and by group in the help message. Signed-off-by: Laurent Pinchart Reviewed-by: Jani Nikula --- tests/modetest/modetest.c | 49 ++- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index e0d7d72..2bb4b19 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p, int plane_count, kms_destroy(&kms); } -static char optstr[] = "ecpmfs:P:v"; - #define min(a, b) ((a) < (b) ? (a) : (b)) static int parse_connector(struct connector *c, const char *arg) @@ -896,15 +894,20 @@ static int parse_plane(struct plane *p, const char *arg) static void usage(char *name) { - fprintf(stderr, "usage: %s [-ecpmf]\n", name); - fprintf(stderr, "\t-e\tlist encoders\n"); + fprintf(stderr, "usage: %s [-cefmPpsv]\n", name); + + fprintf(stderr, "\n Query options:\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); - fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); - fprintf(stderr, "\t-m\tlist modes\n"); + fprintf(stderr, "\t-e\tlist encoders\n"); fprintf(stderr, "\t-f\tlist framebuffers\n"); - fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); - fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); + fprintf(stderr, "\t-m\tlist modes\n"); + fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); + + fprintf(stderr, "\n Test options:\n\n"); fprintf(stderr, "\t-P :x[@]\tset a plane\n"); + fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); + fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); + fprintf(stderr, "\n\tDefault is to dump all info.\n"); exit(0); } @@ -932,6 +935,8 @@ static int page_flipping_supported(void) #endif } +static char optstr[] = "cefmP:ps:v"; + int main(int argc, char **argv) { int c; @@ -946,34 +951,34 @@ int main(int argc, char **argv) opterr = 0; while ((c = getopt(argc, argv, optstr)) != -1) { switch (c) { - case 'e': - encoders = 1; - break; case 'c': connectors = 1; break; - case 'p': - crtcs = 1; - planes = 1; + case 'e': + encoders = 1; + break; + case 'f': + framebuffers = 1; break; case 'm': modes = 1; break; - case 'f': - framebuffers = 1; + case 'P': + if (parse_plane(&plane_args[plane_count], optarg) < 0) + usage(argv[0]); + plane_count++; break; - case 'v': - test_vsync = 1; + case 'p': + crtcs = 1; + planes = 1; break; case 's': if (parse_connector(&con_args[count], optarg) < 0) usage(argv[0]); count++; break; - case 'P': - if (parse_plane(&plane_args[plane_count], optarg) < 0) - usage(argv[0]); - plane_count++; + case 'v': + test_vsync = 1; break; default: usage(argv[0]); -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 05/23] modetest: Add a command line parameter to drop master after mode set
If the -d parameter is specified, modetest will drop master permissions after setting the mode. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index f7ebd63..e079aad 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -894,7 +894,7 @@ static int parse_plane(struct plane *p, const char *arg) static void usage(char *name) { - fprintf(stderr, "usage: %s [-cefMmPpsv]\n", name); + fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name); fprintf(stderr, "\n Query options:\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); @@ -909,6 +909,7 @@ static void usage(char *name) fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); fprintf(stderr, "\n Generic options:\n\n"); + fprintf(stderr, "\t-d\tdrop master after mode set\n"); fprintf(stderr, "\t-M module\tuse the given driver\n"); fprintf(stderr, "\n\tDefault is to dump all info.\n"); @@ -938,12 +939,13 @@ static int page_flipping_supported(void) #endif } -static char optstr[] = "cefM:mP:ps:v"; +static char optstr[] = "cdefM:mP:ps:v"; int main(int argc, char **argv) { int c; int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers = 0; + int drop_master = 0; int test_vsync = 0; const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos" }; char *module = NULL; @@ -958,6 +960,9 @@ int main(int argc, char **argv) case 'c': connectors = 1; break; + case 'd': + drop_master = 1; + break; case 'e': encoders = 1; break; @@ -1044,6 +1049,8 @@ int main(int argc, char **argv) if (count > 0) { set_mode(con_args, count, plane_args, plane_count, test_vsync); + if (drop_master) + drmDropMaster(fd); getchar(); } -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 04/23] modetest: Add a command line parameter to select the driver
If the -M parameter is specified, modetest will use the requested device name instead of trying its builtin list of device names. Signed-off-by: Laurent Pinchart Reviewed-by: Jani Nikula --- tests/modetest/modetest.c | 50 --- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 2bb4b19..f7ebd63 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -894,7 +894,7 @@ static int parse_plane(struct plane *p, const char *arg) static void usage(char *name) { - fprintf(stderr, "usage: %s [-cefmPpsv]\n", name); + fprintf(stderr, "usage: %s [-cefMmPpsv]\n", name); fprintf(stderr, "\n Query options:\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); @@ -908,6 +908,9 @@ static void usage(char *name) fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); + fprintf(stderr, "\n Generic options:\n\n"); + fprintf(stderr, "\t-M module\tuse the given driver\n"); + fprintf(stderr, "\n\tDefault is to dump all info.\n"); exit(0); } @@ -935,7 +938,7 @@ static int page_flipping_supported(void) #endif } -static char optstr[] = "cefmP:ps:v"; +static char optstr[] = "cefM:mP:ps:v"; int main(int argc, char **argv) { @@ -943,6 +946,7 @@ int main(int argc, char **argv) int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers = 0; int test_vsync = 0; const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos" }; + char *module = NULL; unsigned int i; int count = 0, plane_count = 0; struct connector con_args[2]; @@ -960,6 +964,9 @@ int main(int argc, char **argv) case 'f': framebuffers = 1; break; + case 'M': + module = optarg; + break; case 'm': modes = 1; break; @@ -986,30 +993,41 @@ int main(int argc, char **argv) } } - if (argc == 1) - encoders = connectors = crtcs = planes = modes = framebuffers = 1; + if (module) { + fd = drmOpen(module, NULL); + if (fd < 0) { + fprintf(stderr, "failed to open device '%s'.\n", module); + return 1; + } + + /* Preserve the default behaviour or dumping all information. */ + argc--; + } else { + for (i = 0; i < ARRAY_SIZE(modules); i++) { + printf("trying to open device '%s'...", modules[i]); + fd = drmOpen(modules[i], NULL); + if (fd < 0) { + printf("failed.\n"); + } else { + printf("success.\n"); + break; + } + } - for (i = 0; i < ARRAY_SIZE(modules); i++) { - printf("trying to load module %s...", modules[i]); - fd = drmOpen(modules[i], NULL); if (fd < 0) { - printf("failed.\n"); - } else { - printf("success.\n"); - break; + fprintf(stderr, "no device found.\n"); + return 1; } } + if (argc == 1) + encoders = connectors = crtcs = planes = modes = framebuffers = 1; + if (test_vsync && !page_flipping_supported()) { fprintf(stderr, "page flipping not supported by drm.\n"); return -1; } - if (i == ARRAY_SIZE(modules)) { - fprintf(stderr, "failed to load any modules, aborting.\n"); - return -1; - } - resources = drmModeGetResources(fd); if (!resources) { fprintf(stderr, "drmModeGetResources failed: %s\n", -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 06/23] modetest: Retrieve all resources in one go
Instead of retrieving resources as they are needed, retrieve them all (except property blobs) in one go at startup. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 408 +- 1 file changed, 261 insertions(+), 147 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index e079aad..bf5ca54 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -57,7 +57,44 @@ #include "buffers.h" -drmModeRes *resources; +struct crtc { + drmModeCrtc *crtc; + drmModeObjectProperties *props; + drmModePropertyRes **props_info; +}; + +struct encoder { + drmModeEncoder *encoder; +}; + +struct connector { + drmModeConnector *connector; + drmModeObjectProperties *props; + drmModePropertyRes **props_info; +}; + +struct fb { + drmModeFB *fb; +}; + +struct plane { + drmModePlane *plane; + drmModeObjectProperties *props; + drmModePropertyRes **props_info; +}; + +struct resources { + drmModeRes *res; + drmModePlaneRes *plane_res; + + struct crtc *crtcs; + struct encoder *encoders; + struct connector *connectors; + struct fb *fbs; + struct plane *planes; +}; + +struct resources *resources; int fd, modes; #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) @@ -166,21 +203,17 @@ static void dump_encoders(void) printf("Encoders:\n"); printf("id\tcrtc\ttype\tpossible crtcs\tpossible clones\t\n"); - for (i = 0; i < resources->count_encoders; i++) { - encoder = drmModeGetEncoder(fd, resources->encoders[i]); - - if (!encoder) { - fprintf(stderr, "could not get encoder %i: %s\n", - resources->encoders[i], strerror(errno)); + for (i = 0; i < resources->res->count_encoders; i++) { + encoder = resources->encoders[i].encoder; + if (!encoder) continue; - } + printf("%d\t%d\t%s\t0x%08x\t0x%08x\n", encoder->encoder_id, encoder->crtc_id, encoder_type_str(encoder->encoder_type), encoder->possible_crtcs, encoder->possible_clones); - drmModeFreeEncoder(encoder); } printf("\n"); } @@ -230,13 +263,9 @@ dump_blob(uint32_t blob_id) } static void -dump_prop(uint32_t prop_id, uint64_t value) +dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value) { int i; - drmModePropertyPtr prop; - - prop = drmModeGetProperty(fd, prop_id); - printf("\t%d", prop_id); if (!prop) { printf("\n"); @@ -297,25 +326,19 @@ dump_prop(uint32_t prop_id, uint64_t value) dump_blob(value); else printf(" %"PRIu64"\n", value); - - drmModeFreeProperty(prop); } static void dump_connectors(void) { - drmModeConnector *connector; int i, j; printf("Connectors:\n"); printf("id\tencoder\tstatus\t\ttype\tsize (mm)\tmodes\tencoders\n"); - for (i = 0; i < resources->count_connectors; i++) { - connector = drmModeGetConnector(fd, resources->connectors[i]); - - if (!connector) { - fprintf(stderr, "could not get connector %i: %s\n", - resources->connectors[i], strerror(errno)); + for (i = 0; i < resources->res->count_connectors; i++) { + struct connector *_connector = &resources->connectors[i]; + drmModeConnector *connector = _connector->connector; + if (!connector) continue; - } printf("%d\t%d\t%s\t%s\t%dx%d\t\t%d\t", connector->connector_id, @@ -335,35 +358,32 @@ static void dump_connectors(void) "vss vse vtot)\n"); for (j = 0; j < connector->count_modes; j++) dump_mode(&connector->modes[j]); + } + if (_connector->props) { printf(" props:\n"); - for (j = 0; j < connector->count_props; j++) - dump_prop(connector->props[j], - connector->prop_values[j]); + for (j = 0; j < (int)_connector->props->count_props; j++) + dump_prop(_connector->props_info[j], + _connector->props->props[j], + _connector->props->prop_values[j]); } - - drmModeFreeConnector(connector); } printf("\n"); } static void dump_crtcs(void) { - drmModeCrtc *crtc; - drmModeObjectPropertiesPtr props; in
[PATCH v5 07/23] modetest: Don't limit mode set and planes to two instances
Configuring mode on more than two connectors or two planes is perfectly valid. Support it. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 20 ++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index bf5ca54..39f4d59 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -1067,8 +1067,8 @@ int main(int argc, char **argv) char *module = NULL; unsigned int i; int count = 0, plane_count = 0; - struct connector_arg con_args[2]; - struct plane_arg plane_args[2] = { { 0, }, }; + struct connector_arg *con_args = NULL; + struct plane_arg *plane_args = NULL; opterr = 0; while ((c = getopt(argc, argv, optstr)) != -1) { @@ -1092,8 +1092,16 @@ int main(int argc, char **argv) modes = 1; break; case 'P': + plane_args = realloc(plane_args, +(plane_count + 1) * sizeof *plane_args); + if (plane_args == NULL) { + fprintf(stderr, "memory allocation failed\n"); + return 1; + } + if (parse_plane(&plane_args[plane_count], optarg) < 0) usage(argv[0]); + plane_count++; break; case 'p': @@ -1101,8 +1109,16 @@ int main(int argc, char **argv) planes = 1; break; case 's': + con_args = realloc(con_args, + (count + 1) * sizeof *con_args); + if (con_args == NULL) { + fprintf(stderr, "memory allocation failed\n"); + return 1; + } + if (parse_connector(&con_args[count], optarg) < 0) usage(argv[0]); + count++; break; case 'v': -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 08/23] modetest: Add a command line parameter to set properties
The -w parameter can be used to set a property value from the command line, using the target object ID and the property name. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 108 +- 1 file changed, 106 insertions(+), 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 39f4d59..b269608 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -709,6 +709,80 @@ connector_find_mode(struct connector_arg *c) } +/* - + * Properties + */ + +struct property_arg { + uint32_t obj_id; + uint32_t obj_type; + char name[DRM_PROP_NAME_LEN+1]; + uint32_t prop_id; + uint64_t value; +}; + +static void set_property(struct property_arg *p) +{ + drmModeObjectProperties *props; + drmModePropertyRes **props_info; + const char *obj_type; + int ret; + int i; + + p->obj_type = 0; + p->prop_id = 0; + +#define find_object(_res, __res, type, Type) \ + do { \ + for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) { \ + struct type *obj = &(_res)->type##s[i]; \ + if (obj->type->type##_id != p->obj_id) \ + continue; \ + p->obj_type = DRM_MODE_OBJECT_##Type; \ + obj_type = #Type; \ + props = obj->props; \ + props_info = obj->props_info; \ + } \ + } while(0) \ + + find_object(resources, res, crtc, CRTC); + if (p->obj_type == 0) + find_object(resources, res, connector, CONNECTOR); + if (p->obj_type == 0) + find_object(resources, plane_res, plane, PLANE); + if (p->obj_type == 0) { + fprintf(stderr, "Object %i not found, can't set property\n", + p->obj_id); + return; + } + + if (!props) { + fprintf(stderr, "%s %i has no properties\n", + obj_type, p->obj_id); + return; + } + + for (i = 0; i < (int)props->count_props; ++i) { + if (!props_info[i]) + continue; + if (strcmp(props_info[i]->name, p->name) == 0) + break; + } + + if (i == (int)props->count_props) { + fprintf(stderr, "%s %i has no %s property\n", + obj_type, p->obj_id, p->name); + return; + } + + p->prop_id = props->props[i]; + + ret = drmModeObjectSetProperty(fd, p->obj_id, p->obj_type, p->prop_id, p->value); + if (ret < 0) + fprintf(stderr, "failed to set %s %i property %s to %" PRIu64 ": %s\n", + obj_type, p->obj_id, p->name, p->value, strerror(errno)); +} + /* -- */ static void @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const char *arg) return 0; } +static int parse_property(struct property_arg *p, const char *arg) +{ + if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p->value) != 3) + return -1; + + p->obj_type = 0; + p->name[DRM_PROP_NAME_LEN] = '\0'; + + return 0; +} + static void usage(char *name) { - fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name); + fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name); fprintf(stderr, "\n Query options:\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); @@ -1023,6 +1108,7 @@ static void usage(char *name) fprintf(stderr, "\t-P :x[@]\tset a plane\n"); fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); + fprintf(stderr, "\t-w ::\tset property\n"); fprintf(stderr, "\n Generic options:\n\n"); fprintf(stderr, "\t-d\tdrop master after mode set\n"); @@ -1055,7 +1141,7 @@ static int page_flipping_supported(void) #endif } -static char optstr[] = "cdefM:mP:ps:v"; +static char optstr[] = "cdefM:mP:ps:vw:"; int main(int argc, char **argv) { @@ -1067,8 +1153,10 @@ int main(int argc, char **argv) char *module = NULL; unsigned int i; int count = 0, plane_count = 0; + unsigned int prop_count = 0; struct connector_arg *con_args = NULL; struct plane_arg
[PATCH v5 09/23] modetest: Allow specifying plane position
Extend the -P option to allow specifying the plane x and y offsets. The position is optional, if not specified the plane will be positioned at the center of the screen as before. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 67 --- 1 file changed, 52 insertions(+), 15 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index b269608..65ff766 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -40,6 +40,7 @@ #include "config.h" #include +#include #include #include #include @@ -645,6 +646,8 @@ struct connector_arg { struct plane_arg { uint32_t con_id; /* the id of connector to bind to */ + bool has_position; + int32_t x, y; uint32_t w, h; unsigned int fb_id; char format_str[5]; /* need to leave room for terminating \0 */ @@ -855,11 +858,16 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p) return -1; } - /* ok, boring.. but for now put in middle of screen: */ - crtc_x = c->mode->hdisplay / 3; - crtc_y = c->mode->vdisplay / 3; - crtc_w = crtc_x; - crtc_h = crtc_y; + if (!p->has_position) { + /* Default to the middle of the screen */ + crtc_x = (c->mode->hdisplay - p->w) / 2; + crtc_y = (c->mode->vdisplay - p->h) / 2; + } else { + crtc_x = p->x; + crtc_y = p->y; + } + crtc_w = p->w; + crtc_h = p->h; /* note src coords (last 4 args) are in Q16 format */ if (drmModeSetPlane(fd, plane_id, c->crtc, p->fb_id, @@ -1065,18 +1073,47 @@ static int parse_connector(struct connector_arg *c, const char *arg) return 0; } -static int parse_plane(struct plane_arg *p, const char *arg) +static int parse_plane(struct plane_arg *plane, const char *p) { - strcpy(p->format_str, "XR24"); + char *end; - if (sscanf(arg, "%d:%dx%d@%4s", &p->con_id, &p->w, &p->h, p->format_str) != 4 && - sscanf(arg, "%d:%dx%d", &p->con_id, &p->w, &p->h) != 3) - return -1; + memset(plane, 0, sizeof *plane); - p->fourcc = format_fourcc(p->format_str); - if (p->fourcc == 0) { - fprintf(stderr, "unknown format %s\n", p->format_str); - return -1; + plane->con_id = strtoul(p, &end, 10); + if (*end != ':') + return -EINVAL; + + p = end + 1; + plane->w = strtoul(p, &end, 10); + if (*end != 'x') + return -EINVAL; + + p = end + 1; + plane->h = strtoul(p, &end, 10); + + if (*end == '+' || *end == '-') { + plane->x = strtol(end, &end, 10); + if (*end != '+' && *end != '-') + return -EINVAL; + plane->y = strtol(end, &end, 10); + + plane->has_position = true; + } + + if (*end == '@') { + p = end + 1; + if (strlen(p) != 4) + return -EINVAL; + + strcpy(plane->format_str, p); + } else { + strcpy(plane->format_str, "XR24"); + } + + plane->fourcc = format_fourcc(plane->format_str); + if (plane->fourcc == 0) { + fprintf(stderr, "unknown format %s\n", plane->format_str); + return -EINVAL; } return 0; @@ -1105,7 +1142,7 @@ static void usage(char *name) fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); fprintf(stderr, "\n Test options:\n\n"); - fprintf(stderr, "\t-P :x[@]\tset a plane\n"); + fprintf(stderr, "\t-P :x[++][@]\tset a plane\n"); fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); fprintf(stderr, "\t-w ::\tset property\n"); -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 11/23] modetest: Remove the -m argument
The argument isn't used, remove it. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 258b958..b736f2b 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -96,7 +96,7 @@ struct resources { }; struct resources *resources; -int fd, modes; +int fd; #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) @@ -1138,7 +1138,6 @@ static void usage(char *name) fprintf(stderr, "\t-c\tlist connectors\n"); fprintf(stderr, "\t-e\tlist encoders\n"); fprintf(stderr, "\t-f\tlist framebuffers\n"); - fprintf(stderr, "\t-m\tlist modes\n"); fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); fprintf(stderr, "\n Test options:\n\n"); @@ -1178,7 +1177,7 @@ static int page_flipping_supported(void) #endif } -static char optstr[] = "cdefM:mP:ps:vw:"; +static char optstr[] = "cdefM:P:ps:vw:"; int main(int argc, char **argv) { @@ -1213,9 +1212,6 @@ int main(int argc, char **argv) case 'M': module = optarg; break; - case 'm': - modes = 1; - break; case 'P': plane_args = realloc(plane_args, (plane_count + 1) * sizeof *plane_args); @@ -1296,7 +1292,7 @@ int main(int argc, char **argv) } if (argc == 1) - encoders = connectors = crtcs = planes = modes = framebuffers = 1; + encoders = connectors = crtcs = planes = framebuffers = 1; if (test_vsync && !page_flipping_supported()) { fprintf(stderr, "page flipping not supported by drm.\n"); -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 10/23] modetest: Print the plane ID when setting up a plane
As modetest automatically selects an unused plan, providing the plane ID allows modifying plane properties for the selected planes. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 65ff766..258b958 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -838,14 +838,14 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p) plane_id = ovr->plane_id; } - fprintf(stderr, "testing %dx%d@%s overlay plane\n", - p->w, p->h, p->format_str); - if (!plane_id) { - fprintf(stderr, "failed to find plane!\n"); + fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc); return -1; } + fprintf(stderr, "testing %dx%d@%s overlay plane %u\n", + p->w, p->h, p->format_str, plane_id); + plane_bo = create_test_buffer(kms, p->fourcc, p->w, p->h, handles, pitches, offsets, PATTERN_TILES); if (plane_bo == NULL) -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 13/23] modetest: Compute CRTC pipe number as needed
Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 4a6566f..c4e4dda 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -640,7 +640,6 @@ struct connector_arg { drmModeModeInfo *mode; drmModeEncoder *encoder; int crtc; - int pipe; unsigned int fb_id[2], current_fb_id; struct timeval start; @@ -703,15 +702,6 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) if (c->crtc == -1) c->crtc = c->encoder->crtc_id; - - /* and figure out which crtc index it is: */ - for (i = 0; i < dev->resources->res->count_crtcs; i++) { - if (c->crtc == (int)dev->resources->res->crtcs[i]) { - c->pipe = i; - break; - } - } - } /* - @@ -829,15 +819,30 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) struct kms_bo *plane_bo; uint32_t plane_flags = 0; int crtc_x, crtc_y, crtc_w, crtc_h; + unsigned int pipe; unsigned int i; - /* find an unused plane which can be connected to our crtc */ + /* Find an unused plane which can be connected to our CRTC. Find the +* CRTC index first, then iterate over available planes. +*/ + for (i = 0; i < (unsigned int)dev->resources->res->count_crtcs; i++) { + if (c->crtc == (int)dev->resources->res->crtcs[i]) { + pipe = i; + break; + } + } + + if (pipe == (unsigned int)dev->resources->res->count_crtcs) { + fprintf(stderr, "CRTC %u not found\n", c->crtc); + return -1; + } + for (i = 0; i < dev->resources->plane_res->count_planes && !plane_id; i++) { ovr = dev->resources->planes[i].plane; if (!ovr) continue; - if ((ovr->possible_crtcs & (1 << c->pipe)) && !ovr->crtc_id) + if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id) plane_id = ovr->plane_id; } -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 12/23] modetest: Create a device structure
Instead of passing the device fd and resources as global variables group them in a device structure and pass it explictly to all functions that need it. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 199 -- 1 file changed, 102 insertions(+), 97 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index b736f2b..4a6566f 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -95,8 +95,12 @@ struct resources { struct plane *planes; }; -struct resources *resources; -int fd; +struct device { + int fd; + + struct resources *resources; + struct kms_driver *kms; +}; #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) @@ -197,15 +201,15 @@ static const char *mode_flag_names[] = { static bit_name_fn(mode_flag) -static void dump_encoders(void) +static void dump_encoders(struct device *dev) { drmModeEncoder *encoder; int i; printf("Encoders:\n"); printf("id\tcrtc\ttype\tpossible crtcs\tpossible clones\t\n"); - for (i = 0; i < resources->res->count_encoders; i++) { - encoder = resources->encoders[i].encoder; + for (i = 0; i < dev->resources->res->count_encoders; i++) { + encoder = dev->resources->encoders[i].encoder; if (!encoder) continue; @@ -240,14 +244,13 @@ static void dump_mode(drmModeModeInfo *mode) printf("\n"); } -static void -dump_blob(uint32_t blob_id) +static void dump_blob(struct device *dev, uint32_t blob_id) { uint32_t i; unsigned char *blob_data; drmModePropertyBlobPtr blob; - blob = drmModeGetPropertyBlob(fd, blob_id); + blob = drmModeGetPropertyBlob(dev->fd, blob_id); if (!blob) return; @@ -263,8 +266,8 @@ dump_blob(uint32_t blob_id) drmModeFreePropertyBlob(blob); } -static void -dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value) +static void dump_prop(struct device *dev, drmModePropertyPtr prop, + uint32_t prop_id, uint64_t value) { int i; printf("\t%d", prop_id); @@ -316,7 +319,7 @@ dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value) if (prop->flags & DRM_MODE_PROP_BLOB) { printf("\t\tblobs:\n"); for (i = 0; i < prop->count_blobs; i++) - dump_blob(prop->blob_ids[i]); + dump_blob(dev, prop->blob_ids[i]); printf("\n"); } else { assert(prop->count_blobs == 0); @@ -324,19 +327,19 @@ dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value) printf("\t\tvalue:"); if (prop->flags & DRM_MODE_PROP_BLOB) - dump_blob(value); + dump_blob(dev, value); else printf(" %"PRIu64"\n", value); } -static void dump_connectors(void) +static void dump_connectors(struct device *dev) { int i, j; printf("Connectors:\n"); printf("id\tencoder\tstatus\t\ttype\tsize (mm)\tmodes\tencoders\n"); - for (i = 0; i < resources->res->count_connectors; i++) { - struct connector *_connector = &resources->connectors[i]; + for (i = 0; i < dev->resources->res->count_connectors; i++) { + struct connector *_connector = &dev->resources->connectors[i]; drmModeConnector *connector = _connector->connector; if (!connector) continue; @@ -364,7 +367,7 @@ static void dump_connectors(void) if (_connector->props) { printf(" props:\n"); for (j = 0; j < (int)_connector->props->count_props; j++) - dump_prop(_connector->props_info[j], + dump_prop(dev, _connector->props_info[j], _connector->props->props[j], _connector->props->prop_values[j]); } @@ -372,15 +375,15 @@ static void dump_connectors(void) printf("\n"); } -static void dump_crtcs(void) +static void dump_crtcs(struct device *dev) { int i; uint32_t j; printf("CRTCs:\n"); printf("id\tfb\tpos\tsize\n"); - for (i = 0; i < resources->res->count_crtcs; i++) { - struct crtc *_crtc = &resources->crtcs[i]; + for (i = 0; i < dev->resources->res->count_crtcs; i++) { + struct crtc *_crtc = &dev->resources->crtcs[i]; drmModeCrtc *crtc = _crtc->crtc; if (!crtc) continue; @@ -395,7 +398,7 @@ static void dump_crtcs(void) if (_crtc->props) { printf(" props:\n"); for (j = 0; j < _crtc->props->count_props; j++) - du
[PATCH v5 15/23] modetest: Store the crtc in the connector_arg structure
This prepares the code for the split in separate functions of CRTC and planes setup. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 58 ++- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index a7b8b43..e5f12ce 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -634,11 +634,12 @@ error: */ struct connector_arg { uint32_t id; + uint32_t crtc_id; char mode_str[64]; char format_str[5]; unsigned int fourcc; drmModeModeInfo *mode; - int crtc; + struct crtc *crtc; unsigned int fb_id[2], current_fb_id; struct timeval start; @@ -690,18 +691,33 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) return; } - /* Now get the encoder */ - for (i = 0; i < dev->resources->res->count_encoders; i++) { - encoder = dev->resources->encoders[i].encoder; - if (!encoder) - continue; + /* If the CRTC ID was specified, get the corresponding CRTC. Otherwise +* locate a CRTC that can be attached to the connector. +*/ + if (c->crtc_id == (uint32_t)-1) { + for (i = 0; i < dev->resources->res->count_encoders; i++) { + encoder = dev->resources->encoders[i].encoder; + if (!encoder) + continue; + + if (encoder->encoder_id == connector->encoder_id) { + c->crtc_id = encoder->crtc_id; + break; + } + } + } + + if (c->crtc_id == (uint32_t)-1) + return; - if (encoder->encoder_id == connector->encoder_id) + for (i = 0; i < dev->resources->res->count_crtcs; i++) { + struct crtc *crtc = &dev->resources->crtcs[i]; + + if (c->crtc_id == crtc->crtc->crtc_id) { + c->crtc = crtc; break; + } } - - if (c->crtc == -1) - c->crtc = encoder->crtc_id; } /* - @@ -796,7 +812,7 @@ page_flip_handler(int fd, unsigned int frame, else new_fb_id = c->fb_id[0]; - drmModePageFlip(fd, c->crtc, new_fb_id, + drmModePageFlip(fd, c->crtc_id, new_fb_id, DRM_MODE_PAGE_FLIP_EVENT, c); c->current_fb_id = new_fb_id; c->swap_count++; @@ -826,14 +842,14 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) * CRTC index first, then iterate over available planes. */ for (i = 0; i < (unsigned int)dev->resources->res->count_crtcs; i++) { - if (c->crtc == (int)dev->resources->res->crtcs[i]) { + if (c->crtc_id == dev->resources->res->crtcs[i]) { pipe = i; break; } } if (pipe == (unsigned int)dev->resources->res->count_crtcs) { - fprintf(stderr, "CRTC %u not found\n", c->crtc); + fprintf(stderr, "CRTC %u not found\n", c->crtc_id); return -1; } @@ -847,7 +863,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) } if (!plane_id) { - fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc); + fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc_id); return -1; } @@ -878,7 +894,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) crtc_h = p->h; /* note src coords (last 4 args) are in Q16 format */ - if (drmModeSetPlane(dev->fd, plane_id, c->crtc, p->fb_id, + if (drmModeSetPlane(dev->fd, plane_id, c->crtc_id, p->fb_id, plane_flags, crtc_x, crtc_y, crtc_w, crtc_h, 0, 0, p->w << 16, p->h << 16)) { fprintf(stderr, "failed to enable plane: %s\n", @@ -886,7 +902,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) return -1; } - ovr->crtc_id = c->crtc; + ovr->crtc_id = c->crtc_id; return 0; } @@ -930,9 +946,9 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count, continue; printf("setting mode %s@%s on connector %d, crtc %d\n", - c[i].mode_str, c[i].format_str, c[i].id, c[i].crtc); + c[i].mode_str, c[i].format_str, c[i].id, c[i].crtc_id); - ret = drmModeSetCrtc(dev->fd, c[i].crtc, fb_id, x, 0, + ret = drmModeSetCrtc(dev->fd
[PATCH v5 14/23] modetest: Remove the struct connector_arg encoder field
The field is no needed, make it a local variable where used. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index c4e4dda..a7b8b43 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -638,7 +638,6 @@ struct connector_arg { char format_str[5]; unsigned int fourcc; drmModeModeInfo *mode; - drmModeEncoder *encoder; int crtc; unsigned int fb_id[2], current_fb_id; struct timeval start; @@ -659,6 +658,7 @@ struct plane_arg { static void connector_find_mode(struct device *dev, struct connector_arg *c) { drmModeConnector *connector; + drmModeEncoder *encoder; int i, j; /* First, find the connector & mode */ @@ -692,16 +692,16 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) /* Now get the encoder */ for (i = 0; i < dev->resources->res->count_encoders; i++) { - c->encoder = dev->resources->encoders[i].encoder; - if (!c->encoder) + encoder = dev->resources->encoders[i].encoder; + if (!encoder) continue; - if (c->encoder->encoder_id == connector->encoder_id) + if (encoder->encoder_id == connector->encoder_id) break; } if (c->crtc == -1) - c->crtc = c->encoder->crtc_id; + c->crtc = encoder->crtc_id; } /* - -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 16/23] modetest: Store the mode in the crtc structure
This prepares the code for the split in separate functions of CRTC and planes setup. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index e5f12ce..c46db59 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -62,6 +62,7 @@ struct crtc { drmModeCrtc *crtc; drmModeObjectProperties *props; drmModePropertyRes **props_info; + drmModeModeInfo *mode; }; struct encoder { @@ -524,6 +525,7 @@ static void free_resources(struct resources *res) static struct resources *get_resources(struct device *dev) { struct resources *res; + int i; res = malloc(sizeof *res); if (res == 0) @@ -598,6 +600,9 @@ static struct resources *get_resources(struct device *dev) get_properties(res, res, crtc, CRTC); get_properties(res, res, connector, CONNECTOR); + for (i = 0; i < res->res->count_crtcs; ++i) + res->crtcs[i].mode = &res->crtcs[i].crtc->mode; + res->plane_res = drmModeGetPlaneResources(dev->fd); if (!res->plane_res) { fprintf(stderr, "drmModeGetPlaneResources failed: %s\n", @@ -714,6 +719,7 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) struct crtc *crtc = &dev->resources->crtcs[i]; if (c->crtc_id == crtc->crtc->crtc_id) { + crtc->mode = c->mode; c->crtc = crtc; break; } @@ -884,8 +890,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) if (!p->has_position) { /* Default to the middle of the screen */ - crtc_x = (c->mode->hdisplay - p->w) / 2; - crtc_y = (c->mode->vdisplay - p->h) / 2; + crtc_x = (c->crtc->mode->hdisplay - p->w) / 2; + crtc_y = (c->crtc->mode->vdisplay - p->h) / 2; } else { crtc_x = p->x; crtc_y = p->y; -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 17/23] modetest: Give the CRTC ID to the -P option
Planes are associated with CRTCs, not connectors. Don't try to be too clever, use the CRTC ID in the -P option. This prepares for splitting CRTC and planes setup. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 32 +--- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index c46db59..0dfdaa5 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -652,7 +652,7 @@ struct connector_arg { }; struct plane_arg { - uint32_t con_id; /* the id of connector to bind to */ + uint32_t crtc_id; /* the id of CRTC to bind to */ bool has_position; int32_t x, y; uint32_t w, h; @@ -832,8 +832,7 @@ page_flip_handler(int fd, unsigned int frame, } } -static int -set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) +static int set_plane(struct device *dev, struct plane_arg *p) { drmModePlane *ovr; uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */ @@ -841,6 +840,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) struct kms_bo *plane_bo; uint32_t plane_flags = 0; int crtc_x, crtc_y, crtc_w, crtc_h; + struct crtc *crtc; unsigned int pipe; unsigned int i; @@ -848,14 +848,15 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) * CRTC index first, then iterate over available planes. */ for (i = 0; i < (unsigned int)dev->resources->res->count_crtcs; i++) { - if (c->crtc_id == dev->resources->res->crtcs[i]) { + if (p->crtc_id == dev->resources->res->crtcs[i]) { + crtc = &dev->resources->crtcs[i]; pipe = i; break; } } - if (pipe == (unsigned int)dev->resources->res->count_crtcs) { - fprintf(stderr, "CRTC %u not found\n", c->crtc_id); + if (!crtc) { + fprintf(stderr, "CRTC %u not found\n", p->crtc_id); return -1; } @@ -869,7 +870,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) } if (!plane_id) { - fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc_id); + fprintf(stderr, "no unused plane available for CRTC %u\n", + crtc->crtc->crtc_id); return -1; } @@ -890,8 +892,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) if (!p->has_position) { /* Default to the middle of the screen */ - crtc_x = (c->crtc->mode->hdisplay - p->w) / 2; - crtc_y = (c->crtc->mode->vdisplay - p->h) / 2; + crtc_x = (crtc->mode->hdisplay - p->w) / 2; + crtc_y = (crtc->mode->vdisplay - p->h) / 2; } else { crtc_x = p->x; crtc_y = p->y; @@ -900,7 +902,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) crtc_h = p->h; /* note src coords (last 4 args) are in Q16 format */ - if (drmModeSetPlane(dev->fd, plane_id, c->crtc_id, p->fb_id, + if (drmModeSetPlane(dev->fd, plane_id, crtc->crtc->crtc_id, p->fb_id, plane_flags, crtc_x, crtc_y, crtc_w, crtc_h, 0, 0, p->w << 16, p->h << 16)) { fprintf(stderr, "failed to enable plane: %s\n", @@ -908,7 +910,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) return -1; } - ovr->crtc_id = c->crtc_id; + ovr->crtc_id = crtc->crtc->crtc_id; return 0; } @@ -969,8 +971,8 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count, /* if we have a plane/overlay to show, set that up now: */ for (j = 0; j < plane_count; j++) - if (p[j].con_id == c[i].id) - if (set_plane(dev, &c[i], &p[j])) + if (p[j].crtc_id == c[i].crtc_id) + if (set_plane(dev, &p[j])) return; } @@ -1099,7 +1101,7 @@ static int parse_plane(struct plane_arg *plane, const char *p) memset(plane, 0, sizeof *plane); - plane->con_id = strtoul(p, &end, 10); + plane->crtc_id = strtoul(p, &end, 10); if (*end != ':') return -EINVAL; @@ -1161,7 +1163,7 @@ static void usage(char *name) fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); fprintf(stderr, "\n Test options:\n\n"); - fprintf(stderr, "\t-P :x[++][@]\tset a plane\n"); + fprintf(stderr, "\t-P :x[++][@]\tset a plane\n"); fprintf(stderr, "\t-s [@]:[@]\tset a mode\n");
[PATCH v5 18/23] modetest: Split mode setting and plane setup
There's not reason to require setting a mode to test planes. Split the two operations. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 106 +++--- 1 file changed, 73 insertions(+), 33 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 0dfdaa5..adbd073 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -101,6 +101,14 @@ struct device { struct resources *resources; struct kms_driver *kms; + + struct { + unsigned int width; + unsigned int height; + + unsigned int fb_id; + struct kms_bo *bo; + } mode; }; #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) @@ -840,7 +848,7 @@ static int set_plane(struct device *dev, struct plane_arg *p) struct kms_bo *plane_bo; uint32_t plane_flags = 0; int crtc_x, crtc_y, crtc_w, crtc_h; - struct crtc *crtc; + struct crtc *crtc = NULL; unsigned int pipe; unsigned int i; @@ -915,36 +923,37 @@ static int set_plane(struct device *dev, struct plane_arg *p) return 0; } -static void set_mode(struct device *dev, struct connector_arg *c, int count, -struct plane_arg *p, int plane_count, int page_flip) +static void set_mode(struct device *dev, struct connector_arg *c, unsigned int count) { - struct kms_bo *bo, *other_bo; - unsigned int fb_id, other_fb_id; - int i, j, ret, width, height, x; uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */ - drmEventContext evctx; + unsigned int fb_id; + struct kms_bo *bo; + unsigned int i; + int ret, x; + + dev->mode.width = 0; + dev->mode.height = 0; - width = 0; - height = 0; for (i = 0; i < count; i++) { connector_find_mode(dev, &c[i]); if (c[i].mode == NULL) continue; - width += c[i].mode->hdisplay; - if (height < c[i].mode->vdisplay) - height = c[i].mode->vdisplay; + dev->mode.width += c[i].mode->hdisplay; + if (dev->mode.height < c[i].mode->vdisplay) + dev->mode.height = c[i].mode->vdisplay; } - bo = create_test_buffer(dev->kms, c->fourcc, width, height, handles, - pitches, offsets, PATTERN_SMPTE); + bo = create_test_buffer(dev->kms, c->fourcc, + dev->mode.width, dev->mode.height, + handles, pitches, offsets, PATTERN_SMPTE); if (bo == NULL) return; - ret = drmModeAddFB2(dev->fd, width, height, c->fourcc, + ret = drmModeAddFB2(dev->fd, dev->mode.width, dev->mode.height, c->fourcc, handles, pitches, offsets, &fb_id, 0); if (ret) { fprintf(stderr, "failed to add fb (%ux%u): %s\n", - width, height, strerror(errno)); + dev->mode.width, dev->mode.height, strerror(errno)); return; } @@ -968,24 +977,39 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count, fprintf(stderr, "failed to set mode: %s\n", strerror(errno)); return; } - - /* if we have a plane/overlay to show, set that up now: */ - for (j = 0; j < plane_count; j++) - if (p[j].crtc_id == c[i].crtc_id) - if (set_plane(dev, &p[j])) - return; } - if (!page_flip) - return; + dev->mode.bo = bo; + dev->mode.fb_id = fb_id; +} + +static void set_planes(struct device *dev, struct plane_arg *p, unsigned int count) +{ + unsigned int i; + + /* set up planes/overlays */ + for (i = 0; i < count; i++) + if (set_plane(dev, &p[i])) + return; +} - other_bo = create_test_buffer(dev->kms, c->fourcc, width, height, handles, - pitches, offsets, PATTERN_PLAIN); +static void test_page_flip(struct device *dev, struct connector_arg *c, unsigned int count) +{ + uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */ + unsigned int other_fb_id; + struct kms_bo *other_bo; + drmEventContext evctx; + unsigned int i; + int ret; + + other_bo = create_test_buffer(dev->kms, c->fourcc, + dev->mode.width, dev->mode.height, + handles, pitches, offsets, PATTERN_PLAIN); if (other_bo == NULL) return; - ret = drmModeAddFB2(dev->fd, width, height, c->fourcc, handles, pitches, offsets, -
[PATCH v5 19/23] modetest: Rename struct connector_arg to struct pipe_arg
This prepares the code for handling multiple connectors in a single pipeline in a cloned configuration. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 162 -- 1 file changed, 85 insertions(+), 77 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index adbd073..6be8c75 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -635,7 +635,7 @@ error: } /* - - * Connectors and planes + * Pipes and planes */ /* @@ -645,8 +645,8 @@ error: * Then you need to find the encoder attached to that connector so you * can bind it with a free crtc. */ -struct connector_arg { - uint32_t id; +struct pipe_arg { + uint32_t con_id; uint32_t crtc_id; char mode_str[64]; char format_str[5]; @@ -669,14 +669,14 @@ struct plane_arg { unsigned int fourcc; }; -static void connector_find_mode(struct device *dev, struct connector_arg *c) +static void pipe_find_mode(struct device *dev, struct pipe_arg *pipe) { drmModeConnector *connector; drmModeEncoder *encoder; int i, j; /* First, find the connector & mode */ - c->mode = NULL; + pipe->mode = NULL; for (i = 0; i < dev->resources->res->count_connectors; i++) { connector = dev->resources->connectors[i].connector; if (!connector) @@ -685,50 +685,50 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) if (!connector->count_modes) continue; - if (connector->connector_id != c->id) + if (connector->connector_id != pipe->con_id) continue; for (j = 0; j < connector->count_modes; j++) { - c->mode = &connector->modes[j]; - if (!strcmp(c->mode->name, c->mode_str)) + pipe->mode = &connector->modes[j]; + if (!strcmp(pipe->mode->name, pipe->mode_str)) break; } /* Found it, break out */ - if (c->mode) + if (pipe->mode) break; } - if (!c->mode) { - fprintf(stderr, "failed to find mode \"%s\"\n", c->mode_str); + if (!pipe->mode) { + fprintf(stderr, "failed to find mode \"%s\"\n", pipe->mode_str); return; } /* If the CRTC ID was specified, get the corresponding CRTC. Otherwise * locate a CRTC that can be attached to the connector. */ - if (c->crtc_id == (uint32_t)-1) { + if (pipe->crtc_id == (uint32_t)-1) { for (i = 0; i < dev->resources->res->count_encoders; i++) { encoder = dev->resources->encoders[i].encoder; if (!encoder) continue; if (encoder->encoder_id == connector->encoder_id) { - c->crtc_id = encoder->crtc_id; + pipe->crtc_id = encoder->crtc_id; break; } } } - if (c->crtc_id == (uint32_t)-1) + if (pipe->crtc_id == (uint32_t)-1) return; for (i = 0; i < dev->resources->res->count_crtcs; i++) { struct crtc *crtc = &dev->resources->crtcs[i]; - if (c->crtc_id == crtc->crtc->crtc_id) { - crtc->mode = c->mode; - c->crtc = crtc; + if (pipe->crtc_id == crtc->crtc->crtc_id) { + crtc->mode = pipe->mode; + pipe->crtc = crtc; break; } } @@ -815,28 +815,28 @@ static void page_flip_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec, void *data) { - struct connector_arg *c; + struct pipe_arg *pipe; unsigned int new_fb_id; struct timeval end; double t; - c = data; - if (c->current_fb_id == c->fb_id[0]) - new_fb_id = c->fb_id[1]; + pipe = data; + if (pipe->current_fb_id == pipe->fb_id[0]) + new_fb_id = pipe->fb_id[1]; else - new_fb_id = c->fb_id[0]; + new_fb_id = pipe->fb_id[0]; - drmModePageFlip(fd, c->crtc_id, new_fb_id, - DRM_MODE_PAGE_FLIP_EVENT, c); - c->current_fb_id = new_fb_id; - c->swap_count++; - if (c->swap_count == 60) { + drmModePageFlip(fd, pipe->crtc_id, new_fb_id, + DRM_MODE_PAGE_FLIP_EVENT, pipe); + pipe->current_fb_id = new_fb_id; + pipe->swap_count++; + if (pipe->swap_count == 60)
[PATCH v5 20/23] modetest: Support pipes with multiple connectors
The -s argument can now take a list of connectors. Configure all of them in cloned mode using a single CRTC. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 211 ++ 1 file changed, 157 insertions(+), 54 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 6be8c75..945ba84 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -40,6 +40,7 @@ #include "config.h" #include +#include #include #include #include @@ -634,6 +635,34 @@ error: return NULL; } +static drmModeConnector *get_connector_by_id(struct device *dev, uint32_t id) +{ + drmModeConnector *connector; + int i; + + for (i = 0; i < dev->resources->res->count_connectors; i++) { + connector = dev->resources->connectors[i].connector; + if (connector && connector->connector_id == id) + return connector; + } + + return NULL; +} + +static drmModeEncoder *get_encoder_by_id(struct device *dev, uint32_t id) +{ + drmModeEncoder *encoder; + int i; + + for (i = 0; i < dev->resources->res->count_encoders; i++) { + encoder = dev->resources->encoders[i].encoder; + if (encoder && encoder->encoder_id == id) + return encoder; + } + + return NULL; +} + /* - * Pipes and planes */ @@ -646,7 +675,8 @@ error: * can bind it with a free crtc. */ struct pipe_arg { - uint32_t con_id; + uint32_t *con_ids; + unsigned int num_cons; uint32_t crtc_id; char mode_str[64]; char format_str[5]; @@ -669,69 +699,114 @@ struct plane_arg { unsigned int fourcc; }; -static void pipe_find_mode(struct device *dev, struct pipe_arg *pipe) +static drmModeModeInfo * +connector_find_mode(struct device *dev, uint32_t con_id, const char *mode_str) { drmModeConnector *connector; - drmModeEncoder *encoder; - int i, j; + drmModeModeInfo *mode; + int i; - /* First, find the connector & mode */ - pipe->mode = NULL; - for (i = 0; i < dev->resources->res->count_connectors; i++) { - connector = dev->resources->connectors[i].connector; + connector = get_connector_by_id(dev, con_id); + if (!connector || !connector->count_modes) + return NULL; + + for (i = 0; i < connector->count_modes; i++) { + mode = &connector->modes[i]; + if (!strcmp(mode->name, mode_str)) + return mode; + } + + return NULL; +} + +static struct crtc *pipe_find_crtc(struct device *dev, struct pipe_arg *pipe) +{ + uint32_t possible_crtcs = ~0; + uint32_t active_crtcs = 0; + unsigned int crtc_idx; + unsigned int i; + int j; + + for (i = 0; i < pipe->num_cons; ++i) { + drmModeConnector *connector; + drmModeEncoder *encoder; + + connector = get_connector_by_id(dev, pipe->con_ids[i]); if (!connector) - continue; + return NULL; - if (!connector->count_modes) - continue; + encoder = get_encoder_by_id(dev, connector->encoder_id); + if (!encoder) + return NULL; - if (connector->connector_id != pipe->con_id) - continue; + possible_crtcs &= encoder->possible_crtcs; - for (j = 0; j < connector->count_modes; j++) { - pipe->mode = &connector->modes[j]; - if (!strcmp(pipe->mode->name, pipe->mode_str)) + for (j = 0; j < dev->resources->res->count_crtcs; ++j) { + drmModeCrtc *crtc = dev->resources->crtcs[j].crtc; + if (crtc && crtc->crtc_id == encoder->crtc_id) { + active_crtcs |= 1 << j; break; + } } - - /* Found it, break out */ - if (pipe->mode) - break; } - if (!pipe->mode) { - fprintf(stderr, "failed to find mode \"%s\"\n", pipe->mode_str); - return; + if (!possible_crtcs) + return NULL; + + /* Return the first possible and active CRTC if one exists, or the first +* possible CRTC otherwise. +*/ + if (possible_crtcs & active_crtcs) + crtc_idx = ffs(possible_crtcs & active_crtcs); + else + crtc_idx = ffs(possible_crtcs); + + return &dev->resources->crtcs[crtc_idx - 1]; +} + +static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe) +{ + drmModeModeInfo *mode; + int i; + +
[PATCH v5 21/23] modetest: Try all possible encoders for a connector
When building the pipeline, instead of using only the encoders attached to a connector, take all possible encoders into account to locate a CRTC. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 945ba84..58ef1fc 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -635,6 +635,19 @@ error: return NULL; } +static int get_crtc_index(struct device *dev, uint32_t id) +{ + int i; + + for (i = 0; i < dev->resources->res->count_crtcs; ++i) { + drmModeCrtc *crtc = dev->resources->crtcs[i].crtc; + if (crtc && crtc->crtc_id == id) + return i; + } + + return -1; +} + static drmModeConnector *get_connector_by_id(struct device *dev, uint32_t id) { drmModeConnector *connector; @@ -728,26 +741,28 @@ static struct crtc *pipe_find_crtc(struct device *dev, struct pipe_arg *pipe) int j; for (i = 0; i < pipe->num_cons; ++i) { + uint32_t crtcs_for_connector = 0; drmModeConnector *connector; drmModeEncoder *encoder; + int idx; connector = get_connector_by_id(dev, pipe->con_ids[i]); if (!connector) return NULL; - encoder = get_encoder_by_id(dev, connector->encoder_id); - if (!encoder) - return NULL; + for (j = 0; j < connector->count_encoders; ++j) { + encoder = get_encoder_by_id(dev, connector->encoders[j]); + if (!encoder) + continue; - possible_crtcs &= encoder->possible_crtcs; + crtcs_for_connector |= encoder->possible_crtcs; - for (j = 0; j < dev->resources->res->count_crtcs; ++j) { - drmModeCrtc *crtc = dev->resources->crtcs[j].crtc; - if (crtc && crtc->crtc_id == encoder->crtc_id) { - active_crtcs |= 1 << j; - break; - } + idx = get_crtc_index(dev, encoder->crtc_id); + if (idx >= 0) + active_crtcs |= 1 << idx; } + + possible_crtcs &= crtcs_for_connector; } if (!possible_crtcs) -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 23/23] modetest: Allocate NV buffers large enough for the two planes
Multiple the image height by 1.5 for NV12/NV21 and by 2 for NV16/NV61 to make room for the chroma plane. Signed-off-by: Laurent Pinchart --- tests/modetest/buffers.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index 1575bf6..8206ce3 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -1038,12 +1038,29 @@ create_test_buffer(struct kms_driver *kms, unsigned int format, unsigned int handles[4], unsigned int pitches[4], unsigned int offsets[4], enum fill_pattern pattern) { + unsigned int virtual_height; struct kms_bo *bo; void *planes[3] = { 0, }; void *virtual; int ret; - bo = allocate_buffer(kms, width, height, &pitches[0]); + switch (format) { + case DRM_FORMAT_NV12: + case DRM_FORMAT_NV21: + virtual_height = height * 3 / 2; + break; + + case DRM_FORMAT_NV16: + case DRM_FORMAT_NV61: + virtual_height = height * 2; + break; + + default: + virtual_height = height; + break; + } + + bo = allocate_buffer(kms, width, virtual_height, &pitches[0]); if (!bo) return NULL; -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 22/23] modetest: Fix line stride in SMPTE YUV packet pattern generator
The line stride passed to the function is expressed in bytes, there's no need to multiply it by 2. Signed-off-by: Laurent Pinchart --- tests/modetest/buffers.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index 1ca3be5..1575bf6 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -337,13 +337,13 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, for (y = 0; y < height * 6 / 9; ++y) { for (x = 0; x < width; ++x) y_mem[2*x] = colors_top[x * 7 / width].y; - y_mem += stride * 2; + y_mem += stride; } for (; y < height * 7 / 9; ++y) { for (x = 0; x < width; ++x) y_mem[2*x] = colors_middle[x * 7 / width].y; - y_mem += stride * 2; + y_mem += stride; } for (; y < height; ++y) { @@ -354,7 +354,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, / (width / 7) + 4].y; for (; x < width; ++x) y_mem[2*x] = colors_bottom[7].y; - y_mem += stride * 2; + y_mem += stride; } /* Chroma */ @@ -363,7 +363,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, c_mem[2*x+u] = colors_top[x * 7 / width].u; c_mem[2*x+v] = colors_top[x * 7 / width].v; } - c_mem += stride * 2; + c_mem += stride; } for (; y < height * 7 / 9; ++y) { @@ -371,7 +371,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, c_mem[2*x+u] = colors_middle[x * 7 / width].u; c_mem[2*x+v] = colors_middle[x * 7 / width].v; } - c_mem += stride * 2; + c_mem += stride; } for (; y < height; ++y) { @@ -389,7 +389,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, c_mem[2*x+u] = colors_bottom[7].u; c_mem[2*x+v] = colors_bottom[7].v; } - c_mem += stride * 2; + c_mem += stride; } } -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 00/23] modetest enhancements
On Friday 14 June 2013 23:21:11 Laurent Pinchart wrote: > Hello, > > After a (too) long delay here's the fifth version of my modetest > enhancements patch set. And I've of course sent the wrong version :-/ v6 (hopefully final) is on the way. Sorry for the noise. > Beside various cleanups, these patches allow dropping master after mode set, > configuring more than two pipes and planes, setting properties from the > command line, setting plane positions and configuring pipes with multiple > connectors in cloned mode. > > The code has been tested with the R-Car DU DRM driver. > > Changes since v4: > > - Update the usage line with the newly added parameters > - Fixed a bad copy&paste in the -w argument handling > - Use widthxheight[+-]x[+-]y to specify plane size and position > - Fixed two bugs in buffer allocation and pattern generation (22/23 and > 23/23) > > Laurent Pinchart (23): > modetest: Fix warnings > modetest: Remove extern declarations of opt(arg|ind|err|opt) > modetest: Sort command line arguments > modetest: Add a command line parameter to select the driver > modetest: Add a command line parameter to drop master after mode set > modetest: Retrieve all resources in one go > modetest: Don't limit mode set and planes to two instances > modetest: Add a command line parameter to set properties > modetest: Allow specifying plane position > modetest: Print the plane ID when setting up a plane > modetest: Remove the -m argument > modetest: Create a device structure > modetest: Compute CRTC pipe number as needed > modetest: Remove the struct connector_arg encoder field > modetest: Store the crtc in the connector_arg structure > modetest: Store the mode in the crtc structure > modetest: Give the CRTC ID to the -P option > modetest: Split mode setting and plane setup > modetest: Rename struct connector_arg to struct pipe_arg > modetest: Support pipes with multiple connectors > modetest: Try all possible encoders for a connector > modetest: Fix line stride in SMPTE YUV packet pattern generator > modetest: Allocate NV buffers large enough for the two planes > > tests/modetest/Makefile.am |4 +- > tests/modetest/buffers.c | 44 +- > tests/modetest/buffers.h |5 +- > tests/modetest/modetest.c | 1195 + > 4 files changed, 882 insertions(+), 366 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 00/23] modetest enhancements
Hello, Here's the sixth (and hopefully final) version of my modeset enhancements patch set. Beside various cleanups, these patches allow dropping master after mode set, configuring more than two pipes and planes, setting properties from the command line, setting plane positions and configuring pipes with multiple connectors in cloned mode. The code has been tested with the R-Car DU DRM driver. Changes since v5: - Fixed a bug in the -M argument handling that broke the default behaviour Changes since v4: - Update the usage line with the newly added parameters - Fixed a bad copy&paste in the -w argument handling - Use widthxheight[+-]x[+-]y to specify plane size and position - Fixed two bugs in buffer allocation and pattern generation (22/23 and 23/23) Laurent Pinchart (23): modetest: Fix warnings modetest: Remove extern declarations of opt(arg|ind|err|opt) modetest: Sort command line arguments modetest: Add a command line parameter to select the driver modetest: Add a command line parameter to drop master after mode set modetest: Retrieve all resources in one go modetest: Don't limit mode set and planes to two instances modetest: Add a command line parameter to set properties modetest: Allow specifying plane position modetest: Print the plane ID when setting up a plane modetest: Remove the -m argument modetest: Create a device structure modetest: Compute CRTC pipe number as needed modetest: Remove the struct connector_arg encoder field modetest: Store the crtc in the connector_arg structure modetest: Store the mode in the crtc structure modetest: Give the CRTC ID to the -P option modetest: Split mode setting and plane setup modetest: Rename struct connector_arg to struct pipe_arg modetest: Support pipes with multiple connectors modetest: Try all possible encoders for a connector modetest: Fix line stride in SMPTE YUV packet pattern generator modetest: Allocate NV buffers large enough for the two planes tests/modetest/Makefile.am |4 +- tests/modetest/buffers.c | 44 +- tests/modetest/buffers.h |5 +- tests/modetest/modetest.c | 1197 +++- 4 files changed, 884 insertions(+), 366 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 01/23] modetest: Fix warnings
Enable all standard automake warnings except for -Wpointer-arith (as the test pattern generation code uses void pointer arithmetics) and fix them. Signed-off-by: Laurent Pinchart --- tests/modetest/Makefile.am | 4 +++- tests/modetest/buffers.c | 13 +++-- tests/modetest/buffers.h | 5 +++-- tests/modetest/modetest.c | 44 ++-- 4 files changed, 35 insertions(+), 31 deletions(-) diff --git a/tests/modetest/Makefile.am b/tests/modetest/Makefile.am index 410c632..6e7ff14 100644 --- a/tests/modetest/Makefile.am +++ b/tests/modetest/Makefile.am @@ -1,4 +1,6 @@ -AM_CFLAGS = \ +AM_CFLAGS = $(filter-out -Wpointer-arith, $(WARN_CFLAGS)) + +AM_CFLAGS += \ -I$(top_srcdir)/include/drm \ -I$(top_srcdir)/libkms/ \ -I$(top_srcdir) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index b75cca7..1ca3be5 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -999,8 +999,8 @@ fill_pattern(unsigned int format, enum fill_pattern pattern, void *planes[3], */ static struct kms_bo * -allocate_buffer(struct kms_driver *kms, - int width, int height, int *stride) +allocate_buffer(struct kms_driver *kms, unsigned int width, unsigned int height, + unsigned int *stride) { struct kms_bo *bo; unsigned bo_attribs[] = { @@ -1034,13 +1034,14 @@ allocate_buffer(struct kms_driver *kms, struct kms_bo * create_test_buffer(struct kms_driver *kms, unsigned int format, - int width, int height, int handles[4], - int pitches[4], int offsets[4], enum fill_pattern pattern) + unsigned int width, unsigned int height, + unsigned int handles[4], unsigned int pitches[4], + unsigned int offsets[4], enum fill_pattern pattern) { struct kms_bo *bo; - int ret, stride; - void *planes[3]; + void *planes[3] = { 0, }; void *virtual; + int ret; bo = allocate_buffer(kms, width, height, &pitches[0]); if (!bo) diff --git a/tests/modetest/buffers.h b/tests/modetest/buffers.h index 2b15ce5..e320389 100644 --- a/tests/modetest/buffers.h +++ b/tests/modetest/buffers.h @@ -37,8 +37,9 @@ enum fill_pattern { }; struct kms_bo *create_test_buffer(struct kms_driver *kms, unsigned int format, - int width, int height, int handles[4], int pitches[4], - int offsets[4], enum fill_pattern pattern); + unsigned int width, unsigned int height, + unsigned int handles[4], unsigned int pitches[4], + unsigned int offsets[4], enum fill_pattern pattern); unsigned int format_fourcc(const char *name); diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 8afd2b1..47704db 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -64,11 +64,11 @@ int fd, modes; struct type_name { int type; - char *name; + const char *name; }; #define type_name_fn(res) \ -char * res##_str(int type) { \ +const char * res##_str(int type) { \ unsigned int i; \ for (i = 0; i < ARRAY_SIZE(res##_names); i++) { \ if (res##_names[i].type == type)\ @@ -85,7 +85,7 @@ struct type_name encoder_type_names[] = { { DRM_MODE_ENCODER_TVDAC, "TVDAC" }, }; -type_name_fn(encoder_type) +static type_name_fn(encoder_type) struct type_name connector_status_names[] = { { DRM_MODE_CONNECTED, "connected" }, @@ -93,7 +93,7 @@ struct type_name connector_status_names[] = { { DRM_MODE_UNKNOWNCONNECTION, "unknown" }, }; -type_name_fn(connector_status) +static type_name_fn(connector_status) struct type_name connector_type_names[] = { { DRM_MODE_CONNECTOR_Unknown, "unknown" }, @@ -113,11 +113,11 @@ struct type_name connector_type_names[] = { { DRM_MODE_CONNECTOR_eDP, "eDP" }, }; -type_name_fn(connector_type) +static type_name_fn(connector_type) #define bit_name_fn(res) \ -char * res##_str(int type) { \ - int i; \ +const char * res##_str(int type) { \ + unsigned int i; \ const char *sep = ""; \ for (i = 0; i < ARRAY_SIZE(res##_names); i++) { \ if (type & (1 << i)) { \ @@ -138,7 +138,7 @@ static const char *mode_type_names[] = { "driver", }; -bit_name_fn(mode_type) +static bit_name_fn(mode_type) static const char *mode_flag_names[] = { "phsync", @@ -157,9 +157,9 @@ static const char *mode_flag_names[] = { "clkdiv2" }; -bit_name_fn(mode_flag) +static bit_name_fn(mode_flag) -void dump_encoders(vo
[PATCH v6 02/23] modetest: Remove extern declarations of opt(arg|ind|err|opt)
Those variables are declared in unistd.h, there's no need to redeclare them here. Signed-off-by: Laurent Pinchart Reviewed-by: Jani Nikula --- tests/modetest/modetest.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 47704db..e0d7d72 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p, int plane_count, kms_destroy(&kms); } -extern char *optarg; -extern int optind, opterr, optopt; static char optstr[] = "ecpmfs:P:v"; #define min(a, b) ((a) < (b) ? (a) : (b)) -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 04/23] modetest: Add a command line parameter to select the driver
If the -M parameter is specified, modetest will use the requested device name instead of trying its builtin list of device names. Signed-off-by: Laurent Pinchart Reviewed-by: Jani Nikula --- tests/modetest/modetest.c | 50 +-- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 2bb4b19..5802c8e 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -894,7 +894,7 @@ static int parse_plane(struct plane *p, const char *arg) static void usage(char *name) { - fprintf(stderr, "usage: %s [-cefmPpsv]\n", name); + fprintf(stderr, "usage: %s [-cefMmPpsv]\n", name); fprintf(stderr, "\n Query options:\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); @@ -908,6 +908,9 @@ static void usage(char *name) fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); + fprintf(stderr, "\n Generic options:\n\n"); + fprintf(stderr, "\t-M module\tuse the given driver\n"); + fprintf(stderr, "\n\tDefault is to dump all info.\n"); exit(0); } @@ -935,7 +938,7 @@ static int page_flipping_supported(void) #endif } -static char optstr[] = "cefmP:ps:v"; +static char optstr[] = "cefM:mP:ps:v"; int main(int argc, char **argv) { @@ -943,13 +946,17 @@ int main(int argc, char **argv) int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers = 0; int test_vsync = 0; const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos" }; + char *module = NULL; unsigned int i; int count = 0, plane_count = 0; struct connector con_args[2]; struct plane plane_args[2] = { { 0, }, }; + unsigned int args = 0; opterr = 0; while ((c = getopt(argc, argv, optstr)) != -1) { + args++; + switch (c) { case 'c': connectors = 1; @@ -960,6 +967,11 @@ int main(int argc, char **argv) case 'f': framebuffers = 1; break; + case 'M': + module = optarg; + /* Preserve the default behaviour of dumping all information. */ + args--; + break; case 'm': modes = 1; break; @@ -986,17 +998,30 @@ int main(int argc, char **argv) } } - if (argc == 1) + if (!args) encoders = connectors = crtcs = planes = modes = framebuffers = 1; - for (i = 0; i < ARRAY_SIZE(modules); i++) { - printf("trying to load module %s...", modules[i]); - fd = drmOpen(modules[i], NULL); + if (module) { + fd = drmOpen(module, NULL); if (fd < 0) { - printf("failed.\n"); - } else { - printf("success.\n"); - break; + fprintf(stderr, "failed to open device '%s'.\n", module); + return 1; + } + } else { + for (i = 0; i < ARRAY_SIZE(modules); i++) { + printf("trying to open device '%s'...", modules[i]); + fd = drmOpen(modules[i], NULL); + if (fd < 0) { + printf("failed.\n"); + } else { + printf("success.\n"); + break; + } + } + + if (fd < 0) { + fprintf(stderr, "no device found.\n"); + return 1; } } @@ -1005,11 +1030,6 @@ int main(int argc, char **argv) return -1; } - if (i == ARRAY_SIZE(modules)) { - fprintf(stderr, "failed to load any modules, aborting.\n"); - return -1; - } - resources = drmModeGetResources(fd); if (!resources) { fprintf(stderr, "drmModeGetResources failed: %s\n", -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 03/23] modetest: Sort command line arguments
The current mostly random sort order hinders code readability. Sort the options alphabetically in the code, and by group in the help message. Signed-off-by: Laurent Pinchart Reviewed-by: Jani Nikula --- tests/modetest/modetest.c | 49 ++- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index e0d7d72..2bb4b19 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -835,8 +835,6 @@ set_mode(struct connector *c, int count, struct plane *p, int plane_count, kms_destroy(&kms); } -static char optstr[] = "ecpmfs:P:v"; - #define min(a, b) ((a) < (b) ? (a) : (b)) static int parse_connector(struct connector *c, const char *arg) @@ -896,15 +894,20 @@ static int parse_plane(struct plane *p, const char *arg) static void usage(char *name) { - fprintf(stderr, "usage: %s [-ecpmf]\n", name); - fprintf(stderr, "\t-e\tlist encoders\n"); + fprintf(stderr, "usage: %s [-cefmPpsv]\n", name); + + fprintf(stderr, "\n Query options:\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); - fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); - fprintf(stderr, "\t-m\tlist modes\n"); + fprintf(stderr, "\t-e\tlist encoders\n"); fprintf(stderr, "\t-f\tlist framebuffers\n"); - fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); - fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); + fprintf(stderr, "\t-m\tlist modes\n"); + fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); + + fprintf(stderr, "\n Test options:\n\n"); fprintf(stderr, "\t-P :x[@]\tset a plane\n"); + fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); + fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); + fprintf(stderr, "\n\tDefault is to dump all info.\n"); exit(0); } @@ -932,6 +935,8 @@ static int page_flipping_supported(void) #endif } +static char optstr[] = "cefmP:ps:v"; + int main(int argc, char **argv) { int c; @@ -946,34 +951,34 @@ int main(int argc, char **argv) opterr = 0; while ((c = getopt(argc, argv, optstr)) != -1) { switch (c) { - case 'e': - encoders = 1; - break; case 'c': connectors = 1; break; - case 'p': - crtcs = 1; - planes = 1; + case 'e': + encoders = 1; + break; + case 'f': + framebuffers = 1; break; case 'm': modes = 1; break; - case 'f': - framebuffers = 1; + case 'P': + if (parse_plane(&plane_args[plane_count], optarg) < 0) + usage(argv[0]); + plane_count++; break; - case 'v': - test_vsync = 1; + case 'p': + crtcs = 1; + planes = 1; break; case 's': if (parse_connector(&con_args[count], optarg) < 0) usage(argv[0]); count++; break; - case 'P': - if (parse_plane(&plane_args[plane_count], optarg) < 0) - usage(argv[0]); - plane_count++; + case 'v': + test_vsync = 1; break; default: usage(argv[0]); -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 05/23] modetest: Add a command line parameter to drop master after mode set
If the -d parameter is specified, modetest will drop master permissions after setting the mode. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 5802c8e..218d26a 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -894,7 +894,7 @@ static int parse_plane(struct plane *p, const char *arg) static void usage(char *name) { - fprintf(stderr, "usage: %s [-cefMmPpsv]\n", name); + fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name); fprintf(stderr, "\n Query options:\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); @@ -909,6 +909,7 @@ static void usage(char *name) fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); fprintf(stderr, "\n Generic options:\n\n"); + fprintf(stderr, "\t-d\tdrop master after mode set\n"); fprintf(stderr, "\t-M module\tuse the given driver\n"); fprintf(stderr, "\n\tDefault is to dump all info.\n"); @@ -938,12 +939,13 @@ static int page_flipping_supported(void) #endif } -static char optstr[] = "cefM:mP:ps:v"; +static char optstr[] = "cdefM:mP:ps:v"; int main(int argc, char **argv) { int c; int encoders = 0, connectors = 0, crtcs = 0, planes = 0, framebuffers = 0; + int drop_master = 0; int test_vsync = 0; const char *modules[] = { "i915", "radeon", "nouveau", "vmwgfx", "omapdrm", "exynos" }; char *module = NULL; @@ -961,6 +963,9 @@ int main(int argc, char **argv) case 'c': connectors = 1; break; + case 'd': + drop_master = 1; + break; case 'e': encoders = 1; break; @@ -1046,6 +1051,8 @@ int main(int argc, char **argv) if (count > 0) { set_mode(con_args, count, plane_args, plane_count, test_vsync); + if (drop_master) + drmDropMaster(fd); getchar(); } -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 06/23] modetest: Retrieve all resources in one go
Instead of retrieving resources as they are needed, retrieve them all (except property blobs) in one go at startup. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 408 +- 1 file changed, 261 insertions(+), 147 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 218d26a..8ad138e 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -57,7 +57,44 @@ #include "buffers.h" -drmModeRes *resources; +struct crtc { + drmModeCrtc *crtc; + drmModeObjectProperties *props; + drmModePropertyRes **props_info; +}; + +struct encoder { + drmModeEncoder *encoder; +}; + +struct connector { + drmModeConnector *connector; + drmModeObjectProperties *props; + drmModePropertyRes **props_info; +}; + +struct fb { + drmModeFB *fb; +}; + +struct plane { + drmModePlane *plane; + drmModeObjectProperties *props; + drmModePropertyRes **props_info; +}; + +struct resources { + drmModeRes *res; + drmModePlaneRes *plane_res; + + struct crtc *crtcs; + struct encoder *encoders; + struct connector *connectors; + struct fb *fbs; + struct plane *planes; +}; + +struct resources *resources; int fd, modes; #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) @@ -166,21 +203,17 @@ static void dump_encoders(void) printf("Encoders:\n"); printf("id\tcrtc\ttype\tpossible crtcs\tpossible clones\t\n"); - for (i = 0; i < resources->count_encoders; i++) { - encoder = drmModeGetEncoder(fd, resources->encoders[i]); - - if (!encoder) { - fprintf(stderr, "could not get encoder %i: %s\n", - resources->encoders[i], strerror(errno)); + for (i = 0; i < resources->res->count_encoders; i++) { + encoder = resources->encoders[i].encoder; + if (!encoder) continue; - } + printf("%d\t%d\t%s\t0x%08x\t0x%08x\n", encoder->encoder_id, encoder->crtc_id, encoder_type_str(encoder->encoder_type), encoder->possible_crtcs, encoder->possible_clones); - drmModeFreeEncoder(encoder); } printf("\n"); } @@ -230,13 +263,9 @@ dump_blob(uint32_t blob_id) } static void -dump_prop(uint32_t prop_id, uint64_t value) +dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value) { int i; - drmModePropertyPtr prop; - - prop = drmModeGetProperty(fd, prop_id); - printf("\t%d", prop_id); if (!prop) { printf("\n"); @@ -297,25 +326,19 @@ dump_prop(uint32_t prop_id, uint64_t value) dump_blob(value); else printf(" %"PRIu64"\n", value); - - drmModeFreeProperty(prop); } static void dump_connectors(void) { - drmModeConnector *connector; int i, j; printf("Connectors:\n"); printf("id\tencoder\tstatus\t\ttype\tsize (mm)\tmodes\tencoders\n"); - for (i = 0; i < resources->count_connectors; i++) { - connector = drmModeGetConnector(fd, resources->connectors[i]); - - if (!connector) { - fprintf(stderr, "could not get connector %i: %s\n", - resources->connectors[i], strerror(errno)); + for (i = 0; i < resources->res->count_connectors; i++) { + struct connector *_connector = &resources->connectors[i]; + drmModeConnector *connector = _connector->connector; + if (!connector) continue; - } printf("%d\t%d\t%s\t%s\t%dx%d\t\t%d\t", connector->connector_id, @@ -335,35 +358,32 @@ static void dump_connectors(void) "vss vse vtot)\n"); for (j = 0; j < connector->count_modes; j++) dump_mode(&connector->modes[j]); + } + if (_connector->props) { printf(" props:\n"); - for (j = 0; j < connector->count_props; j++) - dump_prop(connector->props[j], - connector->prop_values[j]); + for (j = 0; j < (int)_connector->props->count_props; j++) + dump_prop(_connector->props_info[j], + _connector->props->props[j], + _connector->props->prop_values[j]); } - - drmModeFreeConnector(connector); } printf("\n"); } static void dump_crtcs(void) { - drmModeCrtc *crtc; - drmModeObjectPropertiesPtr props; in
[PATCH v6 07/23] modetest: Don't limit mode set and planes to two instances
Configuring mode on more than two connectors or two planes is perfectly valid. Support it. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 20 ++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 8ad138e..778af62 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -1067,8 +1067,8 @@ int main(int argc, char **argv) char *module = NULL; unsigned int i; int count = 0, plane_count = 0; - struct connector_arg con_args[2]; - struct plane_arg plane_args[2] = { { 0, }, }; + struct connector_arg *con_args = NULL; + struct plane_arg *plane_args = NULL; unsigned int args = 0; opterr = 0; @@ -1097,8 +1097,16 @@ int main(int argc, char **argv) modes = 1; break; case 'P': + plane_args = realloc(plane_args, +(plane_count + 1) * sizeof *plane_args); + if (plane_args == NULL) { + fprintf(stderr, "memory allocation failed\n"); + return 1; + } + if (parse_plane(&plane_args[plane_count], optarg) < 0) usage(argv[0]); + plane_count++; break; case 'p': @@ -1106,8 +1114,16 @@ int main(int argc, char **argv) planes = 1; break; case 's': + con_args = realloc(con_args, + (count + 1) * sizeof *con_args); + if (con_args == NULL) { + fprintf(stderr, "memory allocation failed\n"); + return 1; + } + if (parse_connector(&con_args[count], optarg) < 0) usage(argv[0]); + count++; break; case 'v': -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 08/23] modetest: Add a command line parameter to set properties
The -w parameter can be used to set a property value from the command line, using the target object ID and the property name. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 108 +- 1 file changed, 106 insertions(+), 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 778af62..858d480 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -709,6 +709,80 @@ connector_find_mode(struct connector_arg *c) } +/* - + * Properties + */ + +struct property_arg { + uint32_t obj_id; + uint32_t obj_type; + char name[DRM_PROP_NAME_LEN+1]; + uint32_t prop_id; + uint64_t value; +}; + +static void set_property(struct property_arg *p) +{ + drmModeObjectProperties *props; + drmModePropertyRes **props_info; + const char *obj_type; + int ret; + int i; + + p->obj_type = 0; + p->prop_id = 0; + +#define find_object(_res, __res, type, Type) \ + do { \ + for (i = 0; i < (int)(_res)->__res->count_##type##s; ++i) { \ + struct type *obj = &(_res)->type##s[i]; \ + if (obj->type->type##_id != p->obj_id) \ + continue; \ + p->obj_type = DRM_MODE_OBJECT_##Type; \ + obj_type = #Type; \ + props = obj->props; \ + props_info = obj->props_info; \ + } \ + } while(0) \ + + find_object(resources, res, crtc, CRTC); + if (p->obj_type == 0) + find_object(resources, res, connector, CONNECTOR); + if (p->obj_type == 0) + find_object(resources, plane_res, plane, PLANE); + if (p->obj_type == 0) { + fprintf(stderr, "Object %i not found, can't set property\n", + p->obj_id); + return; + } + + if (!props) { + fprintf(stderr, "%s %i has no properties\n", + obj_type, p->obj_id); + return; + } + + for (i = 0; i < (int)props->count_props; ++i) { + if (!props_info[i]) + continue; + if (strcmp(props_info[i]->name, p->name) == 0) + break; + } + + if (i == (int)props->count_props) { + fprintf(stderr, "%s %i has no %s property\n", + obj_type, p->obj_id, p->name); + return; + } + + p->prop_id = props->props[i]; + + ret = drmModeObjectSetProperty(fd, p->obj_id, p->obj_type, p->prop_id, p->value); + if (ret < 0) + fprintf(stderr, "failed to set %s %i property %s to %" PRIu64 ": %s\n", + obj_type, p->obj_id, p->name, p->value, strerror(errno)); +} + /* -- */ static void @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const char *arg) return 0; } +static int parse_property(struct property_arg *p, const char *arg) +{ + if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p->value) != 3) + return -1; + + p->obj_type = 0; + p->name[DRM_PROP_NAME_LEN] = '\0'; + + return 0; +} + static void usage(char *name) { - fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name); + fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name); fprintf(stderr, "\n Query options:\n\n"); fprintf(stderr, "\t-c\tlist connectors\n"); @@ -1023,6 +1108,7 @@ static void usage(char *name) fprintf(stderr, "\t-P :x[@]\tset a plane\n"); fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); + fprintf(stderr, "\t-w ::\tset property\n"); fprintf(stderr, "\n Generic options:\n\n"); fprintf(stderr, "\t-d\tdrop master after mode set\n"); @@ -1055,7 +1141,7 @@ static int page_flipping_supported(void) #endif } -static char optstr[] = "cdefM:mP:ps:v"; +static char optstr[] = "cdefM:mP:ps:vw:"; int main(int argc, char **argv) { @@ -1067,8 +1153,10 @@ int main(int argc, char **argv) char *module = NULL; unsigned int i; int count = 0, plane_count = 0; + unsigned int prop_count = 0; struct connector_arg *con_args = NULL; struct plane_arg
[PATCH v6 09/23] modetest: Allow specifying plane position
Extend the -P option to allow specifying the plane x and y offsets. The position is optional, if not specified the plane will be positioned at the center of the screen as before. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 67 --- 1 file changed, 52 insertions(+), 15 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 858d480..897a66c 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -40,6 +40,7 @@ #include "config.h" #include +#include #include #include #include @@ -645,6 +646,8 @@ struct connector_arg { struct plane_arg { uint32_t con_id; /* the id of connector to bind to */ + bool has_position; + int32_t x, y; uint32_t w, h; unsigned int fb_id; char format_str[5]; /* need to leave room for terminating \0 */ @@ -855,11 +858,16 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p) return -1; } - /* ok, boring.. but for now put in middle of screen: */ - crtc_x = c->mode->hdisplay / 3; - crtc_y = c->mode->vdisplay / 3; - crtc_w = crtc_x; - crtc_h = crtc_y; + if (!p->has_position) { + /* Default to the middle of the screen */ + crtc_x = (c->mode->hdisplay - p->w) / 2; + crtc_y = (c->mode->vdisplay - p->h) / 2; + } else { + crtc_x = p->x; + crtc_y = p->y; + } + crtc_w = p->w; + crtc_h = p->h; /* note src coords (last 4 args) are in Q16 format */ if (drmModeSetPlane(fd, plane_id, c->crtc, p->fb_id, @@ -1065,18 +1073,47 @@ static int parse_connector(struct connector_arg *c, const char *arg) return 0; } -static int parse_plane(struct plane_arg *p, const char *arg) +static int parse_plane(struct plane_arg *plane, const char *p) { - strcpy(p->format_str, "XR24"); + char *end; - if (sscanf(arg, "%d:%dx%d@%4s", &p->con_id, &p->w, &p->h, p->format_str) != 4 && - sscanf(arg, "%d:%dx%d", &p->con_id, &p->w, &p->h) != 3) - return -1; + memset(plane, 0, sizeof *plane); - p->fourcc = format_fourcc(p->format_str); - if (p->fourcc == 0) { - fprintf(stderr, "unknown format %s\n", p->format_str); - return -1; + plane->con_id = strtoul(p, &end, 10); + if (*end != ':') + return -EINVAL; + + p = end + 1; + plane->w = strtoul(p, &end, 10); + if (*end != 'x') + return -EINVAL; + + p = end + 1; + plane->h = strtoul(p, &end, 10); + + if (*end == '+' || *end == '-') { + plane->x = strtol(end, &end, 10); + if (*end != '+' && *end != '-') + return -EINVAL; + plane->y = strtol(end, &end, 10); + + plane->has_position = true; + } + + if (*end == '@') { + p = end + 1; + if (strlen(p) != 4) + return -EINVAL; + + strcpy(plane->format_str, p); + } else { + strcpy(plane->format_str, "XR24"); + } + + plane->fourcc = format_fourcc(plane->format_str); + if (plane->fourcc == 0) { + fprintf(stderr, "unknown format %s\n", plane->format_str); + return -EINVAL; } return 0; @@ -1105,7 +1142,7 @@ static void usage(char *name) fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); fprintf(stderr, "\n Test options:\n\n"); - fprintf(stderr, "\t-P :x[@]\tset a plane\n"); + fprintf(stderr, "\t-P :x[++][@]\tset a plane\n"); fprintf(stderr, "\t-s [@]:[@]\tset a mode\n"); fprintf(stderr, "\t-v\ttest vsynced page flipping\n"); fprintf(stderr, "\t-w ::\tset property\n"); -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 10/23] modetest: Print the plane ID when setting up a plane
As modetest automatically selects an unused plan, providing the plane ID allows modifying plane properties for the selected planes. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 897a66c..7f7a3a2 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -838,14 +838,14 @@ set_plane(struct kms_driver *kms, struct connector_arg *c, struct plane_arg *p) plane_id = ovr->plane_id; } - fprintf(stderr, "testing %dx%d@%s overlay plane\n", - p->w, p->h, p->format_str); - if (!plane_id) { - fprintf(stderr, "failed to find plane!\n"); + fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc); return -1; } + fprintf(stderr, "testing %dx%d@%s overlay plane %u\n", + p->w, p->h, p->format_str, plane_id); + plane_bo = create_test_buffer(kms, p->fourcc, p->w, p->h, handles, pitches, offsets, PATTERN_TILES); if (plane_bo == NULL) -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 11/23] modetest: Remove the -m argument
The argument isn't used, remove it. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 7f7a3a2..89bfc53 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -96,7 +96,7 @@ struct resources { }; struct resources *resources; -int fd, modes; +int fd; #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) @@ -1138,7 +1138,6 @@ static void usage(char *name) fprintf(stderr, "\t-c\tlist connectors\n"); fprintf(stderr, "\t-e\tlist encoders\n"); fprintf(stderr, "\t-f\tlist framebuffers\n"); - fprintf(stderr, "\t-m\tlist modes\n"); fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); fprintf(stderr, "\n Test options:\n\n"); @@ -1178,7 +1177,7 @@ static int page_flipping_supported(void) #endif } -static char optstr[] = "cdefM:mP:ps:vw:"; +static char optstr[] = "cdefM:P:ps:vw:"; int main(int argc, char **argv) { @@ -1218,9 +1217,6 @@ int main(int argc, char **argv) /* Preserve the default behaviour of dumping all information. */ args--; break; - case 'm': - modes = 1; - break; case 'P': plane_args = realloc(plane_args, (plane_count + 1) * sizeof *plane_args); @@ -1274,7 +1270,7 @@ int main(int argc, char **argv) } if (!args) - encoders = connectors = crtcs = planes = modes = framebuffers = 1; + encoders = connectors = crtcs = planes = framebuffers = 1; if (module) { fd = drmOpen(module, NULL); -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 12/23] modetest: Create a device structure
Instead of passing the device fd and resources as global variables group them in a device structure and pass it explictly to all functions that need it. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 201 -- 1 file changed, 103 insertions(+), 98 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 89bfc53..cfcb989 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -95,8 +95,12 @@ struct resources { struct plane *planes; }; -struct resources *resources; -int fd; +struct device { + int fd; + + struct resources *resources; + struct kms_driver *kms; +}; #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) @@ -197,15 +201,15 @@ static const char *mode_flag_names[] = { static bit_name_fn(mode_flag) -static void dump_encoders(void) +static void dump_encoders(struct device *dev) { drmModeEncoder *encoder; int i; printf("Encoders:\n"); printf("id\tcrtc\ttype\tpossible crtcs\tpossible clones\t\n"); - for (i = 0; i < resources->res->count_encoders; i++) { - encoder = resources->encoders[i].encoder; + for (i = 0; i < dev->resources->res->count_encoders; i++) { + encoder = dev->resources->encoders[i].encoder; if (!encoder) continue; @@ -240,14 +244,13 @@ static void dump_mode(drmModeModeInfo *mode) printf("\n"); } -static void -dump_blob(uint32_t blob_id) +static void dump_blob(struct device *dev, uint32_t blob_id) { uint32_t i; unsigned char *blob_data; drmModePropertyBlobPtr blob; - blob = drmModeGetPropertyBlob(fd, blob_id); + blob = drmModeGetPropertyBlob(dev->fd, blob_id); if (!blob) return; @@ -263,8 +266,8 @@ dump_blob(uint32_t blob_id) drmModeFreePropertyBlob(blob); } -static void -dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value) +static void dump_prop(struct device *dev, drmModePropertyPtr prop, + uint32_t prop_id, uint64_t value) { int i; printf("\t%d", prop_id); @@ -316,7 +319,7 @@ dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value) if (prop->flags & DRM_MODE_PROP_BLOB) { printf("\t\tblobs:\n"); for (i = 0; i < prop->count_blobs; i++) - dump_blob(prop->blob_ids[i]); + dump_blob(dev, prop->blob_ids[i]); printf("\n"); } else { assert(prop->count_blobs == 0); @@ -324,19 +327,19 @@ dump_prop(drmModePropertyPtr prop, uint32_t prop_id, uint64_t value) printf("\t\tvalue:"); if (prop->flags & DRM_MODE_PROP_BLOB) - dump_blob(value); + dump_blob(dev, value); else printf(" %"PRIu64"\n", value); } -static void dump_connectors(void) +static void dump_connectors(struct device *dev) { int i, j; printf("Connectors:\n"); printf("id\tencoder\tstatus\t\ttype\tsize (mm)\tmodes\tencoders\n"); - for (i = 0; i < resources->res->count_connectors; i++) { - struct connector *_connector = &resources->connectors[i]; + for (i = 0; i < dev->resources->res->count_connectors; i++) { + struct connector *_connector = &dev->resources->connectors[i]; drmModeConnector *connector = _connector->connector; if (!connector) continue; @@ -364,7 +367,7 @@ static void dump_connectors(void) if (_connector->props) { printf(" props:\n"); for (j = 0; j < (int)_connector->props->count_props; j++) - dump_prop(_connector->props_info[j], + dump_prop(dev, _connector->props_info[j], _connector->props->props[j], _connector->props->prop_values[j]); } @@ -372,15 +375,15 @@ static void dump_connectors(void) printf("\n"); } -static void dump_crtcs(void) +static void dump_crtcs(struct device *dev) { int i; uint32_t j; printf("CRTCs:\n"); printf("id\tfb\tpos\tsize\n"); - for (i = 0; i < resources->res->count_crtcs; i++) { - struct crtc *_crtc = &resources->crtcs[i]; + for (i = 0; i < dev->resources->res->count_crtcs; i++) { + struct crtc *_crtc = &dev->resources->crtcs[i]; drmModeCrtc *crtc = _crtc->crtc; if (!crtc) continue; @@ -395,7 +398,7 @@ static void dump_crtcs(void) if (_crtc->props) { printf(" props:\n"); for (j = 0; j < _crtc->props->count_props; j++) - du
[PATCH v6 13/23] modetest: Compute CRTC pipe number as needed
Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index cfcb989..a1a683f 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -640,7 +640,6 @@ struct connector_arg { drmModeModeInfo *mode; drmModeEncoder *encoder; int crtc; - int pipe; unsigned int fb_id[2], current_fb_id; struct timeval start; @@ -703,15 +702,6 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) if (c->crtc == -1) c->crtc = c->encoder->crtc_id; - - /* and figure out which crtc index it is: */ - for (i = 0; i < dev->resources->res->count_crtcs; i++) { - if (c->crtc == (int)dev->resources->res->crtcs[i]) { - c->pipe = i; - break; - } - } - } /* - @@ -829,15 +819,30 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) struct kms_bo *plane_bo; uint32_t plane_flags = 0; int crtc_x, crtc_y, crtc_w, crtc_h; + unsigned int pipe; unsigned int i; - /* find an unused plane which can be connected to our crtc */ + /* Find an unused plane which can be connected to our CRTC. Find the +* CRTC index first, then iterate over available planes. +*/ + for (i = 0; i < (unsigned int)dev->resources->res->count_crtcs; i++) { + if (c->crtc == (int)dev->resources->res->crtcs[i]) { + pipe = i; + break; + } + } + + if (pipe == (unsigned int)dev->resources->res->count_crtcs) { + fprintf(stderr, "CRTC %u not found\n", c->crtc); + return -1; + } + for (i = 0; i < dev->resources->plane_res->count_planes && !plane_id; i++) { ovr = dev->resources->planes[i].plane; if (!ovr) continue; - if ((ovr->possible_crtcs & (1 << c->pipe)) && !ovr->crtc_id) + if ((ovr->possible_crtcs & (1 << pipe)) && !ovr->crtc_id) plane_id = ovr->plane_id; } -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 14/23] modetest: Remove the struct connector_arg encoder field
The field is no needed, make it a local variable where used. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index a1a683f..2c3e093 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -638,7 +638,6 @@ struct connector_arg { char format_str[5]; unsigned int fourcc; drmModeModeInfo *mode; - drmModeEncoder *encoder; int crtc; unsigned int fb_id[2], current_fb_id; struct timeval start; @@ -659,6 +658,7 @@ struct plane_arg { static void connector_find_mode(struct device *dev, struct connector_arg *c) { drmModeConnector *connector; + drmModeEncoder *encoder; int i, j; /* First, find the connector & mode */ @@ -692,16 +692,16 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) /* Now get the encoder */ for (i = 0; i < dev->resources->res->count_encoders; i++) { - c->encoder = dev->resources->encoders[i].encoder; - if (!c->encoder) + encoder = dev->resources->encoders[i].encoder; + if (!encoder) continue; - if (c->encoder->encoder_id == connector->encoder_id) + if (encoder->encoder_id == connector->encoder_id) break; } if (c->crtc == -1) - c->crtc = c->encoder->crtc_id; + c->crtc = encoder->crtc_id; } /* - -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 15/23] modetest: Store the crtc in the connector_arg structure
This prepares the code for the split in separate functions of CRTC and planes setup. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 58 ++- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 2c3e093..6fbaf09 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -634,11 +634,12 @@ error: */ struct connector_arg { uint32_t id; + uint32_t crtc_id; char mode_str[64]; char format_str[5]; unsigned int fourcc; drmModeModeInfo *mode; - int crtc; + struct crtc *crtc; unsigned int fb_id[2], current_fb_id; struct timeval start; @@ -690,18 +691,33 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) return; } - /* Now get the encoder */ - for (i = 0; i < dev->resources->res->count_encoders; i++) { - encoder = dev->resources->encoders[i].encoder; - if (!encoder) - continue; + /* If the CRTC ID was specified, get the corresponding CRTC. Otherwise +* locate a CRTC that can be attached to the connector. +*/ + if (c->crtc_id == (uint32_t)-1) { + for (i = 0; i < dev->resources->res->count_encoders; i++) { + encoder = dev->resources->encoders[i].encoder; + if (!encoder) + continue; + + if (encoder->encoder_id == connector->encoder_id) { + c->crtc_id = encoder->crtc_id; + break; + } + } + } + + if (c->crtc_id == (uint32_t)-1) + return; - if (encoder->encoder_id == connector->encoder_id) + for (i = 0; i < dev->resources->res->count_crtcs; i++) { + struct crtc *crtc = &dev->resources->crtcs[i]; + + if (c->crtc_id == crtc->crtc->crtc_id) { + c->crtc = crtc; break; + } } - - if (c->crtc == -1) - c->crtc = encoder->crtc_id; } /* - @@ -796,7 +812,7 @@ page_flip_handler(int fd, unsigned int frame, else new_fb_id = c->fb_id[0]; - drmModePageFlip(fd, c->crtc, new_fb_id, + drmModePageFlip(fd, c->crtc_id, new_fb_id, DRM_MODE_PAGE_FLIP_EVENT, c); c->current_fb_id = new_fb_id; c->swap_count++; @@ -826,14 +842,14 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) * CRTC index first, then iterate over available planes. */ for (i = 0; i < (unsigned int)dev->resources->res->count_crtcs; i++) { - if (c->crtc == (int)dev->resources->res->crtcs[i]) { + if (c->crtc_id == dev->resources->res->crtcs[i]) { pipe = i; break; } } if (pipe == (unsigned int)dev->resources->res->count_crtcs) { - fprintf(stderr, "CRTC %u not found\n", c->crtc); + fprintf(stderr, "CRTC %u not found\n", c->crtc_id); return -1; } @@ -847,7 +863,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) } if (!plane_id) { - fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc); + fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc_id); return -1; } @@ -878,7 +894,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) crtc_h = p->h; /* note src coords (last 4 args) are in Q16 format */ - if (drmModeSetPlane(dev->fd, plane_id, c->crtc, p->fb_id, + if (drmModeSetPlane(dev->fd, plane_id, c->crtc_id, p->fb_id, plane_flags, crtc_x, crtc_y, crtc_w, crtc_h, 0, 0, p->w << 16, p->h << 16)) { fprintf(stderr, "failed to enable plane: %s\n", @@ -886,7 +902,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) return -1; } - ovr->crtc_id = c->crtc; + ovr->crtc_id = c->crtc_id; return 0; } @@ -930,9 +946,9 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count, continue; printf("setting mode %s@%s on connector %d, crtc %d\n", - c[i].mode_str, c[i].format_str, c[i].id, c[i].crtc); + c[i].mode_str, c[i].format_str, c[i].id, c[i].crtc_id); - ret = drmModeSetCrtc(dev->fd, c[i].crtc, fb_id, x, 0, + ret = drmModeSetCrtc(dev->fd
[PATCH v6 16/23] modetest: Store the mode in the crtc structure
This prepares the code for the split in separate functions of CRTC and planes setup. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 6fbaf09..3de611e 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -62,6 +62,7 @@ struct crtc { drmModeCrtc *crtc; drmModeObjectProperties *props; drmModePropertyRes **props_info; + drmModeModeInfo *mode; }; struct encoder { @@ -524,6 +525,7 @@ static void free_resources(struct resources *res) static struct resources *get_resources(struct device *dev) { struct resources *res; + int i; res = malloc(sizeof *res); if (res == 0) @@ -598,6 +600,9 @@ static struct resources *get_resources(struct device *dev) get_properties(res, res, crtc, CRTC); get_properties(res, res, connector, CONNECTOR); + for (i = 0; i < res->res->count_crtcs; ++i) + res->crtcs[i].mode = &res->crtcs[i].crtc->mode; + res->plane_res = drmModeGetPlaneResources(dev->fd); if (!res->plane_res) { fprintf(stderr, "drmModeGetPlaneResources failed: %s\n", @@ -714,6 +719,7 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) struct crtc *crtc = &dev->resources->crtcs[i]; if (c->crtc_id == crtc->crtc->crtc_id) { + crtc->mode = c->mode; c->crtc = crtc; break; } @@ -884,8 +890,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) if (!p->has_position) { /* Default to the middle of the screen */ - crtc_x = (c->mode->hdisplay - p->w) / 2; - crtc_y = (c->mode->vdisplay - p->h) / 2; + crtc_x = (c->crtc->mode->hdisplay - p->w) / 2; + crtc_y = (c->crtc->mode->vdisplay - p->h) / 2; } else { crtc_x = p->x; crtc_y = p->y; -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 18/23] modetest: Split mode setting and plane setup
There's not reason to require setting a mode to test planes. Split the two operations. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 104 -- 1 file changed, 72 insertions(+), 32 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 1a48cc3..93a0864 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -101,6 +101,14 @@ struct device { struct resources *resources; struct kms_driver *kms; + + struct { + unsigned int width; + unsigned int height; + + unsigned int fb_id; + struct kms_bo *bo; + } mode; }; #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) @@ -840,7 +848,7 @@ static int set_plane(struct device *dev, struct plane_arg *p) struct kms_bo *plane_bo; uint32_t plane_flags = 0; int crtc_x, crtc_y, crtc_w, crtc_h; - struct crtc *crtc; + struct crtc *crtc = NULL; unsigned int pipe; unsigned int i; @@ -915,36 +923,37 @@ static int set_plane(struct device *dev, struct plane_arg *p) return 0; } -static void set_mode(struct device *dev, struct connector_arg *c, int count, -struct plane_arg *p, int plane_count, int page_flip) +static void set_mode(struct device *dev, struct connector_arg *c, unsigned int count) { - struct kms_bo *bo, *other_bo; - unsigned int fb_id, other_fb_id; - int i, j, ret, width, height, x; uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */ - drmEventContext evctx; + unsigned int fb_id; + struct kms_bo *bo; + unsigned int i; + int ret, x; + + dev->mode.width = 0; + dev->mode.height = 0; - width = 0; - height = 0; for (i = 0; i < count; i++) { connector_find_mode(dev, &c[i]); if (c[i].mode == NULL) continue; - width += c[i].mode->hdisplay; - if (height < c[i].mode->vdisplay) - height = c[i].mode->vdisplay; + dev->mode.width += c[i].mode->hdisplay; + if (dev->mode.height < c[i].mode->vdisplay) + dev->mode.height = c[i].mode->vdisplay; } - bo = create_test_buffer(dev->kms, c->fourcc, width, height, handles, - pitches, offsets, PATTERN_SMPTE); + bo = create_test_buffer(dev->kms, c->fourcc, + dev->mode.width, dev->mode.height, + handles, pitches, offsets, PATTERN_SMPTE); if (bo == NULL) return; - ret = drmModeAddFB2(dev->fd, width, height, c->fourcc, + ret = drmModeAddFB2(dev->fd, dev->mode.width, dev->mode.height, c->fourcc, handles, pitches, offsets, &fb_id, 0); if (ret) { fprintf(stderr, "failed to add fb (%ux%u): %s\n", - width, height, strerror(errno)); + dev->mode.width, dev->mode.height, strerror(errno)); return; } @@ -968,24 +977,39 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count, fprintf(stderr, "failed to set mode: %s\n", strerror(errno)); return; } - - /* if we have a plane/overlay to show, set that up now: */ - for (j = 0; j < plane_count; j++) - if (p[j].crtc_id == c[i].crtc_id) - if (set_plane(dev, &p[j])) - return; } - if (!page_flip) - return; + dev->mode.bo = bo; + dev->mode.fb_id = fb_id; +} + +static void set_planes(struct device *dev, struct plane_arg *p, unsigned int count) +{ + unsigned int i; + + /* set up planes/overlays */ + for (i = 0; i < count; i++) + if (set_plane(dev, &p[i])) + return; +} + +static void test_page_flip(struct device *dev, struct connector_arg *c, unsigned int count) +{ + uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */ + unsigned int other_fb_id; + struct kms_bo *other_bo; + drmEventContext evctx; + unsigned int i; + int ret; - other_bo = create_test_buffer(dev->kms, c->fourcc, width, height, handles, - pitches, offsets, PATTERN_PLAIN); + other_bo = create_test_buffer(dev->kms, c->fourcc, + dev->mode.width, dev->mode.height, + handles, pitches, offsets, PATTERN_PLAIN); if (other_bo == NULL) return; - ret = drmModeAddFB2(dev->fd, width, height, c->fourcc, handles, pitches, offsets, -
[PATCH v6 17/23] modetest: Give the CRTC ID to the -P option
Planes are associated with CRTCs, not connectors. Don't try to be too clever, use the CRTC ID in the -P option. This prepares for splitting CRTC and planes setup. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 32 +--- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 3de611e..1a48cc3 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -652,7 +652,7 @@ struct connector_arg { }; struct plane_arg { - uint32_t con_id; /* the id of connector to bind to */ + uint32_t crtc_id; /* the id of CRTC to bind to */ bool has_position; int32_t x, y; uint32_t w, h; @@ -832,8 +832,7 @@ page_flip_handler(int fd, unsigned int frame, } } -static int -set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) +static int set_plane(struct device *dev, struct plane_arg *p) { drmModePlane *ovr; uint32_t handles[4], pitches[4], offsets[4] = {0}; /* we only use [0] */ @@ -841,6 +840,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) struct kms_bo *plane_bo; uint32_t plane_flags = 0; int crtc_x, crtc_y, crtc_w, crtc_h; + struct crtc *crtc; unsigned int pipe; unsigned int i; @@ -848,14 +848,15 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) * CRTC index first, then iterate over available planes. */ for (i = 0; i < (unsigned int)dev->resources->res->count_crtcs; i++) { - if (c->crtc_id == dev->resources->res->crtcs[i]) { + if (p->crtc_id == dev->resources->res->crtcs[i]) { + crtc = &dev->resources->crtcs[i]; pipe = i; break; } } - if (pipe == (unsigned int)dev->resources->res->count_crtcs) { - fprintf(stderr, "CRTC %u not found\n", c->crtc_id); + if (!crtc) { + fprintf(stderr, "CRTC %u not found\n", p->crtc_id); return -1; } @@ -869,7 +870,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) } if (!plane_id) { - fprintf(stderr, "no unused plane available for CRTC %u\n", c->crtc_id); + fprintf(stderr, "no unused plane available for CRTC %u\n", + crtc->crtc->crtc_id); return -1; } @@ -890,8 +892,8 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) if (!p->has_position) { /* Default to the middle of the screen */ - crtc_x = (c->crtc->mode->hdisplay - p->w) / 2; - crtc_y = (c->crtc->mode->vdisplay - p->h) / 2; + crtc_x = (crtc->mode->hdisplay - p->w) / 2; + crtc_y = (crtc->mode->vdisplay - p->h) / 2; } else { crtc_x = p->x; crtc_y = p->y; @@ -900,7 +902,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) crtc_h = p->h; /* note src coords (last 4 args) are in Q16 format */ - if (drmModeSetPlane(dev->fd, plane_id, c->crtc_id, p->fb_id, + if (drmModeSetPlane(dev->fd, plane_id, crtc->crtc->crtc_id, p->fb_id, plane_flags, crtc_x, crtc_y, crtc_w, crtc_h, 0, 0, p->w << 16, p->h << 16)) { fprintf(stderr, "failed to enable plane: %s\n", @@ -908,7 +910,7 @@ set_plane(struct device *dev, struct connector_arg *c, struct plane_arg *p) return -1; } - ovr->crtc_id = c->crtc_id; + ovr->crtc_id = crtc->crtc->crtc_id; return 0; } @@ -969,8 +971,8 @@ static void set_mode(struct device *dev, struct connector_arg *c, int count, /* if we have a plane/overlay to show, set that up now: */ for (j = 0; j < plane_count; j++) - if (p[j].con_id == c[i].id) - if (set_plane(dev, &c[i], &p[j])) + if (p[j].crtc_id == c[i].crtc_id) + if (set_plane(dev, &p[j])) return; } @@ -1099,7 +1101,7 @@ static int parse_plane(struct plane_arg *plane, const char *p) memset(plane, 0, sizeof *plane); - plane->con_id = strtoul(p, &end, 10); + plane->crtc_id = strtoul(p, &end, 10); if (*end != ':') return -EINVAL; @@ -1161,7 +1163,7 @@ static void usage(char *name) fprintf(stderr, "\t-p\tlist CRTCs and planes (pipes)\n"); fprintf(stderr, "\n Test options:\n\n"); - fprintf(stderr, "\t-P :x[++][@]\tset a plane\n"); + fprintf(stderr, "\t-P :x[++][@]\tset a plane\n"); fprintf(stderr, "\t-s [@]:[@]\tset a mode\n");
[PATCH v6 19/23] modetest: Rename struct connector_arg to struct pipe_arg
This prepares the code for handling multiple connectors in a single pipeline in a cloned configuration. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 162 -- 1 file changed, 85 insertions(+), 77 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 93a0864..922d9b0 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -635,7 +635,7 @@ error: } /* - - * Connectors and planes + * Pipes and planes */ /* @@ -645,8 +645,8 @@ error: * Then you need to find the encoder attached to that connector so you * can bind it with a free crtc. */ -struct connector_arg { - uint32_t id; +struct pipe_arg { + uint32_t con_id; uint32_t crtc_id; char mode_str[64]; char format_str[5]; @@ -669,14 +669,14 @@ struct plane_arg { unsigned int fourcc; }; -static void connector_find_mode(struct device *dev, struct connector_arg *c) +static void pipe_find_mode(struct device *dev, struct pipe_arg *pipe) { drmModeConnector *connector; drmModeEncoder *encoder; int i, j; /* First, find the connector & mode */ - c->mode = NULL; + pipe->mode = NULL; for (i = 0; i < dev->resources->res->count_connectors; i++) { connector = dev->resources->connectors[i].connector; if (!connector) @@ -685,50 +685,50 @@ static void connector_find_mode(struct device *dev, struct connector_arg *c) if (!connector->count_modes) continue; - if (connector->connector_id != c->id) + if (connector->connector_id != pipe->con_id) continue; for (j = 0; j < connector->count_modes; j++) { - c->mode = &connector->modes[j]; - if (!strcmp(c->mode->name, c->mode_str)) + pipe->mode = &connector->modes[j]; + if (!strcmp(pipe->mode->name, pipe->mode_str)) break; } /* Found it, break out */ - if (c->mode) + if (pipe->mode) break; } - if (!c->mode) { - fprintf(stderr, "failed to find mode \"%s\"\n", c->mode_str); + if (!pipe->mode) { + fprintf(stderr, "failed to find mode \"%s\"\n", pipe->mode_str); return; } /* If the CRTC ID was specified, get the corresponding CRTC. Otherwise * locate a CRTC that can be attached to the connector. */ - if (c->crtc_id == (uint32_t)-1) { + if (pipe->crtc_id == (uint32_t)-1) { for (i = 0; i < dev->resources->res->count_encoders; i++) { encoder = dev->resources->encoders[i].encoder; if (!encoder) continue; if (encoder->encoder_id == connector->encoder_id) { - c->crtc_id = encoder->crtc_id; + pipe->crtc_id = encoder->crtc_id; break; } } } - if (c->crtc_id == (uint32_t)-1) + if (pipe->crtc_id == (uint32_t)-1) return; for (i = 0; i < dev->resources->res->count_crtcs; i++) { struct crtc *crtc = &dev->resources->crtcs[i]; - if (c->crtc_id == crtc->crtc->crtc_id) { - crtc->mode = c->mode; - c->crtc = crtc; + if (pipe->crtc_id == crtc->crtc->crtc_id) { + crtc->mode = pipe->mode; + pipe->crtc = crtc; break; } } @@ -815,28 +815,28 @@ static void page_flip_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec, void *data) { - struct connector_arg *c; + struct pipe_arg *pipe; unsigned int new_fb_id; struct timeval end; double t; - c = data; - if (c->current_fb_id == c->fb_id[0]) - new_fb_id = c->fb_id[1]; + pipe = data; + if (pipe->current_fb_id == pipe->fb_id[0]) + new_fb_id = pipe->fb_id[1]; else - new_fb_id = c->fb_id[0]; + new_fb_id = pipe->fb_id[0]; - drmModePageFlip(fd, c->crtc_id, new_fb_id, - DRM_MODE_PAGE_FLIP_EVENT, c); - c->current_fb_id = new_fb_id; - c->swap_count++; - if (c->swap_count == 60) { + drmModePageFlip(fd, pipe->crtc_id, new_fb_id, + DRM_MODE_PAGE_FLIP_EVENT, pipe); + pipe->current_fb_id = new_fb_id; + pipe->swap_count++; + if (pipe->swap_count == 60)
[PATCH v6 20/23] modetest: Support pipes with multiple connectors
The -s argument can now take a list of connectors. Configure all of them in cloned mode using a single CRTC. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 211 ++ 1 file changed, 157 insertions(+), 54 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 922d9b0..450c86d 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -40,6 +40,7 @@ #include "config.h" #include +#include #include #include #include @@ -634,6 +635,34 @@ error: return NULL; } +static drmModeConnector *get_connector_by_id(struct device *dev, uint32_t id) +{ + drmModeConnector *connector; + int i; + + for (i = 0; i < dev->resources->res->count_connectors; i++) { + connector = dev->resources->connectors[i].connector; + if (connector && connector->connector_id == id) + return connector; + } + + return NULL; +} + +static drmModeEncoder *get_encoder_by_id(struct device *dev, uint32_t id) +{ + drmModeEncoder *encoder; + int i; + + for (i = 0; i < dev->resources->res->count_encoders; i++) { + encoder = dev->resources->encoders[i].encoder; + if (encoder && encoder->encoder_id == id) + return encoder; + } + + return NULL; +} + /* - * Pipes and planes */ @@ -646,7 +675,8 @@ error: * can bind it with a free crtc. */ struct pipe_arg { - uint32_t con_id; + uint32_t *con_ids; + unsigned int num_cons; uint32_t crtc_id; char mode_str[64]; char format_str[5]; @@ -669,69 +699,114 @@ struct plane_arg { unsigned int fourcc; }; -static void pipe_find_mode(struct device *dev, struct pipe_arg *pipe) +static drmModeModeInfo * +connector_find_mode(struct device *dev, uint32_t con_id, const char *mode_str) { drmModeConnector *connector; - drmModeEncoder *encoder; - int i, j; + drmModeModeInfo *mode; + int i; - /* First, find the connector & mode */ - pipe->mode = NULL; - for (i = 0; i < dev->resources->res->count_connectors; i++) { - connector = dev->resources->connectors[i].connector; + connector = get_connector_by_id(dev, con_id); + if (!connector || !connector->count_modes) + return NULL; + + for (i = 0; i < connector->count_modes; i++) { + mode = &connector->modes[i]; + if (!strcmp(mode->name, mode_str)) + return mode; + } + + return NULL; +} + +static struct crtc *pipe_find_crtc(struct device *dev, struct pipe_arg *pipe) +{ + uint32_t possible_crtcs = ~0; + uint32_t active_crtcs = 0; + unsigned int crtc_idx; + unsigned int i; + int j; + + for (i = 0; i < pipe->num_cons; ++i) { + drmModeConnector *connector; + drmModeEncoder *encoder; + + connector = get_connector_by_id(dev, pipe->con_ids[i]); if (!connector) - continue; + return NULL; - if (!connector->count_modes) - continue; + encoder = get_encoder_by_id(dev, connector->encoder_id); + if (!encoder) + return NULL; - if (connector->connector_id != pipe->con_id) - continue; + possible_crtcs &= encoder->possible_crtcs; - for (j = 0; j < connector->count_modes; j++) { - pipe->mode = &connector->modes[j]; - if (!strcmp(pipe->mode->name, pipe->mode_str)) + for (j = 0; j < dev->resources->res->count_crtcs; ++j) { + drmModeCrtc *crtc = dev->resources->crtcs[j].crtc; + if (crtc && crtc->crtc_id == encoder->crtc_id) { + active_crtcs |= 1 << j; break; + } } - - /* Found it, break out */ - if (pipe->mode) - break; } - if (!pipe->mode) { - fprintf(stderr, "failed to find mode \"%s\"\n", pipe->mode_str); - return; + if (!possible_crtcs) + return NULL; + + /* Return the first possible and active CRTC if one exists, or the first +* possible CRTC otherwise. +*/ + if (possible_crtcs & active_crtcs) + crtc_idx = ffs(possible_crtcs & active_crtcs); + else + crtc_idx = ffs(possible_crtcs); + + return &dev->resources->crtcs[crtc_idx - 1]; +} + +static int pipe_find_crtc_and_mode(struct device *dev, struct pipe_arg *pipe) +{ + drmModeModeInfo *mode; + int i; + +
[PATCH v6 21/23] modetest: Try all possible encoders for a connector
When building the pipeline, instead of using only the encoders attached to a connector, take all possible encoders into account to locate a CRTC. Signed-off-by: Laurent Pinchart --- tests/modetest/modetest.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c index 450c86d..e49838b 100644 --- a/tests/modetest/modetest.c +++ b/tests/modetest/modetest.c @@ -635,6 +635,19 @@ error: return NULL; } +static int get_crtc_index(struct device *dev, uint32_t id) +{ + int i; + + for (i = 0; i < dev->resources->res->count_crtcs; ++i) { + drmModeCrtc *crtc = dev->resources->crtcs[i].crtc; + if (crtc && crtc->crtc_id == id) + return i; + } + + return -1; +} + static drmModeConnector *get_connector_by_id(struct device *dev, uint32_t id) { drmModeConnector *connector; @@ -728,26 +741,28 @@ static struct crtc *pipe_find_crtc(struct device *dev, struct pipe_arg *pipe) int j; for (i = 0; i < pipe->num_cons; ++i) { + uint32_t crtcs_for_connector = 0; drmModeConnector *connector; drmModeEncoder *encoder; + int idx; connector = get_connector_by_id(dev, pipe->con_ids[i]); if (!connector) return NULL; - encoder = get_encoder_by_id(dev, connector->encoder_id); - if (!encoder) - return NULL; + for (j = 0; j < connector->count_encoders; ++j) { + encoder = get_encoder_by_id(dev, connector->encoders[j]); + if (!encoder) + continue; - possible_crtcs &= encoder->possible_crtcs; + crtcs_for_connector |= encoder->possible_crtcs; - for (j = 0; j < dev->resources->res->count_crtcs; ++j) { - drmModeCrtc *crtc = dev->resources->crtcs[j].crtc; - if (crtc && crtc->crtc_id == encoder->crtc_id) { - active_crtcs |= 1 << j; - break; - } + idx = get_crtc_index(dev, encoder->crtc_id); + if (idx >= 0) + active_crtcs |= 1 << idx; } + + possible_crtcs &= crtcs_for_connector; } if (!possible_crtcs) -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 22/23] modetest: Fix line stride in SMPTE YUV packet pattern generator
The line stride passed to the function is expressed in bytes, there's no need to multiply it by 2. Signed-off-by: Laurent Pinchart --- tests/modetest/buffers.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index 1ca3be5..1575bf6 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -337,13 +337,13 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, for (y = 0; y < height * 6 / 9; ++y) { for (x = 0; x < width; ++x) y_mem[2*x] = colors_top[x * 7 / width].y; - y_mem += stride * 2; + y_mem += stride; } for (; y < height * 7 / 9; ++y) { for (x = 0; x < width; ++x) y_mem[2*x] = colors_middle[x * 7 / width].y; - y_mem += stride * 2; + y_mem += stride; } for (; y < height; ++y) { @@ -354,7 +354,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, / (width / 7) + 4].y; for (; x < width; ++x) y_mem[2*x] = colors_bottom[7].y; - y_mem += stride * 2; + y_mem += stride; } /* Chroma */ @@ -363,7 +363,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, c_mem[2*x+u] = colors_top[x * 7 / width].u; c_mem[2*x+v] = colors_top[x * 7 / width].v; } - c_mem += stride * 2; + c_mem += stride; } for (; y < height * 7 / 9; ++y) { @@ -371,7 +371,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, c_mem[2*x+u] = colors_middle[x * 7 / width].u; c_mem[2*x+v] = colors_middle[x * 7 / width].v; } - c_mem += stride * 2; + c_mem += stride; } for (; y < height; ++y) { @@ -389,7 +389,7 @@ fill_smpte_yuv_packed(const struct yuv_info *yuv, unsigned char *mem, c_mem[2*x+u] = colors_bottom[7].u; c_mem[2*x+v] = colors_bottom[7].v; } - c_mem += stride * 2; + c_mem += stride; } } -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 23/23] modetest: Allocate NV buffers large enough for the two planes
Multiple the image height by 1.5 for NV12/NV21 and by 2 for NV16/NV61 to make room for the chroma plane. Signed-off-by: Laurent Pinchart --- tests/modetest/buffers.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index 1575bf6..8206ce3 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -1038,12 +1038,29 @@ create_test_buffer(struct kms_driver *kms, unsigned int format, unsigned int handles[4], unsigned int pitches[4], unsigned int offsets[4], enum fill_pattern pattern) { + unsigned int virtual_height; struct kms_bo *bo; void *planes[3] = { 0, }; void *virtual; int ret; - bo = allocate_buffer(kms, width, height, &pitches[0]); + switch (format) { + case DRM_FORMAT_NV12: + case DRM_FORMAT_NV21: + virtual_height = height * 3 / 2; + break; + + case DRM_FORMAT_NV16: + case DRM_FORMAT_NV61: + virtual_height = height * 2; + break; + + default: + virtual_height = height; + break; + } + + bo = allocate_buffer(kms, width, virtual_height, &pitches[0]); if (!bo) return NULL; -- 1.8.1.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/6] fb refcount and crtc helper patches
Hi all, Russell found a refcount bug in my fb refcounting conversion, but to also fix i915.ko I've opted for a slightly different approach. While at it I've thought it would be good to backport some of the semantic changes we've implented in i915's ->set_config callback since we've forked our own modeset infrastructure. The intel code receive lots more refactorings and cleanups (most of them right after the code fork) which would be simple to backport, but I'm kinda swamped (hint, hint, ...). But these few patches are the important bits. Cheers, Daniel Daniel Vetter (6): drm/crtc-helpers: Enforce sane set_config api drm/crtc-helper: no need to check for fb->depth/bpp drm/crtc-helper: explicit DPMS on after modeset drm/crtc-helper: don't disable disconnected outputs drm: check that ->set_config properly updates the fb drm: fix fb leak in setcrtc drivers/gpu/drm/drm_crtc.c| 25 +++- drivers/gpu/drm/drm_crtc_helper.c | 47 + include/drm/drm_crtc.h|4 3 files changed, 45 insertions(+), 31 deletions(-) -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6] drm/crtc-helpers: Enforce sane set_config api
There's no point in trying to clean up after driver-bugs, so just blow up. Furthermore it's an interface abuse to set no mode but have an fb and aslo to try to set an fb without enough connectors. These two spefici cases of interface abuse have been committed by the fb helper, but that's been fixed meanwhile in commit 7e53f3a423146745a4e4bb93362d488dfad502a8 Author: Daniel Vetter Date: Mon Jan 21 10:52:17 2013 +0100 drm/fb-helper: fixup set_config semantics The i915 driver has been shipping since a while with these BUGs with no reports, so should be save. Note that this drops an ugly case where we clear crtc->fb behind the upper levels back and so cause a refcounting mayhem, which Russell Kins spotted while trying to hunt down a drm framebuffer leak. Reported-by: Russell King Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc_helper.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index f554516..e57cfec 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -565,14 +565,13 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) DRM_DEBUG_KMS("\n"); - if (!set) - return -EINVAL; + BUG_ON(!set); + BUG_ON(!set->crtc); + BUG_ON(!set->crtc->helper_private); - if (!set->crtc) - return -EINVAL; - - if (!set->crtc->helper_private) - return -EINVAL; + /* Enforce sane interface api - has been abused by the fb helper. */ + BUG_ON(!set->mode && set->fb); + BUG_ON(set->fb && set->num_connectors == 0); crtc_funcs = set->crtc->helper_private; -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] drm/crtc-helper: no need to check for fb->depth/bpp
... since we already check for fb->pixel_format, which encodes all this. The other two fields are only for backwards compat of older drivers (and we might want to look into eventually just killing them). Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc_helper.c |5 - 1 file changed, 5 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index e57cfec..1ace9c1 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -645,11 +645,6 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) mode_changed = true; } else if (set->fb == NULL) { mode_changed = true; - } else if (set->fb->depth != set->crtc->fb->depth) { - mode_changed = true; - } else if (set->fb->bits_per_pixel != - set->crtc->fb->bits_per_pixel) { - mode_changed = true; } else if (set->fb->pixel_format != set->crtc->fb->pixel_format) { mode_changed = true; -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/6] drm/crtc-helper: explicit DPMS on after modeset
Atm the crtc helper implementation of set_config has really inconsisten semantics: If just an fb update is good enough, dpms state will be left as-is, but if we do a full modeset we force everything to dpms on. This change has already been applied to the i915 modeset code in commit e3de42b68478a8c95dd27520e9adead2af9477a5 Author: Imre Deak Date: Fri May 3 19:44:07 2013 +0200 drm/i915: force full modeset if the connector is in DPMS OFF mode which according to Greg KH seems to aim for a new record in most Bugzilla: links in a commit message. The history of this dpms forcing is pretty interesting. This patch here is an almost-revert of commit 811aaa55ba21ab37407018cfc01770d6b037d3fb Author: Keith Packard Date: Thu Feb 3 16:57:28 2011 -0800 drm: Only set DPMS ON when actually configuring a mode which fixed the bug of trying to dpms on disabled outputs, but introduced the new discrepancy between an fb update only and full modesets. The actual introduction of this goes back to commit bf9dc102e284a5aa78c73fc9d72e11d5ccd8669f Author: Keith Packard Date: Fri Nov 26 10:45:58 2010 -0800 drm: Set connector DPMS status to ON in drm_crtc_helper_set_config And if you'd dig around in the i915 driver code there's even more fun around forcing dpms on and losing our heads and temper of the resulting inconsistencies. Especially the DP re-training code had tons of funny stuff in it. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc_helper.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 1ace9c1..738a429 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -754,12 +754,6 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) ret = -EINVAL; goto fail; } - DRM_DEBUG_KMS("Setting connector DPMS state to on\n"); - for (i = 0; i < set->num_connectors; i++) { - DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id, - drm_get_connector_name(set->connectors[i])); - set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON); - } } drm_helper_disable_unused_functions(dev); } else if (fb_changed) { @@ -777,6 +771,22 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set) } } + /* +* crtc set_config helpers implicit set the crtc and all connected +* encoders to DPMS on for a full mode set. But for just an fb update it +* doesn't do that. To not confuse userspace, do an explicit DPMS_ON +* unconditionally. This will also ensure driver internal dpms state is +* consistent again. +*/ + if (set->crtc->enabled) { + DRM_DEBUG_KMS("Setting connector DPMS state to on\n"); + for (i = 0; i < set->num_connectors; i++) { + DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id, + drm_get_connector_name(set->connectors[i])); + set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON); + } + } + kfree(save_connectors); kfree(save_encoders); kfree(save_crtcs); -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] drm/crtc-helper: don't disable disconnected outputs
This has originally been added in commit a3a0544b2c84e1d7a2022b558ecf66d8c6a8dd93 Author: Dave Airlie Date: Mon Aug 31 15:16:30 2009 +1000 drm/kms: add explicit encoder disable function and detach harder. I think it's the wrong thing to do for a few reasons: - It's policy in the kernel. Userspace gets hotplug events when an output disconnects, and it can inquire about all the routing that is already set up. If userspace wants to disable a disconnected output it can simply do so itself. - The reason for adding it given in the commit message was to improve use of shared encoders. But the disable_unused_functions call only happens after the modeset has been completed, so it won't help too much in making the modest succeed. - We (at least in drm/i915, but iirc all other drivers work the same) ensure that if the user accidentally yanks the cable and then replugs, the same configuration will keep on working without any userspace actions required. For most outputs that's the case by default, and DP can have the same semantics with a simple re-train handler fired off from the hpd replug event. This breaks it since if the user is unlucky and hits the mouse, resulting in a panning of e.g. the integrated panel that modeset might kill the accidentally yanked output. Which isn't too great. i915 has applied this behaviour change as part of the bit modeset review, thus far without resulting in an angry crowd with pitchforks: commit b0a2658acb5bf9ca86b4aab011b7106de3af0add Author: Daniel Vetter Date: Tue Dec 18 09:37:54 2012 +0100 drm/i915: don't disable disconnected outputs Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc_helper.c |7 --- 1 file changed, 7 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 738a429..917c803 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -279,13 +279,6 @@ void drm_helper_disable_unused_functions(struct drm_device *dev) struct drm_connector *connector; struct drm_crtc *crtc; - list_for_each_entry(connector, &dev->mode_config.connector_list, head) { - if (!connector->encoder) - continue; - if (connector->status == connector_status_disconnected) - connector->encoder = NULL; - } - list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { if (!drm_helper_encoder_in_use(encoder)) { drm_encoder_disable(encoder); -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/6] drm: check that ->set_config properly updates the fb
Historically drm lacked fb refcounting, so the updating of crtc->fb was done by the lower levels at a point convenient to get their own refcounting (e.g. refcounts for the underlying gem bo, pinning refcounts) right. With the introduction of refcounted fbs the drm core handled the fb refcounts, but still relied on drivers to update the crtc->fb pointer (this approach required the least invasive changes in drivers). Enforce this contract with a WARN_ON. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index baee575..bcee25a 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1939,6 +1939,9 @@ int drm_mode_set_config_internal(struct drm_mode_set *set) ret = crtc->funcs->set_config(set); if (ret == 0) { + /* crtc->fb must be updated by ->set_config, enforces this. */ + WARN_ON(fb != crtc->fb); + if (old_fb) drm_framebuffer_unreference(old_fb); if (fb) -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6] drm: fix fb leak in setcrtc
Drivers are allowed (actually have to) disable unrelated crtcs in their ->set_config callback (when we steal all the connectors from that crtc). If they do that they'll clear crtc->fb to NULL. Which results in a refcount leak, since the drm core is keeping track of that reference. To fix this track the old fb of all crtcs and adjust references for all of them. Of course, since we only hold an additional reference for the fb for the current crtc we need to increase refcounts before we drop the old one. This approach has the benefit that it inches us a bit closer to an atomic modeset world, where we want to update the config of all crtcs in one step. This regression has been introduce in the framebuffer refcount conversion, specifically in commit b0d1232589df5575c5971224ac4cb30e7e525884 Author: Daniel Vetter Date: Tue Dec 11 01:07:12 2012 +0100 drm: refcounting for crtc framebuffers Reported-by: Russell King Cc: Russell King Cc: sta...@vger.kernel.org Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_crtc.c | 22 -- include/drm/drm_crtc.h |4 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index bcee25a..3486a00 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1931,21 +1931,31 @@ out: int drm_mode_set_config_internal(struct drm_mode_set *set) { struct drm_crtc *crtc = set->crtc; - struct drm_framebuffer *fb, *old_fb; + struct drm_framebuffer *fb; + struct drm_crtc *tmp; int ret; - old_fb = crtc->fb; + /* +* NOTE: ->set_config can also disable other crtcs (if we steal all +* connectors from it), hence we need to refcount the fbs across all +* crtcs. Atomic modeset will have saner semantics ... +*/ + list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head) + tmp->old_fb = tmp->fb; + fb = set->fb; ret = crtc->funcs->set_config(set); if (ret == 0) { /* crtc->fb must be updated by ->set_config, enforces this. */ WARN_ON(fb != crtc->fb); + } - if (old_fb) - drm_framebuffer_unreference(old_fb); - if (fb) - drm_framebuffer_reference(fb); + list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head) { + if (tmp->fb) + drm_framebuffer_reference(tmp->fb); + if (tmp->old_fb) + drm_framebuffer_unreference(tmp->old_fb); } return ret; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 53c33e2..07d1dbbb 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -409,6 +409,10 @@ struct drm_crtc { /* framebuffer the connector is currently bound to */ struct drm_framebuffer *fb; + /* Temporary tracking of the old fb while a modeset is ongoing. Used +* by drm_mode_set_config_internal to implement correct refcounting. */ + struct drm_framebuffer *old_fb; + bool enabled; /* Requested mode from modesetting. */ -- 1.7.10.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver
On Sat, Jun 15, 2013 at 12:15 AM, Russell King - ARM Linux wrote: > On Fri, Jun 14, 2013 at 09:50:22PM +0200, Daniel Vetter wrote: >> On Fri, Jun 14, 2013 at 4:42 PM, Russell King - ARM Linux >> wrote: >> > If you're happy with the patch I supplied, that's probably the minimal fix >> > which should go to stable kernels (I'm using 3.9 here) - this also counts >> > as a "user visible bug". It's something I've tripped over which causes >> > exhausts memory and can prevent the X server from starting up. >> > >> > If you want me to package the patch up with a commit message and sign-off.. >> >> Your patch doesn't fix drm/i915 (since we don't use the crtc helpers >> any more). And I don't think it's good to have the refcounting >> partially in the drm core and partially in drivers. > > Let me check what you mean by that. I hope you mean that the drm core, > drm helpers and drivers are individually responsible for dropping any > refcount that they obtain on any object. > > In other words, if the drm core takes a refcount on object X, then the > DRM core must be the code base which drops that refcount. Yeah, that's the idea. But since with the currently quirky drm core ->set_config callback the drivers need to drop the reference if any other crtc loses all its connectors. My patch also drops the ref for that fb in the drm core functions. The reason for that is that we want to switch to a global ->set_config function anyway (usually called atomic modeset) to push the configuration for all the crtcs into the hw in one step (or fail it completely ofc). The current approach leads to too much flickering and some funny hacks in drivers ... Of course such gradual transitions over mutliple kernel release (we're shuffling backend code in drm/i915 since about 3.7 for this) leaves a few ugly intermediat states. But I prefer to move into the overall right direction with a bit of ugliness than doing such big transitions in one big swoop. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] i915: Don't provide ACPI backlight interface if firmware expects Windows 8
On Sat, Jun 15, 2013 at 12:14:42PM +0800, Aaron Lu wrote: > On 06/15/2013 09:38 AM, Matthew Garrett wrote: > > Well, Windows 8 will only use the ACPI backlight interface if the GPU > > driver decides to, right? So the logic for deciding whether to remove > > the ACPI backlight control or not should be left up to the GPU. There's > > I don't know this. From the document I googled, Microsoft suggests under > win8, backlight should be controlled by the graphics driver for smooth > brightness level change, instead of ACPI or other methods. So it is > possible that OEM will not test the ACPI interface well and thus the > interface is likely broken. I don't see why GPU driver has any better > knowledge on which systems the firmware interface is broken or not. The vendor will presumably have tested that backlight control works - if the GPU driver uses the ACPI interface and backlight control is broken, then the vendor would fix it. > > no harm in refusing to expose a working method if there's another > > working method, but there is harm in exposing a broken one and expecting > > userspace to know the difference. > > BTW, the proposed solution is not meant to solve win8 problems alone, it > should make solving other problems easy and make individual backlight > control interface provider module independent with each other such as > the platform drivers will not need to check if ACPI video driver will > control backlight or not and can always create backlight interface(its > default priority is lower that ACPI video driver's so will not be taken > by user space by default, showing the same behavior of the current code). Sure, but it still requires the replacement of existing userspace. We need to fix the problem with existing userspace, too. > The current acpi_backlight=video/vendor kernel command line is pretty > misleading, for laptops that do not have vendor backlight interface, > adding acpi_backlight=vendor actually makes the system using the GPU's > interface. Some laptops are using this switch to work around problems in > ACPI video driver and users think they are using vendor interface. > That's why I think we need a new command line as the > backlight.force_interface=raw/firmware/platform. No, I think we need to fix the bugs that currently require users to pass options. > Instead of letting individual driver to make decisions on which > backlight interface this system should use(either in platform driver as > we currently did, see acer-wmi and asus-wmi, or GPU driver as this case), > I think we need a better and clear way to handle such things. For > example, suppose an acer laptop: vendor does not support backlight, ACPI > video's backlight interface is broken and GPU's works, to make it work, > user will need to select acer-wmi module while this module does not have > anything to do with the functionality, but simply because it serves as > the backlight manager for acer laptops. How do these machines work under Windows? Until we know the answer to that, we don't know what the correct way to handle the problem is under Linux. -- Matthew Garrett | mj...@srcf.ucam.org ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 65730] no sound via HDMI for Radeon 6850 mesa driver
https://bugs.freedesktop.org/show_bug.cgi?id=65730 --- Comment #4 from Jason --- Before I being, I must make a disclaimer that I am not a Linux genius by any means, but a moderate user, able and willing to find my way around things. I do occasionally assist one of the pcsx2 emulator devs with me applying and testing various patches for the code. Sometimes I need help, though, finding the correct file to patch, as I am not a coder. Anyways, back to the topic at hand. I'm assuming the file in question should be r600_hdmi.c? I have checked using catfish file searcher and also manually for this file. This includes the /lib/modules/3.8.0-23-generic/kernel/drivers/gpu/drm/radeon directory. All I have is radeon.ko in that directory. In other words, I don't see any trace of where r600_hdmi.c is or should be anywhere in my file system. Again I apologize for the inconvenenience. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130614/285c88b3/attachment.html>
[Bug 64201] OpenCL usage result segmentation fault on r600g with HD6850.
https://bugs.freedesktop.org/show_bug.cgi?id=64201 --- Comment #33 from Erdem U. Alt?nyurt --- Yes. I got same version... Now, the error is gone. (I clean llvm and mesa repo, updated from trunk and rebuild.) bfgminer --benchmark generate error of: [2013-06-14 03:24:24] Error -11: Building Program (clBuildProgram) int') with an expression of incompatible type 'unsigned int __attribute__((ext_vector_type(2)))' uint r = rot(W[3].x,25u)^rot(W[3].x,14u)^((W[3].x)>>3U); ^ ~~ [2013-06-14 03:24:24] Failed to init GPU thread 0, disabling device 0 [2013-06-14 03:24:24] Restarting the GPU from the "bfgminer --benchmark -v1" locks the GPU and gpu doesn't restart... Thanks. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130614/77405a83/attachment.html>
[PATCH v3] drm: Renesas R-Car Display Unit DRM driver
Hi Daniel, On Friday 07 June 2013 10:50:55 Daniel Vetter wrote: > On Fri, Jun 07, 2013 at 09:44:45AM +0200, Laurent Pinchart wrote: > > On Wednesday 05 June 2013 10:55:05 Daniel Vetter wrote: > > > On Wed, Jun 05, 2013 at 03:51:53AM +0200, Laurent Pinchart wrote: > > > > On Tuesday 04 June 2013 20:36:20 Daniel Vetter wrote: > > > > > On Tue, Jun 4, 2013 at 8:03 PM, Laurent Pinchart wrote: > > > > > > On Tuesday 04 June 2013 16:12:36 Daniel Vetter wrote: > > > > > >> On Tue, Jun 04, 2013 at 04:53:40AM +0200, Laurent Pinchart wrote: > > > [snip] > > > > > > > > >> > +static int rcar_du_vga_connector_get_modes(struct > > > > > >> > drm_connector > > > > > >> > *connector) > > > > > >> > +{ > > > > > >> > + return drm_add_modes_noedid(connector, 1280, 768); > > > > > >> > +} > > > > > >> > > > > > >> This (and the dummy detect function below) looks a bit funny, > > > > > >> since it essentially overrides the default behaviour already > > > > > >> provided by the crtc helpers. Until rcar has at least proper > > > > > >> detect support for VGA > > > > > > > > > > > > I would add that but the DDC signals are not connected on the > > > > > > boards I have access to :-/ > > > > > > > > > > > >> I'd just kill this and use the connector force support (and > > > > > >> manually adding the right resolutions). > > > > > > > > > > > > Looks like that's a candidate for better documentation... How does > > > > > > force support work ? > > > > > > > > > > Grep for DRM_FORCE_ON, iirc it can be set on the commandline, where > > > > > you can also force a specific mode. The best I could find wrt docs > > > > > is the kerneldoc for drm_mode_parse_command_line_for_connector. With > > > > > a bit more reading it looks like it's intermingled with the fbdev > > > > > helper code, but should be fairly easy to extract and used by your > > > > > driver. > > > > > > > > It makes sense to force the connector state from command line, but I'm > > > > not sure if the same mechanism is the best solution here. As the > > > > driver has no way to know the connector state, the best we can do is > > > > guess what modes are supported. I can just return 0 in the get_modes > > > > handler, but then the core will not call drm_add_modes_noedid(), and > > > > modes will need to be added manually. > > > > > > > > Is your point that for a board on which the VGA connector state can't > > > > be detected, the user should always be responsible for adding all the > > > > modes supported by the VGA monitor on the command line ? > > > > > > My point is that we already have both an established code for connected > > > outputs without EDID to add fallback modes and means to force connectors > > > to certain states. Your code here seems to reinvent that wheel, so I > > > wonder what we should/need to improve in the common code to suit your > > > needs. > > > > The currently available code might suit my needs, it might just be that I > > fail to see how to use it properly. > > > > Regarding the "code for connected outputs without EDID to add fallback > > modes" you're referring to, is that > > > > if (count == 0 && connector->status == connector_status_connected) > > count = drm_add_modes_noedid(connector, 1024, 768); > > > > in drm_helper_probe_single_connector_modes() ? That function will only be > > called if the connector status is connector_status_connected. There are > > two ways to enforce that: > > > > - returning connector_status_connected from the connector detect() > > operation, which seems to defeat the purpose of having > > connector_status_unknown completely. > > We might want to add such a default mode also for unknown, I'm not sure. > Userspace policy is to first try to light up any connected outputs, and if > there's none try to light up any unknown outputs. Not sure whether userspace > (i.e. X) will automatically add a default mode. fbcon might also handle this > less gracefully. > > Personally I'm ok with extending this to unknown, it shouldn't really hurt > (since we already try really hard not to leak unknown anywhere visible). Do you mean something like diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index f554516..9aae384 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -160,7 +160,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector, #endif count = (*connector_funcs->get_modes)(connector); - if (count == 0 && connector->status == connector_status_connected) + if (count == 0 && (connector->status == connector_status_connected || + connector->status == connector_status_unknown)) count = drm_add_modes_noedid(connector, 1024, 768); if (count == 0) goto prune; If so I can submit a proper patch. > > - setting connector->force to DRM_FORCE_ON. Are drivers allowed to do so > > themselves at initializa
[RFC PATCH] dmabuf-sync: Introduce buffer synchronization framework
Hi Russell, > -Original Message- > From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk] > Sent: Friday, June 14, 2013 2:26 AM > To: Inki Dae > Cc: maarten.lankhorst at canonical.com; daniel at ffwll.ch; robdclark at > gmail.com; > linux-fbdev at vger.kernel.org; dri-devel at lists.freedesktop.org; > kyungmin.park at samsung.com; myungjoo.ham at samsung.com; yj44.cho at > samsung.com; > linux-arm-kernel at lists.infradead.org; linux-media at vger.kernel.org > Subject: Re: [RFC PATCH] dmabuf-sync: Introduce buffer synchronization > framework > > On Thu, Jun 13, 2013 at 05:28:08PM +0900, Inki Dae wrote: > > This patch adds a buffer synchronization framework based on DMA BUF[1] > > and reservation[2] to use dma-buf resource, and based on ww-mutexes[3] > > for lock mechanism. > > > > The purpose of this framework is not only to couple cache operations, > > and buffer access control to CPU and DMA but also to provide easy-to-use > > interfaces for device drivers and potentially user application > > (not implemented for user applications, yet). And this framework can be > > used for all dma devices using system memory as dma buffer, especially > > for most ARM based SoCs. > > > > The mechanism of this framework has the following steps, > > 1. Register dmabufs to a sync object - A task gets a new sync object > and > > can add one or more dmabufs that the task wants to access. > > This registering should be performed when a device context or an > event > > context such as a page flip event is created or before CPU accesses a > shared > > buffer. > > > > dma_buf_sync_get(a sync object, a dmabuf); > > > > 2. Lock a sync object - A task tries to lock all dmabufs added in its > own > > sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid > dead > > lock issue and for race condition between CPU and CPU, CPU and DMA, > and DMA > > and DMA. Taking a lock means that others cannot access all locked > dmabufs > > until the task that locked the corresponding dmabufs, unlocks all the > locked > > dmabufs. > > This locking should be performed before DMA or CPU accesses these > dmabufs. > > > > dma_buf_sync_lock(a sync object); > > > > 3. Unlock a sync object - The task unlocks all dmabufs added in its > own sync > > object. The unlock means that the DMA or CPU accesses to the dmabufs > have > > been completed so that others may access them. > > This unlocking should be performed after DMA or CPU has completed > accesses > > to the dmabufs. > > > > dma_buf_sync_unlock(a sync object); > > > > 4. Unregister one or all dmabufs from a sync object - A task > unregisters > > the given dmabufs from the sync object. This means that the task > dosen't > > want to lock the dmabufs. > > The unregistering should be performed after DMA or CPU has completed > > accesses to the dmabufs or when dma_buf_sync_lock() is failed. > > > > dma_buf_sync_put(a sync object, a dmabuf); > > dma_buf_sync_put_all(a sync object); > > > > The described steps may be summarized as: > > get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put > > > > This framework includes the following two features. > > 1. read (shared) and write (exclusive) locks - A task is required to > declare > > the access type when the task tries to register a dmabuf; > > READ, WRITE, READ DMA, or WRITE DMA. > > > > The below is example codes, > > struct dmabuf_sync *sync; > > > > sync = dmabuf_sync_init(NULL, "test sync"); > > > > dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ); > > ... > > > > And the below can be used as access types: > > DMA_BUF_ACCESS_READ, > > - CPU will access a buffer for read. > > DMA_BUF_ACCESS_WRITE, > > - CPU will access a buffer for read or write. > > DMA_BUF_ACCESS_READ | DMA_BUF_ACCESS_DMA, > > - DMA will access a buffer for read > > DMA_BUF_ACCESS_WRITE | DMA_BUF_ACCESS_DMA, > > - DMA will access a buffer for read or write. > > > > 2. Mandatory resource releasing - a task cannot hold a lock > indefinitely. > > A task may never try to unlock a buffer after taking a lock to the > buffer. > > In this case, a timer handler to the corresponding sync object is > called > > in five (default) seconds and then the timed-out buffer is unlocked > by work > > queue handler to avoid lockups and to enforce resources of the buffer. > > > > The below is how to use: > > 1. Allocate and Initialize a sync object: > > struct dmabuf_sync *sync; > > > > sync = dmabuf_sync_init(NULL, "test sync"); > > ... > > > > 2. Add a dmabuf to the sync object when setting up dma buffer > relevant > >registers: > > dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ); > > ... > > > > 3. Lock all dmabu
[RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with pmu reg control
Hello Kishon, On 2013? 06? 13? 21:54, Kishon Vijay Abraham I wrote: > Hi, > > On Thursday 13 June 2013 04:51 PM, Inki Dae wrote: >> >> >>> -Original Message- >>> From: Sylwester Nawrocki [mailto:s.nawrocki at samsung.com] >>> Sent: Thursday, June 13, 2013 5:56 PM >>> To: Rahul Sharma >>> Cc: Rahul Sharma; Inki Dae; linux-samsung-soc at vger.kernel.org; >>> devicetree- >>> discuss at lists.ozlabs.org; DRI mailing list; Kukjin Kim; Seung-Woo Kim; >>> Sean Paul; sunil joshi; Kishon Vijay Abraham I; Stephen Warren; >>> grant.likely at linaro.org >>> Subject: Re: [RFC 0/2] exynos5250/hdmi: replace dummy hdmiphy clock with >>> pmu reg control >>> >>> Hi, >>> >>> On 06/13/2013 06:26 AM, Rahul Sharma wrote: Mr. Dae, Thanks for your valuable inputs. I posted it as RFC because, I also have received comments to register hdmiphy as a clock controller. As we always configure it for specific frequency, hdmi-phy looks similar to a PLL. But it really doesn't belong to that class. Secondly prior to exynos5420, it was a i2c device. I am not sure we can register a I2C device as a clock controller. I wanted to discuss and explore this option here. >>> >>> Have you considered using the generic PHY framework for those HDMI >>> PHY devices [1] ? I guess we could add a dedicated group of ops for >>> video PHYs, similarly as is is done with struct v4l2_subdev_ops. For >>> configuring things like the carrier/pixel clock frequency or anything >>> what's common across the video PHYs. >>> >>> Perhaps you could have a look and see if this framework would be >>> useful for HDMI and possibly point out anything what might be missing ? >>> >>> I'm not sure it it really solves the issues specific to the Exynos >>> HDMI but at least with a generic PHY driver the PHY module would be >>> separate from the PHY controller, as often same HDMI DPHY can be used >>> with various types of a HDMI controller. So this would allow to not >>> duplicate the HDMI PHY drivers in the long-term perspective. >> >> Yeah, at least, it seems that we could use PHY module to control PMU >> register, HDMI_PHY_CONTROL. However, PHY module provides only init/on/off >> callbacks. As you may know, HDMIPHY needs i2c interfaces to control >> HDMIPHY >> clock. So with PHY module, HDMIPHY driver could enable PMU more >> generically, >> but also has to use existing i2c stuff to control HDMIPHY clock. I had a >> quick review to Generic PHY Framework[v6] but I didn't see that the PHY >> module could generically support more features such as i2c stuff. > > I don't think PHY framework needs to provide i2c interfaces to program > certain configurations. Instead in one of the callbacks (init/on/off) > PHY driver can program whatever it wants using any of the interfaces it > needs. IMO PHY framework should work independent of the interfaces. In exnoys hdmi case, i2c interface is not the exact issue. In exynos hdmi, hdmiphy should send i2c configuration about video clock information as the video mode information including resolution, bit per pixel, refresh rate passed from drm subsystem. So init/on/off callbacks of phy framework are not enough for exynos hdmiphy and it should have a callback to set video mode. Do you have plan to add driver specific extend callback pointers to phy framework? Currently, hdmi directly calls phy operations, but Rahul's another patch set, mentioned by Inki, divides hdmi and hdmiphy and hdmi and hdmiphy is connected with exynos hdmi own sub driver callback operations. IMHO, if phy framework can support extend callback feature, then this own sub driver callbacks can be replaced with phy framework at long term view. Thanks and Regards, - Seung-Woo Kim > > For example, twl4030 phy driver actually uses i2c to program its > registers but still it uses the PHY framework [1]. > > [1] --> http://www.gossamer-threads.com/lists/linux/kernel/1729414 > > Thanks > Kishon > -- > To unsubscribe from this list: send the line "unsubscribe > linux-samsung-soc" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Seung-Woo Kim Samsung Software R&D Center --
[PATCH 1/9] drm/exynos: use SoC name to identify hdmi version
Hello Rahul, On 2013? 06? 11? 23:11, Rahul Sharma wrote: > Exynos hdmi IP version is named after hdmi specification version i.e. > 1.3 and 1.4. This versioning mechanism is not sufficient to handle > the diversity in the hdmi/phy IPs which are present across the exynos > SoC family. > > This patch changes the hdmi version to the name of the SoC in which > the IP was introduced for the first time. Same version is applicable > to all subsequent SoCs having the same IP version. > > Exynos4210 has 1.3 HDMI, i2c mapped phy with configuration set. > Exynos5250 has 1.4 HDMI, i2c mapped phy with configuration set. > Exynos5420 has 1.4 HDMI, Platform Bus mapped phy with configuration set. > > Based on the HDMI IP version we cannot decide to pick Exynos5250 phy conf > and use i2c for data transfer or Exynos5420 phy confs and platform bus > calls for communication. Considering your other patch to divide hdmi and hdmiphy, how do you think using hdmiphy version parsed from hdmiphy dt binding from phy code instead of using hdmi version for both hdmi and hdmiphy? If that, this SoC identifying hdmi version is not necessary because there is no change at least in hdmi side. And IMO, it seems easy to merge hdmiphy related patch first before merging patch for exynos5420. > > Signed-off-by: Rahul Sharma > --- > drivers/gpu/drm/exynos/exynos_hdmi.c | 249 > +- > drivers/gpu/drm/exynos/regs-hdmi.h | 78 +-- > 2 files changed, 164 insertions(+), 163 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > b/drivers/gpu/drm/exynos/exynos_hdmi.c > index 75a6bf3..9384ffc 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -73,9 +73,9 @@ enum HDMI_PACKET_TYPE { > HDMI_PACKET_TYPE_AUI = HDMI_PACKET_TYPE_INFOFRAME + 4 > }; > > -enum hdmi_type { > - HDMI_TYPE13, > - HDMI_TYPE14, > +enum hdmi_version { > + HDMI_VER_EXYNOS4210, > + HDMI_VER_EXYNOS4212, > }; -- Seung-Woo Kim Samsung Software R&D Center --
[PATCH 5/9] drm/exynos: add support for exynos5420 mixer
Hello Rahul, This patch looks good to me just with mixer part of 2nd patch because there is no hdmiphy related things. On 2013? 06? 11? 23:11, Rahul Sharma wrote: > Add support for exynos5420 mixer IP in the drm mixer driver. > > Signed-off-by: Rahul Sharma > --- > drivers/gpu/drm/exynos/exynos_mixer.c | 49 > + > drivers/gpu/drm/exynos/regs-mixer.h |7 + > 2 files changed, 44 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c > b/drivers/gpu/drm/exynos/exynos_mixer.c > index 58dfd3f..101d5bb 100644 > --- a/drivers/gpu/drm/exynos/exynos_mixer.c > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c > @@ -78,6 +78,7 @@ struct mixer_resources { > enum mixer_version_id { > MXR_VER_0_0_0_16, > MXR_VER_16_0_33_0, > + MXR_VER_128_0_0_184, > }; > > struct mixer_context { > @@ -283,17 +284,19 @@ static void mixer_cfg_scan(struct mixer_context *ctx, > unsigned int height) > val = (ctx->interlace ? MXR_CFG_SCAN_INTERLACE : > MXR_CFG_SCAN_PROGRASSIVE); > > - /* choosing between porper HD and SD mode */ > - if (height <= 480) > - val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD; > - else if (height <= 576) > - val |= MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD; > - else if (height <= 720) > - val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD; > - else if (height <= 1080) > - val |= MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD; > - else > - val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD; > + if (ctx->mxr_ver != MXR_VER_128_0_0_184) { > + /* choosing between proper HD and SD mode */ > + if (height <= 480) > + val |= MXR_CFG_SCAN_NTSC | MXR_CFG_SCAN_SD; > + else if (height <= 576) > + val |= MXR_CFG_SCAN_PAL | MXR_CFG_SCAN_SD; > + else if (height <= 720) > + val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD; > + else if (height <= 1080) > + val |= MXR_CFG_SCAN_HD_1080 | MXR_CFG_SCAN_HD; > + else > + val |= MXR_CFG_SCAN_HD_720 | MXR_CFG_SCAN_HD; > + } > > mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_SCAN_MASK); > } > @@ -557,6 +560,14 @@ static void mixer_graph_buffer(struct mixer_context > *ctx, int win) > /* setup geometry */ > mixer_reg_write(res, MXR_GRAPHIC_SPAN(win), win_data->fb_width); > > + /* setup display size */ > + if (ctx->mxr_ver == MXR_VER_128_0_0_184 && > + win == MIXER_DEFAULT_WIN) { > + val = MXR_MXR_RES_HEIGHT(win_data->fb_height); > + val |= MXR_MXR_RES_WIDTH(win_data->fb_width); > + mixer_reg_write(res, MXR_RESOLUTION, val); > + } > + > val = MXR_GRP_WH_WIDTH(win_data->crtc_width); > val |= MXR_GRP_WH_HEIGHT(win_data->crtc_height); > val |= MXR_GRP_WH_H_SCALE(x_ratio); > @@ -581,7 +592,8 @@ static void mixer_graph_buffer(struct mixer_context *ctx, > int win) > mixer_cfg_layer(ctx, win, true); > > /* layer update mandatory for mixer 16.0.33.0 */ > - if (ctx->mxr_ver == MXR_VER_16_0_33_0) > + if (ctx->mxr_ver == MXR_VER_16_0_33_0 || > + ctx->mxr_ver == MXR_VER_128_0_0_184) > mixer_layer_update(ctx); > > mixer_run(ctx); > @@ -822,6 +834,7 @@ static void mixer_win_disable(void *ctx, int win) > > static int mixer_check_mode(void *ctx, struct drm_display_mode *mode) > { > + struct mixer_context *mixer_ctx = ctx; > u32 w, h; > > w = mode->hdisplay; > @@ -831,6 +844,10 @@ static int mixer_check_mode(void *ctx, struct > drm_display_mode *mode) > mode->hdisplay, mode->vdisplay, mode->vrefresh, > (mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0); > > + if (mixer_ctx->mxr_ver == MXR_VER_0_0_0_16 || > + mixer_ctx->mxr_ver == MXR_VER_128_0_0_184) > + return 0; > + > if ((w >= 464 && w <= 720 && h >= 261 && h <= 576) || > (w >= 1024 && w <= 1280 && h >= 576 && h <= 720) || > (w >= 1664 && w <= 1920 && h >= 936 && h <= 1080)) > @@ -1127,6 +1144,11 @@ static int vp_resources_init(struct > exynos_drm_hdmi_context *ctx, > return 0; > } > > +static struct mixer_drv_data exynos5420_mxr_drv_data = { > + .version = MXR_VER_128_0_0_184, > + .is_vp_enabled = 0, > +}; > + > static struct mixer_drv_data exynos5250_mxr_drv_data = { > .version = MXR_VER_16_0_33_0, > .is_vp_enabled = 0, > @@ -1151,6 +1173,9 @@ static struct platform_device_id mixer_driver_types[] = > { > > static struct of_device_id mixer_match_types[] = { > { > + .compatible = "samsung,exynos5420-mixer", > + .data = &exynos5420_mxr_drv_data, > + }, { > .compatible = "samsung,exynos5250-mixer", > .data = &exynos5250_mxr_drv_data,
[PATCH 7/9] drm/exynos: use of_get_named_gpio to get hdmi hpd gpio
Hello Rahul, this patch is not related with others and it looks good to me. On 2013? 06? 11? 23:11, Rahul Sharma wrote: > Cleanup by removing flags variable from drm_hdmi_dt_parse_pdata > which is not used anywhere. Swtiching to of_get_named_gpio instead > of of_get_named_gpio_flags solved this. > > Signed-off-by: Rahul Sharma Acked-by: Seung-Woo Kim > --- > drivers/gpu/drm/exynos/exynos_hdmi.c |3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c > b/drivers/gpu/drm/exynos/exynos_hdmi.c > index 1eb5ffb..fc6a9b0 100644 > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c > @@ -2081,7 +2081,6 @@ static struct s5p_hdmi_platform_data > *drm_hdmi_dt_parse_pdata > { > struct device_node *np = dev->of_node; > struct s5p_hdmi_platform_data *pd; > - enum of_gpio_flags flags; > u32 value; > > pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); > @@ -2095,7 +2094,7 @@ static struct s5p_hdmi_platform_data > *drm_hdmi_dt_parse_pdata > goto err_data; > } > > - pd->hpd_gpio = of_get_named_gpio_flags(np, "hpd-gpio", 0, &flags); > + pd->hpd_gpio = of_get_named_gpio(np, "hpd-gpio", 0); > > return pd; > > -- Seung-Woo Kim Samsung Software R&D Center --
[Bug 64257] RS880 issues with r600-llvm-compiler
https://bugs.freedesktop.org/show_bug.cgi?id=64257 --- Comment #64 from Marc Dietrich --- patch in comment 60 does not fix the issues (no change). -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130614/50422e87/attachment.html>
[Bug 65438] GTK apps crash X11 since last update of LLVMM
https://bugs.freedesktop.org/show_bug.cgi?id=65438 Michel D?nzer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #12 from Michel D?nzer --- Tom committed his fix as revision 183937. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130614/5a4c817a/attachment.html>
[Bug 65730] no sound via HDMI for Radeon 6850 mesa driver
https://bugs.freedesktop.org/show_bug.cgi?id=65730 --- Comment #5 from Andy Furniss --- (In reply to comment #4) > Before I being, I must make a disclaimer that I am not a Linux genius by any > means, but a moderate user, able and willing to find my way around things. > I do occasionally assist one of the pcsx2 emulator devs with me applying and > testing various patches for the code. Sometimes I need help, though, > finding the correct file to patch, as I am not a coder. Anyways, back to > the topic at hand. > > I'm assuming the file in question should be r600_hdmi.c? > > I have checked using catfish file searcher and also manually for this file. > This includes the > /lib/modules/3.8.0-23-generic/kernel/drivers/gpu/drm/radeon directory. All > I have is radeon.ko in that directory. In other words, I don't see any > trace of where r600_hdmi.c is or should be anywhere in my file system. > > Again I apologize for the inconvenenience. I am not a dev and don't know ubuntu or any distro really. You would need to be building your own kernels and so have the kernel source tree to find hdmi.c. One thing that puzzles me is the use of grub command line to pass module parameters - I thought that would be if radeon was built into kernel, but as you show you are using a module. If you search for how to set module parameters in ubuntu you may have more luck - it will involve putting option radeon audio=1 somewhere and regenerating something ... -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130614/d4bfa6cc/attachment.html>
[PATCH] drm/exynos: make sure to handle an error case to vm_mmap call
vm_mmap function returns unsigned long so addr type should be unsigned long. a pointer or address variable is required to use unsigned long or uint64_t type for 64bits address support. So this patch makes sure that addr has unsigned long type and also exynos_drm_gem_mmap_ioctl returns correct error type. Signed-off-by: Inki Dae Signed-off-by: Kyungmin Park --- drivers/gpu/drm/exynos/exynos_drm_gem.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c index 5af1478..c3f15e7 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c @@ -420,7 +420,7 @@ int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data, { struct drm_exynos_gem_mmap *args = data; struct drm_gem_object *obj; - unsigned int addr; + unsigned long addr; if (!(dev->driver->driver_features & DRIVER_GEM)) { DRM_ERROR("does not support GEM.\n"); @@ -462,14 +462,14 @@ int exynos_drm_gem_mmap_ioctl(struct drm_device *dev, void *data, drm_gem_object_unreference(obj); - if (IS_ERR((void *)addr)) { + if (IS_ERR_VALUE(addr)) { /* check filp->f_op, filp->private_data are restored */ if (file_priv->filp->f_op == &exynos_drm_gem_fops) { file_priv->filp->f_op = fops_get(dev->driver->fops); file_priv->filp->private_data = file_priv; } mutex_unlock(&dev->struct_mutex); - return PTR_ERR((void *)addr); + return (int)addr; } mutex_unlock(&dev->struct_mutex); -- 1.7.5.4
[Bug 65730] no sound via HDMI for Radeon 6850 mesa driver
https://bugs.freedesktop.org/show_bug.cgi?id=65730 --- Comment #6 from Andy Furniss --- (In reply to comment #5) > option radeon audio=1 Oops, should be options radeon audio=1 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130614/ea4cde3f/attachment-0001.html>