Re: [PATCH 4/4] drm/tilcdc: add support for LCD panels (v4)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 24, 2013 at 7:08 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Tue, Jan 22, 2013 at 04:36:25PM -0600, Rob Clark wrote:
>> Add an output panel driver for LCD panels.  Tested with LCD3 cape on
>> beaglebone.
>>
>> v1: original
>> v2: s/of_find_node_by_name()/of_get_child_by_name()/ from Pantelis
>>     Antoniou
>> v3: add backlight support
>> v4: rebase to latest of video timing helpers
>>
>> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx>
>
> So given that I'm utterly lacking clue about all things of (which seems to
> be where all the magic in this patch lies) I'm just gonna ask a few funny
> questions.
>
> [cut]
>
>> +static int panel_connector_get_modes(struct drm_connector *connector)
>> +{
>> +     struct drm_device *dev = connector->dev;
>> +     struct panel_connector *panel_connector = to_panel_connector(connector);
>> +     struct display_timings *timings = panel_connector->mod->timings;
>> +     int i;
>> +
>> +     for (i = 0; i < timings->num_timings; i++) {
>> +             struct drm_display_mode *mode = drm_mode_create(dev);
>> +             struct videomode vm;
>> +
>> +             if (videomode_from_timing(timings, &vm, i))
>> +                     break;
>> +
>> +             drm_display_mode_from_videomode(&vm, mode);
>
> Why do we need to jump through the intermediate videomode thing here? Is
> that a deficiency of the of/videomode stuff?

there is a helper that cut through the intermediate videomode
structure, although it assumed you are doing that at the point where
you still have the 'struct device_node *' (in the probe code) which
didn't really fit well for how I had structured things.  So I just
skipped it.  I guess I could have gone straight to array of
drm_display_mode in the probe call, and then copied them here in
get_modes() but this just seemed a bit easier.

> [cut]
>
>> +     ret |= of_property_read_u32(info_np, "ac-bias", &info->ac_bias);
>> +     ret |= of_property_read_u32(info_np, "ac-bias-intrpt", &info->ac_bias_intrpt);
>> +     ret |= of_property_read_u32(info_np, "dma-burst-sz", &info->dma_burst_sz);
>> +     ret |= of_property_read_u32(info_np, "bpp", &info->bpp);
>> +     ret |= of_property_read_u32(info_np, "fdd", &info->fdd);
>> +     ret |= of_property_read_u32(info_np, "sync-edge", &info->sync_edge);
>> +     ret |= of_property_read_u32(info_np, "sync-ctrl", &info->sync_ctrl);
>> +     ret |= of_property_read_u32(info_np, "raster-order", &info->raster_order);
>> +     ret |= of_property_read_u32(info_np, "fifo-th", &info->fifo_th);
>
> Shouldn't these values all be documented somewhere in the devictree docs?
> Or are they somewhat standardized?

Yeah, I guess I need to add DT docs..  I didn't realize this earlier.

BR,
-R
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux