* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-08-16 9:52 UTC (permalink / raw)
To: Alex Courbot
Cc: Stephen Warren, Simon Glass, Grant Likely, Rob Herring,
Mark Brown, Anton Vorontsov, David Woodhouse, Arnd Bergmann,
Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <502CBB0C.70102@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 6392 bytes --]
On Thu, Aug 16, 2012 at 06:19:08PM +0900, Alex Courbot wrote:
> On 08/16/2012 04:42 PM, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> >
> >On Thu, Aug 16, 2012 at 03:08:55PM +0900, Alexandre Courbot wrote:
[...]
> >>+Usage by Drivers and Resources Management
> >>+-----------------------------------------
> >>+Power sequences make use of resources that must be properly allocated and
> >>+managed. The power_seq_build() function builds a power sequence from the
> >>+platform data. It also takes care of resolving and allocating the resources
> >>+referenced by the sequence if needed:
> >>+
> >>+ struct power_seq *power_seq_build(struct device *dev, struct list_head *ress,
> >>+ struct platform_power_seq *pseq);
> >>+
> >>+The 'dev' argument is the device in the name of which the resources are to be
> >>+allocated.
> >>+
> >>+The 'ress' argument is a list to which the resolved resources are appended. This
> >>+avoids allocating a resource referenced in several power sequences multiple
> >>+times.
> >>+
> >>+On success, the function returns a devm allocated resolved sequence that is
> >>+ready to be passed to power_seq_run(). In case of failure, and error code is
> >>+returned.
> >>+
> >>+A resolved power sequence returned by power_seq_build can be run by
> >>+power_run_run():
> >>+
> >>+ int power_seq_run(power_seq *seq);
> >>+
> >>+It returns 0 if the sequence has successfully been run, or an error code if a
> >>+problem occured.
> >>+
> >>+There is no need to explicitly free the resources used by the sequence as they
> >>+are devm-allocated.
> >
> >I had some comments about this particular interface for creating
> >sequences in the last series. My point was that explicitly requiring
> >drivers to manage a list of already allocated resources may be too much
> >added complexity. Power sequences should be easy to use, and I find the
> >requirement for a separately managed list of resources cumbersome.
> >
> >What I proposed last time was to collect all power sequences under a
> >common parent object, which in turn would take care of managing the
> >resources.
>
> Yes, I remember that. While I see why you don't like this list,
> having a common parent object to all sequences will not reduce the
> number of arguments to pass to power_seq_build() (which is the only
> function that has to handle this list now). Also having the list of
> resources at hand is needed for some drivers: for instance,
> pwm-backlight needs to check that exactly one PWM has been
> allocated, and takes a reference to it from this list in order to
> control the brightness.
I'm not complaining about the additional argument to power_seq_build()
but about the missing encapsulation. I just think that keeping a list
external to the power sequencing code is error-prone. Drivers could do
just about anything with it between calls to power_seq_build(). If you
do all of this internally, then you don't depend on the driver at all
and power sequencing code can just do the right thing.
Obtaining a reference to the PWM, or any other resource for that matter,
from the power sequence could be done via an explicit API.
> Ideally we could embed the list into the device structure, but I
> don't see how we can do that without modifying it (and we don't want
> to modify it). Another solution would be to keep a static mapping
> table that associates a device to its power_seq related resources
> within power_seq.c. If we protect it for concurrent access this
> should make it possible to make resources management transparent.
> How does this sound? Only drawback I see is that we would need to
> explicitly clean it up through a dedicated function when the driver
> exits.
I don't think that's much better. Since the power sequences will be very
tightly coupled to a specific device, tying the sequences and their
resources to the device makes a lot of sense. Keeping a global list of
resources doesn't in my opinion.
> >>+static int power_seq_step_run(struct power_seq_step *step)
> >>+{
> >>+ struct platform_power_seq_step *pdata = &step->pdata;
> >>+ int err = 0;
> >>+
> >>+ switch (pdata->type) {
> >>+ case POWER_SEQ_DELAY:
> >>+ usleep_range(pdata->delay.delay_us,
> >>+ pdata->delay.delay_us + 1000);
> >>+ break;
> >>+#ifdef CONFIG_REGULATOR
> >>+ case POWER_SEQ_REGULATOR:
> >>+ if (pdata->regulator.enable)
> >>+ err = regulator_enable(step->resource->regulator);
> >>+ else
> >>+ err = regulator_disable(step->resource->regulator);
> >>+ break;
> >>+#endif
> >>+#ifdef CONFIG_PWM
> >>+ case POWER_SEQ_PWM:
> >>+ if (pdata->gpio.enable)
> >>+ err = pwm_enable(step->resource->pwm);
> >>+ else
> >>+ pwm_disable(step->resource->pwm);
> >>+ break;
> >>+#endif
> >>+#ifdef CONFIG_GPIOLIB
> >>+ case POWER_SEQ_GPIO:
> >>+ gpio_set_value_cansleep(pdata->gpio.gpio, pdata->gpio.enable);
> >>+ break;
> >>+#endif
> >>+ /*
> >>+ * should never happen unless the sequence includes a step which
> >>+ * type does not have support compiled in
> >
> >I think this should be "whose type"? I also remember commenting on the
> >whole #ifdef'ery here. I really don't think it is necessary. At least
> >for regulators I know that the functions can be used even if the
> >subsystem itself isn't supported. The same seems to hold for GPIO and we
> >can probably add something similar for PWM.
>
> Actually I kept them because I don't really like the empty function
> definitions in the regulator framework. They all return 0 as if the
> function completed successfully - here we should at least warn the
> user that proper support for that resource is missing.
>
> >
> >It might also be a good idea to just skip unsupported resource types
> >when the sequence is built, accompanied by runtime warnings that the
> >type is not supported.
>
> Agreed.
If you do this, then I think the above #ifdef'ery becomes obsolete
because any errors that could potentially be hidden have already been
caught when the list was built.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v4] fbdev: sh_mobile_lcdc: use dma_mmap_coherent
From: Hideki EIRAKU @ 2012-08-16 10:13 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: linux-fbdev, linux-kernel, Marek Szyprowski, Laurent Pinchart,
Katsuya MATSUBARA, Damian Hobson-Garcia, Hideki EIRAKU
fb_mmap() implemented in fbmem.c uses smem_start as the physical
address of the frame buffer. In the sh_mobile_lcdc driver, the
smem_start is a dma_addr_t that is not a physical address when IOMMU is
enabled. dma_mmap_coherent() maps the address correctly.
Signed-off-by: Hideki EIRAKU <hdk@igel.co.jp>
---
drivers/video/sh_mobile_lcdcfb.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 699487c..bccfd7e 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -1614,6 +1614,15 @@ static int sh_mobile_lcdc_overlay_blank(int blank, struct fb_info *info)
return 1;
}
+static int
+sh_mobile_lcdc_overlay_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+ struct sh_mobile_lcdc_overlay *ovl = info->par;
+
+ return dma_mmap_coherent(ovl->channel->lcdc->dev, vma, ovl->fb_mem,
+ ovl->dma_handle, ovl->fb_size);
+}
+
static struct fb_ops sh_mobile_lcdc_overlay_ops = {
.owner = THIS_MODULE,
.fb_read = fb_sys_read,
@@ -1626,6 +1635,7 @@ static struct fb_ops sh_mobile_lcdc_overlay_ops = {
.fb_ioctl = sh_mobile_lcdc_overlay_ioctl,
.fb_check_var = sh_mobile_lcdc_overlay_check_var,
.fb_set_par = sh_mobile_lcdc_overlay_set_par,
+ .fb_mmap = sh_mobile_lcdc_overlay_mmap,
};
static void
@@ -2093,6 +2103,15 @@ static int sh_mobile_lcdc_blank(int blank, struct fb_info *info)
return 0;
}
+static int
+sh_mobile_lcdc_mmap(struct fb_info *info, struct vm_area_struct *vma)
+{
+ struct sh_mobile_lcdc_chan *ch = info->par;
+
+ return dma_mmap_coherent(ch->lcdc->dev, vma, ch->fb_mem,
+ ch->dma_handle, ch->fb_size);
+}
+
static struct fb_ops sh_mobile_lcdc_ops = {
.owner = THIS_MODULE,
.fb_setcolreg = sh_mobile_lcdc_setcolreg,
@@ -2108,6 +2127,7 @@ static struct fb_ops sh_mobile_lcdc_ops = {
.fb_release = sh_mobile_lcdc_release,
.fb_check_var = sh_mobile_lcdc_check_var,
.fb_set_par = sh_mobile_lcdc_set_par,
+ .fb_mmap = sh_mobile_lcdc_mmap,
};
static void
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v3 3/4] media: videobuf2-dma-contig: use dma_mmap_coherent if available
From: Hideki EIRAKU @ 2012-08-16 10:13 UTC (permalink / raw)
To: m.szyprowski
Cc: laurent.pinchart, linux, pawel, kyungmin.park, mchehab,
FlorianSchandinat, perex, tiwai, t.stanislaws, linux-arm-kernel,
linux-kernel, linux-media, linux-fbdev, alsa-devel, matsu,
dhobsong
In-Reply-To: <012701cd74ac$6a617060$3f245120$%szyprowski@samsung.com>
Hello,
From: Marek Szyprowski <m.szyprowski@samsung.com>
Subject: RE: [PATCH v3 3/4] media: videobuf2-dma-contig: use dma_mmap_coherent if available
Date: Tue, 07 Aug 2012 16:53:25 +0200
> I'm sorry for bringing this issue now, once you have already created v3 of your
> patches, but similar patch has been already proposed some time ago. It is already
> processed together with general videobuf2-dma-contig redesign and dma-buf extensions
> by Tomasz Stanislawski.
>
> See post http://thread.gmane.org/gmane.comp.video.dri.devel/70402/focusI461 and
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/49438
>
> It doesn't use conditional code inside videobuf2 allocator and rely entirely on
> dma-mapping subsystem to provide a working dma_mmap_coherent/writecombine/attrs()
> function. When it was posted, it relied on the dma-mapping extensions, which now
> have been finally merged to v3.6-rc1. Now I wonder if there are any architectures,
> which don't use dma_map_ops based dma-mapping framework, which might use
> videobuf2-dma-conting module.
Thank you for telling me about videobuf2-dma-contig and v3.6-rc1. The
videobuf2-dma-contig patch I sent is now unnecessary. So I will
remove the patch. I will remove the patch defining
ARCH_HAS_DMA_MMAP_COHERENT too because the v3.6-rc1 kernel has generic
dma_mmap_coherent() API for every architecture.
I will also remove the Laurent's patch I sent because it was related
to ARCH_HAS_DMA_MMAP_COHERENT.
The remaining patch is sh_mobile_lcdc. I will remove ifdefs from the
patch and re-post it as a patch v4.
--
Hideki EIRAKU <hdk@igel.co.jp>
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-08-16 10:33 UTC (permalink / raw)
To: Thierry Reding
Cc: Stephen Warren, Simon Glass, Grant Likely, Rob Herring,
Mark Brown, Anton Vorontsov, David Woodhouse, Arnd Bergmann,
Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <20120816095251.GA30646@avionic-0098.mockup.avionic-design.de>
On 08/16/2012 06:52 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, Aug 16, 2012 at 06:19:08PM +0900, Alex Courbot wrote:
>> On 08/16/2012 04:42 PM, Thierry Reding wrote:
>>>> Old Signed by an unknown key
>>>
>>> On Thu, Aug 16, 2012 at 03:08:55PM +0900, Alexandre Courbot wrote:
> [...]
>>>> +Usage by Drivers and Resources Management
>>>> +-----------------------------------------
>>>> +Power sequences make use of resources that must be properly allocated and
>>>> +managed. The power_seq_build() function builds a power sequence from the
>>>> +platform data. It also takes care of resolving and allocating the resources
>>>> +referenced by the sequence if needed:
>>>> +
>>>> + struct power_seq *power_seq_build(struct device *dev, struct list_head *ress,
>>>> + struct platform_power_seq *pseq);
>>>> +
>>>> +The 'dev' argument is the device in the name of which the resources are to be
>>>> +allocated.
>>>> +
>>>> +The 'ress' argument is a list to which the resolved resources are appended. This
>>>> +avoids allocating a resource referenced in several power sequences multiple
>>>> +times.
>>>> +
>>>> +On success, the function returns a devm allocated resolved sequence that is
>>>> +ready to be passed to power_seq_run(). In case of failure, and error code is
>>>> +returned.
>>>> +
>>>> +A resolved power sequence returned by power_seq_build can be run by
>>>> +power_run_run():
>>>> +
>>>> + int power_seq_run(power_seq *seq);
>>>> +
>>>> +It returns 0 if the sequence has successfully been run, or an error code if a
>>>> +problem occured.
>>>> +
>>>> +There is no need to explicitly free the resources used by the sequence as they
>>>> +are devm-allocated.
>>>
>>> I had some comments about this particular interface for creating
>>> sequences in the last series. My point was that explicitly requiring
>>> drivers to manage a list of already allocated resources may be too much
>>> added complexity. Power sequences should be easy to use, and I find the
>>> requirement for a separately managed list of resources cumbersome.
>>>
>>> What I proposed last time was to collect all power sequences under a
>>> common parent object, which in turn would take care of managing the
>>> resources.
>>
>> Yes, I remember that. While I see why you don't like this list,
>> having a common parent object to all sequences will not reduce the
>> number of arguments to pass to power_seq_build() (which is the only
>> function that has to handle this list now). Also having the list of
>> resources at hand is needed for some drivers: for instance,
>> pwm-backlight needs to check that exactly one PWM has been
>> allocated, and takes a reference to it from this list in order to
>> control the brightness.
>
> I'm not complaining about the additional argument to power_seq_build()
> but about the missing encapsulation. I just think that keeping a list
> external to the power sequencing code is error-prone. Drivers could do
> just about anything with it between calls to power_seq_build(). If you
> do all of this internally, then you don't depend on the driver at all
> and power sequencing code can just do the right thing.
On the opposite side, I am concerned about over-encapsulation. :) IIRC
you proposed to have a top structure to hold the power sequences, their
resources and the associated device. Power sequences would then have a
name and be run through a 2 arguments power_seq_run():
power_seq_run(sequences, "up");
There are two things that bother me with this solution. First is that
addressing power sequences by name looks a little bit overkill, when a
single pointer should be enough. It would also complicate the design.
Second thing is that this design would place the power sequences
structure on top of the device - in effect, you could perfectly have
several of these structures all using the same device and failing to see
each other's resources. While that would be a error from the device
driver's side, the design allows it.
>
> Obtaining a reference to the PWM, or any other resource for that matter,
> from the power sequence could be done via an explicit API.
>
>> Ideally we could embed the list into the device structure, but I
>> don't see how we can do that without modifying it (and we don't want
>> to modify it). Another solution would be to keep a static mapping
>> table that associates a device to its power_seq related resources
>> within power_seq.c. If we protect it for concurrent access this
>> should make it possible to make resources management transparent.
>> How does this sound? Only drawback I see is that we would need to
>> explicitly clean it up through a dedicated function when the driver
>> exits.
>
> I don't think that's much better. Since the power sequences will be very
> tightly coupled to a specific device, tying the sequences and their
> resources to the device makes a lot of sense. Keeping a global list of
> resources doesn't in my opinion.
That is not what would happen actually - what I proposed is to have a
mapping (hash map, or more likely binary tree) between a device and the
list_head of the resources for that device. In C++ (forgive me, this
makes the types more explicit) that would be:
static std::map<struct device *, struct list_head> device_resources;
That way you would have exactly one list per device, could keep
resource-management totally transparent without exposing the list_head,
and keep the API and design simple.
For special cases (like pwm-backlight which needs to get the PWM), the
list_head could be obtained through a dedicated API.
Alex.
^ permalink raw reply
* RE: [PATCH v3 3/4] media: videobuf2-dma-contig: use dma_mmap_coherent if available
From: Marek Szyprowski @ 2012-08-16 10:39 UTC (permalink / raw)
To: 'Hideki EIRAKU'
Cc: laurent.pinchart, linux, pawel, kyungmin.park, mchehab,
FlorianSchandinat, perex, tiwai, Tomasz Stanislawski,
linux-arm-kernel, linux-kernel, linux-media, linux-fbdev,
alsa-devel, matsu, dhobsong
In-Reply-To: <20120816.191358.127675610.hdk@igel.co.jp>
Hello,
On Thursday, August 16, 2012 12:14 PM Hideki EIRAKU wrote:
> From: Marek Szyprowski <m.szyprowski@samsung.com>
> Subject: RE: [PATCH v3 3/4] media: videobuf2-dma-contig: use dma_mmap_coherent if available
> Date: Tue, 07 Aug 2012 16:53:25 +0200
>
> > I'm sorry for bringing this issue now, once you have already created v3 of your
> > patches, but similar patch has been already proposed some time ago. It is already
> > processed together with general videobuf2-dma-contig redesign and dma-buf extensions
> > by Tomasz Stanislawski.
> >
> > See post http://thread.gmane.org/gmane.comp.video.dri.devel/70402/focusI461 and
> > http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/49438
> >
> > It doesn't use conditional code inside videobuf2 allocator and rely entirely on
> > dma-mapping subsystem to provide a working dma_mmap_coherent/writecombine/attrs()
> > function. When it was posted, it relied on the dma-mapping extensions, which now
> > have been finally merged to v3.6-rc1. Now I wonder if there are any architectures,
> > which don't use dma_map_ops based dma-mapping framework, which might use
> > videobuf2-dma-conting module.
>
> Thank you for telling me about videobuf2-dma-contig and v3.6-rc1. The
> videobuf2-dma-contig patch I sent is now unnecessary. So I will
> remove the patch. I will remove the patch defining
> ARCH_HAS_DMA_MMAP_COHERENT too because the v3.6-rc1 kernel has generic
> dma_mmap_coherent() API for every architecture.
Just to let you know - Tomasz has posted an updated version of the dma-buf/vb2-dma-contig
patches:
http://www.spinics.net/lists/linux-media/msg51768.html
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
^ permalink raw reply
* Re: [PATCH v4 1/3] Runtime Interpreted Power Sequences
From: Thierry Reding @ 2012-08-16 10:52 UTC (permalink / raw)
To: Alex Courbot
Cc: Stephen Warren, Simon Glass, Grant Likely, Rob Herring,
Mark Brown, Anton Vorontsov, David Woodhouse, Arnd Bergmann,
Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org
In-Reply-To: <502CCC77.2010005@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 6396 bytes --]
On Thu, Aug 16, 2012 at 07:33:27PM +0900, Alex Courbot wrote:
> On 08/16/2012 06:52 PM, Thierry Reding wrote:
> >* PGP Signed by an unknown key
> >
> >On Thu, Aug 16, 2012 at 06:19:08PM +0900, Alex Courbot wrote:
> >>On 08/16/2012 04:42 PM, Thierry Reding wrote:
> >>>>Old Signed by an unknown key
> >>>
> >>>On Thu, Aug 16, 2012 at 03:08:55PM +0900, Alexandre Courbot wrote:
> >[...]
> >>>>+Usage by Drivers and Resources Management
> >>>>+-----------------------------------------
> >>>>+Power sequences make use of resources that must be properly allocated and
> >>>>+managed. The power_seq_build() function builds a power sequence from the
> >>>>+platform data. It also takes care of resolving and allocating the resources
> >>>>+referenced by the sequence if needed:
> >>>>+
> >>>>+ struct power_seq *power_seq_build(struct device *dev, struct list_head *ress,
> >>>>+ struct platform_power_seq *pseq);
> >>>>+
> >>>>+The 'dev' argument is the device in the name of which the resources are to be
> >>>>+allocated.
> >>>>+
> >>>>+The 'ress' argument is a list to which the resolved resources are appended. This
> >>>>+avoids allocating a resource referenced in several power sequences multiple
> >>>>+times.
> >>>>+
> >>>>+On success, the function returns a devm allocated resolved sequence that is
> >>>>+ready to be passed to power_seq_run(). In case of failure, and error code is
> >>>>+returned.
> >>>>+
> >>>>+A resolved power sequence returned by power_seq_build can be run by
> >>>>+power_run_run():
> >>>>+
> >>>>+ int power_seq_run(power_seq *seq);
> >>>>+
> >>>>+It returns 0 if the sequence has successfully been run, or an error code if a
> >>>>+problem occured.
> >>>>+
> >>>>+There is no need to explicitly free the resources used by the sequence as they
> >>>>+are devm-allocated.
> >>>
> >>>I had some comments about this particular interface for creating
> >>>sequences in the last series. My point was that explicitly requiring
> >>>drivers to manage a list of already allocated resources may be too much
> >>>added complexity. Power sequences should be easy to use, and I find the
> >>>requirement for a separately managed list of resources cumbersome.
> >>>
> >>>What I proposed last time was to collect all power sequences under a
> >>>common parent object, which in turn would take care of managing the
> >>>resources.
> >>
> >>Yes, I remember that. While I see why you don't like this list,
> >>having a common parent object to all sequences will not reduce the
> >>number of arguments to pass to power_seq_build() (which is the only
> >>function that has to handle this list now). Also having the list of
> >>resources at hand is needed for some drivers: for instance,
> >>pwm-backlight needs to check that exactly one PWM has been
> >>allocated, and takes a reference to it from this list in order to
> >>control the brightness.
> >
> >I'm not complaining about the additional argument to power_seq_build()
> >but about the missing encapsulation. I just think that keeping a list
> >external to the power sequencing code is error-prone. Drivers could do
> >just about anything with it between calls to power_seq_build(). If you
> >do all of this internally, then you don't depend on the driver at all
> >and power sequencing code can just do the right thing.
>
> On the opposite side, I am concerned about over-encapsulation. :)
> IIRC you proposed to have a top structure to hold the power
> sequences, their resources and the associated device. Power
> sequences would then have a name and be run through a 2 arguments
> power_seq_run():
>
> power_seq_run(sequences, "up");
>
> There are two things that bother me with this solution. First is
> that addressing power sequences by name looks a little bit overkill,
> when a single pointer should be enough. It would also complicate the
> design. Second thing is that this design would place the power
> sequences structure on top of the device - in effect, you could
> perfectly have several of these structures all using the same device
> and failing to see each other's resources. While that would be a
> error from the device driver's side, the design allows it.
I see. Perhaps I'm just bugged by the interface being a simple list. If
it was something just a little more sophisticated, like a very primitive
resource manager attached to one device, I would be appeased. Maybe an
opaque structure that carries the list and hides it for drivers would do
as well.
> >Obtaining a reference to the PWM, or any other resource for that matter,
> >from the power sequence could be done via an explicit API.
> >
> >>Ideally we could embed the list into the device structure, but I
> >>don't see how we can do that without modifying it (and we don't want
> >>to modify it). Another solution would be to keep a static mapping
> >>table that associates a device to its power_seq related resources
> >>within power_seq.c. If we protect it for concurrent access this
> >>should make it possible to make resources management transparent.
> >>How does this sound? Only drawback I see is that we would need to
> >>explicitly clean it up through a dedicated function when the driver
> >>exits.
> >
> >I don't think that's much better. Since the power sequences will be very
> >tightly coupled to a specific device, tying the sequences and their
> >resources to the device makes a lot of sense. Keeping a global list of
> >resources doesn't in my opinion.
>
> That is not what would happen actually - what I proposed is to have
> a mapping (hash map, or more likely binary tree) between a device
> and the list_head of the resources for that device. In C++ (forgive
> me, this makes the types more explicit) that would be:
>
> static std::map<struct device *, struct list_head> device_resources;
>
> That way you would have exactly one list per device, could keep
> resource-management totally transparent without exposing the
> list_head, and keep the API and design simple.
>
> For special cases (like pwm-backlight which needs to get the PWM),
> the list_head could be obtained through a dedicated API.
I understand. You could use an idr (include/linux/idr.h) for this
purpose. However I don't know if this would be any better than the
above.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 1/6] OMAPDSS: DSI: Maintain copy of operation mode in driver data
From: Tomi Valkeinen @ 2012-08-16 11:19 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345102594-6222-2-git-send-email-archit@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> The DSI driver currently relies on the omap_dss_device struct to know the mode
> of operation of the DSI protocol(command or video mode). This makes the DSI
> interface driver dependent on the omap_dss_device struct.
>
> Make the DSI driver data maintain it's own operation mode field. The panel
> driver is expected to call omapdss_dsi_set_operation_mode() before the interface
> is enabled.
>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> drivers/video/omap2/displays/panel-taal.c | 1 +
> drivers/video/omap2/dss/dsi.c | 42 +++++++++++++++++++++--------
> include/video/omapdss.h | 2 ++
> 3 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
> index d220f19..649247f 100644
> --- a/drivers/video/omap2/displays/panel-taal.c
> +++ b/drivers/video/omap2/displays/panel-taal.c
> @@ -1063,6 +1063,7 @@ static int taal_power_on(struct omap_dss_device *dssdev)
> omapdss_dsi_set_size(dssdev, dssdev->panel.timings.x_res,
> dssdev->panel.timings.y_res);
> omapdss_dsi_set_pixel_format(dssdev, dssdev->panel.dsi_pix_fmt);
> + omapdss_dsi_set_operation_mode(dssdev, dssdev->panel.dsi_mode);
Taal is always in cmd mode. Shouldn't we just use a hardcoded value
here?
Actually I think the same goes for the pix_fmt above. It's always the
same for Taal.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH V4 1/6] OMAPDSS: DISPC: cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-16 11:30 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <1344859135-4470-1-git-send-email-cmahapatra@ti.com>
All the cpu_is checks have been moved to dispc_init_features function providing
a much more generic and cleaner interface. The OMAP version and revision
specific functions and data are initialized by dispc_features structure which is
local to dispc.c.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dispc.c | 428 +++++++++++++++++++++++++--------------
1 file changed, 273 insertions(+), 155 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 5b289c5..0415845 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -81,6 +81,23 @@ struct dispc_irq_stats {
unsigned irqs[32];
};
+struct dispc_features {
+ int hp_max;
+ int vp_max;
+ int sw_max;
+ int sw_start;
+ int fp_start;
+ int bp_start;
+ int (*calc_scaling) (enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk);
+ unsigned long (*calc_core_clk) (enum omap_channel channel,
+ u16 width, u16 height, u16 out_width, u16 out_height);
+};
+
static struct {
struct platform_device *pdev;
void __iomem *base;
@@ -101,6 +118,8 @@ static struct {
bool ctx_valid;
u32 ctx[DISPC_SZ_REGS / sizeof(u32)];
+ const struct dispc_features *feat;
+
#ifdef CONFIG_OMAP2_DSS_COLLECT_IRQ_STATS
spinlock_t irq_stats_lock;
struct dispc_irq_stats irq_stats;
@@ -1939,7 +1958,18 @@ static unsigned long calc_core_clk_five_taps(enum omap_channel channel,
return core_clk;
}
-static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
+static unsigned long calc_core_clk_24xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height)
+{
+ unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+ if (height > out_height && width > out_width)
+ return pclk * 4;
+ else
+ return pclk * 2;
+}
+
+static unsigned long calc_core_clk_34xx(enum omap_channel channel, u16 width,
u16 height, u16 out_width, u16 out_height)
{
unsigned int hf, vf;
@@ -1958,25 +1988,163 @@ static unsigned long calc_core_clk(enum omap_channel channel, u16 width,
hf = 2;
else
hf = 1;
-
if (height > out_height)
vf = 2;
else
vf = 1;
- if (cpu_is_omap24xx()) {
- if (vf > 1 && hf > 1)
- return pclk * 4;
- else
- return pclk * 2;
- } else if (cpu_is_omap34xx()) {
- return pclk * vf * hf;
- } else {
- if (hf > 1)
- return DIV_ROUND_UP(pclk, out_width) * width;
- else
- return pclk;
+ return pclk * vf * hf;
+}
+
+static unsigned long calc_core_clk_44xx(enum omap_channel channel, u16 width,
+ u16 height, u16 out_width, u16 out_height)
+{
+ unsigned long pclk = dispc_mgr_pclk_rate(channel);
+
+ if (width > out_width)
+ return DIV_ROUND_UP(pclk, out_width) * width;
+ else
+ return pclk;
+}
+
+static int dispc_ovl_calc_scaling_24xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ int error;
+ u16 in_width, in_height;
+ int min_factor = min(*decim_x, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+ *five_taps = false;
+
+ do {
+ in_height = DIV_ROUND_UP(height, *decim_y);
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width,
+ in_height, out_width, out_height);
+ error = (in_width > maxsinglelinewidth || !*core_clk ||
+ *core_clk > dispc_core_clk_rate());
+ if (error) {
+ if (*decim_x = *decim_y) {
+ *decim_x = min_factor;
+ ++*decim_y;
+ } else {
+ swap(*decim_x, *decim_y);
+ if (*decim_x < *decim_y)
+ ++*decim_x;
+ }
+ }
+ } while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+ if (in_width > maxsinglelinewidth) {
+ DSSERR("Cannot scale max input width exceeded");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int dispc_ovl_calc_scaling_34xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ int error;
+ u16 in_width, in_height;
+ int min_factor = min(*decim_x, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+ do {
+ in_height = DIV_ROUND_UP(height, *decim_y);
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ *core_clk = calc_core_clk_five_taps(channel, mgr_timings,
+ in_width, in_height, out_width, out_height, color_mode);
+
+ error = check_horiz_timing_omap3(channel, mgr_timings, pos_x,
+ in_width, in_height, out_width, out_height);
+
+ if (in_width > maxsinglelinewidth)
+ if (in_height > out_height &&
+ in_height < out_height * 2)
+ *five_taps = false;
+ if (!*five_taps)
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width,
+ in_height, out_width, out_height);
+
+ error = (error || in_width > maxsinglelinewidth * 2 ||
+ (in_width > maxsinglelinewidth && *five_taps) ||
+ !*core_clk || *core_clk > dispc_core_clk_rate());
+ if (error) {
+ if (*decim_x = *decim_y) {
+ *decim_x = min_factor;
+ ++*decim_y;
+ } else {
+ swap(*decim_x, *decim_y);
+ if (*decim_x < *decim_y)
+ ++*decim_x;
+ }
+ }
+ } while (*decim_x <= *x_predecim && *decim_y <= *y_predecim && error);
+
+ if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width, height,
+ out_width, out_height)){
+ DSSERR("horizontal timing too tight\n");
+ return -EINVAL;
}
+
+ if (in_width > (maxsinglelinewidth * 2)) {
+ DSSERR("Cannot setup scaling");
+ DSSERR("width exceeds maximum width possible");
+ return -EINVAL;
+ }
+
+ if (in_width > maxsinglelinewidth && *five_taps) {
+ DSSERR("cannot setup scaling with five taps");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int dispc_ovl_calc_scaling_44xx(enum omap_channel channel,
+ const struct omap_video_timings *mgr_timings,
+ u16 width, u16 height, u16 out_width, u16 out_height,
+ enum omap_color_mode color_mode, bool *five_taps,
+ int *x_predecim, int *y_predecim, int *decim_x, int *decim_y,
+ u16 pos_x, unsigned long *core_clk)
+{
+ u16 in_width, in_width_max;
+ int decim_x_min = *decim_x;
+ u16 in_height = DIV_ROUND_UP(height, *decim_y);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
+
+ in_width_max = dispc_core_clk_rate() /
+ DIV_ROUND_UP(dispc_mgr_pclk_rate(channel), out_width);
+ *decim_x = DIV_ROUND_UP(width, in_width_max);
+
+ *decim_x = *decim_x > decim_x_min ? *decim_x : decim_x_min;
+ if (*decim_x > *x_predecim)
+ return -EINVAL;
+
+ do {
+ in_width = DIV_ROUND_UP(width, *decim_x);
+ } while (*decim_x <= *x_predecim &&
+ in_width > maxsinglelinewidth && ++*decim_x);
+
+ if (in_width > maxsinglelinewidth) {
+ DSSERR("Cannot scale width exceeds max line width");
+ return -EINVAL;
+ }
+
+ *core_clk = dispc.feat->calc_core_clk(channel, in_width, in_height,
+ out_width, out_height);
+ return 0;
}
static int dispc_ovl_calc_scaling(enum omap_plane plane,
@@ -1988,12 +2156,9 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
- const int maxsinglelinewidth - dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
const int max_decim_limit = 16;
unsigned long core_clk = 0;
- int decim_x, decim_y, error, min_factor;
- u16 in_width, in_height, in_width_max = 0;
+ int decim_x, decim_y, ret;
if (width = out_width && height = out_height)
return 0;
@@ -2017,118 +2182,17 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
decim_x = DIV_ROUND_UP(DIV_ROUND_UP(width, out_width), maxdownscale);
decim_y = DIV_ROUND_UP(DIV_ROUND_UP(height, out_height), maxdownscale);
- min_factor = min(decim_x, decim_y);
-
if (decim_x > *x_predecim || out_width > width * 8)
return -EINVAL;
if (decim_y > *y_predecim || out_height > height * 8)
return -EINVAL;
- if (cpu_is_omap24xx()) {
- *five_taps = false;
-
- do {
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width = DIV_ROUND_UP(width, decim_x);
- core_clk = calc_core_clk(channel, in_width, in_height,
- out_width, out_height);
- error = (in_width > maxsinglelinewidth || !core_clk ||
- core_clk > dispc_core_clk_rate());
- if (error) {
- if (decim_x = decim_y) {
- decim_x = min_factor;
- decim_y++;
- } else {
- swap(decim_x, decim_y);
- if (decim_x < decim_y)
- decim_x++;
- }
- }
- } while (decim_x <= *x_predecim && decim_y <= *y_predecim &&
- error);
-
- if (in_width > maxsinglelinewidth) {
- DSSERR("Cannot scale max input width exceeded");
- return -EINVAL;
- }
- } else if (cpu_is_omap34xx()) {
-
- do {
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width = DIV_ROUND_UP(width, decim_x);
- core_clk = calc_core_clk_five_taps(channel, mgr_timings,
- in_width, in_height, out_width, out_height,
- color_mode);
-
- error = check_horiz_timing_omap3(channel, mgr_timings,
- pos_x, in_width, in_height, out_width,
- out_height);
-
- if (in_width > maxsinglelinewidth)
- if (in_height > out_height &&
- in_height < out_height * 2)
- *five_taps = false;
- if (!*five_taps)
- core_clk = calc_core_clk(channel, in_width,
- in_height, out_width, out_height);
- error = (error || in_width > maxsinglelinewidth * 2 ||
- (in_width > maxsinglelinewidth && *five_taps) ||
- !core_clk || core_clk > dispc_core_clk_rate());
- if (error) {
- if (decim_x = decim_y) {
- decim_x = min_factor;
- decim_y++;
- } else {
- swap(decim_x, decim_y);
- if (decim_x < decim_y)
- decim_x++;
- }
- }
- } while (decim_x <= *x_predecim && decim_y <= *y_predecim
- && error);
-
- if (check_horiz_timing_omap3(channel, mgr_timings, pos_x, width,
- height, out_width, out_height)){
- DSSERR("horizontal timing too tight\n");
- return -EINVAL;
- }
-
- if (in_width > (maxsinglelinewidth * 2)) {
- DSSERR("Cannot setup scaling");
- DSSERR("width exceeds maximum width possible");
- return -EINVAL;
- }
-
- if (in_width > maxsinglelinewidth && *five_taps) {
- DSSERR("cannot setup scaling with five taps");
- return -EINVAL;
- }
- } else {
- int decim_x_min = decim_x;
- in_height = DIV_ROUND_UP(height, decim_y);
- in_width_max = dispc_core_clk_rate() /
- DIV_ROUND_UP(dispc_mgr_pclk_rate(channel),
- out_width);
- decim_x = DIV_ROUND_UP(width, in_width_max);
-
- decim_x = decim_x > decim_x_min ? decim_x : decim_x_min;
- if (decim_x > *x_predecim)
- return -EINVAL;
-
- do {
- in_width = DIV_ROUND_UP(width, decim_x);
- } while (decim_x <= *x_predecim &&
- in_width > maxsinglelinewidth && decim_x++);
-
- if (in_width > maxsinglelinewidth) {
- DSSERR("Cannot scale width exceeds max line width");
- return -EINVAL;
- }
-
- core_clk = calc_core_clk(channel, in_width, in_height,
- out_width, out_height);
- }
+ ret = dispc.feat->calc_scaling(channel, mgr_timings, width, height,
+ out_width, out_height, color_mode, five_taps, x_predecim,
+ y_predecim, &decim_x, &decim_y, pos_x, &core_clk);
+ if (ret)
+ return ret;
DSSDBG("required core clk rate = %lu Hz\n", core_clk);
DSSDBG("current core clk rate = %lu Hz\n", dispc_core_clk_rate());
@@ -2604,24 +2668,13 @@ static bool _dispc_mgr_size_ok(u16 width, u16 height)
static bool _dispc_lcd_timings_ok(int hsw, int hfp, int hbp,
int vsw, int vfp, int vbp)
{
- if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
- if (hsw < 1 || hsw > 64 ||
- hfp < 1 || hfp > 256 ||
- hbp < 1 || hbp > 256 ||
- vsw < 1 || vsw > 64 ||
- vfp < 0 || vfp > 255 ||
- vbp < 0 || vbp > 255)
- return false;
- } else {
- if (hsw < 1 || hsw > 256 ||
- hfp < 1 || hfp > 4096 ||
- hbp < 1 || hbp > 4096 ||
- vsw < 1 || vsw > 256 ||
- vfp < 0 || vfp > 4095 ||
- vbp < 0 || vbp > 4095)
- return false;
- }
-
+ if (hsw < 1 || hsw > dispc.feat->sw_max ||
+ hfp < 1 || hfp > dispc.feat->hp_max ||
+ hbp < 1 || hbp > dispc.feat->hp_max ||
+ vsw < 1 || vsw > dispc.feat->sw_max ||
+ vfp < 0 || vfp > dispc.feat->vp_max ||
+ vbp < 0 || vbp > dispc.feat->vp_max)
+ return false;
return true;
}
@@ -2653,19 +2706,12 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, int hsw,
u32 timing_h, timing_v, l;
bool onoff, rf, ipc;
- if (cpu_is_omap24xx() || omap_rev() < OMAP3430_REV_ES3_0) {
- timing_h = FLD_VAL(hsw-1, 5, 0) | FLD_VAL(hfp-1, 15, 8) |
- FLD_VAL(hbp-1, 27, 20);
-
- timing_v = FLD_VAL(vsw-1, 5, 0) | FLD_VAL(vfp, 15, 8) |
- FLD_VAL(vbp, 27, 20);
- } else {
- timing_h = FLD_VAL(hsw-1, 7, 0) | FLD_VAL(hfp-1, 19, 8) |
- FLD_VAL(hbp-1, 31, 20);
-
- timing_v = FLD_VAL(vsw-1, 7, 0) | FLD_VAL(vfp, 19, 8) |
- FLD_VAL(vbp, 31, 20);
- }
+ timing_h = FLD_VAL(hsw-1, dispc.feat->sw_start, 0) |
+ FLD_VAL(hfp-1, dispc.feat->fp_start, 8) |
+ FLD_VAL(hbp-1, dispc.feat->bp_start, 20);
+ timing_v = FLD_VAL(vsw-1, dispc.feat->sw_start, 0) |
+ FLD_VAL(vfp, dispc.feat->fp_start, 8) |
+ FLD_VAL(vbp, dispc.feat->bp_start, 20);
dispc_write_reg(DISPC_TIMING_H(channel), timing_h);
dispc_write_reg(DISPC_TIMING_V(channel), timing_v);
@@ -3671,6 +3717,74 @@ static void _omap_dispc_initial_config(void)
dispc_ovl_enable_zorder_planes();
}
+static const struct dispc_features omap24xx_dispc_features __initconst = {
+ .hp_max = 256,
+ .vp_max = 255,
+ .sw_max = 64,
+ .sw_start = 5,
+ .fp_start = 15,
+ .bp_start = 27,
+ .calc_scaling = dispc_ovl_calc_scaling_24xx,
+ .calc_core_clk = calc_core_clk_24xx,
+};
+
+static const struct dispc_features omap34xx_rev1_0_dispc_features __initconst = {
+ .hp_max = 256,
+ .vp_max = 255,
+ .sw_max = 64,
+ .sw_start = 5,
+ .fp_start = 15,
+ .bp_start = 27,
+ .calc_scaling = dispc_ovl_calc_scaling_34xx,
+ .calc_core_clk = calc_core_clk_34xx,
+};
+
+static const struct dispc_features omap34xx_rev3_0_dispc_features __initconst = {
+ .hp_max = 4096,
+ .vp_max = 4095,
+ .sw_max = 256,
+ .sw_start = 7,
+ .fp_start = 19,
+ .bp_start = 31,
+ .calc_scaling = dispc_ovl_calc_scaling_34xx,
+ .calc_core_clk = calc_core_clk_34xx,
+};
+
+static const struct dispc_features omap44xx_dispc_features __initconst = {
+ .hp_max = 4096,
+ .vp_max = 4095,
+ .sw_max = 256,
+ .sw_start = 7,
+ .fp_start = 19,
+ .bp_start = 31,
+ .calc_scaling = dispc_ovl_calc_scaling_44xx,
+ .calc_core_clk = calc_core_clk_44xx,
+};
+
+static int __init dispc_init_features(struct device *dev)
+{
+ dispc.feat = devm_kzalloc(dev, sizeof(*dispc.feat), GFP_KERNEL);
+ if (!dispc.feat) {
+ dev_err(dev, "Failed to allocate DISPC Features\n");
+ return -ENOMEM;
+ }
+
+ if (cpu_is_omap24xx()) {
+ dispc.feat = &omap24xx_dispc_features;
+ } else if (cpu_is_omap34xx()) {
+ if (omap_rev() < OMAP3430_REV_ES3_0)
+ dispc.feat = &omap34xx_rev1_0_dispc_features;
+ else
+ dispc.feat = &omap34xx_rev3_0_dispc_features;
+ } else if (cpu_is_omap44xx()) {
+ dispc.feat = &omap44xx_dispc_features;
+ } else {
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
/* DISPC HW IP initialisation */
static int __init omap_dispchw_probe(struct platform_device *pdev)
{
@@ -3725,6 +3839,10 @@ static int __init omap_dispchw_probe(struct platform_device *pdev)
dispc.dss_clk = clk;
+ r = dispc_init_features(&dispc.pdev->dev);
+ if (r)
+ return r;
+
pm_runtime_enable(&pdev->dev);
r = dispc_runtime_get();
--
1.7.10
changes since V1
V2 : moved dispc_ops structure definitions from dss_features.c to dispc.c
renamed dispc_ops to dispc_features
V3 : moved dispc_features definitions close to dispc_init_features thereby
eliminating various functions declarations from top of dispc.c
used __initdata before dispc_features definitions and __init before
dispc_init_features()
initialized dispc.feat with devm_kzalloc
removed definitions of _dispc_lcd_timings_ok_24xx,
_dispc_lcd_timings_ok_44xx, _dispc_mgr_set_lcd_timings_hv_24xx and
_dispc_mgr_set_lcd_timings_hv_44xx replacing them with appropiate
variables
V4 : replaced __initdata with __initconst as per guidelines in
include/linux/init.h
renamed various dispc_features structures to give consistent naming
removed dispc.feat deinitialization code from omap_dispchw_remove()
added a check for omap4 in dispc_init_features()
^ permalink raw reply related
* [PATCH V4 3/6] OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
From: Chandrabhanu Mahapatra @ 2012-08-16 11:30 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <1344859176-4512-1-git-send-email-cmahapatra@ti.com>
All the cpu_is checks have been moved to dss_init_features function providing a
much more generic and cleaner interface. The OMAP version and revision specific
initializations in various functions are cleaned and the necessary data are
moved to dss_features structure which is local to dss.c.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dss.c | 114 ++++++++++++++++++++++++++---------------
1 file changed, 73 insertions(+), 41 deletions(-)
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 7b1c6ac..a9cb84b 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -31,6 +31,7 @@
#include <linux/clk.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/gfp.h>
#include <video/omapdss.h>
@@ -65,6 +66,12 @@ struct dss_reg {
static int dss_runtime_get(void);
static void dss_runtime_put(void);
+struct dss_features {
+ u16 fck_div_max;
+ int dss_fck_multiplier;
+ char *clk_name;
+};
+
static struct {
struct platform_device *pdev;
void __iomem *base;
@@ -83,6 +90,8 @@ static struct {
bool ctx_valid;
u32 ctx[DSS_SZ_REGS / sizeof(u32)];
+
+ const struct dss_features *feat;
} dss;
static const char * const dss_generic_clk_source_names[] = {
@@ -91,6 +100,30 @@ static const char * const dss_generic_clk_source_names[] = {
[OMAP_DSS_CLK_SRC_FCK] = "DSS_FCK",
};
+static const struct dss_features omap24xx_dss_features __initconst = {
+ .fck_div_max = 16,
+ .dss_fck_multiplier = 2,
+ .clk_name = NULL,
+};
+
+static const struct dss_features omap34xx_dss_features __initconst = {
+ .fck_div_max = 16,
+ .dss_fck_multiplier = 2,
+ .clk_name = "dpll4_m4_ck",
+};
+
+static const struct dss_features omap3630_dss_features __initconst = {
+ .fck_div_max = 32,
+ .dss_fck_multiplier = 1,
+ .clk_name = "dpll4_m4_ck",
+};
+
+static const struct dss_features omap44xx_dss_features __initconst = {
+ .fck_div_max = 32,
+ .dss_fck_multiplier = 1,
+ .clk_name = "dpll_per_m5x2_ck",
+};
+
static inline void dss_write_reg(const struct dss_reg idx, u32 val)
{
__raw_writel(val, dss.base + idx.idx);
@@ -236,7 +269,6 @@ const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src)
return dss_generic_clk_source_names[clk_src];
}
-
void dss_dump_clocks(struct seq_file *s)
{
unsigned long dpll4_ck_rate;
@@ -259,18 +291,10 @@ void dss_dump_clocks(struct seq_file *s)
seq_printf(s, "dpll4_ck %lu\n", dpll4_ck_rate);
- if (cpu_is_omap3630() || cpu_is_omap44xx())
- seq_printf(s, "%s (%s) = %lu / %lu = %lu\n",
- fclk_name, fclk_real_name,
- dpll4_ck_rate,
- dpll4_ck_rate / dpll4_m4_ck_rate,
- fclk_rate);
- else
- seq_printf(s, "%s (%s) = %lu / %lu * 2 = %lu\n",
- fclk_name, fclk_real_name,
- dpll4_ck_rate,
- dpll4_ck_rate / dpll4_m4_ck_rate,
- fclk_rate);
+ seq_printf(s, "%s (%s) = %lu / %lu * %d = %lu\n",
+ fclk_name, fclk_real_name, dpll4_ck_rate,
+ dpll4_ck_rate / dpll4_m4_ck_rate,
+ dss.feat->dss_fck_multiplier, fclk_rate);
} else {
seq_printf(s, "%s (%s) = %lu\n",
fclk_name, fclk_real_name,
@@ -470,7 +494,7 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
unsigned long fck, max_dss_fck;
- u16 fck_div, fck_div_max = 16;
+ u16 fck_div;
int match = 0;
int min_fck_per_pck;
@@ -480,9 +504,8 @@ int dss_calc_clock_div(unsigned long req_pck, struct dss_clock_info *dss_cinfo,
max_dss_fck = dss_feat_get_param_max(FEAT_PARAM_DSS_FCK);
fck = clk_get_rate(dss.dss_clk);
- if (req_pck = dss.cache_req_pck &&
- ((cpu_is_omap34xx() && prate = dss.cache_prate) ||
- dss.cache_dss_cinfo.fck = fck)) {
+ if (req_pck = dss.cache_req_pck && prate = dss.cache_prate &&
+ dss.cache_dss_cinfo.fck = fck) {
DSSDBG("dispc clock info found from cache.\n");
*dss_cinfo = dss.cache_dss_cinfo;
*dispc_cinfo = dss.cache_dispc_cinfo;
@@ -519,16 +542,10 @@ retry:
goto found;
} else {
- if (cpu_is_omap3630() || cpu_is_omap44xx())
- fck_div_max = 32;
-
- for (fck_div = fck_div_max; fck_div > 0; --fck_div) {
+ for (fck_div = dss.feat->fck_div_max; fck_div > 0; --fck_div) {
struct dispc_clock_info cur_dispc;
- if (fck_div_max = 32)
- fck = prate / fck_div;
- else
- fck = prate / fck_div * 2;
+ fck = prate / fck_div * dss.feat->dss_fck_multiplier;
if (fck > max_dss_fck)
continue;
@@ -633,22 +650,11 @@ static int dss_get_clocks(void)
dss.dss_clk = clk;
- if (cpu_is_omap34xx()) {
- clk = clk_get(NULL, "dpll4_m4_ck");
- if (IS_ERR(clk)) {
- DSSERR("Failed to get dpll4_m4_ck\n");
- r = PTR_ERR(clk);
- goto err;
- }
- } else if (cpu_is_omap44xx()) {
- clk = clk_get(NULL, "dpll_per_m5x2_ck");
- if (IS_ERR(clk)) {
- DSSERR("Failed to get dpll_per_m5x2_ck\n");
- r = PTR_ERR(clk);
- goto err;
- }
- } else { /* omap24xx */
- clk = NULL;
+ clk = clk_get(NULL, dss.feat->clk_name);
+ if (IS_ERR(clk)) {
+ DSSERR("Failed to get %s\n", dss.feat->clk_name);
+ r = PTR_ERR(clk);
+ goto err;
}
dss.dpll4_m4_ck = clk;
@@ -704,6 +710,28 @@ void dss_debug_dump_clocks(struct seq_file *s)
}
#endif
+static int __init dss_init_features(struct device *dev)
+{
+ dss.feat = devm_kzalloc(dev, sizeof(*dss.feat), GFP_KERNEL);
+ if (!dss.feat) {
+ dev_err(dev, "Failed to allocate local DSS Features\n");
+ return -ENOMEM;
+ }
+
+ if (cpu_is_omap24xx())
+ dss.feat = &omap24xx_dss_features;
+ else if (cpu_is_omap34xx())
+ dss.feat = &omap34xx_dss_features;
+ else if (cpu_is_omap3630())
+ dss.feat = &omap3630_dss_features;
+ else if (cpu_is_omap44xx())
+ dss.feat = &omap44xx_dss_features;
+ else
+ return -ENODEV;
+
+ return 0;
+}
+
/* DSS HW IP initialisation */
static int __init omap_dsshw_probe(struct platform_device *pdev)
{
@@ -750,6 +778,10 @@ static int __init omap_dsshw_probe(struct platform_device *pdev)
dss.lcd_clk_source[0] = OMAP_DSS_CLK_SRC_FCK;
dss.lcd_clk_source[1] = OMAP_DSS_CLK_SRC_FCK;
+ r = dss_init_features(&dss.pdev->dev);
+ if (r)
+ return r;
+
rev = dss_read_reg(DSS_REVISION);
printk(KERN_INFO "OMAP DSS rev %d.%d\n",
FLD_GET(rev, 7, 4), FLD_GET(rev, 3, 0));
--
1.7.10
changes since V1
V2 : moved dss_ops structure definitions from dss_features.c to dss.c
renamed dss_ops to dss_features
removed dss_get_clk_24xx(), dss_get_clk_34xx(), dss_get_clk_44xx,
set_dump_clk_str_24_34() and set_dump_clk_str() replacing them
with appropiate variables in dss_features structure
V3 : used __initdata before dss_features definitions and __init before
dss_init_features()
initialized dss.feat with devm_kzalloc
removed definitions of check_dss_cinfo_fck(),
check_dss_cinfo_fck_34xx(), dss_get_clk_24xx(), dss_get_clk_34xx()
and dss_get_clk_44xx() replacing them with appropiate variables
V4 : replaced __initdata with __initconst as per guidelines in
include/linux/init.h
renamed various dss_features structures to give consistent naming
removed dss.feat deinitialization code from omap_dispchw_remove()
replaced #include <linux/dma-mapping.h> with <linux/gfx.h>
renamed factor variable of dss_features structure with
dss_fck_multiplier
added a check for omap4 in dss_init_features()
^ permalink raw reply related
* Re: [PATCH 3/6] OMAPDSS: DSI: Maintain copy of video mode timings in driver data
From: Tomi Valkeinen @ 2012-08-16 11:31 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345102594-6222-4-git-send-email-archit@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> The DSI driver currently relies on the omap_dss_device struct to receive the
> video mode timings requested by the panel driver. This makes the DSI interface
> driver dependent on the omap_dss_device struct.
>
> Make the DSI driver data maintain it's own video mode timings field. The panel
> driver is expected to call omapdss_dsi_set_videomode_timings() to configure the
> video mode timings before the interface is enabled. The function takes in a
> void pointer rather than a pointer to omap_dss_dsi_videomode_timings struct.
> This is because this function will finally be an output op shared across
> different outputs to set custom or private timings.
I don't think the function should take a void * in any case. If we want
to share the function, it should take a struct that perhaps contains an
union of rfbi and dsi timings.
But I'm not sure if there's any benefit for that...
So do you see us having just one set_timings, which would take either
the normal video timings, rfbi timings or dsi timings?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 6/6] OMAPDSS: VENC: Maintian copy of video output polarity in private data
From: Tomi Valkeinen @ 2012-08-16 11:38 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345102594-6222-7-git-send-email-archit@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2294 bytes --]
On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> The VENC driver currently relies on the omap_dss_device struct to configure the
> video output polarity. This makes the VENC interface driver dependent on the
> omap_dss_device struct.
>
> Make the VENC driver data maintain it's own polarity field. A panel driver
> is expected to call omapdss_venc_set_vid_out_polarity() before enabling the
> interface.
>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> drivers/video/omap2/dss/dss.h | 2 ++
> drivers/video/omap2/dss/venc.c | 13 ++++++++++++-
> drivers/video/omap2/dss/venc_panel.c | 6 ++++++
> 3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index c17d298..b2cf5530 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -479,6 +479,8 @@ u32 omapdss_venc_get_wss(struct omap_dss_device *dssdev);
> int omapdss_venc_set_wss(struct omap_dss_device *dssdev, u32 wss);
> void omapdss_venc_set_type(struct omap_dss_device *dssdev,
> enum omap_dss_venc_type type);
> +void omapdss_venc_set_vid_out_polarity(struct omap_dss_device *dssdev,
> + enum omap_dss_signal_level vid_out_pol);
> int venc_panel_init(void);
> void venc_panel_exit(void);
>
> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
> index 2d90fcf..8cb372f 100644
> --- a/drivers/video/omap2/dss/venc.c
> +++ b/drivers/video/omap2/dss/venc.c
> @@ -303,6 +303,7 @@ static struct {
>
> struct omap_video_timings timings;
> enum omap_dss_venc_type type;
> + enum omap_dss_signal_level vid_out_pol;
> } venc;
>
> static inline void venc_write_reg(int idx, u32 val)
> @@ -447,7 +448,7 @@ static int venc_power_on(struct omap_dss_device *dssdev)
> else /* S-Video */
> l |= (1 << 0) | (1 << 2);
>
> - if (dssdev->phy.venc.invert_polarity == false)
> + if (venc.vid_out_pol == OMAPDSS_SIG_ACTIVE_HIGH)
> l |= 1 << 3;
Are you sure this is correct? I know practically nothing about analog
TV, but the TRM doesn't seem to say much about that bit, except it can
be used to "invert the video output". It doesn't say there's an
active/inactive level for the signal.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 3/6] OMAPDSS: DSI: Maintain copy of video mode timings in driver data
From: Archit Taneja @ 2012-08-16 11:58 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345116666.15132.4.camel@lappyti>
On Thursday 16 August 2012 05:01 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
>> The DSI driver currently relies on the omap_dss_device struct to receive the
>> video mode timings requested by the panel driver. This makes the DSI interface
>> driver dependent on the omap_dss_device struct.
>>
>> Make the DSI driver data maintain it's own video mode timings field. The panel
>> driver is expected to call omapdss_dsi_set_videomode_timings() to configure the
>> video mode timings before the interface is enabled. The function takes in a
>> void pointer rather than a pointer to omap_dss_dsi_videomode_timings struct.
>> This is because this function will finally be an output op shared across
>> different outputs to set custom or private timings.
>
> I don't think the function should take a void * in any case. If we want
> to share the function, it should take a struct that perhaps contains an
> union of rfbi and dsi timings.
>
> But I'm not sure if there's any benefit for that...
>
> So do you see us having just one set_timings, which would take either
> the normal video timings, rfbi timings or dsi timings?
I thought of having 2 timing ops, one is a standard "modeline like"
set_timings(), and the other a vague-ish set_custom_timings(). For
us(OMAP), we need to use it for DSI videomode and RFBI, we may reduce
that to only RFBI later if we calculate for DSI timings automatically.
For these extra timings to be consistent across SoCs, we would need to
align to get a common struct of some sort, which could then have unions
as you said. For now, I thought having a void pointer might suffice.
Archit
^ permalink raw reply
* Re: [PATCH 3/6] OMAPDSS: DSI: Maintain copy of video mode timings in driver data
From: Tomi Valkeinen @ 2012-08-16 12:14 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <502CE016.6060404@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2237 bytes --]
On Thu, 2012-08-16 at 17:27 +0530, Archit Taneja wrote:
> On Thursday 16 August 2012 05:01 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> >> The DSI driver currently relies on the omap_dss_device struct to receive the
> >> video mode timings requested by the panel driver. This makes the DSI interface
> >> driver dependent on the omap_dss_device struct.
> >>
> >> Make the DSI driver data maintain it's own video mode timings field. The panel
> >> driver is expected to call omapdss_dsi_set_videomode_timings() to configure the
> >> video mode timings before the interface is enabled. The function takes in a
> >> void pointer rather than a pointer to omap_dss_dsi_videomode_timings struct.
> >> This is because this function will finally be an output op shared across
> >> different outputs to set custom or private timings.
> >
> > I don't think the function should take a void * in any case. If we want
> > to share the function, it should take a struct that perhaps contains an
> > union of rfbi and dsi timings.
> >
> > But I'm not sure if there's any benefit for that...
> >
> > So do you see us having just one set_timings, which would take either
> > the normal video timings, rfbi timings or dsi timings?
>
> I thought of having 2 timing ops, one is a standard "modeline like"
> set_timings(), and the other a vague-ish set_custom_timings(). For
> us(OMAP), we need to use it for DSI videomode and RFBI, we may reduce
> that to only RFBI later if we calculate for DSI timings automatically.
>
> For these extra timings to be consistent across SoCs, we would need to
> align to get a common struct of some sort, which could then have unions
> as you said. For now, I thought having a void pointer might suffice.
I would only use void * when it's a must, i.e. when really anything can
be passed as a parameter. In our case, I think having just two separate
functions is best.
I think the timings in videomode and rfbi structs should be SoC
independent even now, at least more or less. But you're right, it should
be verified. And any names in the structs should be according to DBI or
DSI spec, not OMAP. But this is for later.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v4] fbdev: sh_mobile_lcdc: use dma_mmap_coherent
From: Laurent Pinchart @ 2012-08-16 12:16 UTC (permalink / raw)
To: Hideki EIRAKU
Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel,
Marek Szyprowski, Katsuya MATSUBARA, Damian Hobson-Garcia
In-Reply-To: <1345112000-22738-1-git-send-email-hdk@igel.co.jp>
Hi Eiraku-san,
On Thursday 16 August 2012 19:13:20 Hideki EIRAKU wrote:
> fb_mmap() implemented in fbmem.c uses smem_start as the physical
> address of the frame buffer. In the sh_mobile_lcdc driver, the
> smem_start is a dma_addr_t that is not a physical address when IOMMU is
> enabled. dma_mmap_coherent() maps the address correctly.
>
> Signed-off-by: Hideki EIRAKU <hdk@igel.co.jp>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
I will push the patch to v3.7 through my tree.
> ---
> drivers/video/sh_mobile_lcdcfb.c | 20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/sh_mobile_lcdcfb.c
> b/drivers/video/sh_mobile_lcdcfb.c index 699487c..bccfd7e 100644
> --- a/drivers/video/sh_mobile_lcdcfb.c
> +++ b/drivers/video/sh_mobile_lcdcfb.c
> @@ -1614,6 +1614,15 @@ static int sh_mobile_lcdc_overlay_blank(int blank,
> struct fb_info *info) return 1;
> }
>
> +static int
> +sh_mobile_lcdc_overlay_mmap(struct fb_info *info, struct vm_area_struct
> *vma) +{
> + struct sh_mobile_lcdc_overlay *ovl = info->par;
> +
> + return dma_mmap_coherent(ovl->channel->lcdc->dev, vma, ovl->fb_mem,
> + ovl->dma_handle, ovl->fb_size);
> +}
> +
> static struct fb_ops sh_mobile_lcdc_overlay_ops = {
> .owner = THIS_MODULE,
> .fb_read = fb_sys_read,
> @@ -1626,6 +1635,7 @@ static struct fb_ops sh_mobile_lcdc_overlay_ops = {
> .fb_ioctl = sh_mobile_lcdc_overlay_ioctl,
> .fb_check_var = sh_mobile_lcdc_overlay_check_var,
> .fb_set_par = sh_mobile_lcdc_overlay_set_par,
> + .fb_mmap = sh_mobile_lcdc_overlay_mmap,
> };
>
> static void
> @@ -2093,6 +2103,15 @@ static int sh_mobile_lcdc_blank(int blank, struct
> fb_info *info) return 0;
> }
>
> +static int
> +sh_mobile_lcdc_mmap(struct fb_info *info, struct vm_area_struct *vma)
> +{
> + struct sh_mobile_lcdc_chan *ch = info->par;
> +
> + return dma_mmap_coherent(ch->lcdc->dev, vma, ch->fb_mem,
> + ch->dma_handle, ch->fb_size);
> +}
> +
> static struct fb_ops sh_mobile_lcdc_ops = {
> .owner = THIS_MODULE,
> .fb_setcolreg = sh_mobile_lcdc_setcolreg,
> @@ -2108,6 +2127,7 @@ static struct fb_ops sh_mobile_lcdc_ops = {
> .fb_release = sh_mobile_lcdc_release,
> .fb_check_var = sh_mobile_lcdc_check_var,
> .fb_set_par = sh_mobile_lcdc_set_par,
> + .fb_mmap = sh_mobile_lcdc_mmap,
> };
>
> static void
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 1/6] OMAPDSS: DSI: Maintain copy of operation mode in driver data
From: Archit Taneja @ 2012-08-16 12:23 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345115964.15132.2.camel@lappyti>
On Thursday 16 August 2012 04:49 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
>> The DSI driver currently relies on the omap_dss_device struct to know the mode
>> of operation of the DSI protocol(command or video mode). This makes the DSI
>> interface driver dependent on the omap_dss_device struct.
>>
>> Make the DSI driver data maintain it's own operation mode field. The panel
>> driver is expected to call omapdss_dsi_set_operation_mode() before the interface
>> is enabled.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>> drivers/video/omap2/displays/panel-taal.c | 1 +
>> drivers/video/omap2/dss/dsi.c | 42 +++++++++++++++++++++--------
>> include/video/omapdss.h | 2 ++
>> 3 files changed, 34 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
>> index d220f19..649247f 100644
>> --- a/drivers/video/omap2/displays/panel-taal.c
>> +++ b/drivers/video/omap2/displays/panel-taal.c
>> @@ -1063,6 +1063,7 @@ static int taal_power_on(struct omap_dss_device *dssdev)
>> omapdss_dsi_set_size(dssdev, dssdev->panel.timings.x_res,
>> dssdev->panel.timings.y_res);
>> omapdss_dsi_set_pixel_format(dssdev, dssdev->panel.dsi_pix_fmt);
>> + omapdss_dsi_set_operation_mode(dssdev, dssdev->panel.dsi_mode);
>
> Taal is always in cmd mode. Shouldn't we just use a hardcoded value
> here?
>
> Actually I think the same goes for the pix_fmt above. It's always the
> same for Taal.
Yes, I'll do this. I guess we could gradually remove some of these
fields from omap_dss_device? Can't we? I mean, if there are no panels
which can ever support command and video mode together, then we can just
leave it to the driver to hold this information.
Archit
^ permalink raw reply
* Re: [PATCH 1/6] OMAPDSS: DSI: Maintain copy of operation mode in driver data
From: Tomi Valkeinen @ 2012-08-16 12:23 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
In-Reply-To: <502CE36A.5020700@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]
On Thu, 2012-08-16 at 17:41 +0530, Archit Taneja wrote:
> On Thursday 16 August 2012 04:49 PM, Tomi Valkeinen wrote:
> > On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
> >> The DSI driver currently relies on the omap_dss_device struct to know the mode
> >> of operation of the DSI protocol(command or video mode). This makes the DSI
> >> interface driver dependent on the omap_dss_device struct.
> >>
> >> Make the DSI driver data maintain it's own operation mode field. The panel
> >> driver is expected to call omapdss_dsi_set_operation_mode() before the interface
> >> is enabled.
> >>
> >> Signed-off-by: Archit Taneja <archit@ti.com>
> >> ---
> >> drivers/video/omap2/displays/panel-taal.c | 1 +
> >> drivers/video/omap2/dss/dsi.c | 42 +++++++++++++++++++++--------
> >> include/video/omapdss.h | 2 ++
> >> 3 files changed, 34 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
> >> index d220f19..649247f 100644
> >> --- a/drivers/video/omap2/displays/panel-taal.c
> >> +++ b/drivers/video/omap2/displays/panel-taal.c
> >> @@ -1063,6 +1063,7 @@ static int taal_power_on(struct omap_dss_device *dssdev)
> >> omapdss_dsi_set_size(dssdev, dssdev->panel.timings.x_res,
> >> dssdev->panel.timings.y_res);
> >> omapdss_dsi_set_pixel_format(dssdev, dssdev->panel.dsi_pix_fmt);
> >> + omapdss_dsi_set_operation_mode(dssdev, dssdev->panel.dsi_mode);
> >
> > Taal is always in cmd mode. Shouldn't we just use a hardcoded value
> > here?
> >
> > Actually I think the same goes for the pix_fmt above. It's always the
> > same for Taal.
>
> Yes, I'll do this. I guess we could gradually remove some of these
> fields from omap_dss_device? Can't we? I mean, if there are no panels
> which can ever support command and video mode together, then we can just
> leave it to the driver to hold this information.
Yes, we can remove them. Even if there was a panel taht supports both
cmd and video mode, it would be the panel driver's job to handle it, and
any parameters passed to the driver would happen via the panel's own
platform data, not via dssdev.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 6/6] OMAPDSS: VENC: Maintian copy of video output polarity in private data
From: Archit Taneja @ 2012-08-16 12:39 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1345117118.15132.7.camel@lappyti>
On Thursday 16 August 2012 05:08 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-16 at 13:06 +0530, Archit Taneja wrote:
>> The VENC driver currently relies on the omap_dss_device struct to configure the
>> video output polarity. This makes the VENC interface driver dependent on the
>> omap_dss_device struct.
>>
>> Make the VENC driver data maintain it's own polarity field. A panel driver
>> is expected to call omapdss_venc_set_vid_out_polarity() before enabling the
>> interface.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>> drivers/video/omap2/dss/dss.h | 2 ++
>> drivers/video/omap2/dss/venc.c | 13 ++++++++++++-
>> drivers/video/omap2/dss/venc_panel.c | 6 ++++++
>> 3 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
>> index c17d298..b2cf5530 100644
>> --- a/drivers/video/omap2/dss/dss.h
>> +++ b/drivers/video/omap2/dss/dss.h
>> @@ -479,6 +479,8 @@ u32 omapdss_venc_get_wss(struct omap_dss_device *dssdev);
>> int omapdss_venc_set_wss(struct omap_dss_device *dssdev, u32 wss);
>> void omapdss_venc_set_type(struct omap_dss_device *dssdev,
>> enum omap_dss_venc_type type);
>> +void omapdss_venc_set_vid_out_polarity(struct omap_dss_device *dssdev,
>> + enum omap_dss_signal_level vid_out_pol);
>> int venc_panel_init(void);
>> void venc_panel_exit(void);
>>
>> diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
>> index 2d90fcf..8cb372f 100644
>> --- a/drivers/video/omap2/dss/venc.c
>> +++ b/drivers/video/omap2/dss/venc.c
>> @@ -303,6 +303,7 @@ static struct {
>>
>> struct omap_video_timings timings;
>> enum omap_dss_venc_type type;
>> + enum omap_dss_signal_level vid_out_pol;
>> } venc;
>>
>> static inline void venc_write_reg(int idx, u32 val)
>> @@ -447,7 +448,7 @@ static int venc_power_on(struct omap_dss_device *dssdev)
>> else /* S-Video */
>> l |= (1 << 0) | (1 << 2);
>>
>> - if (dssdev->phy.venc.invert_polarity = false)
>> + if (venc.vid_out_pol = OMAPDSS_SIG_ACTIVE_HIGH)
>> l |= 1 << 3;
>
> Are you sure this is correct? I know practically nothing about analog
> TV, but the TRM doesn't seem to say much about that bit, except it can
> be used to "invert the video output". It doesn't say there's an
> active/inactive level for the signal.
Well, the code works :), I'm also not totally sure about whether it
should be represented as a 2-level signal, it seemed like it would be
nicer to give it a signal level rather than keeping it as a bool, which
could vary for other SoC's?
I am considering not to pass this via the panel driver for now, I don't
know if all VENC IPs needs to do this, maybe it's okay to leave this
dssdev reference for now?
Archit
>
> Tomi
>
^ permalink raw reply
* [PATCH 00/19] SH Mobile LCDC panel cleanup (including board code)
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Hi everybody,
As part of a general panel API implementation (more on that a bit later), I've
cleaned up panel support in the LCDC driver and in the related board code. The
result is 19 patches independent of the generic panel API that I would like to
get reviewed by the respective board maintainers.
Laurent Pinchart (19):
fbdev: sh_mobile_lcdc: Get display dimensions from the channel
structure
fbdev: sh_mobile_lcdc: Rename mode argument to modes
fbdev: sh_mobile_lcdc: Remove priv argument from channel and overlay
init
ARM: mach-shmobile: ag5evm: Add LCDC tx_dev field to platform data
fbdev: sh_mipi_dsi: Add channel field to platform data
ARM: mach-shmobile: Initiliaze the new sh_mipi_dsi_info channel field
fbdev: sh_mipi_dsi: Use the sh_mipi_dsi_info channel field
fbdev: sh_mipi_dsi: Use the LCDC entity default mode
fbdev: sh_mipi_dsi: Remove last reference to LCDC platform data
ARM: mach-shmobile: Remove the unused sh_mipi_dsi_info lcd_chan field
fbdev: sh_mipi_dsi: Remove the unused sh_mipi_dsi_info lcd_chan field
fbdev: sh_mobile_lcdc: Store the backlight brightness internally
ARM: mach-shmobile: mackerel: Removed unused get_brightness callback
sh: ap325rxa: Remove unused get_brightness LCDC callback
sh: ecovec24: Remove unused get_brightness LCDC callback
fbdev: sh_mobile_lcdc: Remove unused get_brightness pdata callback
ARM: mach-shmobile: ag5evm: Use the backlight API for brightness
control
sh: kfr2r09: Use the backlight API for brightness control
fbdev: sh_mobile_lcdc: Make sh_mobile_lcdc_sys_bus_ops static
arch/arm/mach-shmobile/board-ag5evm.c | 198 ++++++++++++++-------------
arch/arm/mach-shmobile/board-ap4evb.c | 4 +-
arch/arm/mach-shmobile/board-mackerel.c | 6 -
arch/sh/boards/mach-ap325rxa/setup.c | 6 -
arch/sh/boards/mach-ecovec24/setup.c | 6 -
arch/sh/boards/mach-kfr2r09/lcd_wqvga.c | 16 +--
arch/sh/boards/mach-kfr2r09/setup.c | 7 +-
arch/sh/include/mach-kfr2r09/mach/kfr2r09.h | 6 +-
drivers/video/sh_mipi_dsi.c | 69 ++++------
drivers/video/sh_mobile_lcdcfb.c | 54 ++++----
drivers/video/sh_mobile_lcdcfb.h | 1 +
include/video/sh_mipi_dsi.h | 4 +-
include/video/sh_mobile_lcdc.h | 1 -
13 files changed, 172 insertions(+), 206 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 01/19] fbdev: sh_mobile_lcdc: Get display dimensions from the channel structure
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Get the display dimensions directly from the channel structure instead
of recomputing them from the monitor specs or accessing the platform
data.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index bccfd7e..7375dbf 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -586,8 +586,8 @@ static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch,
* Just turn on, if we run a resume here, the
* logo disappears.
*/
- info->var.width = monspec->max_x * 10;
- info->var.height = monspec->max_y * 10;
+ info->var.width = ch->display.width;
+ info->var.height = ch->display.height;
sh_mobile_lcdc_display_on(ch);
} else {
/* New monitor or have to wake up */
@@ -2248,8 +2248,8 @@ sh_mobile_lcdc_channel_fb_init(struct sh_mobile_lcdc_chan *ch,
*/
var = &info->var;
fb_videomode_to_var(var, mode);
- var->width = ch->cfg->panel_cfg.width;
- var->height = ch->cfg->panel_cfg.height;
+ var->width = ch->display.width;
+ var->height = ch->display.height;
var->xres_virtual = ch->xres_virtual;
var->yres_virtual = ch->yres_virtual;
var->activate = FB_ACTIVATE_NOW;
--
1.7.8.6
^ permalink raw reply related
* [PATCH 02/19] fbdev: sh_mobile_lcdc: Rename mode argument to modes
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
The sh_mobile_lcdc_channel_fb_init() mode argument is used to pass a
list of modes, rename it to modes.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 7375dbf..82b53f7 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -2187,7 +2187,7 @@ sh_mobile_lcdc_channel_fb_cleanup(struct sh_mobile_lcdc_chan *ch)
static int __devinit
sh_mobile_lcdc_channel_fb_init(struct sh_mobile_lcdc_chan *ch,
- const struct fb_videomode *mode,
+ const struct fb_videomode *modes,
unsigned int num_modes)
{
struct sh_mobile_lcdc_priv *priv = ch->lcdc;
@@ -2213,7 +2213,7 @@ sh_mobile_lcdc_channel_fb_init(struct sh_mobile_lcdc_chan *ch,
info->pseudo_palette = &ch->pseudo_palette;
info->par = ch;
- fb_videomode_to_modelist(mode, num_modes, &info->modelist);
+ fb_videomode_to_modelist(modes, num_modes, &info->modelist);
ret = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0);
if (ret < 0) {
@@ -2247,7 +2247,7 @@ sh_mobile_lcdc_channel_fb_init(struct sh_mobile_lcdc_chan *ch,
* default.
*/
var = &info->var;
- fb_videomode_to_var(var, mode);
+ fb_videomode_to_var(var, modes);
var->width = ch->display.width;
var->height = ch->display.height;
var->xres_virtual = ch->xres_virtual;
--
1.7.8.6
^ permalink raw reply related
* [PATCH 03/19] fbdev: sh_mobile_lcdc: Remove priv argument from channel and overlay init
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
The channel and overlay init functions operate on a channel and an
overlay, don't pass the priv parameter explicitly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mobile_lcdcfb.c | 35 +++++++++++++++++------------------
1 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 82b53f7..c169581 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -2536,10 +2536,10 @@ static int __devinit sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *
}
static int __devinit
-sh_mobile_lcdc_overlay_init(struct sh_mobile_lcdc_priv *priv,
- struct sh_mobile_lcdc_overlay *ovl)
+sh_mobile_lcdc_overlay_init(struct sh_mobile_lcdc_overlay *ovl)
{
const struct sh_mobile_lcdc_format_info *format;
+ struct device *dev = ovl->channel->lcdc->dev;
int ret;
if (ovl->cfg->fourcc = 0)
@@ -2548,7 +2548,7 @@ sh_mobile_lcdc_overlay_init(struct sh_mobile_lcdc_priv *priv,
/* Validate the format. */
format = sh_mobile_format_info(ovl->cfg->fourcc);
if (format = NULL) {
- dev_err(priv->dev, "Invalid FOURCC %08x\n", ovl->cfg->fourcc);
+ dev_err(dev, "Invalid FOURCC %08x\n", ovl->cfg->fourcc);
return -EINVAL;
}
@@ -2576,10 +2576,10 @@ sh_mobile_lcdc_overlay_init(struct sh_mobile_lcdc_priv *priv,
/* Allocate frame buffer memory. */
ovl->fb_size = ovl->cfg->max_xres * ovl->cfg->max_yres
* format->bpp / 8 * 2;
- ovl->fb_mem = dma_alloc_coherent(priv->dev, ovl->fb_size,
- &ovl->dma_handle, GFP_KERNEL);
+ ovl->fb_mem = dma_alloc_coherent(dev, ovl->fb_size, &ovl->dma_handle,
+ GFP_KERNEL);
if (!ovl->fb_mem) {
- dev_err(priv->dev, "unable to allocate buffer\n");
+ dev_err(dev, "unable to allocate buffer\n");
return -ENOMEM;
}
@@ -2591,11 +2591,11 @@ sh_mobile_lcdc_overlay_init(struct sh_mobile_lcdc_priv *priv,
}
static int __devinit
-sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_priv *priv,
- struct sh_mobile_lcdc_chan *ch)
+sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_chan *ch)
{
const struct sh_mobile_lcdc_format_info *format;
const struct sh_mobile_lcdc_chan_cfg *cfg = ch->cfg;
+ struct device *dev = ch->lcdc->dev;
const struct fb_videomode *max_mode;
const struct fb_videomode *mode;
unsigned int num_modes;
@@ -2608,7 +2608,7 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_priv *priv,
/* Validate the format. */
format = sh_mobile_format_info(cfg->fourcc);
if (format = NULL) {
- dev_err(priv->dev, "Invalid FOURCC %08x.\n", cfg->fourcc);
+ dev_err(dev, "Invalid FOURCC %08x.\n", cfg->fourcc);
return -EINVAL;
}
@@ -2624,7 +2624,7 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_priv *priv,
/* NV12/NV21 buffers must have even number of lines */
if ((cfg->fourcc = V4L2_PIX_FMT_NV12 ||
cfg->fourcc = V4L2_PIX_FMT_NV21) && (mode->yres & 0x1)) {
- dev_err(priv->dev, "yres must be multiple of 2 for "
+ dev_err(dev, "yres must be multiple of 2 for "
"YCbCr420 mode.\n");
return -EINVAL;
}
@@ -2638,7 +2638,7 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_priv *priv,
if (!max_size)
max_size = MAX_XRES * MAX_YRES;
else
- dev_dbg(priv->dev, "Found largest videomode %ux%u\n",
+ dev_dbg(dev, "Found largest videomode %ux%u\n",
max_mode->xres, max_mode->yres);
if (cfg->lcd_modes = NULL) {
@@ -2672,10 +2672,10 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_priv *priv,
/* Allocate frame buffer memory. */
ch->fb_size = max_size * format->bpp / 8 * 2;
- ch->fb_mem = dma_alloc_coherent(priv->dev, ch->fb_size, &ch->dma_handle,
+ ch->fb_mem = dma_alloc_coherent(dev, ch->fb_size, &ch->dma_handle,
GFP_KERNEL);
if (ch->fb_mem = NULL) {
- dev_err(priv->dev, "unable to allocate buffer\n");
+ dev_err(dev, "unable to allocate buffer\n");
return -ENOMEM;
}
@@ -2683,8 +2683,7 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_priv *priv,
if (cfg->tx_dev) {
if (!cfg->tx_dev->dev.driver ||
!try_module_get(cfg->tx_dev->dev.driver->owner)) {
- dev_warn(priv->dev,
- "unable to get transmitter device\n");
+ dev_warn(dev, "unable to get transmitter device\n");
return -EINVAL;
}
ch->tx_dev = platform_get_drvdata(cfg->tx_dev);
@@ -2792,9 +2791,9 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
pm_runtime_enable(&pdev->dev);
for (i = 0; i < num_channels; i++) {
- struct sh_mobile_lcdc_chan *ch = priv->ch + i;
+ struct sh_mobile_lcdc_chan *ch = &priv->ch[i];
- error = sh_mobile_lcdc_channel_init(priv, ch);
+ error = sh_mobile_lcdc_channel_init(ch);
if (error)
goto err1;
}
@@ -2805,7 +2804,7 @@ static int __devinit sh_mobile_lcdc_probe(struct platform_device *pdev)
ovl->cfg = &pdata->overlays[i];
ovl->channel = &priv->ch[0];
- error = sh_mobile_lcdc_overlay_init(priv, ovl);
+ error = sh_mobile_lcdc_overlay_init(ovl);
if (error)
goto err1;
}
--
1.7.8.6
^ permalink raw reply related
* [PATCH 04/19] ARM: mach-shmobile: ag5evm: Add LCDC tx_dev field to platform data
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Reference the MIPI-DSI transceiver in the LCDC platform data and make
sure it gets registered before the LCDC.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/mach-shmobile/board-ag5evm.c | 157 +++++++++++++++++----------------
1 files changed, 80 insertions(+), 77 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index 5a6f22f..afae573 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -199,6 +199,84 @@ static struct platform_device irda_device = {
.num_resources = ARRAY_SIZE(irda_resources),
};
+/* MIPI-DSI */
+static struct resource mipidsi0_resources[] = {
+ [0] = {
+ .name = "DSI0",
+ .start = 0xfeab0000,
+ .end = 0xfeab3fff,
+ .flags = IORESOURCE_MEM,
+ },
+ [1] = {
+ .name = "DSI0",
+ .start = 0xfeab4000,
+ .end = 0xfeab7fff,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static int sh_mipi_set_dot_clock(struct platform_device *pdev,
+ void __iomem *base,
+ int enable)
+{
+ struct clk *pck, *phy;
+ int ret;
+
+ pck = clk_get(&pdev->dev, "dsip_clk");
+ if (IS_ERR(pck)) {
+ ret = PTR_ERR(pck);
+ goto sh_mipi_set_dot_clock_pck_err;
+ }
+
+ phy = clk_get(&pdev->dev, "dsiphy_clk");
+ if (IS_ERR(phy)) {
+ ret = PTR_ERR(phy);
+ goto sh_mipi_set_dot_clock_phy_err;
+ }
+
+ if (enable) {
+ clk_set_rate(pck, clk_round_rate(pck, 24000000));
+ clk_set_rate(phy, clk_round_rate(pck, 510000000));
+ clk_enable(pck);
+ clk_enable(phy);
+ } else {
+ clk_disable(pck);
+ clk_disable(phy);
+ }
+
+ ret = 0;
+
+ clk_put(phy);
+sh_mipi_set_dot_clock_phy_err:
+ clk_put(pck);
+sh_mipi_set_dot_clock_pck_err:
+ return ret;
+}
+
+static struct sh_mobile_lcdc_info lcdc0_info;
+
+static struct sh_mipi_dsi_info mipidsi0_info = {
+ .data_format = MIPI_RGB888,
+ .lcd_chan = &lcdc0_info.ch[0],
+ .lane = 2,
+ .vsynw_offset = 20,
+ .clksrc = 1,
+ .flags = SH_MIPI_DSI_HSABM |
+ SH_MIPI_DSI_SYNC_PULSES_MODE |
+ SH_MIPI_DSI_HSbyteCLK,
+ .set_dot_clock = sh_mipi_set_dot_clock,
+};
+
+static struct platform_device mipidsi0_device = {
+ .name = "sh-mipi-dsi",
+ .num_resources = ARRAY_SIZE(mipidsi0_resources),
+ .resource = mipidsi0_resources,
+ .id = 0,
+ .dev = {
+ .platform_data = &mipidsi0_info,
+ },
+};
+
static unsigned char lcd_backlight_seq[3][2] = {
{ 0x04, 0x07 },
{ 0x23, 0x80 },
@@ -261,6 +339,7 @@ static struct sh_mobile_lcdc_info lcdc0_info = {
.display_on = lcd_backlight_on,
.display_off = lcd_backlight_reset,
},
+ .tx_dev = &mipidsi0_device,
}
};
@@ -288,82 +367,6 @@ static struct platform_device lcdc0_device = {
},
};
-/* MIPI-DSI */
-static struct resource mipidsi0_resources[] = {
- [0] = {
- .name = "DSI0",
- .start = 0xfeab0000,
- .end = 0xfeab3fff,
- .flags = IORESOURCE_MEM,
- },
- [1] = {
- .name = "DSI0",
- .start = 0xfeab4000,
- .end = 0xfeab7fff,
- .flags = IORESOURCE_MEM,
- },
-};
-
-static int sh_mipi_set_dot_clock(struct platform_device *pdev,
- void __iomem *base,
- int enable)
-{
- struct clk *pck, *phy;
- int ret;
-
- pck = clk_get(&pdev->dev, "dsip_clk");
- if (IS_ERR(pck)) {
- ret = PTR_ERR(pck);
- goto sh_mipi_set_dot_clock_pck_err;
- }
-
- phy = clk_get(&pdev->dev, "dsiphy_clk");
- if (IS_ERR(phy)) {
- ret = PTR_ERR(phy);
- goto sh_mipi_set_dot_clock_phy_err;
- }
-
- if (enable) {
- clk_set_rate(pck, clk_round_rate(pck, 24000000));
- clk_set_rate(phy, clk_round_rate(pck, 510000000));
- clk_enable(pck);
- clk_enable(phy);
- } else {
- clk_disable(pck);
- clk_disable(phy);
- }
-
- ret = 0;
-
- clk_put(phy);
-sh_mipi_set_dot_clock_phy_err:
- clk_put(pck);
-sh_mipi_set_dot_clock_pck_err:
- return ret;
-}
-
-static struct sh_mipi_dsi_info mipidsi0_info = {
- .data_format = MIPI_RGB888,
- .lcd_chan = &lcdc0_info.ch[0],
- .lane = 2,
- .vsynw_offset = 20,
- .clksrc = 1,
- .flags = SH_MIPI_DSI_HSABM |
- SH_MIPI_DSI_SYNC_PULSES_MODE |
- SH_MIPI_DSI_HSbyteCLK,
- .set_dot_clock = sh_mipi_set_dot_clock,
-};
-
-static struct platform_device mipidsi0_device = {
- .name = "sh-mipi-dsi",
- .num_resources = ARRAY_SIZE(mipidsi0_resources),
- .resource = mipidsi0_resources,
- .id = 0,
- .dev = {
- .platform_data = &mipidsi0_info,
- },
-};
-
/* SDHI0 */
static struct sh_mobile_sdhi_info sdhi0_info = {
.dma_slave_tx = SHDMA_SLAVE_SDHI0_TX,
@@ -460,8 +463,8 @@ static struct platform_device *ag5evm_devices[] __initdata = {
&fsi_device,
&mmc_device,
&irda_device,
- &lcdc0_device,
&mipidsi0_device,
+ &lcdc0_device,
&sdhi0_device,
&sdhi1_device,
};
--
1.7.8.6
^ permalink raw reply related
* [PATCH 05/19] fbdev: sh_mipi_dsi: Add channel field to platform data
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
The channel field stores the LCDC channel corresponding to the MIPI-DSI
transmitter. The information is currently obtained through the lcd_chan
field that will be removed.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
include/video/sh_mipi_dsi.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/video/sh_mipi_dsi.h b/include/video/sh_mipi_dsi.h
index 06c67fb..eae71d9 100644
--- a/include/video/sh_mipi_dsi.h
+++ b/include/video/sh_mipi_dsi.h
@@ -48,6 +48,7 @@ struct sh_mobile_lcdc_chan_cfg;
struct sh_mipi_dsi_info {
enum sh_mipi_dsi_data_fmt data_format;
struct sh_mobile_lcdc_chan_cfg *lcd_chan;
+ int channel;
int lane;
unsigned long flags;
u32 clksrc;
--
1.7.8.6
^ permalink raw reply related
* [PATCH 06/19] ARM: mach-shmobile: Initiliaze the new sh_mipi_dsi_info channel field
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/mach-shmobile/board-ag5evm.c | 1 +
arch/arm/mach-shmobile/board-ap4evb.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ag5evm.c b/arch/arm/mach-shmobile/board-ag5evm.c
index afae573..5523bc8 100644
--- a/arch/arm/mach-shmobile/board-ag5evm.c
+++ b/arch/arm/mach-shmobile/board-ag5evm.c
@@ -258,6 +258,7 @@ static struct sh_mobile_lcdc_info lcdc0_info;
static struct sh_mipi_dsi_info mipidsi0_info = {
.data_format = MIPI_RGB888,
.lcd_chan = &lcdc0_info.ch[0],
+ .channel = LCDC_CHAN_MAINLCD,
.lane = 2,
.vsynw_offset = 20,
.clksrc = 1,
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index ace6024..5fbd176 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -532,6 +532,7 @@ static struct sh_mobile_lcdc_info lcdc_info;
static struct sh_mipi_dsi_info mipidsi0_info = {
.data_format = MIPI_RGB888,
.lcd_chan = &lcdc_info.ch[0],
+ .channel = LCDC_CHAN_MAINLCD,
.lane = 2,
.vsynw_offset = 17,
.phyctrl = 0x6 << 8,
--
1.7.8.6
^ permalink raw reply related
* [PATCH 07/19] fbdev: sh_mipi_dsi: Use the sh_mipi_dsi_info channel field
From: Laurent Pinchart @ 2012-08-16 13:00 UTC (permalink / raw)
To: linux-fbdev
Get the LCDC channel selector from the sh_mipi_dsi_info channel field
directly instead of accessing the LCDC platform data through the
lcd_chan field.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/video/sh_mipi_dsi.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/sh_mipi_dsi.c b/drivers/video/sh_mipi_dsi.c
index 3951fda..7f6ce65 100644
--- a/drivers/video/sh_mipi_dsi.c
+++ b/drivers/video/sh_mipi_dsi.c
@@ -369,7 +369,7 @@ static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
/* setup LCD panel */
/* cf. drivers/video/omap/lcd_mipid.c */
- sh_mipi_dcs(ch->chan, MIPI_DCS_EXIT_SLEEP_MODE);
+ sh_mipi_dcs(pdata->channel, MIPI_DCS_EXIT_SLEEP_MODE);
msleep(120);
/*
* [7] - Page Address Mode
@@ -381,11 +381,11 @@ static int sh_mipi_setup(struct sh_mipi *mipi, struct sh_mipi_dsi_info *pdata)
* [1] - Flip Horizontal
* [0] - Flip Vertical
*/
- sh_mipi_dcs_param(ch->chan, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
+ sh_mipi_dcs_param(pdata->channel, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
/* cf. set_data_lines() */
- sh_mipi_dcs_param(ch->chan, MIPI_DCS_SET_PIXEL_FORMAT,
+ sh_mipi_dcs_param(pdata->channel, MIPI_DCS_SET_PIXEL_FORMAT,
pixfmt << 4);
- sh_mipi_dcs(ch->chan, MIPI_DCS_SET_DISPLAY_ON);
+ sh_mipi_dcs(pdata->channel, MIPI_DCS_SET_DISPLAY_ON);
/* Enable timeout counters */
iowrite32(0x00000f00, base + DSICTRL);
--
1.7.8.6
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox