Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-12 17:01, Archit Taneja wrote:

 So, what I'm saying is that we should stick to output-dispc_channel. We
 iterate through all the panels, and by using output-dispc_channel, we
 get the manager for an output, and map that manager to a crtc, and make
 sure the number of unique managers we finally use is equal to NUM_CRTC.
 
 Does that sound good?

Yes, I think that sounds good.

 Tomi




signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-12 17:01, Archit Taneja wrote:

> So, what I'm saying is that we should stick to output->dispc_channel. We
> iterate through all the panels, and by using output->dispc_channel, we
> get the manager for an output, and map that manager to a crtc, and make
> sure the number of unique managers we finally use is equal to NUM_CRTC.
> 
> Does that sound good?

Yes, I think that sounds good.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: 

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Tuesday 12 March 2013 07:59 PM, Tomi Valkeinen wrote:
> On 2013-03-12 16:01, Archit Taneja wrote:
>> On Tuesday 12 March 2013 07:07 PM, Tomi Valkeinen wrote:
>
>>> So, I don't disagree with you. But I don't quite understand why we could
>>> not use the fixed channels for now? They should work in all the boards
>>> we have, right? Or is there something with DRM that forces the driver to
>>> select the channel dynamically?
>>
>> I think we can use fixed channels, but if the number of different fixed
>> channels crosses the number of crtcs the kernel wants, then we would
>> need to atleast change the channels of some of the outputs.
>>
>> For example, suppose omapdrm is asked to use only 2 crtcs, and it picks
>> up LCD2 and TV managers. Now if there is some panel which says it's
>> recommended channel is LCD, then things won't work.
>
> Are you saying omapdrm picks the managers for the crtcs before knowing
> what panels there are? That can't work right... We need to know what
> outputs are to be used before we can select the managers. Or, we always
> need crtcs for all the managers.

That's how it is right now.

>
> If we do know the panels, and thus outputs, then the managers to be used
> are found easily from output->dispc_channel.

Yes, my patch tries to do the same, but it could assign a manger which 
isn't the recommended channel. It can pick one from the list of 
supported channels. I've explained it a bit more below.

>
> But, of course, the crtc to manager mapping could be changed (if omapdrm
> supports this). If omapdrm is asked to use only 1 crtc, but there are
> two panels, then only one panel can be used at a time, and the manager
> for the crtc needs to be changed when the panel to be used is changed.
> But even in this case used manager is clear, it comes from
> output->dispc_channel.

This is something I don't know that can be done or not, or if it can be 
done easily. A crtc isn't purely an overlay manager. It also needs to 
have one plane associated to it. So, if we want to change the overlay 
manager tied to a crtc on the fly, we should make sure that it's still 
connected to a plane pointing to the same buffer. This needs a better 
understanding of drm internals. I guess Rob could answer this better.

>
>> At the moment, omapdrm maps a crtc with a manger using a function called
>> pipe2chan() which just selects a manager with the biggest channel no. So
>> if the kernel is configured to have num_crtcs as 1. The single crtc will
>> be mapped to LCD2. This method is wrong, as it doesn't even look at the
>> type of panels at all. For an omap5 panda, the most suitable manager to
>> map to the crtc would be TV(for hdmi).
>>
>> I think what we probably need to do is to combine both the methods. I.e,
>> make each output connectible to only one channel, and also iterate
>> through the panels in omapdrm to find the most suitable channels. So in
>> my patch, instead of looking at all the supported managers for an
>> output(checking with dss_feat_get_supported_outputs() on each manager),
>> I just look at the recommended channel, and try to map that manager.
>
> I don't know, I feel like I'm not understanding something here =).

:)

I think what I said above is equivalent to what you said here:

"If we do know the panels, and thus outputs, then the managers to be 
used are found easily from output->dispc_channel."

Instead of using output->dispc_channel, the patch I made tried to get 
the first supported channel(in increasing order of channel index) for an 
output. If that channel was already taken/reserved by a previous output, 
it tries to take the next supported channel if there are enough crtcs left.

My patch would break things if it chooses a manager for an output for 
which we haven't written the necessary code yet(switching clock sources 
etc).

So, what I'm saying is that we should stick to output->dispc_channel. We 
iterate through all the panels, and by using output->dispc_channel, we 
get the manager for an output, and map that manager to a crtc, and make 
sure the number of unique managers we finally use is equal to NUM_CRTC.

Does that sound good?

Archit

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Tuesday 12 March 2013 07:07 PM, Tomi Valkeinen wrote:
> On 2013-03-12 14:57, Archit Taneja wrote:
>
 We could also use the recommended channel way for omapdrm, I can't
 figure out what's the better approach at the moment.
>>>
>>> Hmm, I think it'd be safer to use the recommended channel from omapdss
>>> for now. The current omapdss code doesn't really let you use any other
>>> channel than the recommended one (which was thus renamed to
>>> dispc_channel in my later version).
>>
>> I think omapdss needs to give the freedom to set a different dispc
>> manager for an output. This is because in omapdrm crtcs are sort of
>> floating, and when a connector(and it's encoder) needs to be enabled, it
>> tries to look search for a crtc it can connect to. If omapdss sort of
>> fixes the output->manager link, which is not how it should work.
>>
>> I think it is fine for us to use this approach for omapfb when creating
>> the links, but not within dss.
>>
>> For example, the code here:
>>
>> -/*
>> - * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
>> - * source for DPI is SoC integration detail, not something that should
>> - * be configured in the dssdev
>> - */
>> -dsidev = dpi_get_dsidev(dssdev->channel);
>> +dsidev = dpi_get_dsidev(dpi.output.dispc_channel);
>>
>> We are sort of using the recommended channel to figure out which DSI pll
>> to use. We don't have much of an option here because dpi_init_display()
>> happens much earlier. But if it were to connect to a different manager
>> later on, we would get into trouble.
>
> Right. It should be changed to allow dynamic dispc channel changes (as I
> said in my mail). But that requires changing how the output drivers and
> the output.c work. Perhaps nothing too difficult, but just something
> that has not been done =).
>
>>> Or does your patch do a better job at selecting the outputs (I'm mostly
>>> thinking about OMAP5 here, which has a bit more conflicts with the mgrs
>>> and outputs than earlier omaps).
>>
>> My approach is very drm oriented, the problem with omapdrm right now is
>> that we want to limit the number of crtcs(which map to managers). The
>> lesser the number of crtcs, the more free planes we have for overlaying.
>>
>> If through bootargs or CONFIG, we set NUM_CRTCs to 2. We can only set up
>> only 2 overlay managers. My patch just tries to figure out which are the
>> best 2 managers to choose out of the total number of managers, in such a
>> way that most encoders/panels on the platform are supported.
>>
>> The drm framework(I think) keeps the connections between crtc and
>> encoders flexible through the possible_crtcs flag maintained by
>> encoders. It figures out which crtc is free, and tries to use that one
>> at that moment. Once that encoder is done using it, the crtc can be used
>> by another encoder later. We can also change the crtc of an
>> encoder(after it's off).
>>
>> So omapdss fixing the dispc channel for an output doesn't seem suitable
>> for drm.
>
> So, I don't disagree with you. But I don't quite understand why we could
> not use the fixed channels for now? They should work in all the boards
> we have, right? Or is there something with DRM that forces the driver to
> select the channel dynamically?

I think we can use fixed channels, but if the number of different fixed 
channels crosses the number of crtcs the kernel wants, then we would 
need to atleast change the channels of some of the outputs.

For example, suppose omapdrm is asked to use only 2 crtcs, and it picks 
up LCD2 and TV managers. Now if there is some panel which says it's 
recommended channel is LCD, then things won't work.

At the moment, omapdrm maps a crtc with a manger using a function called 
pipe2chan() which just selects a manager with the biggest channel no. So 
if the kernel is configured to have num_crtcs as 1. The single crtc will 
be mapped to LCD2. This method is wrong, as it doesn't even look at the 
type of panels at all. For an omap5 panda, the most suitable manager to 
map to the crtc would be TV(for hdmi).

I think what we probably need to do is to combine both the methods. I.e, 
make each output connectible to only one channel, and also iterate 
through the panels in omapdrm to find the most suitable channels. So in 
my patch, instead of looking at all the supported managers for an 
output(checking with dss_feat_get_supported_outputs() on each manager), 
I just look at the recommended channel, and try to map that manager.

This way, we have a better approach than the pipe2chan() thing, but we 
restrict each encoder to connect to only one crtc, which we could extend 
later if the need arises.

Should we go with this approach then?

>
> As long as omapdss doesn't handle switching the clock sources (and
> perhaps some other settings also, like OMAP5's DPI video source
> selection. we've never had support for changing the channel later.),
> having omapdrm select the channels could lead to 

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Tuesday 12 March 2013 04:08 PM, Tomi Valkeinen wrote:
> On 2013-03-12 08:07, Archit Taneja wrote:
>> On Monday 11 March 2013 05:58 PM, Tomi Valkeinen wrote:
>>> On 2013-03-05 16:17, Archit Taneja wrote:
 The support outputs struct for overlay managers is incorrect for
 OMAP4. Make
 these changes:

 - DPI isn't supported via the LCD1 overlay manager, remove DPI as a
 supported
 output.
 - the TV manager can suppport DPI, but the omapdss driver doesn't
 support that
 yet, we require some muxing at the DSS level, and we also need to
 configure
 the hdmi pll in the DPI driver so that the TV manager has a pixel
 clock. We
 don't support that yet.

 Signed-off-by: Archit Taneja 
 ---
drivers/video/omap2/dss/dss_features.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)

 diff --git a/drivers/video/omap2/dss/dss_features.c
 b/drivers/video/omap2/dss/dss_features.c
 index d7d66ef..7f791ae 100644
 --- a/drivers/video/omap2/dss/dss_features.c
 +++ b/drivers/video/omap2/dss/dss_features.c
 @@ -202,12 +202,10 @@ static const enum omap_dss_output_id
 omap3630_dss_supported_outputs[] = {

static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
/* OMAP_DSS_CHANNEL_LCD */
 -OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
 -OMAP_DSS_OUTPUT_DSI1,
 +OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,

/* OMAP_DSS_CHANNEL_DIGIT */
 -OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
 -OMAP_DSS_OUTPUT_DPI,
 +OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,

/* OMAP_DSS_CHANNEL_LCD2 */
OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |

>>>
>>> Thanks, I'll apply this to omapdss fixes branch.
>>
>> Hi, just one point here, this patch is a prerequisite for the patch 2/4
>> in this series. So we need to make sure that the 2/4 patch is not
>> without this one in a kernel.
>>
>> Tomi,
>>
>> About patch '2/4', could you have a look at it too? It basically tries
>> to do a dynamic assignment of channels to outputs. I worked on this
>> before you posted the misc series with recommended_channel for outputs.
>> This patch tries to figure out managers with supported_outputs. It isn't
>> the most optimal way, as it can't back track and chose a better manager,
>> but it still seems to do a reasonable job.
>>
>> We could also use the recommended channel way for omapdrm, I can't
>> figure out what's the better approach at the moment.
>
> Hmm, I think it'd be safer to use the recommended channel from omapdss
> for now. The current omapdss code doesn't really let you use any other
> channel than the recommended one (which was thus renamed to
> dispc_channel in my later version).

I think omapdss needs to give the freedom to set a different dispc 
manager for an output. This is because in omapdrm crtcs are sort of 
floating, and when a connector(and it's encoder) needs to be enabled, it 
tries to look search for a crtc it can connect to. If omapdss sort of 
fixes the output->manager link, which is not how it should work.

I think it is fine for us to use this approach for omapfb when creating 
the links, but not within dss.

For example, the code here:

-   /*
-* XXX We shouldn't need dssdev->channel for this. The dsi pll clock
-* source for DPI is SoC integration detail, not something that should
-* be configured in the dssdev
-*/
-   dsidev = dpi_get_dsidev(dssdev->channel);
+   dsidev = dpi_get_dsidev(dpi.output.dispc_channel);

We are sort of using the recommended channel to figure out which DSI pll 
to use. We don't have much of an option here because dpi_init_display() 
happens much earlier. But if it were to connect to a different manager 
later on, we would get into trouble.

>
> Or does your patch do a better job at selecting the outputs (I'm mostly
> thinking about OMAP5 here, which has a bit more conflicts with the mgrs
> and outputs than earlier omaps).

My approach is very drm oriented, the problem with omapdrm right now is 
that we want to limit the number of crtcs(which map to managers). The 
lesser the number of crtcs, the more free planes we have for overlaying.

If through bootargs or CONFIG, we set NUM_CRTCs to 2. We can only set up 
only 2 overlay managers. My patch just tries to figure out which are the 
best 2 managers to choose out of the total number of managers, in such a 
way that most encoders/panels on the platform are supported.

The drm framework(I think) keeps the connections between crtc and 
encoders flexible through the possible_crtcs flag maintained by 
encoders. It figures out which crtc is free, and tries to use that one 
at that moment. Once that encoder is done using it, the crtc can be used 
by another encoder later. We can also change the crtc of an 
encoder(after it's off).

So omapdss fixing the dispc 

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-12 16:01, Archit Taneja wrote:
> On Tuesday 12 March 2013 07:07 PM, Tomi Valkeinen wrote:

>> So, I don't disagree with you. But I don't quite understand why we could
>> not use the fixed channels for now? They should work in all the boards
>> we have, right? Or is there something with DRM that forces the driver to
>> select the channel dynamically?
> 
> I think we can use fixed channels, but if the number of different fixed
> channels crosses the number of crtcs the kernel wants, then we would
> need to atleast change the channels of some of the outputs.
> 
> For example, suppose omapdrm is asked to use only 2 crtcs, and it picks
> up LCD2 and TV managers. Now if there is some panel which says it's
> recommended channel is LCD, then things won't work.

Are you saying omapdrm picks the managers for the crtcs before knowing
what panels there are? That can't work right... We need to know what
outputs are to be used before we can select the managers. Or, we always
need crtcs for all the managers.

If we do know the panels, and thus outputs, then the managers to be used
are found easily from output->dispc_channel.

But, of course, the crtc to manager mapping could be changed (if omapdrm
supports this). If omapdrm is asked to use only 1 crtc, but there are
two panels, then only one panel can be used at a time, and the manager
for the crtc needs to be changed when the panel to be used is changed.
But even in this case used manager is clear, it comes from
output->dispc_channel.

> At the moment, omapdrm maps a crtc with a manger using a function called
> pipe2chan() which just selects a manager with the biggest channel no. So
> if the kernel is configured to have num_crtcs as 1. The single crtc will
> be mapped to LCD2. This method is wrong, as it doesn't even look at the
> type of panels at all. For an omap5 panda, the most suitable manager to
> map to the crtc would be TV(for hdmi).
> 
> I think what we probably need to do is to combine both the methods. I.e,
> make each output connectible to only one channel, and also iterate
> through the panels in omapdrm to find the most suitable channels. So in
> my patch, instead of looking at all the supported managers for an
> output(checking with dss_feat_get_supported_outputs() on each manager),
> I just look at the recommended channel, and try to map that manager.

I don't know, I feel like I'm not understanding something here =).

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: 

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-12 14:57, Archit Taneja wrote:

>>> We could also use the recommended channel way for omapdrm, I can't
>>> figure out what's the better approach at the moment.
>>
>> Hmm, I think it'd be safer to use the recommended channel from omapdss
>> for now. The current omapdss code doesn't really let you use any other
>> channel than the recommended one (which was thus renamed to
>> dispc_channel in my later version).
> 
> I think omapdss needs to give the freedom to set a different dispc
> manager for an output. This is because in omapdrm crtcs are sort of
> floating, and when a connector(and it's encoder) needs to be enabled, it
> tries to look search for a crtc it can connect to. If omapdss sort of
> fixes the output->manager link, which is not how it should work.
> 
> I think it is fine for us to use this approach for omapfb when creating
> the links, but not within dss.
> 
> For example, the code here:
> 
> -/*
> - * XXX We shouldn't need dssdev->channel for this. The dsi pll clock
> - * source for DPI is SoC integration detail, not something that should
> - * be configured in the dssdev
> - */
> -dsidev = dpi_get_dsidev(dssdev->channel);
> +dsidev = dpi_get_dsidev(dpi.output.dispc_channel);
> 
> We are sort of using the recommended channel to figure out which DSI pll
> to use. We don't have much of an option here because dpi_init_display()
> happens much earlier. But if it were to connect to a different manager
> later on, we would get into trouble.

Right. It should be changed to allow dynamic dispc channel changes (as I
said in my mail). But that requires changing how the output drivers and
the output.c work. Perhaps nothing too difficult, but just something
that has not been done =).

>> Or does your patch do a better job at selecting the outputs (I'm mostly
>> thinking about OMAP5 here, which has a bit more conflicts with the mgrs
>> and outputs than earlier omaps).
> 
> My approach is very drm oriented, the problem with omapdrm right now is
> that we want to limit the number of crtcs(which map to managers). The
> lesser the number of crtcs, the more free planes we have for overlaying.
> 
> If through bootargs or CONFIG, we set NUM_CRTCs to 2. We can only set up
> only 2 overlay managers. My patch just tries to figure out which are the
> best 2 managers to choose out of the total number of managers, in such a
> way that most encoders/panels on the platform are supported.
> 
> The drm framework(I think) keeps the connections between crtc and
> encoders flexible through the possible_crtcs flag maintained by
> encoders. It figures out which crtc is free, and tries to use that one
> at that moment. Once that encoder is done using it, the crtc can be used
> by another encoder later. We can also change the crtc of an
> encoder(after it's off).
> 
> So omapdss fixing the dispc channel for an output doesn't seem suitable
> for drm.

So, I don't disagree with you. But I don't quite understand why we could
not use the fixed channels for now? They should work in all the boards
we have, right? Or is there something with DRM that forces the driver to
select the channel dynamically?

As long as omapdss doesn't handle switching the clock sources (and
perhaps some other settings also, like OMAP5's DPI video source
selection. we've never had support for changing the channel later.),
having omapdrm select the channels could lead to problems if it happens
to select the wrong channel.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: 

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-12 08:07, Archit Taneja wrote:
> On Monday 11 March 2013 05:58 PM, Tomi Valkeinen wrote:
>> On 2013-03-05 16:17, Archit Taneja wrote:
>>> The support outputs struct for overlay managers is incorrect for
>>> OMAP4. Make
>>> these changes:
>>>
>>> - DPI isn't supported via the LCD1 overlay manager, remove DPI as a
>>> supported
>>>output.
>>> - the TV manager can suppport DPI, but the omapdss driver doesn't
>>> support that
>>>yet, we require some muxing at the DSS level, and we also need to
>>> configure
>>>the hdmi pll in the DPI driver so that the TV manager has a pixel
>>> clock. We
>>>don't support that yet.
>>>
>>> Signed-off-by: Archit Taneja 
>>> ---
>>>   drivers/video/omap2/dss/dss_features.c |6 ++
>>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/dss/dss_features.c
>>> b/drivers/video/omap2/dss/dss_features.c
>>> index d7d66ef..7f791ae 100644
>>> --- a/drivers/video/omap2/dss/dss_features.c
>>> +++ b/drivers/video/omap2/dss/dss_features.c
>>> @@ -202,12 +202,10 @@ static const enum omap_dss_output_id
>>> omap3630_dss_supported_outputs[] = {
>>>
>>>   static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
>>>   /* OMAP_DSS_CHANNEL_LCD */
>>> -OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>>> -OMAP_DSS_OUTPUT_DSI1,
>>> +OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
>>>
>>>   /* OMAP_DSS_CHANNEL_DIGIT */
>>> -OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
>>> -OMAP_DSS_OUTPUT_DPI,
>>> +OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
>>>
>>>   /* OMAP_DSS_CHANNEL_LCD2 */
>>>   OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>>>
>>
>> Thanks, I'll apply this to omapdss fixes branch.
> 
> Hi, just one point here, this patch is a prerequisite for the patch 2/4
> in this series. So we need to make sure that the 2/4 patch is not
> without this one in a kernel.
> 
> Tomi,
> 
> About patch '2/4', could you have a look at it too? It basically tries
> to do a dynamic assignment of channels to outputs. I worked on this
> before you posted the misc series with recommended_channel for outputs.
> This patch tries to figure out managers with supported_outputs. It isn't
> the most optimal way, as it can't back track and chose a better manager,
> but it still seems to do a reasonable job.
> 
> We could also use the recommended channel way for omapdrm, I can't
> figure out what's the better approach at the moment.

Hmm, I think it'd be safer to use the recommended channel from omapdss
for now. The current omapdss code doesn't really let you use any other
channel than the recommended one (which was thus renamed to
dispc_channel in my later version).

Or does your patch do a better job at selecting the outputs (I'm mostly
thinking about OMAP5 here, which has a bit more conflicts with the mgrs
and outputs than earlier omaps).

But at some point I think we should fix those issues, and let omapdrm
decide how to connect the managers and outputs.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: 

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Monday 11 March 2013 05:58 PM, Tomi Valkeinen wrote:
> On 2013-03-05 16:17, Archit Taneja wrote:
>> The support outputs struct for overlay managers is incorrect for OMAP4. Make
>> these changes:
>>
>> - DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
>>output.
>> - the TV manager can suppport DPI, but the omapdss driver doesn't support 
>> that
>>yet, we require some muxing at the DSS level, and we also need to 
>> configure
>>the hdmi pll in the DPI driver so that the TV manager has a pixel clock. 
>> We
>>don't support that yet.
>>
>> Signed-off-by: Archit Taneja 
>> ---
>>   drivers/video/omap2/dss/dss_features.c |6 ++
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/dss_features.c 
>> b/drivers/video/omap2/dss/dss_features.c
>> index d7d66ef..7f791ae 100644
>> --- a/drivers/video/omap2/dss/dss_features.c
>> +++ b/drivers/video/omap2/dss/dss_features.c
>> @@ -202,12 +202,10 @@ static const enum omap_dss_output_id 
>> omap3630_dss_supported_outputs[] = {
>>
>>   static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
>>  /* OMAP_DSS_CHANNEL_LCD */
>> -OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>> -OMAP_DSS_OUTPUT_DSI1,
>> +OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
>>
>>  /* OMAP_DSS_CHANNEL_DIGIT */
>> -OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
>> -OMAP_DSS_OUTPUT_DPI,
>> +OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
>>
>>  /* OMAP_DSS_CHANNEL_LCD2 */
>>  OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
>>
>
> Thanks, I'll apply this to omapdss fixes branch.

Hi, just one point here, this patch is a prerequisite for the patch 2/4 
in this series. So we need to make sure that the 2/4 patch is not 
without this one in a kernel.

Tomi,

About patch '2/4', could you have a look at it too? It basically tries 
to do a dynamic assignment of channels to outputs. I worked on this 
before you posted the misc series with recommended_channel for outputs. 
This patch tries to figure out managers with supported_outputs. It isn't 
the most optimal way, as it can't back track and chose a better manager, 
but it still seems to do a reasonable job.

We could also use the recommended channel way for omapdrm, I can't 
figure out what's the better approach at the moment.

Archit

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Monday 11 March 2013 05:58 PM, Tomi Valkeinen wrote:

On 2013-03-05 16:17, Archit Taneja wrote:

The support outputs struct for overlay managers is incorrect for OMAP4. Make
these changes:

- DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
   output.
- the TV manager can suppport DPI, but the omapdss driver doesn't support that
   yet, we require some muxing at the DSS level, and we also need to configure
   the hdmi pll in the DPI driver so that the TV manager has a pixel clock. We
   don't support that yet.

Signed-off-by: Archit Taneja arc...@ti.com
---
  drivers/video/omap2/dss/dss_features.c |6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c 
b/drivers/video/omap2/dss/dss_features.c
index d7d66ef..7f791ae 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -202,12 +202,10 @@ static const enum omap_dss_output_id 
omap3630_dss_supported_outputs[] = {

  static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
/* OMAP_DSS_CHANNEL_LCD */
-   OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-   OMAP_DSS_OUTPUT_DSI1,
+   OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,

/* OMAP_DSS_CHANNEL_DIGIT */
-   OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
-   OMAP_DSS_OUTPUT_DPI,
+   OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,

/* OMAP_DSS_CHANNEL_LCD2 */
OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |



Thanks, I'll apply this to omapdss fixes branch.


Hi, just one point here, this patch is a prerequisite for the patch 2/4 
in this series. So we need to make sure that the 2/4 patch is not 
without this one in a kernel.


Tomi,

About patch '2/4', could you have a look at it too? It basically tries 
to do a dynamic assignment of channels to outputs. I worked on this 
before you posted the misc series with recommended_channel for outputs. 
This patch tries to figure out managers with supported_outputs. It isn't 
the most optimal way, as it can't back track and chose a better manager, 
but it still seems to do a reasonable job.


We could also use the recommended channel way for omapdrm, I can't 
figure out what's the better approach at the moment.


Archit
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-12 08:07, Archit Taneja wrote:
 On Monday 11 March 2013 05:58 PM, Tomi Valkeinen wrote:
 On 2013-03-05 16:17, Archit Taneja wrote:
 The support outputs struct for overlay managers is incorrect for
 OMAP4. Make
 these changes:

 - DPI isn't supported via the LCD1 overlay manager, remove DPI as a
 supported
output.
 - the TV manager can suppport DPI, but the omapdss driver doesn't
 support that
yet, we require some muxing at the DSS level, and we also need to
 configure
the hdmi pll in the DPI driver so that the TV manager has a pixel
 clock. We
don't support that yet.

 Signed-off-by: Archit Taneja arc...@ti.com
 ---
   drivers/video/omap2/dss/dss_features.c |6 ++
   1 file changed, 2 insertions(+), 4 deletions(-)

 diff --git a/drivers/video/omap2/dss/dss_features.c
 b/drivers/video/omap2/dss/dss_features.c
 index d7d66ef..7f791ae 100644
 --- a/drivers/video/omap2/dss/dss_features.c
 +++ b/drivers/video/omap2/dss/dss_features.c
 @@ -202,12 +202,10 @@ static const enum omap_dss_output_id
 omap3630_dss_supported_outputs[] = {

   static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
   /* OMAP_DSS_CHANNEL_LCD */
 -OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
 -OMAP_DSS_OUTPUT_DSI1,
 +OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,

   /* OMAP_DSS_CHANNEL_DIGIT */
 -OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
 -OMAP_DSS_OUTPUT_DPI,
 +OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,

   /* OMAP_DSS_CHANNEL_LCD2 */
   OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |


 Thanks, I'll apply this to omapdss fixes branch.
 
 Hi, just one point here, this patch is a prerequisite for the patch 2/4
 in this series. So we need to make sure that the 2/4 patch is not
 without this one in a kernel.
 
 Tomi,
 
 About patch '2/4', could you have a look at it too? It basically tries
 to do a dynamic assignment of channels to outputs. I worked on this
 before you posted the misc series with recommended_channel for outputs.
 This patch tries to figure out managers with supported_outputs. It isn't
 the most optimal way, as it can't back track and chose a better manager,
 but it still seems to do a reasonable job.
 
 We could also use the recommended channel way for omapdrm, I can't
 figure out what's the better approach at the moment.

Hmm, I think it'd be safer to use the recommended channel from omapdss
for now. The current omapdss code doesn't really let you use any other
channel than the recommended one (which was thus renamed to
dispc_channel in my later version).

Or does your patch do a better job at selecting the outputs (I'm mostly
thinking about OMAP5 here, which has a bit more conflicts with the mgrs
and outputs than earlier omaps).

But at some point I think we should fix those issues, and let omapdrm
decide how to connect the managers and outputs.

 Tomi




signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Tuesday 12 March 2013 04:08 PM, Tomi Valkeinen wrote:

On 2013-03-12 08:07, Archit Taneja wrote:

On Monday 11 March 2013 05:58 PM, Tomi Valkeinen wrote:

On 2013-03-05 16:17, Archit Taneja wrote:

The support outputs struct for overlay managers is incorrect for
OMAP4. Make
these changes:

- DPI isn't supported via the LCD1 overlay manager, remove DPI as a
supported
output.
- the TV manager can suppport DPI, but the omapdss driver doesn't
support that
yet, we require some muxing at the DSS level, and we also need to
configure
the hdmi pll in the DPI driver so that the TV manager has a pixel
clock. We
don't support that yet.

Signed-off-by: Archit Taneja arc...@ti.com
---
   drivers/video/omap2/dss/dss_features.c |6 ++
   1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c
b/drivers/video/omap2/dss/dss_features.c
index d7d66ef..7f791ae 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -202,12 +202,10 @@ static const enum omap_dss_output_id
omap3630_dss_supported_outputs[] = {

   static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
   /* OMAP_DSS_CHANNEL_LCD */
-OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-OMAP_DSS_OUTPUT_DSI1,
+OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,

   /* OMAP_DSS_CHANNEL_DIGIT */
-OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
-OMAP_DSS_OUTPUT_DPI,
+OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,

   /* OMAP_DSS_CHANNEL_LCD2 */
   OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |



Thanks, I'll apply this to omapdss fixes branch.


Hi, just one point here, this patch is a prerequisite for the patch 2/4
in this series. So we need to make sure that the 2/4 patch is not
without this one in a kernel.

Tomi,

About patch '2/4', could you have a look at it too? It basically tries
to do a dynamic assignment of channels to outputs. I worked on this
before you posted the misc series with recommended_channel for outputs.
This patch tries to figure out managers with supported_outputs. It isn't
the most optimal way, as it can't back track and chose a better manager,
but it still seems to do a reasonable job.

We could also use the recommended channel way for omapdrm, I can't
figure out what's the better approach at the moment.


Hmm, I think it'd be safer to use the recommended channel from omapdss
for now. The current omapdss code doesn't really let you use any other
channel than the recommended one (which was thus renamed to
dispc_channel in my later version).


I think omapdss needs to give the freedom to set a different dispc 
manager for an output. This is because in omapdrm crtcs are sort of 
floating, and when a connector(and it's encoder) needs to be enabled, it 
tries to look search for a crtc it can connect to. If omapdss sort of 
fixes the output-manager link, which is not how it should work.


I think it is fine for us to use this approach for omapfb when creating 
the links, but not within dss.


For example, the code here:

-   /*
-* XXX We shouldn't need dssdev-channel for this. The dsi pll clock
-* source for DPI is SoC integration detail, not something that should
-* be configured in the dssdev
-*/
-   dsidev = dpi_get_dsidev(dssdev-channel);
+   dsidev = dpi_get_dsidev(dpi.output.dispc_channel);

We are sort of using the recommended channel to figure out which DSI pll 
to use. We don't have much of an option here because dpi_init_display() 
happens much earlier. But if it were to connect to a different manager 
later on, we would get into trouble.




Or does your patch do a better job at selecting the outputs (I'm mostly
thinking about OMAP5 here, which has a bit more conflicts with the mgrs
and outputs than earlier omaps).


My approach is very drm oriented, the problem with omapdrm right now is 
that we want to limit the number of crtcs(which map to managers). The 
lesser the number of crtcs, the more free planes we have for overlaying.


If through bootargs or CONFIG, we set NUM_CRTCs to 2. We can only set up 
only 2 overlay managers. My patch just tries to figure out which are the 
best 2 managers to choose out of the total number of managers, in such a 
way that most encoders/panels on the platform are supported.


The drm framework(I think) keeps the connections between crtc and 
encoders flexible through the possible_crtcs flag maintained by 
encoders. It figures out which crtc is free, and tries to use that one 
at that moment. Once that encoder is done using it, the crtc can be used 
by another encoder later. We can also change the crtc of an 
encoder(after it's off).


So omapdss fixing the dispc channel for an output doesn't seem suitable 
for drm.


Archit

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Tuesday 12 March 2013 07:07 PM, Tomi Valkeinen wrote:

On 2013-03-12 14:57, Archit Taneja wrote:


We could also use the recommended channel way for omapdrm, I can't
figure out what's the better approach at the moment.


Hmm, I think it'd be safer to use the recommended channel from omapdss
for now. The current omapdss code doesn't really let you use any other
channel than the recommended one (which was thus renamed to
dispc_channel in my later version).


I think omapdss needs to give the freedom to set a different dispc
manager for an output. This is because in omapdrm crtcs are sort of
floating, and when a connector(and it's encoder) needs to be enabled, it
tries to look search for a crtc it can connect to. If omapdss sort of
fixes the output-manager link, which is not how it should work.

I think it is fine for us to use this approach for omapfb when creating
the links, but not within dss.

For example, the code here:

-/*
- * XXX We shouldn't need dssdev-channel for this. The dsi pll clock
- * source for DPI is SoC integration detail, not something that should
- * be configured in the dssdev
- */
-dsidev = dpi_get_dsidev(dssdev-channel);
+dsidev = dpi_get_dsidev(dpi.output.dispc_channel);

We are sort of using the recommended channel to figure out which DSI pll
to use. We don't have much of an option here because dpi_init_display()
happens much earlier. But if it were to connect to a different manager
later on, we would get into trouble.


Right. It should be changed to allow dynamic dispc channel changes (as I
said in my mail). But that requires changing how the output drivers and
the output.c work. Perhaps nothing too difficult, but just something
that has not been done =).


Or does your patch do a better job at selecting the outputs (I'm mostly
thinking about OMAP5 here, which has a bit more conflicts with the mgrs
and outputs than earlier omaps).


My approach is very drm oriented, the problem with omapdrm right now is
that we want to limit the number of crtcs(which map to managers). The
lesser the number of crtcs, the more free planes we have for overlaying.

If through bootargs or CONFIG, we set NUM_CRTCs to 2. We can only set up
only 2 overlay managers. My patch just tries to figure out which are the
best 2 managers to choose out of the total number of managers, in such a
way that most encoders/panels on the platform are supported.

The drm framework(I think) keeps the connections between crtc and
encoders flexible through the possible_crtcs flag maintained by
encoders. It figures out which crtc is free, and tries to use that one
at that moment. Once that encoder is done using it, the crtc can be used
by another encoder later. We can also change the crtc of an
encoder(after it's off).

So omapdss fixing the dispc channel for an output doesn't seem suitable
for drm.


So, I don't disagree with you. But I don't quite understand why we could
not use the fixed channels for now? They should work in all the boards
we have, right? Or is there something with DRM that forces the driver to
select the channel dynamically?


I think we can use fixed channels, but if the number of different fixed 
channels crosses the number of crtcs the kernel wants, then we would 
need to atleast change the channels of some of the outputs.


For example, suppose omapdrm is asked to use only 2 crtcs, and it picks 
up LCD2 and TV managers. Now if there is some panel which says it's 
recommended channel is LCD, then things won't work.


At the moment, omapdrm maps a crtc with a manger using a function called 
pipe2chan() which just selects a manager with the biggest channel no. So 
if the kernel is configured to have num_crtcs as 1. The single crtc will 
be mapped to LCD2. This method is wrong, as it doesn't even look at the 
type of panels at all. For an omap5 panda, the most suitable manager to 
map to the crtc would be TV(for hdmi).


I think what we probably need to do is to combine both the methods. I.e, 
make each output connectible to only one channel, and also iterate 
through the panels in omapdrm to find the most suitable channels. So in 
my patch, instead of looking at all the supported managers for an 
output(checking with dss_feat_get_supported_outputs() on each manager), 
I just look at the recommended channel, and try to map that manager.


This way, we have a better approach than the pipe2chan() thing, but we 
restrict each encoder to connect to only one crtc, which we could extend 
later if the need arises.


Should we go with this approach then?



As long as omapdss doesn't handle switching the clock sources (and
perhaps some other settings also, like OMAP5's DPI video source
selection. we've never had support for changing the channel later.),
having omapdrm select the channels could lead to problems if it happens
to select the wrong channel.


Yes, most of the other combinations wont work because of missing things 
like clock source selection in DSS_CTRL and using the 

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

On Tuesday 12 March 2013 07:59 PM, Tomi Valkeinen wrote:

On 2013-03-12 16:01, Archit Taneja wrote:

On Tuesday 12 March 2013 07:07 PM, Tomi Valkeinen wrote:



So, I don't disagree with you. But I don't quite understand why we could
not use the fixed channels for now? They should work in all the boards
we have, right? Or is there something with DRM that forces the driver to
select the channel dynamically?


I think we can use fixed channels, but if the number of different fixed
channels crosses the number of crtcs the kernel wants, then we would
need to atleast change the channels of some of the outputs.

For example, suppose omapdrm is asked to use only 2 crtcs, and it picks
up LCD2 and TV managers. Now if there is some panel which says it's
recommended channel is LCD, then things won't work.


Are you saying omapdrm picks the managers for the crtcs before knowing
what panels there are? That can't work right... We need to know what
outputs are to be used before we can select the managers. Or, we always
need crtcs for all the managers.


That's how it is right now.



If we do know the panels, and thus outputs, then the managers to be used
are found easily from output-dispc_channel.


Yes, my patch tries to do the same, but it could assign a manger which 
isn't the recommended channel. It can pick one from the list of 
supported channels. I've explained it a bit more below.




But, of course, the crtc to manager mapping could be changed (if omapdrm
supports this). If omapdrm is asked to use only 1 crtc, but there are
two panels, then only one panel can be used at a time, and the manager
for the crtc needs to be changed when the panel to be used is changed.
But even in this case used manager is clear, it comes from
output-dispc_channel.


This is something I don't know that can be done or not, or if it can be 
done easily. A crtc isn't purely an overlay manager. It also needs to 
have one plane associated to it. So, if we want to change the overlay 
manager tied to a crtc on the fly, we should make sure that it's still 
connected to a plane pointing to the same buffer. This needs a better 
understanding of drm internals. I guess Rob could answer this better.





At the moment, omapdrm maps a crtc with a manger using a function called
pipe2chan() which just selects a manager with the biggest channel no. So
if the kernel is configured to have num_crtcs as 1. The single crtc will
be mapped to LCD2. This method is wrong, as it doesn't even look at the
type of panels at all. For an omap5 panda, the most suitable manager to
map to the crtc would be TV(for hdmi).

I think what we probably need to do is to combine both the methods. I.e,
make each output connectible to only one channel, and also iterate
through the panels in omapdrm to find the most suitable channels. So in
my patch, instead of looking at all the supported managers for an
output(checking with dss_feat_get_supported_outputs() on each manager),
I just look at the recommended channel, and try to map that manager.


I don't know, I feel like I'm not understanding something here =).


:)

I think what I said above is equivalent to what you said here:

If we do know the panels, and thus outputs, then the managers to be 
used are found easily from output-dispc_channel.


Instead of using output-dispc_channel, the patch I made tried to get 
the first supported channel(in increasing order of channel index) for an 
output. If that channel was already taken/reserved by a previous output, 
it tries to take the next supported channel if there are enough crtcs left.


My patch would break things if it chooses a manager for an output for 
which we haven't written the necessary code yet(switching clock sources 
etc).


So, what I'm saying is that we should stick to output-dispc_channel. We 
iterate through all the panels, and by using output-dispc_channel, we 
get the manager for an output, and map that manager to a crtc, and make 
sure the number of unique managers we finally use is equal to NUM_CRTC.


Does that sound good?

Archit

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-12 14:57, Archit Taneja wrote:

 We could also use the recommended channel way for omapdrm, I can't
 figure out what's the better approach at the moment.

 Hmm, I think it'd be safer to use the recommended channel from omapdss
 for now. The current omapdss code doesn't really let you use any other
 channel than the recommended one (which was thus renamed to
 dispc_channel in my later version).
 
 I think omapdss needs to give the freedom to set a different dispc
 manager for an output. This is because in omapdrm crtcs are sort of
 floating, and when a connector(and it's encoder) needs to be enabled, it
 tries to look search for a crtc it can connect to. If omapdss sort of
 fixes the output-manager link, which is not how it should work.
 
 I think it is fine for us to use this approach for omapfb when creating
 the links, but not within dss.
 
 For example, the code here:
 
 -/*
 - * XXX We shouldn't need dssdev-channel for this. The dsi pll clock
 - * source for DPI is SoC integration detail, not something that should
 - * be configured in the dssdev
 - */
 -dsidev = dpi_get_dsidev(dssdev-channel);
 +dsidev = dpi_get_dsidev(dpi.output.dispc_channel);
 
 We are sort of using the recommended channel to figure out which DSI pll
 to use. We don't have much of an option here because dpi_init_display()
 happens much earlier. But if it were to connect to a different manager
 later on, we would get into trouble.

Right. It should be changed to allow dynamic dispc channel changes (as I
said in my mail). But that requires changing how the output drivers and
the output.c work. Perhaps nothing too difficult, but just something
that has not been done =).

 Or does your patch do a better job at selecting the outputs (I'm mostly
 thinking about OMAP5 here, which has a bit more conflicts with the mgrs
 and outputs than earlier omaps).
 
 My approach is very drm oriented, the problem with omapdrm right now is
 that we want to limit the number of crtcs(which map to managers). The
 lesser the number of crtcs, the more free planes we have for overlaying.
 
 If through bootargs or CONFIG, we set NUM_CRTCs to 2. We can only set up
 only 2 overlay managers. My patch just tries to figure out which are the
 best 2 managers to choose out of the total number of managers, in such a
 way that most encoders/panels on the platform are supported.
 
 The drm framework(I think) keeps the connections between crtc and
 encoders flexible through the possible_crtcs flag maintained by
 encoders. It figures out which crtc is free, and tries to use that one
 at that moment. Once that encoder is done using it, the crtc can be used
 by another encoder later. We can also change the crtc of an
 encoder(after it's off).
 
 So omapdss fixing the dispc channel for an output doesn't seem suitable
 for drm.

So, I don't disagree with you. But I don't quite understand why we could
not use the fixed channels for now? They should work in all the boards
we have, right? Or is there something with DRM that forces the driver to
select the channel dynamically?

As long as omapdss doesn't handle switching the clock sources (and
perhaps some other settings also, like OMAP5's DPI video source
selection. we've never had support for changing the channel later.),
having omapdrm select the channels could lead to problems if it happens
to select the wrong channel.

 Tomi




signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-12 16:01, Archit Taneja wrote:
 On Tuesday 12 March 2013 07:07 PM, Tomi Valkeinen wrote:

 So, I don't disagree with you. But I don't quite understand why we could
 not use the fixed channels for now? They should work in all the boards
 we have, right? Or is there something with DRM that forces the driver to
 select the channel dynamically?
 
 I think we can use fixed channels, but if the number of different fixed
 channels crosses the number of crtcs the kernel wants, then we would
 need to atleast change the channels of some of the outputs.
 
 For example, suppose omapdrm is asked to use only 2 crtcs, and it picks
 up LCD2 and TV managers. Now if there is some panel which says it's
 recommended channel is LCD, then things won't work.

Are you saying omapdrm picks the managers for the crtcs before knowing
what panels there are? That can't work right... We need to know what
outputs are to be used before we can select the managers. Or, we always
need crtcs for all the managers.

If we do know the panels, and thus outputs, then the managers to be used
are found easily from output-dispc_channel.

But, of course, the crtc to manager mapping could be changed (if omapdrm
supports this). If omapdrm is asked to use only 1 crtc, but there are
two panels, then only one panel can be used at a time, and the manager
for the crtc needs to be changed when the panel to be used is changed.
But even in this case used manager is clear, it comes from
output-dispc_channel.

 At the moment, omapdrm maps a crtc with a manger using a function called
 pipe2chan() which just selects a manager with the biggest channel no. So
 if the kernel is configured to have num_crtcs as 1. The single crtc will
 be mapped to LCD2. This method is wrong, as it doesn't even look at the
 type of panels at all. For an omap5 panda, the most suitable manager to
 map to the crtc would be TV(for hdmi).
 
 I think what we probably need to do is to combine both the methods. I.e,
 make each output connectible to only one channel, and also iterate
 through the panels in omapdrm to find the most suitable channels. So in
 my patch, instead of looking at all the supported managers for an
 output(checking with dss_feat_get_supported_outputs() on each manager),
 I just look at the recommended channel, and try to map that manager.

I don't know, I feel like I'm not understanding something here =).

 Tomi




signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-05 16:17, Archit Taneja wrote:
> The support outputs struct for overlay managers is incorrect for OMAP4. Make
> these changes:
> 
> - DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
>   output.
> - the TV manager can suppport DPI, but the omapdss driver doesn't support that
>   yet, we require some muxing at the DSS level, and we also need to configure
>   the hdmi pll in the DPI driver so that the TV manager has a pixel clock. We
>   don't support that yet.
> 
> Signed-off-by: Archit Taneja 
> ---
>  drivers/video/omap2/dss/dss_features.c |6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dss_features.c 
> b/drivers/video/omap2/dss/dss_features.c
> index d7d66ef..7f791ae 100644
> --- a/drivers/video/omap2/dss/dss_features.c
> +++ b/drivers/video/omap2/dss/dss_features.c
> @@ -202,12 +202,10 @@ static const enum omap_dss_output_id 
> omap3630_dss_supported_outputs[] = {
>  
>  static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
>   /* OMAP_DSS_CHANNEL_LCD */
> - OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
> - OMAP_DSS_OUTPUT_DSI1,
> + OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
>  
>   /* OMAP_DSS_CHANNEL_DIGIT */
> - OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
> - OMAP_DSS_OUTPUT_DPI,
> + OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
>  
>   /* OMAP_DSS_CHANNEL_LCD2 */
>   OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
> 

Thanks, I'll apply this to omapdss fixes branch.

 Tomi


-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 899 bytes
Desc: OpenPGP digital signature
URL: 

Re: [PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Tomi Valkeinen]

On 2013-03-05 16:17, Archit Taneja wrote:
 The support outputs struct for overlay managers is incorrect for OMAP4. Make
 these changes:
 
 - DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
   output.
 - the TV manager can suppport DPI, but the omapdss driver doesn't support that
   yet, we require some muxing at the DSS level, and we also need to configure
   the hdmi pll in the DPI driver so that the TV manager has a pixel clock. We
   don't support that yet.
 
 Signed-off-by: Archit Taneja arc...@ti.com
 ---
  drivers/video/omap2/dss/dss_features.c |6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/video/omap2/dss/dss_features.c 
 b/drivers/video/omap2/dss/dss_features.c
 index d7d66ef..7f791ae 100644
 --- a/drivers/video/omap2/dss/dss_features.c
 +++ b/drivers/video/omap2/dss/dss_features.c
 @@ -202,12 +202,10 @@ static const enum omap_dss_output_id 
 omap3630_dss_supported_outputs[] = {
  
  static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
   /* OMAP_DSS_CHANNEL_LCD */
 - OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
 - OMAP_DSS_OUTPUT_DSI1,
 + OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
  
   /* OMAP_DSS_CHANNEL_DIGIT */
 - OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
 - OMAP_DSS_OUTPUT_DPI,
 + OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
  
   /* OMAP_DSS_CHANNEL_LCD2 */
   OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
 

Thanks, I'll apply this to omapdss fixes branch.

 Tomi




signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

The support outputs struct for overlay managers is incorrect for OMAP4. Make
these changes:

- DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
  output.
- the TV manager can suppport DPI, but the omapdss driver doesn't support that
  yet, we require some muxing at the DSS level, and we also need to configure
  the hdmi pll in the DPI driver so that the TV manager has a pixel clock. We
  don't support that yet.

Signed-off-by: Archit Taneja 
---
 drivers/video/omap2/dss/dss_features.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c 
b/drivers/video/omap2/dss/dss_features.c
index d7d66ef..7f791ae 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -202,12 +202,10 @@ static const enum omap_dss_output_id 
omap3630_dss_supported_outputs[] = {

 static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
/* OMAP_DSS_CHANNEL_LCD */
-   OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-   OMAP_DSS_OUTPUT_DSI1,
+   OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,

/* OMAP_DSS_CHANNEL_DIGIT */
-   OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
-   OMAP_DSS_OUTPUT_DPI,
+   OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,

/* OMAP_DSS_CHANNEL_LCD2 */
OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-- 
1.7.10.4

[PATCH 4/4] omapdss: features: fixed supported outputs for OMAP4

[Archit Taneja]

The support outputs struct for overlay managers is incorrect for OMAP4. Make
these changes:

- DPI isn't supported via the LCD1 overlay manager, remove DPI as a supported
  output.
- the TV manager can suppport DPI, but the omapdss driver doesn't support that
  yet, we require some muxing at the DSS level, and we also need to configure
  the hdmi pll in the DPI driver so that the TV manager has a pixel clock. We
  don't support that yet.

Signed-off-by: Archit Taneja arc...@ti.com
---
 drivers/video/omap2/dss/dss_features.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/dss_features.c 
b/drivers/video/omap2/dss/dss_features.c
index d7d66ef..7f791ae 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -202,12 +202,10 @@ static const enum omap_dss_output_id 
omap3630_dss_supported_outputs[] = {
 
 static const enum omap_dss_output_id omap4_dss_supported_outputs[] = {
/* OMAP_DSS_CHANNEL_LCD */
-   OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-   OMAP_DSS_OUTPUT_DSI1,
+   OMAP_DSS_OUTPUT_DBI | OMAP_DSS_OUTPUT_DSI1,
 
/* OMAP_DSS_CHANNEL_DIGIT */
-   OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI |
-   OMAP_DSS_OUTPUT_DPI,
+   OMAP_DSS_OUTPUT_VENC | OMAP_DSS_OUTPUT_HDMI,
 
/* OMAP_DSS_CHANNEL_LCD2 */
OMAP_DSS_OUTPUT_DPI | OMAP_DSS_OUTPUT_DBI |
-- 
1.7.10.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel