Re: [PATCH 4/4] drm/msm: Fix default fb var width and height

[hali]

Hi Rob,

> On Fri, Mar 6, 2015 at 1:12 PM, Rob Clark <robdclark@xxxxxxxxx> wrote:
>> On Thu, Mar 5, 2015 at 3:20 PM, Hai Li <hali@xxxxxxxxxxxxxx> wrote:
>>> The framebuffer var width and height should reflect the size of
>>> framebuffer memory allocated, which is the entire surface size.
>>>
>>> In case of dual DSI connectors with TILE properties, this change
>>> makes the whole image show on the dual DSI panel, instead of
>>> duplicated images on both sides.
>>>
>>> Signed-off-by: Hai Li <hali@xxxxxxxxxxxxxx>
>>> ---
>>>  drivers/gpu/drm/msm/msm_fbdev.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c
>>> b/drivers/gpu/drm/msm/msm_fbdev.c
>>> index df60f65..d3e8b14 100644
>>> --- a/drivers/gpu/drm/msm/msm_fbdev.c
>>> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
>>> @@ -169,7 +169,8 @@ static int msm_fbdev_create(struct drm_fb_helper
>>> *helper,
>>>         }
>>>
>>>         drm_fb_helper_fill_fix(fbi, fb->pitches[0], fb->depth);
>>> -       drm_fb_helper_fill_var(fbi, helper, sizes->fb_width,
>>> sizes->fb_height);
>>> +       drm_fb_helper_fill_var(fbi, helper,
>>> +                       sizes->surface_width, sizes->surface_height);
>>>
>>
>> so, I believe the intention for separation if surface_width/height and
>> fb_width/height, it to allocate a buffer that is big enough (width and
>> height) for all connected displays (so as to not leave some display
>> scanning out too small of a buffer), but size the fbdev buffer small
>> enough that text would be visible on all screens.  Using
>> surface_width/height here instead of fb_width/height would break that.
>>
>> But I think I have a different idea.. we could implement fb helper
>> func initial_config() to just call drm_pick_crtcs() (which we'd have
>> to export), and then for each of the crtcs[n] connected to a connector
>> with TILE property, populate the offsets[n].x/y.  Or possibly we
>> should just make drm_setup_crtcs() clever enough to do that
>> automatically.. that would result in larger fb_size and the two crtc's
>> scanning out their own parts of the buffer.
>
> Oh, I spoke too soon.. looks like Dave already added it in
> drm_get_tile_offsets()..
>

As discussed in IRC, the TILE information has been set and the framebuffer
is allocated with the full size, not the half.

The problem is, we create the full size framebuffer but only tell the user
(fbcon here, but may be real user space application who will use the
default framebuffer.) the half resolution. I think this mismatch will
cause trouble not only in the case of TILE connectors.

For our specific case of TILE connectors, since the right crtc offset.x
has been set to the half width, while the framebuffer width set to plane
is 0~half_width-1 (got from fb var info), the PIPE takes it wrong settings
and will not fetch the image data from framebuffer.
> BR,
> -R
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/