[PATCH hwc v2 05/18] drm_hwcomposer: Enable resource manager support

[Sean Paul]

On Tue, Apr 17, 2018 at 02:43:17PM +0100, Alexandru-Cosmin Gheorghe wrote:
> Hi Sean,
> 
> On Mon, Apr 16, 2018 at 03:54:02PM -0400, Sean Paul wrote:
> > On Wed, Apr 11, 2018 at 04:22:16PM +0100, Alexandru Gheorghe wrote:
> > > Use the newly added ResourceManager for creating and detecting all the
> > > drm devices instead of assuming that there is only one device.
> > > 
> > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe at arm.com>
> > > ---

<snip />

> > > diff --git a/drmresources.cpp b/drmresources.cpp
> > > index 32dd376..a5ddda0 100644
> > > --- a/drmresources.cpp
> > > +++ b/drmresources.cpp
> > > @@ -42,10 +42,9 @@ DrmResources::~DrmResources() {
> > >    event_listener_.Exit();
> > >  }
> > >  
> > > -int DrmResources::Init() {
> > > -  char path[PROPERTY_VALUE_MAX];
> > > -  property_get("hwc.drm.device", path, "/dev/dri/card0");
> > > -
> > > +int DrmResources::Init(ResourceManager *resource_manager, char *path,
> > > +                       int start_display_index) {
> > > +  resource_manager_ = resource_manager;
> > 
> > You can avoid the backpointer if you just pass the RM to the right places (looks
> > like compositor + composition). Bonus points if you can remove drm_ from those
> > objects once you've done that.
> 
> That's the thing Compositor/Composition already had drm_, hence the
> need of the backpointer. Didn't want to touch that as well. I suppose
> there is no strong reason why both Compositor & Composition shouldn't
> have just a ResourceManager object.

Yeah, exactly what I was thinking. It'll be a bit more refactoring, but worth it
IMO.

<snip />

> > > diff --git a/drmresources.h b/drmresources.h
> > > index 4cca48c..4cdcd87 100644
> > > --- a/drmresources.h
> > > +++ b/drmresources.h
> > > @@ -17,22 +17,26 @@
> > >  #ifndef ANDROID_DRM_H_
> > >  #define ANDROID_DRM_H_
> > >  
> > > +#include <stdint.h>
> > >  #include "drmconnector.h"
> > >  #include "drmcrtc.h"
> > >  #include "drmencoder.h"
> > >  #include "drmeventlistener.h"
> > >  #include "drmplane.h"
> > > -
> > > -#include <stdint.h>
> > 
> > Why this change?
> 
> I blame clang-format-diff-3.8 -i. I suppose it should be in a different
> commit.

I'd rather fix it at the source. If stdint.h include needs to move, that
probably means one of our headers needs to include it. So let's figure out which
needs it and add it there instead of shuffling things here.

Sean

> > 
> > > +#include "platform.h"
> > > +#include "resourcemanager.h"
> > >  
> > >  namespace android {
> > >  
> > > +class ResourceManager;
> > > +
> > >  class DrmResources {
> > >   public:
> > >    DrmResources();
> > >    ~DrmResources();
> > >  
> > > -  int Init();
> > > +  int Init(ResourceManager *resource_manager, char *path,
> > > +           int start_display_index);
> > >  
> > >    int fd() const {
> > >      return fd_.get();
> > > @@ -58,6 +62,7 @@ class DrmResources {
> > >    DrmCrtc *GetCrtcForDisplay(int display) const;
> > >    DrmPlane *GetPlane(uint32_t id) const;
> > >    DrmEventListener *event_listener();
> > > +  ResourceManager *resource_manager();
> > >  
> > >    int GetPlaneProperty(const DrmPlane &plane, const char *prop_name,
> > >                         DrmProperty *property);
> > > @@ -71,6 +76,7 @@ class DrmResources {
> > >  
> > >    int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id);
> > >    int DestroyPropertyBlob(uint32_t blob_id);
> > > +  bool HandlesDisplay(int display) const;
> > >  
> > >   private:
> > >    int TryEncoderForDisplay(int display, DrmEncoder *enc);
> > > @@ -90,6 +96,8 @@ class DrmResources {
> > >  
> > >    std::pair<uint32_t, uint32_t> min_resolution_;
> > >    std::pair<uint32_t, uint32_t> max_resolution_;
> > > +  std::map<int, int> displays_;
> > > +  ResourceManager *resource_manager_;
> > >  };
> > >  }
> > >  
> > > -- 
> > > 2.7.4
> > > 
> > 
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> 
> -- 
> Cheers,
> Alex G

-- 
Sean Paul, Software Engineer, Google / Chromium OS