Re: [PATCH 1/4] drm: Move drm_format_num_planes() to drm_crtc.c

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

 



On Fri, Apr 20, 2012 at 8:49 AM, Ville Syrjälä
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Fri, Apr 20, 2012 at 07:48:07AM -0500, Rob Clark wrote:
>> On Fri, Apr 20, 2012 at 7:29 AM, Dave Airlie <airlied@xxxxxxxxx> wrote:
>> > On Fri, Apr 20, 2012 at 1:23 PM, Ville Syrjälä
>> > <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>> >> On Fri, Apr 20, 2012 at 12:43:03PM +0100, Dave Airlie wrote:
>> >>> On Thu, Apr 5, 2012 at 7:35 PM,  <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>> >>> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> >>> >
>> >>> > There will be a need for this function in drm_crtc.c later. This
>> >>> > avoids making drm_crtc.c depend on drm_crtc_helper.c.
>> >>>
>> >>> Hi Ville,
>> >>>
>> >>> I've applied these 4 patches, but I might have lost some others for
>> >>> you, let me know if there is some other stuff I should be reviewing
>> >>> for -next.
>> >>
>> >> IIRC only one patch fell through the cracks:
>> >> Subject: [PATCH] drm: Unify and fix idr error handling
>> >> Message-Id: <1331834311-30757-1-git-send-email-ville.syrjala@xxxxxxxxxxxxxxx>
>> >
>> > Thanks I'll take a look at that,
>> >
>> >>
>> >> BTW you never gave any statement wrt. removing NV12M and YUV420M from
>> >> drm_fourcc.h. I can send a patch if you agree, and if not, well then
>> >> someone has to actually change the code to treat them exactly the same
>> >> as NV12 and YUV420.
>> >
>> > I'm probably not qualified, if you and jbarnes and say one other
>> > non-Intel person agree, then send the patch with R-b and I'll apply
>> > it.
>>
>> I agree that they seem like the same thing (as their non-M
>> counterparts) to me..  maybe the one exception is if there was hw that
>> somehow *only* supported discontiguous multi-planar.
>
> The way things are currently, anyone can feed the driver either
> contiguous or discontiguous planes using either format.
>
> As a hint to userspace the M version might work, except it still
> doesn't tell you anything on how you need to allocate or align the
> planes. Since memory allocation is driver specific anyway, I see no
> point in overloading the pixel formats with that information. Some
> other mechanism to communicate such information would be needed if
> there is a desire to make it generic.
>
>> I can't really
>> tell if this is the case in current exynos, it seems to only advertise
>> XRGB8888 (but possibly I'm looking in the wrong place).
>>
>> For drivers that have weird requirements, I'd suggest they use the non
>> 7-bit ascii space (ie. one or more of bytes in fourcc is non-ascii, so
>> as to not conflict with potential future standard fourcc's) driver
>> specific format values.
>
> We could define a DRM_FORMAT_DRIVER_SPECIFIC bit just like we have

yeah, that is a good idea

> DRM_FORMAT_BIG_ENDIAN. We still have three bits to choose from. OTOH
> I'm not too worried about standard fourccs since we already differ
> from the standard names anyway.

well, was more just thinking from the point of view of clashes if we
add more standard names later but some driver somewhere was already
using that new fourcc name

Possibly I'm over-thinking this.. but seemed like a reasonable thing
to separate standard and non-standard formats before a bunch of driver
specific formats start cropping up.

BR,
-R

> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux